[flake8-annotations] Fix return type annotations to handle shadowed builtin symbols (ANN201, ANN202, ANN204, ANN205, ANN206)#20612
Conversation
|
| PythonType::None => { | ||
| let (edit, binding) = checker | ||
| .importer() | ||
| .get_or_import_builtin_symbol("None", at, checker.semantic()) |
There was a problem hiding this comment.
None is a keyword: it shouldn’t be imported.
ntBre
added
bug
ntBre
left a comment
ntBre
left a comment
There was a problem hiding this comment.
Thanks! This looks good overall, just a couple of simplification suggestions and one question/possible missed case for the @override method.
..._annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__shadowed_builtins.snap
Outdated
Show resolved
Hide resolved
| .importer() | ||
| .get_or_import_builtin_symbol("str", at, checker.semantic()) | ||
| .ok()?; | ||
| let expr = Expr::Name(ast::ExprName { |
There was a problem hiding this comment.
Can we still preserve the original name helper? Possibly with a generic impl Into<Name> argument instead of &str? That seemed to help with some of the repetition here. It looks like we could possibly even include the get_or_import_builtin call in the helper.
ntBre
left a comment
ntBre
left a comment
There was a problem hiding this comment.
Thank you! I pushed one commit moving the name helper back into type_expr just to keep it more consistent with the old code. I'm also not sure what I was thinking with the impl Into<Name> suggestion, so I just put that back to &str too 😅
| ))); | ||
| if let Some(return_type_str) = return_type { | ||
| // Convert the simple return type to a proper expression that handles shadowed builtins | ||
| let python_type = match return_type_str { |
There was a problem hiding this comment.
This is a bit awkward because we just did basically the same match inside of simple_magic_return_type. I tried moving simple_magic_return_type into this file and making it return a PythonType directly, though, and then we don't have a nice way to get back to the string representation. So I think this is fine as-is.
* origin/main: [`flake8-bugbear`] Include certain guaranteed-mutable expressions: tuples, generators, and assignment expressions (`B006`) (#20024) [`flake8-comprehensions`] Clarify fix safety documentation (`C413`) (#20640) [ty] improve base conda distinction from child conda (#20675) [`ruff`] Extend FA102 with listed PEP 585-compatible APIs (#20659) [`ruff`] Handle argfile expansion errors gracefully (#20691) [`flynt`] Fix f-string quoting for mixed quote joiners (`FLY002`) (#20662) [ty] Fix file root matching for `/` [ruff,ty] Enable tracing's `log` feature [`flake8-annotations`] Fix return type annotations to handle shadowed builtin symbols (`ANN201`, `ANN202`, `ANN204`, `ANN205`, `ANN206`) (#20612) Bump 0.13.3 (#20685) Update benchmarking CI for cargo-codspeed v4 (#20686) [ty] Support single-starred argument for overload call (#20223) [ty] `~T` should never be assignable to `T` (#20606) [`pylint`] Clarify fix safety to include left-hand hashability (`PLR6201`) (#20518) [ty] No union with `Unknown` for module-global symbols (#20664) [`ty`] Reject renaming files to start with slash in Playground (#20666) [ty] Enums: allow multiple aliases to point to the same member (#20669)
3 participants
Summary
Fixes #20610