Skip to content

Commit 885ff2c

Browse files
robsdeduderobsdedude
committed
[ruff] Fix RUF033 breaking with named default expressions
1 parent 316c1b2 commit 885ff2c

File tree

3 files changed

+204
-23
lines changed

3 files changed

+204
-23
lines changed

‎crates/ruff_linter/resources/test/fixtures/ruff/RUF033.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,3 +65,31 @@ class Foo:
6565
bar = "should've used attrs"
6666

6767
def __post_init__(self, bar: str = "ahhh", baz: str = "hmm") -> None: ...
68+
69+
70+
# https://github.com/astral-sh/ruff/issues/18950
71+
@dataclass
72+
class Foo:
73+
def __post_init__(self, bar: int = (x := 1)) -> None:
74+
pass
75+
76+
77+
@dataclass
78+
class Foo:
79+
def __post_init__(
80+
self,
81+
bar: int = (x := 1) # comment
82+
,
83+
baz: int = (y := 2), # comment
84+
) -> None:
85+
pass
86+
87+
88+
@dataclass
89+
class Foo:
90+
def __post_init__(
91+
self,
92+
bar: int = 1, # comment
93+
baz: int = 2, # comment
94+
) -> None:
95+
pass

‎crates/ruff_linter/src/rules/ruff/rules/post_init_default.rs‎

Lines changed: 57 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use ruff_python_ast as ast;
55
use ruff_python_semantic::{Scope, ScopeKind};
66
use ruff_python_trivia::{indentation_at_offset, textwrap};
77
use ruff_source_file::LineRanges;
8-
use ruff_text_size::Ranged;
8+
use ruff_text_size::{Ranged, TextRange};
99

