diff --git a/resources/test/fixtures/A001.py b/resources/test/fixtures/A001.py new file mode 100644 index 0000000000..a1a16f954b --- /dev/null +++ b/resources/test/fixtures/A001.py @@ -0,0 +1,27 @@ +import some as sum +from some import other as int + +print = 1 +copyright: 'annotation' = 2 +(complex := 3) +float = object = 4 +min, max = 5, 6 + +def bytes(): + pass + +class slice: + pass + +try: + ... +except ImportError as ValueError: + ... + +for memoryview, *bytearray in []: + pass + +with open('file') as str, open('file2') as (all, any): + pass + +[0 for sum in ()] diff --git a/resources/test/fixtures/A002.py b/resources/test/fixtures/A002.py new file mode 100644 index 0000000000..66b2aaac9a --- /dev/null +++ b/resources/test/fixtures/A002.py @@ -0,0 +1,9 @@ +def func1(str, /, type, *complex, Exception, **getattr): + pass + + +async def func2(bytes): + pass + + +map([], lambda float: ...) diff --git a/resources/test/fixtures/A003.py b/resources/test/fixtures/A003.py new file mode 100644 index 0000000000..cdf9030bcc --- /dev/null +++ b/resources/test/fixtures/A003.py @@ -0,0 +1,8 @@ +class MyClass: + ImportError = 4 + + def __init__(self): + self.float = 5 # is fine + + def str(self): + pass diff --git a/src/ast/checks.rs b/src/ast/checks.rs index 19baa9d028..73dde5eca6 100644 --- a/src/ast/checks.rs +++ b/src/ast/checks.rs @@ -11,6 +11,7 @@ use crate::ast::operations::SourceCodeLocator; use crate::ast::types::{Binding, BindingKind, CheckLocator, FunctionScope, Scope, ScopeKind}; use crate::autofix::{fixer, fixes}; use crate::checks::{Check, CheckKind, Fix, RejectedCmpop}; +use crate::python::builtins::BUILTINS; /// Check IfTuple compliance. pub fn check_if_tuple(test: &Expr, location: Location) -> Option { @@ -652,3 +653,30 @@ pub fn check_continue_outside_loop( None } } + +// flake8-builtins +pub enum ShadowingType { + Variable, + Argument, + Attribute, +} + +/// Check builtin name shadowing +pub fn check_builtin_shadowing( + name: &str, + location: Location, + node_type: ShadowingType, +) -> Option { + if BUILTINS.contains(&name) { + Some(Check::new( + match node_type { + ShadowingType::Variable => CheckKind::BuiltinVariableShadowing(name.to_string()), + ShadowingType::Argument => CheckKind::BuiltinArgumentShadowing(name.to_string()), + ShadowingType::Attribute => CheckKind::BuiltinAttributeShadowing(name.to_string()), + }, + location, + )) + } else { + None + } +} diff --git a/src/check_ast.rs b/src/check_ast.rs index 307c6490ef..451195e64a 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -246,6 +246,9 @@ where self.checks.push(check); } } + + self.check_builtin_shadowing(name, stmt.location, true); + for expr in decorator_list { self.visit_expr(expr); } @@ -342,6 +345,8 @@ where } } + self.check_builtin_shadowing(name, self.locate_check(stmt.location), false); + for expr in bases { self.visit_expr(expr) } @@ -382,6 +387,10 @@ where }, ) } else { + if let Some(asname) = &alias.node.asname { + self.check_builtin_shadowing(asname, stmt.location, false); + } + self.add_binding( alias .node @@ -504,6 +513,10 @@ where .expect("No current scope found."))]; scope.import_starred = true; } else { + if let Some(asname) = &alias.node.asname { + self.check_builtin_shadowing(asname, stmt.location, false); + } + let binding = Binding { kind: BindingKind::Importation(match module { None => name.clone(), @@ -667,6 +680,9 @@ where self.checks.push(check); } } + + self.check_builtin_shadowing(id, expr.location, true); + let parent = self.parents[*(self.parent_stack.last().expect("No parent found."))]; self.handle_node_store(expr, parent); @@ -977,6 +993,9 @@ where self.checks.push(check); } } + + self.check_builtin_shadowing(name, excepthandler.location, false); + let scope = &self.scopes [*(self.scope_stack.last().expect("No current scope found."))]; if scope.values.contains_key(name) { @@ -1080,6 +1099,8 @@ where self.checks.push(check); } } + + self.check_builtin_arg_shadowing(&arg.node.arg, arg.location); } } @@ -1505,6 +1526,44 @@ impl<'a> Checker<'a> { } } } + + fn check_builtin_shadowing(&mut self, name: &str, location: Location, is_attribute: bool) { + let scope = &self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; + + // flake8-builtins + if is_attribute + && matches!(scope.kind, ScopeKind::Class) + && self.settings.select.contains(&CheckCode::A003) + { + if let Some(check) = checks::check_builtin_shadowing( + name, + self.locate_check(location), + checks::ShadowingType::Attribute, + ) { + self.checks.push(check); + } + } else if self.settings.select.contains(&CheckCode::A001) { + if let Some(check) = checks::check_builtin_shadowing( + name, + self.locate_check(location), + checks::ShadowingType::Variable, + ) { + self.checks.push(check); + } + } + } + + fn check_builtin_arg_shadowing(&mut self, name: &str, location: Location) { + if self.settings.select.contains(&CheckCode::A002) { + if let Some(check) = checks::check_builtin_shadowing( + name, + self.locate_check(location), + checks::ShadowingType::Argument, + ) { + self.checks.push(check); + } + } + } } pub fn check_ast( diff --git a/src/checks.rs b/src/checks.rs index aee8fa5fcb..a13000c182 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -4,7 +4,7 @@ use anyhow::Result; use rustpython_parser::ast::Location; use serde::{Deserialize, Serialize}; -pub const DEFAULT_CHECK_CODES: [CheckCode; 42] = [ +pub const DEFAULT_CHECK_CODES: [CheckCode; 45] = [ CheckCode::E402, CheckCode::E501, CheckCode::E711, @@ -47,9 +47,13 @@ pub const DEFAULT_CHECK_CODES: [CheckCode; 42] = [ CheckCode::F831, CheckCode::F841, CheckCode::F901, + // flake8-builtins + CheckCode::A001, + CheckCode::A002, + CheckCode::A003, ]; -pub const ALL_CHECK_CODES: [CheckCode; 45] = [ +pub const ALL_CHECK_CODES: [CheckCode; 48] = [ CheckCode::E402, CheckCode::E501, CheckCode::E711, @@ -95,6 +99,10 @@ pub const ALL_CHECK_CODES: [CheckCode; 45] = [ CheckCode::M001, CheckCode::R001, CheckCode::R002, + // flake8-builtins + CheckCode::A001, + CheckCode::A002, + CheckCode::A003, ]; #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize, Hash, PartialOrd, Ord)] @@ -144,6 +152,10 @@ pub enum CheckCode { R001, R002, M001, + // flake8-builtins + A001, + A002, + A003, } impl FromStr for CheckCode { @@ -196,6 +208,10 @@ impl FromStr for CheckCode { "R001" => Ok(CheckCode::R001), "R002" => Ok(CheckCode::R002), "M001" => Ok(CheckCode::M001), + // flake8-builtins + "A001" => Ok(CheckCode::A001), + "A002" => Ok(CheckCode::A002), + "A003" => Ok(CheckCode::A003), _ => Err(anyhow::anyhow!("Unknown check code: {s}")), } } @@ -249,6 +265,10 @@ impl CheckCode { CheckCode::R001 => "R001", CheckCode::R002 => "R002", CheckCode::M001 => "M001", + // flake8-builtins + CheckCode::A001 => "A001", + CheckCode::A002 => "A002", + CheckCode::A003 => "A003", } } @@ -309,6 +329,10 @@ impl CheckCode { CheckCode::M001 => CheckKind::UnusedNOQA(None), CheckCode::R001 => CheckKind::UselessObjectInheritance("...".to_string()), CheckCode::R002 => CheckKind::NoAssertEquals, + // flake8-builtins + CheckCode::A001 => CheckKind::BuiltinVariableShadowing("...".to_string()), + CheckCode::A002 => CheckKind::BuiltinArgumentShadowing("...".to_string()), + CheckCode::A003 => CheckKind::BuiltinAttributeShadowing("...".to_string()), } } } @@ -373,6 +397,10 @@ pub enum CheckKind { UnusedVariable(String), UselessObjectInheritance(String), YieldOutsideFunction, + // flake8-builtin + BuiltinVariableShadowing(String), + BuiltinArgumentShadowing(String), + BuiltinAttributeShadowing(String), } impl CheckKind { @@ -426,6 +454,10 @@ impl CheckKind { CheckKind::UselessObjectInheritance(_) => "UselessObjectInheritance", CheckKind::YieldOutsideFunction => "YieldOutsideFunction", CheckKind::UnusedNOQA(_) => "UnusedNOQA", + // flake8-builtins + CheckKind::BuiltinVariableShadowing(_) => "BuiltinVariableShadowing", + CheckKind::BuiltinArgumentShadowing(_) => "BuiltinArgumentShadowing", + CheckKind::BuiltinAttributeShadowing(_) => "BuiltinAttributeShadowing", } } @@ -477,6 +509,10 @@ impl CheckKind { CheckKind::UnusedVariable(_) => &CheckCode::F841, CheckKind::UselessObjectInheritance(_) => &CheckCode::R001, CheckKind::YieldOutsideFunction => &CheckCode::F704, + // flake8-builtins + CheckKind::BuiltinVariableShadowing(_) => &CheckCode::A001, + CheckKind::BuiltinArgumentShadowing(_) => &CheckCode::A002, + CheckKind::BuiltinAttributeShadowing(_) => &CheckCode::A003, } } @@ -611,6 +647,16 @@ impl CheckKind { None => "Unused `noqa` directive".to_string(), Some(code) => format!("Unused `noqa` directive for: {code}"), }, + // flake8-builtins + CheckKind::BuiltinVariableShadowing(name) => { + format!("Variable `{name}` is shadowing a python builtin") + } + CheckKind::BuiltinArgumentShadowing(name) => { + format!("Argument `{name}` is shadowing a python builtin") + } + CheckKind::BuiltinAttributeShadowing(name) => { + format!("class attribute `{name}` is shadowing a python builtin") + } } } diff --git a/src/linter.rs b/src/linter.rs index 3c184e5a5b..c48bdfd881 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -726,4 +726,40 @@ mod tests { insta::assert_yaml_snapshot!(checks); Ok(()) } + + #[test] + fn a001() -> Result<()> { + let mut checks = check_path( + Path::new("./resources/test/fixtures/A001.py"), + &settings::Settings::for_rule(CheckCode::A001), + &fixer::Mode::Generate, + )?; + checks.sort_by_key(|check| check.location); + insta::assert_yaml_snapshot!(checks); + Ok(()) + } + + #[test] + fn a002() -> Result<()> { + let mut checks = check_path( + Path::new("./resources/test/fixtures/A002.py"), + &settings::Settings::for_rule(CheckCode::A002), + &fixer::Mode::Generate, + )?; + checks.sort_by_key(|check| check.location); + insta::assert_yaml_snapshot!(checks); + Ok(()) + } + + #[test] + fn a003() -> Result<()> { + let mut checks = check_path( + Path::new("./resources/test/fixtures/A003.py"), + &settings::Settings::for_rule(CheckCode::A003), + &fixer::Mode::Generate, + )?; + checks.sort_by_key(|check| check.location); + insta::assert_yaml_snapshot!(checks); + Ok(()) + } } diff --git a/src/snapshots/ruff__linter__tests__a001.snap b/src/snapshots/ruff__linter__tests__a001.snap new file mode 100644 index 0000000000..c79ad8e8bf --- /dev/null +++ b/src/snapshots/ruff__linter__tests__a001.snap @@ -0,0 +1,112 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: + BuiltinVariableShadowing: sum + location: + row: 1 + column: 1 + fix: ~ +- kind: + BuiltinVariableShadowing: int + location: + row: 2 + column: 1 + fix: ~ +- kind: + BuiltinVariableShadowing: print + location: + row: 4 + column: 1 + fix: ~ +- kind: + BuiltinVariableShadowing: copyright + location: + row: 5 + column: 1 + fix: ~ +- kind: + BuiltinVariableShadowing: complex + location: + row: 6 + column: 2 + fix: ~ +- kind: + BuiltinVariableShadowing: float + location: + row: 7 + column: 1 + fix: ~ +- kind: + BuiltinVariableShadowing: object + location: + row: 7 + column: 9 + fix: ~ +- kind: + BuiltinVariableShadowing: min + location: + row: 8 + column: 1 + fix: ~ +- kind: + BuiltinVariableShadowing: max + location: + row: 8 + column: 6 + fix: ~ +- kind: + BuiltinVariableShadowing: bytes + location: + row: 10 + column: 1 + fix: ~ +- kind: + BuiltinVariableShadowing: slice + location: + row: 13 + column: 1 + fix: ~ +- kind: + BuiltinVariableShadowing: ValueError + location: + row: 18 + column: 1 + fix: ~ +- kind: + BuiltinVariableShadowing: memoryview + location: + row: 21 + column: 5 + fix: ~ +- kind: + BuiltinVariableShadowing: bytearray + location: + row: 21 + column: 18 + fix: ~ +- kind: + BuiltinVariableShadowing: str + location: + row: 24 + column: 22 + fix: ~ +- kind: + BuiltinVariableShadowing: all + location: + row: 24 + column: 45 + fix: ~ +- kind: + BuiltinVariableShadowing: any + location: + row: 24 + column: 50 + fix: ~ +- kind: + BuiltinVariableShadowing: sum + location: + row: 27 + column: 8 + fix: ~ diff --git a/src/snapshots/ruff__linter__tests__a002.snap b/src/snapshots/ruff__linter__tests__a002.snap new file mode 100644 index 0000000000..c42e9dfd8d --- /dev/null +++ b/src/snapshots/ruff__linter__tests__a002.snap @@ -0,0 +1,46 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: + BuiltinArgumentShadowing: str + location: + row: 1 + column: 11 + fix: ~ +- kind: + BuiltinArgumentShadowing: type + location: + row: 1 + column: 19 + fix: ~ +- kind: + BuiltinArgumentShadowing: complex + location: + row: 1 + column: 26 + fix: ~ +- kind: + BuiltinArgumentShadowing: Exception + location: + row: 1 + column: 35 + fix: ~ +- kind: + BuiltinArgumentShadowing: getattr + location: + row: 1 + column: 48 + fix: ~ +- kind: + BuiltinArgumentShadowing: bytes + location: + row: 5 + column: 17 + fix: ~ +- kind: + BuiltinArgumentShadowing: float + location: + row: 9 + column: 16 + fix: ~ diff --git a/src/snapshots/ruff__linter__tests__a003.snap b/src/snapshots/ruff__linter__tests__a003.snap new file mode 100644 index 0000000000..46de90dacb --- /dev/null +++ b/src/snapshots/ruff__linter__tests__a003.snap @@ -0,0 +1,18 @@ +--- +source: src/linter.rs +assertion_line: 762 +expression: checks +--- +- kind: + BuiltinAttributeShadowing: ImportError + location: + row: 2 + column: 5 + fix: ~ +- kind: + BuiltinAttributeShadowing: str + location: + row: 7 + column: 5 + fix: ~ +