diff --git a/crates/ruff/resources/test/fixtures/pylint/sys_exit_alias_10.py b/crates/ruff/resources/test/fixtures/pylint/sys_exit_alias_10.py new file mode 100644 index 0000000000000..4344aec3d5219 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pylint/sys_exit_alias_10.py @@ -0,0 +1,8 @@ +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from sys import exit as bar + + +def main(): + exit(0) diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP006.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP006_0.py similarity index 100% rename from crates/ruff/resources/test/fixtures/pyupgrade/UP006.py rename to crates/ruff/resources/test/fixtures/pyupgrade/UP006_0.py diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP006_1.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP006_1.py new file mode 100644 index 0000000000000..1f56c64011a15 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP006_1.py @@ -0,0 +1,10 @@ +from __future__ import annotations + +import typing + +if typing.TYPE_CHECKING: + from collections import defaultdict + + +def f(x: typing.DefaultDict[str, str]) -> None: + ... diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP006_2.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP006_2.py new file mode 100644 index 0000000000000..eb4f5e8864f33 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP006_2.py @@ -0,0 +1,8 @@ +import typing + +if typing.TYPE_CHECKING: + from collections import defaultdict + + +def f(x: typing.DefaultDict[str, str]) -> None: + ... diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP006_3.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP006_3.py new file mode 100644 index 0000000000000..463365102feb1 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP006_3.py @@ -0,0 +1,8 @@ +import typing + +if typing.TYPE_CHECKING: + from collections import defaultdict + + +def f(x: "typing.DefaultDict[str, str]") -> None: + ... diff --git a/crates/ruff/src/importer/mod.rs b/crates/ruff/src/importer/mod.rs index 7179459c5b2ae..6649e467c1899 100644 --- a/crates/ruff/src/importer/mod.rs +++ b/crates/ruff/src/importer/mod.rs @@ -70,11 +70,16 @@ impl<'a> Importer<'a> { at: TextSize, semantic_model: &SemanticModel, ) -> Result<(Edit, String)> { - self.get_symbol(module, member, at, semantic_model)? - .map_or_else( - || self.import_symbol(module, member, at, semantic_model), - Ok, - ) + match self.get_symbol(module, member, at, semantic_model) { + None => self.import_symbol(module, member, at, semantic_model), + Some(Resolution::Success(edit, binding)) => Ok((edit, binding)), + Some(Resolution::LateBinding) => { + bail!("Unable to use existing symbol due to late binding") + } + Some(Resolution::IncompatibleContext) => { + bail!("Unable to use existing symbol due to incompatible context") + } + } } /// Return an [`Edit`] to reference an existing symbol, if it's present in the given [`SemanticModel`]. @@ -84,21 +89,25 @@ impl<'a> Importer<'a> { member: &str, at: TextSize, semantic_model: &SemanticModel, - ) -> Result> { + ) -> Option { // If the symbol is already available in the current scope, use it. - let Some((source, binding)) = semantic_model.resolve_qualified_import_name(module, member) else { - return Ok(None); - }; + let imported_name = semantic_model.resolve_qualified_import_name(module, member)?; - // The exception: the symbol source (i.e., the import statement) comes after the current - // location. For example, we could be generating an edit within a function, and the import + // If the symbol source (i.e., the import statement) comes after the current location, + // abort. For example, we could be generating an edit within a function, and the import // could be defined in the module scope, but after the function definition. In this case, // it's unclear whether we can use the symbol (the function could be called between the // import and the current location, and thus the symbol would not be available). It's also // unclear whether should add an import statement at the top of the file, since it could // be shadowed between the import and the current location. - if source.start() > at { - bail!("Unable to use existing symbol `{binding}` due to late-import"); + if imported_name.range().start() > at { + return Some(Resolution::LateBinding); + } + + // If the symbol source (i.e., the import statement) is in a typing-only context, but we're + // in a runtime context, abort. + if imported_name.context().is_typing() && semantic_model.execution_context().is_runtime() { + return Some(Resolution::IncompatibleContext); } // We also add a no-op edit to force conflicts with any other fixes that might try to @@ -118,10 +127,10 @@ impl<'a> Importer<'a> { // By adding this no-op edit, we force the `unused-imports` fix to conflict with the // `sys-exit-alias` fix, and thus will avoid applying both fixes in the same pass. let import_edit = Edit::range_replacement( - self.locator.slice(source.range()).to_string(), - source.range(), + self.locator.slice(imported_name.range()).to_string(), + imported_name.range(), ); - Ok(Some((import_edit, binding))) + Some(Resolution::Success(import_edit, imported_name.into_name())) } /// Generate an [`Edit`] to reference the given symbol. Returns the [`Edit`] necessary to make @@ -220,3 +229,13 @@ impl<'a> Importer<'a> { Ok(Edit::range_replacement(state.to_string(), stmt.range())) } } + +enum Resolution { + /// The symbol is available for use. + Success(Edit, String), + /// The symbol is imported, but the import came after the current location. + LateBinding, + /// The symbol is imported, but in an incompatible context (e.g., in typing-only context, while + /// we're in a runtime context). + IncompatibleContext, +} diff --git a/crates/ruff/src/rules/pylint/mod.rs b/crates/ruff/src/rules/pylint/mod.rs index 0c0a4952a5e62..6534a3efd34e4 100644 --- a/crates/ruff/src/rules/pylint/mod.rs +++ b/crates/ruff/src/rules/pylint/mod.rs @@ -39,6 +39,7 @@ mod tests { #[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_7.py"); "PLR1722_7")] #[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_8.py"); "PLR1722_8")] #[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_9.py"); "PLR1722_9")] + #[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_10.py"); "PLR1722_10")] #[test_case(Rule::ContinueInFinally, Path::new("continue_in_finally.py"); "PLE0116")] #[test_case(Rule::GlobalStatement, Path::new("global_statement.py"); "PLW0603")] #[test_case(Rule::GlobalVariableNotAssigned, Path::new("global_variable_not_assigned.py"); "PLW0602")] diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1722_sys_exit_alias_10.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1722_sys_exit_alias_10.py.snap new file mode 100644 index 0000000000000..b555d867bb79f --- /dev/null +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1722_sys_exit_alias_10.py.snap @@ -0,0 +1,12 @@ +--- +source: crates/ruff/src/rules/pylint/mod.rs +--- +sys_exit_alias_10.py:8:5: PLR1722 Use `sys.exit()` instead of `exit` + | +8 | def main(): +9 | exit(0) + | ^^^^ PLR1722 + | + = help: Replace `exit` with `sys.exit()` + + diff --git a/crates/ruff/src/rules/pyupgrade/mod.rs b/crates/ruff/src/rules/pyupgrade/mod.rs index e702f75279404..7a101bce0303a 100644 --- a/crates/ruff/src/rules/pyupgrade/mod.rs +++ b/crates/ruff/src/rules/pyupgrade/mod.rs @@ -20,7 +20,10 @@ mod tests { #[test_case(Rule::TypeOfPrimitive, Path::new("UP003.py"); "UP003")] #[test_case(Rule::UselessObjectInheritance, Path::new("UP004.py"); "UP004")] #[test_case(Rule::DeprecatedUnittestAlias, Path::new("UP005.py"); "UP005")] - #[test_case(Rule::NonPEP585Annotation, Path::new("UP006.py"); "UP006")] + #[test_case(Rule::NonPEP585Annotation, Path::new("UP006_0.py"); "UP006_0")] + #[test_case(Rule::NonPEP585Annotation, Path::new("UP006_1.py"); "UP006_1")] + #[test_case(Rule::NonPEP585Annotation, Path::new("UP006_2.py"); "UP006_2")] + #[test_case(Rule::NonPEP585Annotation, Path::new("UP006_3.py"); "UP006_3")] #[test_case(Rule::NonPEP604Annotation, Path::new("UP007.py"); "UP007")] #[test_case(Rule::SuperCallWithParameters, Path::new("UP008.py"); "UP008")] #[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_0.py"); "UP009_0")] diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006_0.py.snap similarity index 78% rename from crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006.py.snap rename to crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006_0.py.snap index 31d1624d68fa5..c0d954282b484 100644 --- a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006.py.snap +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006_0.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff/src/rules/pyupgrade/mod.rs --- -UP006.py:4:10: UP006 [*] Use `list` instead of `typing.List` for type annotation +UP006_0.py:4:10: UP006 [*] Use `list` instead of `typing.List` for type annotation | 4 | def f(x: typing.List[str]) -> None: | ^^^^^^^^^^^ UP006 @@ -19,7 +19,7 @@ UP006.py:4:10: UP006 [*] Use `list` instead of `typing.List` for type annotation 6 6 | 7 7 | -UP006.py:11:10: UP006 [*] Use `list` instead of `List` for type annotation +UP006_0.py:11:10: UP006 [*] Use `list` instead of `List` for type annotation | 11 | def f(x: List[str]) -> None: | ^^^^ UP006 @@ -37,7 +37,7 @@ UP006.py:11:10: UP006 [*] Use `list` instead of `List` for type annotation 13 13 | 14 14 | -UP006.py:18:10: UP006 [*] Use `list` instead of `t.List` for type annotation +UP006_0.py:18:10: UP006 [*] Use `list` instead of `t.List` for type annotation | 18 | def f(x: t.List[str]) -> None: | ^^^^^^ UP006 @@ -55,7 +55,7 @@ UP006.py:18:10: UP006 [*] Use `list` instead of `t.List` for type annotation 20 20 | 21 21 | -UP006.py:25:10: UP006 [*] Use `list` instead of `IList` for type annotation +UP006_0.py:25:10: UP006 [*] Use `list` instead of `IList` for type annotation | 25 | def f(x: IList[str]) -> None: | ^^^^^ UP006 @@ -73,7 +73,7 @@ UP006.py:25:10: UP006 [*] Use `list` instead of `IList` for type annotation 27 27 | 28 28 | -UP006.py:29:11: UP006 [*] Use `list` instead of `List` for type annotation +UP006_0.py:29:11: UP006 [*] Use `list` instead of `List` for type annotation | 29 | def f(x: "List[str]") -> None: | ^^^^ UP006 @@ -91,7 +91,7 @@ UP006.py:29:11: UP006 [*] Use `list` instead of `List` for type annotation 31 31 | 32 32 | -UP006.py:33:12: UP006 [*] Use `list` instead of `List` for type annotation +UP006_0.py:33:12: UP006 [*] Use `list` instead of `List` for type annotation | 33 | def f(x: r"List[str]") -> None: | ^^^^ UP006 @@ -109,7 +109,7 @@ UP006.py:33:12: UP006 [*] Use `list` instead of `List` for type annotation 35 35 | 36 36 | -UP006.py:37:11: UP006 [*] Use `list` instead of `List` for type annotation +UP006_0.py:37:11: UP006 [*] Use `list` instead of `List` for type annotation | 37 | def f(x: "List[str]") -> None: | ^^^^ UP006 @@ -127,7 +127,7 @@ UP006.py:37:11: UP006 [*] Use `list` instead of `List` for type annotation 39 39 | 40 40 | -UP006.py:41:13: UP006 [*] Use `list` instead of `List` for type annotation +UP006_0.py:41:13: UP006 [*] Use `list` instead of `List` for type annotation | 41 | def f(x: """List[str]""") -> None: | ^^^^ UP006 @@ -145,7 +145,7 @@ UP006.py:41:13: UP006 [*] Use `list` instead of `List` for type annotation 43 43 | 44 44 | -UP006.py:45:10: UP006 Use `list` instead of `List` for type annotation +UP006_0.py:45:10: UP006 Use `list` instead of `List` for type annotation | 45 | def f(x: "Li" "st[str]") -> None: | ^^^^^^^^^^^^^^ UP006 @@ -153,7 +153,7 @@ UP006.py:45:10: UP006 Use `list` instead of `List` for type annotation | = help: Replace with `list` -UP006.py:49:11: UP006 [*] Use `list` instead of `List` for type annotation +UP006_0.py:49:11: UP006 [*] Use `list` instead of `List` for type annotation | 49 | def f(x: "List['List[str]']") -> None: | ^^^^ UP006 @@ -171,7 +171,7 @@ UP006.py:49:11: UP006 [*] Use `list` instead of `List` for type annotation 51 51 | 52 52 | -UP006.py:49:17: UP006 [*] Use `list` instead of `List` for type annotation +UP006_0.py:49:17: UP006 [*] Use `list` instead of `List` for type annotation | 49 | def f(x: "List['List[str]']") -> None: | ^^^^ UP006 @@ -189,7 +189,7 @@ UP006.py:49:17: UP006 [*] Use `list` instead of `List` for type annotation 51 51 | 52 52 | -UP006.py:53:11: UP006 [*] Use `list` instead of `List` for type annotation +UP006_0.py:53:11: UP006 [*] Use `list` instead of `List` for type annotation | 53 | def f(x: "List['Li' 'st[str]']") -> None: | ^^^^ UP006 @@ -207,7 +207,7 @@ UP006.py:53:11: UP006 [*] Use `list` instead of `List` for type annotation 55 55 | 56 56 | -UP006.py:53:16: UP006 Use `list` instead of `List` for type annotation +UP006_0.py:53:16: UP006 Use `list` instead of `List` for type annotation | 53 | def f(x: "List['Li' 'st[str]']") -> None: | ^^^^^^^^^^^^^^ UP006 @@ -215,7 +215,7 @@ UP006.py:53:16: UP006 Use `list` instead of `List` for type annotation | = help: Replace with `list` -UP006.py:57:10: UP006 Use `list` instead of `List` for type annotation +UP006_0.py:57:10: UP006 Use `list` instead of `List` for type annotation | 57 | def f(x: "Li" "st['List[str]']") -> None: | ^^^^^^^^^^^^^^^^^^^^^^ UP006 @@ -223,7 +223,7 @@ UP006.py:57:10: UP006 Use `list` instead of `List` for type annotation | = help: Replace with `list` -UP006.py:57:10: UP006 Use `list` instead of `List` for type annotation +UP006_0.py:57:10: UP006 Use `list` instead of `List` for type annotation | 57 | def f(x: "Li" "st['List[str]']") -> None: | ^^^^^^^^^^^^^^^^^^^^^^ UP006 @@ -231,7 +231,7 @@ UP006.py:57:10: UP006 Use `list` instead of `List` for type annotation | = help: Replace with `list` -UP006.py:61:10: UP006 [*] Use `collections.deque` instead of `typing.Deque` for type annotation +UP006_0.py:61:10: UP006 [*] Use `collections.deque` instead of `typing.Deque` for type annotation | 61 | def f(x: typing.Deque[str]) -> None: | ^^^^^^^^^^^^ UP006 @@ -257,7 +257,7 @@ UP006.py:61:10: UP006 [*] Use `collections.deque` instead of `typing.Deque` for 63 64 | 64 65 | -UP006.py:65:10: UP006 [*] Use `collections.defaultdict` instead of `typing.DefaultDict` for type annotation +UP006_0.py:65:10: UP006 [*] Use `collections.defaultdict` instead of `typing.DefaultDict` for type annotation | 65 | def f(x: typing.DefaultDict[str, str]) -> None: | ^^^^^^^^^^^^^^^^^^ UP006 diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006_1.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006_1.py.snap new file mode 100644 index 0000000000000..d0b32a6494443 --- /dev/null +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006_1.py.snap @@ -0,0 +1,20 @@ +--- +source: crates/ruff/src/rules/pyupgrade/mod.rs +--- +UP006_1.py:9:10: UP006 [*] Use `collections.defaultdict` instead of `typing.DefaultDict` for type annotation + | + 9 | def f(x: typing.DefaultDict[str, str]) -> None: + | ^^^^^^^^^^^^^^^^^^ UP006 +10 | ... + | + = help: Replace with `collections.defaultdict` + +ℹ Suggested fix +6 6 | from collections import defaultdict +7 7 | +8 8 | +9 |-def f(x: typing.DefaultDict[str, str]) -> None: + 9 |+def f(x: defaultdict[str, str]) -> None: +10 10 | ... + + diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006_2.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006_2.py.snap new file mode 100644 index 0000000000000..58c06666cb75e --- /dev/null +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006_2.py.snap @@ -0,0 +1,12 @@ +--- +source: crates/ruff/src/rules/pyupgrade/mod.rs +--- +UP006_2.py:7:10: UP006 Use `collections.defaultdict` instead of `typing.DefaultDict` for type annotation + | +7 | def f(x: typing.DefaultDict[str, str]) -> None: + | ^^^^^^^^^^^^^^^^^^ UP006 +8 | ... + | + = help: Replace with `collections.defaultdict` + + diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006_3.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006_3.py.snap new file mode 100644 index 0000000000000..3a724af5edbf1 --- /dev/null +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP006_3.py.snap @@ -0,0 +1,20 @@ +--- +source: crates/ruff/src/rules/pyupgrade/mod.rs +--- +UP006_3.py:7:11: UP006 [*] Use `collections.defaultdict` instead of `typing.DefaultDict` for type annotation + | +7 | def f(x: "typing.DefaultDict[str, str]") -> None: + | ^^^^^^^^^^^^^^^^^^ UP006 +8 | ... + | + = help: Replace with `collections.defaultdict` + +ℹ Suggested fix +4 4 | from collections import defaultdict +5 5 | +6 6 | +7 |-def f(x: "typing.DefaultDict[str, str]") -> None: + 7 |+def f(x: "defaultdict[str, str]") -> None: +8 8 | ... + + diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index b7eb31ece9d4e..d4f5f5878dce3 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -4,7 +4,7 @@ use std::path::Path; use bitflags::bitflags; use nohash_hasher::{BuildNoHashHasher, IntMap}; use ruff_text_size::TextRange; -use rustpython_parser::ast::{Expr, Stmt}; +use rustpython_parser::ast::{Expr, Ranged, Stmt}; use smallvec::smallvec; use ruff_python_ast::call_path::{collect_call_path, from_unqualified_name, CallPath}; @@ -350,7 +350,7 @@ impl<'a> SemanticModel<'a> { &self, module: &str, member: &str, - ) -> Option<(&Stmt, String)> { + ) -> Option { self.scopes().enumerate().find_map(|(scope_index, scope)| { scope.binding_ids().find_map(|binding_id| { let binding = &self.bindings[binding_id]; @@ -360,14 +360,18 @@ impl<'a> SemanticModel<'a> { // `import sys as sys2` -> `sys2.exit` BindingKind::Importation(Importation { name, full_name }) => { if full_name == &module { - // Verify that `sys` isn't bound in an inner scope. - if self - .scopes() - .take(scope_index) - .all(|scope| scope.get(name).is_none()) - { - if let Some(source) = binding.source { - return Some((self.stmts[source], format!("{name}.{member}"))); + if let Some(source) = binding.source { + // Verify that `sys` isn't bound in an inner scope. + if self + .scopes() + .take(scope_index) + .all(|scope| scope.get(name).is_none()) + { + return Some(ImportedName { + name: format!("{name}.{member}"), + range: self.stmts[source].range(), + context: binding.context, + }); } } } @@ -378,14 +382,18 @@ impl<'a> SemanticModel<'a> { BindingKind::FromImportation(FromImportation { name, full_name }) => { if let Some((target_module, target_member)) = full_name.split_once('.') { if target_module == module && target_member == member { - // Verify that `join` isn't bound in an inner scope. - if self - .scopes() - .take(scope_index) - .all(|scope| scope.get(name).is_none()) - { - if let Some(source) = binding.source { - return Some((self.stmts[source], (*name).to_string())); + if let Some(source) = binding.source { + // Verify that `join` isn't bound in an inner scope. + if self + .scopes() + .take(scope_index) + .all(|scope| scope.get(name).is_none()) + { + return Some(ImportedName { + name: (*name).to_string(), + range: self.stmts[source].range(), + context: binding.context, + }); } } } @@ -395,14 +403,18 @@ impl<'a> SemanticModel<'a> { // `import os.path ` -> `os.name` BindingKind::SubmoduleImportation(SubmoduleImportation { name, .. }) => { if name == &module { - // Verify that `os` isn't bound in an inner scope. - if self - .scopes() - .take(scope_index) - .all(|scope| scope.get(name).is_none()) - { - if let Some(source) = binding.source { - return Some((self.stmts[source], format!("{name}.{member}"))); + if let Some(source) = binding.source { + // Verify that `os` isn't bound in an inner scope. + if self + .scopes() + .take(scope_index) + .all(|scope| scope.get(name).is_none()) + { + return Some(ImportedName { + name: format!("{name}.{member}"), + range: self.stmts[source].range(), + context: binding.context, + }); } } } @@ -898,3 +910,29 @@ pub enum ResolvedReference { /// The reference is definitively unresolved. NotFound, } + +#[derive(Debug)] +pub struct ImportedName { + /// The name to which the imported symbol is bound. + name: String, + /// The range at which the symbol is imported. + range: TextRange, + /// The context in which the symbol is imported. + context: ExecutionContext, +} + +impl ImportedName { + pub fn into_name(self) -> String { + self.name + } + + pub const fn context(&self) -> ExecutionContext { + self.context + } +} + +impl Ranged for ImportedName { + fn range(&self) -> TextRange { + self.range + } +}