1010
use crate::{Edit, Fix, FixAvailability, Violation};
1111
use crate::{checkers::ast::Checker, importer::ImportRequest};
@@ -117,13 +117,7 @@ pub(crate) fn post_init_default(checker: &Checker, function_def: &ast::StmtFunct
117117

118118
if !stopped_fixes {
119119
diagnostic.try_set_fix(|| {
120-
use_initvar(
121-
current_scope,
122-
function_def,
123-
&parameter.parameter,
124-
default,
125-
checker,
126-
)
120+
use_initvar(current_scope, function_def, &parameter, default, checker)
127121
});
128122
// Need to stop fixes as soon as there is a parameter we cannot fix.
129123
// Otherwise, we risk a syntax error (a parameter without a default
@@ -138,14 +132,14 @@ pub(crate) fn post_init_default(checker: &Checker, function_def: &ast::StmtFunct
138132
fn use_initvar(
139133
current_scope: &Scope,
140134
post_init_def: &ast::StmtFunctionDef,
141-
parameter: &ast::Parameter,
135+
parameter: &ast::ParameterWithDefault,
142136
default: &ast::Expr,
143137
checker: &Checker,
144138
) -> anyhow::Result<Fix> {
145-
if current_scope.has(&parameter.name) {
139+
if current_scope.has(&parameter.parameter.name) {
146140
return Err(anyhow::anyhow!(
147141
"Cannot add a `{}: InitVar` field to the class body, as a field by that name already exists",
148-
parameter.name
142+
parameter.parameter.name
149143
));
150144
}
151145

@@ -160,24 +154,47 @@ fn use_initvar(
160154
// Delete the default value. For example,
161155
// - def __post_init__(self, foo: int = 0) -> None: ...
162156
// + def __post_init__(self, foo: int) -> None: ...
163-
let default_edit = Edit::deletion(parameter.end(), default.end());
157+
debug_assert!(only_default_between_param_name_end_and_param(parameter));
158+
let default_edit = Edit::deletion(parameter.parameter.end(), parameter.end());
164159

165160
// Add `dataclasses.InitVar` field to class body.
166161
let locator = checker.locator();
167162

163+
let initvar_name = ast::Expr::Name(ast::ExprName {
164+
node_index: ast::AtomicNodeIndex::dummy(),
165+
range: TextRange::default(),
166+
id: ast::name::Name::new(initvar_binding),
167+
ctx: ast::ExprContext::Load,
168+
});
169+
170+
let stmt = ast::Stmt::AnnAssign(ast::StmtAnnAssign {
171+
node_index: ast::AtomicNodeIndex::dummy(),
172+
range: TextRange::default(),
173+
target: Box::new(ast::Expr::Name(ast::ExprName {
174+
node_index: ast::AtomicNodeIndex::dummy(),
175+
range: TextRange::default(),
176+
id: parameter.parameter.name.id.clone(),
177+
ctx: ast::ExprContext::Store,
178+
})),
179+
annotation: Box::new(match parameter.annotation() {
180+
None => initvar_name,
181+
Some(annotation) => ast::Expr::Subscript(ast::ExprSubscript {
182+
node_index: ast::AtomicNodeIndex::dummy(),
183+
range: TextRange::default(),
184+
value: Box::new(initvar_name),
185+
slice: Box::new(annotation.clone()),
186+
ctx: ast::ExprContext::Load,
187+
}),
188+
}),
189+
value: Some(Box::new(default.clone())),
190+
simple: true,
191+
});
192+
168193
let content = {
169-
let parameter_name = locator.slice(&parameter.name);
170-
let default = locator.slice(default);
171194
let line_ending = checker.stylist().line_ending().as_str();
172-
173-
if let Some(annotation) = &parameter
174-
.annotation()
175-
.map(|annotation| locator.slice(annotation))
176-
{
177-
format!("{parameter_name}: {initvar_binding}[{annotation}] = {default}{line_ending}")
178-
} else {
179-
format!("{parameter_name}: {initvar_binding} = {default}{line_ending}")
180-
}
195+
let mut content = checker.generator().stmt(&stmt);
196+
content.push_str(line_ending);
197+
content
181198
};
182199

183200
let indentation = indentation_at_offset(post_init_def.start(), checker.source())
@@ -190,3 +207,20 @@ fn use_initvar(
190207
);
191208
Ok(Fix::unsafe_edits(import_edit, [default_edit, initvar_edit]))
192209
}
210+
211+
/// Check that no other construct is between the parameter name and the end of the parameter
212+
/// definition (including the default value).
213+
fn only_default_between_param_name_end_and_param(parameter: &ast::ParameterWithDefault) -> bool {
214+
let ast::ParameterWithDefault {
215+
// Full destructuring to force compilation error when new fields are added.
216+
// New fields potentially need consideration in the logic below.
217+
range: _,
218+
node_index: _,
219+
parameter,
220+
default,
221+
} = parameter;
222+
let default = default
223+
.as_deref()
224+
.expect("called no_expr_after_default on a non-defaulted parameter");
225+
parameter.range.end() < default.range().start()
226+
}

‎crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF033_RUF033.py.snap‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,3 +156,122 @@ RUF033.py:67:59: RUF033 `__post_init__` method with argument defaults
156156
| ^^^^^ RUF033
157157
|
158158
= help: Use `dataclasses.InitVar` instead
159+
160+
RUF033.py:73:41: RUF033 [*] `__post_init__` method with argument defaults
161+
|
162+
71 | @dataclass
163+
72 | class Foo:
164+
73 | def __post_init__(self, bar: int = (x := 1)) -> None:
165+
| ^^^^^^ RUF033
166+
74 | pass
167+
|
168+
= help: Use `dataclasses.InitVar` instead
169+
170+
ℹ Unsafe fix
171+
70 70 | # https://github.com/astral-sh/ruff/issues/18950
172+
71 71 | @dataclass
173+
72 72 | class Foo:
174+
73 |- def __post_init__(self, bar: int = (x := 1)) -> None:
175+
73 |+ bar: InitVar[int] = (x := 1)
176+
74 |+ def __post_init__(self, bar: int) -> None:
177+
74 75 | pass
178+
75 76 |
179+
76 77 |
180+
181+
RUF033.py:81:21: RUF033 [*] `__post_init__` method with argument defaults
182+
|
183+
79 | def __post_init__(
184+
80 | self,
185+
81 | bar: int = (x := 1) # comment
186+
| ^^^^^^ RUF033
187+
82 | ,
188+
83 | baz: int = (y := 2), # comment
189+
|
190+
= help: Use `dataclasses.InitVar` instead
191+
192+
ℹ Unsafe fix
193+
76 76 |
194+
77 77 | @dataclass
195+
78 78 | class Foo:
196+
79 |+ bar: InitVar[int] = (x := 1)
197+
79 80 | def __post_init__(
198+
80 81 | self,
199+
81 |- bar: int = (x := 1) # comment
200+
82 |+ bar: int # comment
201+
82 83 | ,
202+
83 84 | baz: int = (y := 2), # comment
203+
84 85 | ) -> None:
204+
205+
RUF033.py:83:21: RUF033 [*] `__post_init__` method with argument defaults
206+
|
207+
81 | bar: int = (x := 1) # comment
208+
82 | ,
209+
83 | baz: int = (y := 2), # comment
210+
| ^^^^^^ RUF033
211+
84 | ) -> None:
212+
85 | pass
213+
|
214+
= help: Use `dataclasses.InitVar` instead
215+
216+
ℹ Unsafe fix
217+
76 76 |
218+
77 77 | @dataclass
219+
78 78 | class Foo:
220+
79 |+ baz: InitVar[int] = (y := 2)
221+
79 80 | def __post_init__(
222+
80 81 | self,
223+
81 82 | bar: int = (x := 1) # comment
224+
82 83 | ,
225+
83 |- baz: int = (y := 2), # comment
226+
84 |+ baz: int, # comment
227+
84 85 | ) -> None:
228+
85 86 | pass
229+
86 87 |
230+
231+
RUF033.py:92:20: RUF033 [*] `__post_init__` method with argument defaults
232+
|
233+
90 | def __post_init__(
234+
91 | self,
235+
92 | bar: int = 1, # comment
236+
| ^ RUF033
237+
93 | baz: int = 2, # comment
238+
94 | ) -> None:
239+
|
240+
= help: Use `dataclasses.InitVar` instead
241+
242+
ℹ Unsafe fix
243+
87 87 |
244+
88 88 | @dataclass
245+
89 89 | class Foo:
246+
90 |+ bar: InitVar[int] = 1
247+
90 91 | def __post_init__(
248+
91 92 | self,
249+
92 |- bar: int = 1, # comment
250+
93 |+ bar: int, # comment
251+
93 94 | baz: int = 2, # comment
252+
94 95 | ) -> None:
253+
95 96 | pass
254+
255+
RUF033.py:93:20: RUF033 [*] `__post_init__` method with argument defaults
256+
|
257+
91 | self,
258+
92 | bar: int = 1, # comment
259+
93 | baz: int = 2, # comment
260+
| ^ RUF033
261+
94 | ) -> None:
262+
95 | pass
263+
|
264+
= help: Use `dataclasses.InitVar` instead
265+
266+
ℹ Unsafe fix
267+
87 87 |
268+
88 88 | @dataclass
269+
89 89 | class Foo:
270+
90 |+ baz: InitVar[int] = 2
271+
90 91 | def __post_init__(
272+
91 92 | self,
273+
92 93 | bar: int = 1, # comment
274+
93 |- baz: int = 2, # comment
275+
94 |+ baz: int, # comment
276+
94 95 | ) -> None:
277+
95 96 | pass

0 commit comments

Comments
 (0)