Skip to content

Commit f9688bd

Browse files
danparizherntBre
andauthored
[flake8-annotations] Fix return type annotations to handle shadowed builtin symbols (ANN201, ANN202, ANN204, ANN205, ANN206) (#20612)
## Summary Fixes #20610 --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
1 parent 188c0dc commit f9688bd

File tree

5 files changed

+239
-30
lines changed

5 files changed

+239
-30
lines changed
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
from collections import UserString as str
2+
from typing import override
3+
4+
def foo():
5+
return "!"
6+
7+
def _foo():
8+
return "!"
9+
10+
class C:
11+
@override
12+
def __str__(self):
13+
return "!"
14+
15+
@staticmethod
16+
def foo():
17+
return "!"
18+
19+
@classmethod
20+
def bar(cls):
21+
return "!"

crates/ruff_linter/src/rules/flake8_annotations/helpers.rs

Lines changed: 51 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -142,23 +142,25 @@ impl AutoPythonType {
142142
});
143143
Some((expr, vec![no_return_edit]))
144144
}
145-
AutoPythonType::Atom(python_type) => {
146-
let expr = type_expr(python_type)?;
147-
Some((expr, vec![]))
148-
}
145+
AutoPythonType::Atom(python_type) => type_expr(python_type, checker, at),
149146
AutoPythonType::Union(python_types) => {
150147
if target_version >= PythonVersion::PY310 {
151148
// Aggregate all the individual types (e.g., `int`, `float`).
149+
let mut all_edits = Vec::new();
152150
let names = python_types
153151
.iter()
154152
.sorted_unstable()
155-
.map(|python_type| type_expr(*python_type))
153+
.map(|python_type| {
154+
let (expr, mut edits) = type_expr(*python_type, checker, at)?;
155+
all_edits.append(&mut edits);
156+
Some(expr)
157+
})
156158
.collect::<Option<Vec<_>>>()?;
157159

158160
// Wrap in a bitwise union (e.g., `int | float`).
159161
let expr = pep_604_union(&names);
160162

161-
Some((expr, vec![]))
163+
Some((expr, all_edits))
162164
} else {
163165
let python_types = python_types
164166
.into_iter()
@@ -167,20 +169,26 @@ impl AutoPythonType {
167169

168170
match python_types.as_slice() {
169171
[python_type, PythonType::None] | [PythonType::None, python_type] => {
170-
let element = type_expr(*python_type)?;
172+
let (element, mut edits) = type_expr(*python_type, checker, at)?;
171173

172174
// Ex) `Optional[int]`
173175
let (optional_edit, binding) = checker
174176
.typing_importer("Optional", PythonVersion::lowest())?
175177
.import(at)
176178
.ok()?;
177179
let expr = typing_optional(element, Name::from(binding));
178-
Some((expr, vec![optional_edit]))
180+
edits.push(optional_edit);
181+
Some((expr, edits))
179182
}
180183
_ => {
184+
let mut all_edits = Vec::new();
181185
let elements = python_types
182186
.into_iter()
183-
.map(type_expr)
187+
.map(|python_type| {
188+
let (expr, mut edits) = type_expr(python_type, checker, at)?;
189+
all_edits.append(&mut edits);
190+
Some(expr)
191+
})
184192
.collect::<Option<Vec<_>>>()?;
185193

186194
// Ex) `Union[int, str]`
@@ -189,7 +197,8 @@ impl AutoPythonType {
189197
.import(at)
190198
.ok()?;
191199
let expr = typing_union(&elements, Name::from(binding));
192-
Some((expr, vec![union_edit]))
200+
all_edits.push(union_edit);
201+
Some((expr, all_edits))
193202
}
194203
}
195204
}
@@ -199,26 +208,44 @@ impl AutoPythonType {
199208
}
200209

201210
/// Given a [`PythonType`], return an [`Expr`] that resolves to that type.
202-
fn type_expr(python_type: PythonType) -> Option<Expr> {
203-
fn name(name: &str) -> Expr {
204-
Expr::Name(ast::ExprName {
205-
id: name.into(),
211+
///
212+
/// If the [`Expr`] relies on importing any external symbols, those imports will be returned as
213+
/// additional edits.
214+
pub(crate) fn type_expr(
215+
python_type: PythonType,
216+
checker: &Checker,
217+
at: TextSize,
218+
) -> Option<(Expr, Vec<Edit>)> {
219+
fn name(name: &str, checker: &Checker, at: TextSize) -> Option<(Expr, Vec<Edit>)> {
220+
let (edit, binding) = checker
221+
.importer()
222+
.get_or_import_builtin_symbol(name, at, checker.semantic())
223+
.ok()?;
224+
let expr = Expr::Name(ast::ExprName {
225+
id: binding.into(),
206226
range: TextRange::default(),
207227
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
208228
ctx: ExprContext::Load,
209-
})
229+
});
230+
Some((expr, edit.map_or_else(Vec::new, |edit| vec![edit])))
210231
}
211232

212233
match python_type {
213-
PythonType::String => Some(name("str")),
214-
PythonType::Bytes => Some(name("bytes")),
215-
PythonType::Number(number) => match number {
216-
NumberLike::Integer => Some(name("int")),
217-
NumberLike::Float => Some(name("float")),
218-
NumberLike::Complex => Some(name("complex")),
219-
NumberLike::Bool => Some(name("bool")),
220-
},
221-
PythonType::None => Some(name("None")),
234+
PythonType::String => name("str", checker, at),
235+
PythonType::Bytes => name("bytes", checker, at),
236+
PythonType::Number(number) => {
237+
let symbol = match number {
238+
NumberLike::Integer => "int",
239+
NumberLike::Float => "float",
240+
NumberLike::Complex => "complex",
241+
NumberLike::Bool => "bool",
242+
};
243+
name(symbol, checker, at)
244+
}
245+
PythonType::None => {
246+
let expr = Expr::NoneLiteral(ast::ExprNoneLiteral::default());
247+
Some((expr, vec![]))
248+
}
222249
PythonType::Ellipsis => None,
223250
PythonType::Dict => None,
224251
PythonType::List => None,

crates/ruff_linter/src/rules/flake8_annotations/mod.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,4 +229,20 @@ mod tests {
229229
assert_diagnostics!(diagnostics);
230230
Ok(())
231231
}
232+
233+
#[test]
234+
fn shadowed_builtins() -> Result<()> {
235+
let diagnostics = test_path(
236+
Path::new("flake8_annotations/shadowed_builtins.py"),
237+
&LinterSettings::for_rules(vec![
238+
Rule::MissingReturnTypeUndocumentedPublicFunction,
239+
Rule::MissingReturnTypePrivateFunction,
240+
Rule::MissingReturnTypeSpecialMethod,
241+
Rule::MissingReturnTypeStaticMethod,
242+
Rule::MissingReturnTypeClassMethod,
243+
]),
244+
)?;
245+
assert_diagnostics!(diagnostics);
246+
Ok(())
247+
}
232248
}

crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,14 @@ use ruff_python_ast::identifier::Identifier;
44
use ruff_python_ast::visitor::Visitor;
55
use ruff_python_ast::{self as ast, Expr, Stmt};
66
use ruff_python_semantic::Definition;
7+
use ruff_python_semantic::analyze::type_inference::{NumberLike, PythonType};
78
use ruff_python_semantic::analyze::visibility;
89
use ruff_python_stdlib::typing::simple_magic_return_type;
910
use ruff_text_size::Ranged;
1011

1112
use crate::checkers::ast::{Checker, DiagnosticGuard};
1213
use crate::registry::Rule;
13-
use crate::rules::flake8_annotations::helpers::auto_return_type;
14+
use crate::rules::flake8_annotations::helpers::{auto_return_type, type_expr};
1415
use crate::rules::ruff::typing::type_hint_resolves_to_any;
1516
use crate::{Edit, Fix, FixAvailability, Violation};
1617

@@ -825,11 +826,31 @@ pub(crate) fn definition(
825826
},
826827
function.identifier(),
827828
);
828-
if let Some(return_type) = return_type {
829-
diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion(
830-
format!(" -> {return_type}"),
831-
function.parameters.end(),
832-
)));
829+
if let Some(return_type_str) = return_type {
830+
// Convert the simple return type to a proper expression that handles shadowed builtins
831+
let python_type = match return_type_str {
832+
"str" => PythonType::String,
833+
"bytes" => PythonType::Bytes,
834+
"int" => PythonType::Number(NumberLike::Integer),
835+
"float" => PythonType::Number(NumberLike::Float),
836+
"complex" => PythonType::Number(NumberLike::Complex),
837+
"bool" => PythonType::Number(NumberLike::Bool),
838+
"None" => PythonType::None,
839+
_ => return, // Unknown type, skip
840+
};
841+
842+
if let Some((expr, edits)) =
843+
type_expr(python_type, checker, function.parameters.start())
844+
{
845+
let return_type_expr = checker.generator().expr(&expr);
846+
diagnostic.set_fix(Fix::unsafe_edits(
847+
Edit::insertion(
848+
format!(" -> {return_type_expr}"),
849+
function.parameters.end(),
850+
),
851+
edits,
852+
));
853+
}
833854
}
834855
diagnostics.push(diagnostic);
835856
}
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
---
2+
source: crates/ruff_linter/src/rules/flake8_annotations/mod.rs
3+
---
4+
ANN201 [*] Missing return type annotation for public function `foo`
5+
--> shadowed_builtins.py:4:5
6+
|
7+
2 | from typing import override
8+
3 |
9+
4 | def foo():
10+
| ^^^
11+
5 | return "!"
12+
|
13+
help: Add return type annotation: `builtins.str`
14+
1 | from collections import UserString as str
15+
2 | from typing import override
16+
3 + import builtins
17+
4 |
18+
- def foo():
19+
5 + def foo() -> builtins.str:
20+
6 | return "!"
21+
7 |
22+
8 | def _foo():
23+
note: This is an unsafe fix and may change runtime behavior
24+
25+
ANN202 [*] Missing return type annotation for private function `_foo`
26+
--> shadowed_builtins.py:7:5
27+
|
28+
5 | return "!"
29+
6 |
30+
7 | def _foo():
31+
| ^^^^
32+
8 | return "!"
33+
|
34+
help: Add return type annotation: `builtins.str`
35+
1 | from collections import UserString as str
36+
2 | from typing import override
37+
3 + import builtins
38+
4 |
39+
5 | def foo():
40+
6 | return "!"
41+
7 |
42+
- def _foo():
43+
8 + def _foo() -> builtins.str:
44+
9 | return "!"
45+
10 |
46+
11 | class C:
47+
note: This is an unsafe fix and may change runtime behavior
48+
49+
ANN204 [*] Missing return type annotation for special method `__str__`
50+
--> shadowed_builtins.py:12:9
51+
|
52+
10 | class C:
53+
11 | @override
54+
12 | def __str__(self):
55+
| ^^^^^^^
56+
13 | return "!"
57+
|
58+
help: Add return type annotation: `str`
59+
1 | from collections import UserString as str
60+
2 | from typing import override
61+
3 + import builtins
62+
4 |
63+
5 | def foo():
64+
6 | return "!"
65+
--------------------------------------------------------------------------------
66+
10 |
67+
11 | class C:
68+
12 | @override
69+
- def __str__(self):
70+
13 + def __str__(self) -> builtins.str:
71+
14 | return "!"
72+
15 |
73+
16 | @staticmethod
74+
note: This is an unsafe fix and may change runtime behavior
75+
76+
ANN205 [*] Missing return type annotation for staticmethod `foo`
77+
--> shadowed_builtins.py:16:9
78+
|
79+
15 | @staticmethod
80+
16 | def foo():
81+
| ^^^
82+
17 | return "!"
83+
|
84+
help: Add return type annotation: `builtins.str`
85+
1 | from collections import UserString as str
86+
2 | from typing import override
87+
3 + import builtins
88+
4 |
89+
5 | def foo():
90+
6 | return "!"
91+
--------------------------------------------------------------------------------
92+
14 | return "!"
93+
15 |
94+
16 | @staticmethod
95+
- def foo():
96+
17 + def foo() -> builtins.str:
97+
18 | return "!"
98+
19 |
99+
20 | @classmethod
100+
note: This is an unsafe fix and may change runtime behavior
101+
102+
ANN206 [*] Missing return type annotation for classmethod `bar`
103+
--> shadowed_builtins.py:20:9
104+
|
105+
19 | @classmethod
106+
20 | def bar(cls):
107+
| ^^^
108+
21 | return "!"
109+
|
110+
help: Add return type annotation: `builtins.str`
111+
1 | from collections import UserString as str
112+
2 | from typing import override
113+
3 + import builtins
114+
4 |
115+
5 | def foo():
116+
6 | return "!"
117+
--------------------------------------------------------------------------------
118+
18 | return "!"
119+
19 |
120+
20 | @classmethod
121+
- def bar(cls):
122+
21 + def bar(cls) -> builtins.str:
123+
22 | return "!"
124+
note: This is an unsafe fix and may change runtime behavior

0 commit comments

Comments
 (0)