Skip to content

Commit 17b82db

Browse files
committed
fixup! added literal and final use-site type modifiers
1 parent 066ca57 commit 17b82db

10 files changed

Lines changed: 485 additions & 23 deletions

File tree

‎crates/by_transforms/src/lib.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,8 @@ pub fn reverse_transpile(source: &str, config: &Config) -> Result<String, String
619619
let mut not_rev = reverse_transforms::not_type::NotTypeReverse::new(src, &model);
620620
let mut dynamic_keyword_rev =
621621
reverse_transforms::dynamic_keyword::DynamicKeywordReverse::new(&model);
622+
let mut literal_string_rev =
623+
reverse_transforms::literal_string::LiteralStringReverse::new(&model);
622624
let mut type_is_rev = reverse_transforms::type_is::TypeIsReverse::new(src, &model);
623625
let mut identity_rev = reverse_transforms::identity_swap::IdentitySwapReverse::new(src);
624626
let mut tuple_type = reverse_transforms::tuple_type::TupleTypeReverse::new(src, &model);
@@ -649,6 +651,7 @@ pub fn reverse_transpile(source: &str, config: &Config) -> Result<String, String
649651
intersection.visit_stmt(stmt);
650652
not_rev.visit_stmt(stmt);
651653
dynamic_keyword_rev.visit_stmt(stmt);
654+
literal_string_rev.visit_stmt(stmt);
652655
type_is_rev.visit_stmt(stmt);
653656
identity_rev.visit_stmt(stmt);
654657
tuple_type.visit_stmt(stmt);
@@ -700,6 +703,7 @@ pub fn reverse_transpile(source: &str, config: &Config) -> Result<String, String
700703
fixes.extend(intersection.edits);
701704
fixes.extend(not_rev.edits);
702705
fixes.extend(dynamic_keyword_rev.edits);
706+
fixes.extend(literal_string_rev.edits);
703707
fixes.extend(type_is_rev.edits);
704708
fixes.extend(identity_rev.edits);
705709
fixes.extend(unpack.edits);
Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
//! reverse of `crate::transforms::literal_string`:
2+
//! `LiteralString` → `literal str`
3+
//! `typing.LiteralString` → `literal str`
4+
//!
5+
//! only fires in type positions on a name spelled `LiteralString` that also
6+
//! *resolves* to `LiteralString` — the same pair of guards
7+
//! [`dynamic_keyword`](super::dynamic_keyword) uses, so an alias
8+
//! (`MyStr = LiteralString; x: MyStr`) keeps its own spelling and a shadowing
9+
//! binding is left alone.
10+
//!
11+
//! descent into nested type positions (`list[LiteralString]`,
12+
//! `LiteralString | None`, `Annotated[LiteralString, meta]` first arg) is
13+
//! delegated to [`type_expr_walker`], so metadata and `Literal[…]` slices are
14+
//! never touched.
15+
//!
16+
//! once the last reference is rewritten, `from typing import LiteralString` is
17+
//! dead and [`prune_imports`](super::prune_imports) drops it.
18+
19+
use ruff_diagnostics::{Edit, Fix};
20+
use ruff_python_ast::visitor::Visitor;
21+
use ruff_python_ast::{Expr, Stmt};
22+
use ruff_text_size::Ranged;
23+
24+
use crate::transforms::source_util::for_each_annotation_in_stmt;
25+
use crate::transforms::type_expr_walker::{Recurse, TypeExprVisitor, TypePos, walk_one_type_expr};
26+
use crate::type_info::TypeInfo;
27+
28+
pub(crate) struct LiteralStringReverse<'src> {
29+
types: &'src dyn TypeInfo,
30+
pub(crate) edits: Vec<Fix>,
31+
}
32+
33+
impl<'src> LiteralStringReverse<'src> {
34+
pub(crate) fn new(types: &'src dyn TypeInfo) -> Self {
35+
Self {
36+
types,
37+
edits: Vec::new(),
38+
}
39+
}
40+
41+
/// `LiteralString` (bare) or `<mod>.LiteralString`, where the name is
42+
/// spelled `LiteralString` *and* resolves to it
43+
fn is_literal_string(&self, expr: &Expr) -> bool {
44+
let spelled = match expr {
45+
Expr::Name(name) => name.id.as_str() == "LiteralString",
46+
Expr::Attribute(attribute) => attribute.attr.id.as_str() == "LiteralString",
47+
_ => false,
48+
};
49+
spelled && self.types.is_literal_string(expr)
50+
}
51+
52+
fn rewrite_annotation(&mut self, annotation: &Expr) {
53+
walk_one_type_expr(annotation, self);
54+
}
55+
}
56+
57+
impl TypeExprVisitor for LiteralStringReverse<'_> {
58+
fn visit(&mut self, expr: &Expr, _pos: TypePos) -> Recurse {
59+
if self.is_literal_string(expr) {
60+
self.edits.push(Fix::safe_edit(Edit::range_replacement(
61+
"literal str".to_owned(),
62+
expr.range(),
63+
)));
64+
}
65+
Recurse::Descend
66+
}
67+
}
68+
69+
impl<'ast> Visitor<'ast> for LiteralStringReverse<'_> {
70+
fn visit_stmt(&mut self, stmt: &'ast Stmt) {
71+
for_each_annotation_in_stmt(stmt, |annotation| {
72+
self.rewrite_annotation(annotation);
73+
});
74+
}
75+
}
76+
77+
#[cfg(test)]
78+
mod tests {
79+
use crate::{Config, reverse_transpile, transpile};
80+
use indoc::indoc;
81+
82+
/// `Config::test_default` leaves import pruning off so expectations stay
83+
/// stable; the now-dead `from typing import LiteralString` is dropped in
84+
/// real `--reverse` usage, which `dead_import_is_pruned` covers
85+
fn check(input: &str, expected: &str) {
86+
assert_eq!(
87+
reverse_transpile(input, &Config::test_default()).unwrap(),
88+
expected
89+
);
90+
}
91+
92+
#[test]
93+
fn simple_annotation() {
94+
check(
95+
"from typing import LiteralString\nx: LiteralString\n",
96+
"from typing import LiteralString\nx: literal str\n",
97+
);
98+
}
99+
100+
#[test]
101+
fn nested_in_generic() {
102+
check(
103+
"from typing import LiteralString\nx: list[LiteralString]\n",
104+
"from typing import LiteralString\nx: list[literal str]\n",
105+
);
106+
}
107+
108+
#[test]
109+
fn in_union() {
110+
check(
111+
"from typing import LiteralString\nx: LiteralString | None\n",
112+
"from typing import LiteralString\nx: literal str | None\n",
113+
);
114+
}
115+
116+
#[test]
117+
fn function_param_and_return() {
118+
check(
119+
indoc! {"
120+
from typing import LiteralString
121+
def f(x: LiteralString) -> LiteralString: ...
122+
"},
123+
indoc! {"
124+
from typing import LiteralString
125+
def f(x: literal str) -> literal str
126+
"},
127+
);
128+
}
129+
130+
#[test]
131+
fn qualified_typing_literal_string() {
132+
check(
133+
indoc! {"
134+
import typing
135+
x: typing.LiteralString
136+
"},
137+
indoc! {"
138+
import typing
139+
x: literal str
140+
"},
141+
);
142+
}
143+
144+
#[test]
145+
fn annotated_first_arg_only() {
146+
// only the first arg of `Annotated[T, meta]` is a type position; a
147+
// `LiteralString` in the metadata slot is an arbitrary value
148+
check(
149+
"from typing import Annotated, LiteralString\nx: Annotated[LiteralString, LiteralString]\n",
150+
"from typing import Annotated, LiteralString\nx: Annotated[literal str, LiteralString]\n",
151+
);
152+
}
153+
154+
#[test]
155+
fn value_position_unchanged() {
156+
check(
157+
"from typing import LiteralString\nx = LiteralString\n",
158+
"from typing import LiteralString\nx = LiteralString\n",
159+
);
160+
}
161+
162+
#[test]
163+
fn shadowed_unchanged() {
164+
// a local binding shadows the typing import, so the name no longer
165+
// resolves to `LiteralString` — leave it alone
166+
check(
167+
indoc! {"
168+
LiteralString = object()
169+
x: LiteralString
170+
"},
171+
indoc! {"
172+
LiteralString = object()
173+
x: LiteralString
174+
"},
175+
);
176+
}
177+
178+
#[test]
179+
fn dead_import_is_pruned() {
180+
// the real `--reverse` path prunes: once the last reference is a
181+
// keyword, the import supplies nothing
182+
let out = reverse_transpile(
183+
"from typing import LiteralString\nx: LiteralString\n",
184+
&Config::default(),
185+
)
186+
.unwrap();
187+
assert_eq!(out, "x: literal str\n");
188+
}
189+
190+
/// the pair is a real round trip: python in, the same python out
191+
#[test]
192+
fn round_trips_through_the_forward_lowering() {
193+
let python = "from typing import LiteralString\nx: LiteralString\n";
194+
let reversed = reverse_transpile(python, &Config::default()).unwrap();
195+
assert_eq!(reversed, "x: literal str\n");
196+
let forward = transpile(&reversed, &Config::test_default()).unwrap();
197+
assert!(
198+
forward.contains("x: LiteralString"),
199+
"expected the annotation back, got:\n{forward}"
200+
);
201+
}
202+
}

‎crates/by_transforms/src/reverse_transforms/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ pub(crate) mod generics;
2525
pub(crate) mod identity_swap;
2626
pub(crate) mod implementation;
2727
pub(crate) mod intersection;
28+
pub(crate) mod literal_string;
2829
pub(crate) mod literal_types;
2930
pub(crate) mod modifiers;
3031
pub(crate) mod none_chain;

‎crates/by_transforms/src/transforms/ast_driver.rs‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,12 @@ use super::{
3939
dedent_string, dynamic_keyword, empty_declarations, extension, float_const, force_unwrap,
4040
frameworks, generic_call, generics, grapheme_string, identity_swap, if_let, implementation,
4141
implicit_receiver, implicit_typing, inferred_annotation, init_method, just_float, kw_subscript,
42-
literal_types, local_once, main_function, match_type, modifiers, mutable_defaults, none_chain,
43-
optional_type, overload, parametric_is, postfix_await, propagate, properties, protocol_type,
44-
raises_clause, reified_generic, repeated_underscore, sentinel, some_ctor, soundness,
45-
statement_expression, string_tag, super_keyword, symbolic_type_op, top_star, trailing_lambda,
46-
tuple_index, type_fn, type_is, type_reification, typed_dict_literal, typed_lambda,
47-
typeof_keyword, unpack, use_site_variance,
42+
literal_string, literal_types, local_once, main_function, match_type, modifiers,
43+
mutable_defaults, none_chain, optional_type, overload, parametric_is, postfix_await, propagate,
44+
properties, protocol_type, raises_clause, reified_generic, repeated_underscore, sentinel,
45+
some_ctor, soundness, statement_expression, string_tag, super_keyword, symbolic_type_op,
46+
top_star, trailing_lambda, tuple_index, type_fn, type_is, type_reification, typed_dict_literal,
47+
typed_lambda, typeof_keyword, unpack, use_site_variance,
4848
};
4949
use crate::Config;
5050
use crate::type_info::TypeInfo;
@@ -390,6 +390,12 @@ pub(crate) fn run_against_source<'a>(
390390
// up front, from the original parse where `typeof` operands are still
391391
// intact for ty to read. the pass replaces each operation node and must run
392392
// before `typeof` lowering so a `typeof` operand is consumed here
393+
// `literal str` is the one use-site modifier python can spell, so it lowers
394+
// to `LiteralString` rather than being erased with the other markers. it has
395+
// to be collected from the db's own (marker-bearing) parse, since the
396+
// blanked copy the passes walk no longer has a `literal` keyword in it
397+
let literal_string_rewrites = literal_string::collect(parsed_handle.suite(), &semantic_model);
398+
393399
let symbolic_folds =
394400
symbolic_type_op::collect_symbolic_folds(parsed_handle.suite(), &semantic_model);
395401
let symbolic_needs_literal_import = symbolic_folds.needs_literal_import;
@@ -749,9 +755,22 @@ pub(crate) fn run_against_source<'a>(
749755
// spans) both come out clean; only a wider *plain* text edit keeps the
750756
// padding, which is valid Python either way
751757
for range in &blanked.ranges {
758+
// a marker rewritten to `LiteralString` has its whole range replaced
759+
// below; emitting the keyword collapse too would leave two plain text
760+
// edits racing for the same start offset
761+
if literal_string_rewrites.covers(*range) {
762+
continue;
763+
}
752764
ctx.text_edits
753765
.push((*range, use_site_variance::collapsed_to(source_ref, *range)));
754766
}
767+
for (range, replacement) in literal_string_rewrites.edits() {
768+
ctx.text_edits.push((range, replacement));
769+
}
770+
if literal_string_rewrites.needs_import {
771+
ctx.required_imports
772+
.push("from typing import LiteralString".to_owned());
773+
}
755774

756775
ctx.required_imports.sort();
757776
ctx.required_imports.dedup();

0 commit comments

Comments
 (0)