-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[
flake8_builtins] implement builtin-[import,lambda-argument,module]…
…-shadowing
- Loading branch information
Showing
38 changed files
with
582 additions
and
15 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5 changes: 5 additions & 0 deletions
5
crates/ruff_linter/resources/test/fixtures/flake8_builtins/A004.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| import some as sum | ||
| import float | ||
| from some import other as int | ||
| from some import input, exec | ||
| from directory import new as dir |
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
5 changes: 5 additions & 0 deletions
5
crates/ruff_linter/resources/test/fixtures/flake8_builtins/A006.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| lambda print, copyright: print | ||
| lambda x, float, y: x + y | ||
| lambda min, max: min | ||
| lambda id: id | ||
| lambda dir: dir |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
49 changes: 49 additions & 0 deletions
49
crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_import_shadowing.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| use crate::checkers::ast::Checker; | ||
| use crate::rules::flake8_builtins::helpers::shadows_builtin; | ||
| use ruff_diagnostics::{Diagnostic, Violation}; | ||
| use ruff_macros::{derive_message_formats, violation}; | ||
| use ruff_python_ast::Alias; | ||
|
|
||
| /// ## What it does | ||
| /// Checks for any import statement that use the same name as a builtin. | ||
| /// | ||
| /// ## Why is this bad? | ||
| /// Reusing a builtin name for the name of an import increases the | ||
| /// difficulty of reading and maintaining the code, and can cause | ||
| /// non-obvious errors, as readers may mistake the variable for the | ||
| /// builtin and vice versa. | ||
| /// | ||
| /// Builtins can be marked as exceptions to this rule via the | ||
| /// [`lint.flake8-builtins.builtins-ignorelist`] configuration option. | ||
| /// | ||
| /// ## Options | ||
| /// - `lint.flake8-builtins.builtins-ignorelist` | ||
| #[violation] | ||
| pub struct BuiltinImportShadowing { | ||
| name: String, | ||
| } | ||
|
|
||
| impl Violation for BuiltinImportShadowing { | ||
| #[derive_message_formats] | ||
| fn message(&self) -> String { | ||
| let BuiltinImportShadowing { name } = self; | ||
| format!("Import `{name}` is shadowing Python builtin") | ||
| } | ||
| } | ||
|
|
||
| /// A004 | ||
| pub(crate) fn builtin_import_shadowing(checker: &mut Checker, alias: &Alias) { | ||
| let name = alias.asname.as_ref().unwrap_or(&alias.name); | ||
| if shadows_builtin( | ||
| name.as_str(), | ||
| &checker.settings.flake8_builtins.builtins_ignorelist, | ||
| checker.source_type, | ||
| ) { | ||
| checker.diagnostics.push(Diagnostic::new( | ||
| BuiltinImportShadowing { | ||
| name: name.to_string(), | ||
| }, | ||
| name.range, | ||
| )); | ||
| } | ||
| } |
55 changes: 55 additions & 0 deletions
55
crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_lambda_argument_shadowing.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| use ruff_diagnostics::{Diagnostic, Violation}; | ||
| use ruff_macros::{derive_message_formats, violation}; | ||
| use ruff_python_ast::ExprLambda; | ||
|
|
||
| use crate::checkers::ast::Checker; | ||
| use crate::rules::flake8_builtins::helpers::shadows_builtin; | ||
|
|
||
| /// ## What it does | ||
| /// Checks for any lambda argument that use the same name as a builtin. | ||
| /// | ||
| /// ## Why is this bad? | ||
| /// Reusing a builtin name for the name of a lambda argument increases the | ||
| /// difficulty of reading and maintaining the code, and can cause | ||
| /// non-obvious errors, as readers may mistake the variable for the | ||
| /// builtin and vice versa. | ||
| /// | ||
| /// Builtins can be marked as exceptions to this rule via the | ||
| /// [`lint.flake8-builtins.builtins-ignorelist`] configuration option. | ||
| /// | ||
| /// ## Options | ||
| /// - `lint.flake8-builtins.builtins-ignorelist` | ||
| #[violation] | ||
| pub struct BuiltinLambdaArgumentShadowing { | ||
| name: String, | ||
| } | ||
|
|
||
| impl Violation for BuiltinLambdaArgumentShadowing { | ||
| #[derive_message_formats] | ||
| fn message(&self) -> String { | ||
| let BuiltinLambdaArgumentShadowing { name } = self; | ||
| format!("Lambda argument `{name}` is shadowing Python builtin") | ||
| } | ||
| } | ||
|
|
||
| /// A006 | ||
| pub(crate) fn builtin_lambda_argument_shadowing(checker: &mut Checker, lambda: &ExprLambda) { | ||
| let Some(parameters) = lambda.parameters.as_ref() else { | ||
| return; | ||
| }; | ||
| for param in parameters.iter_non_variadic_params() { | ||
| let name = ¶m.parameter.name; | ||
| if shadows_builtin( | ||
| name.as_ref(), | ||
| &checker.settings.flake8_builtins.builtins_ignorelist, | ||
| checker.source_type, | ||
| ) { | ||
| checker.diagnostics.push(Diagnostic::new( | ||
| BuiltinLambdaArgumentShadowing { | ||
| name: name.to_string(), | ||
| }, | ||
| name.range, | ||
| )); | ||
| } | ||
| } | ||
| } |
75 changes: 75 additions & 0 deletions
75
crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_module_shadowing.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| use std::path::Path; | ||
|
|
||
| use ruff_diagnostics::{Diagnostic, Violation}; | ||
| use ruff_macros::{derive_message_formats, violation}; | ||
| use ruff_python_stdlib::path::is_module_file; | ||
| use ruff_python_stdlib::sys::is_known_standard_library; | ||
| use ruff_text_size::TextRange; | ||
|
|
||
| use crate::settings::types::PythonVersion; | ||
|
|
||
| /// ## What it does | ||
| /// Checks for any module that use the same name as a builtin module. | ||
| /// | ||
| /// ## Why is this bad? | ||
| /// Reusing a builtin module name for the name of a module increases the | ||
| /// difficulty of reading and maintaining the code, and can cause | ||
| /// non-obvious errors, as readers may mistake the variable for the | ||
| /// builtin and vice versa. | ||
| /// | ||
| /// Builtin modules can be marked as exceptions to this rule via the | ||
| /// [`lint.flake8-builtins.builtins-allowed-modules`] configuration option. | ||
| /// | ||
| /// ## Options | ||
| /// - `lint.flake8-builtins.builtins-allowed-modules` | ||
| #[violation] | ||
| pub struct BuiltinModuleShadowing { | ||
| name: String, | ||
| } | ||
|
|
||
| impl Violation for BuiltinModuleShadowing { | ||
| #[derive_message_formats] | ||
| fn message(&self) -> String { | ||
| let BuiltinModuleShadowing { name } = self; | ||
| format!("Module `{name}` is shadowing Python builtin module") | ||
| } | ||
| } | ||
|
|
||
| /// A005 | ||
| pub(crate) fn builtin_module_shadowing( | ||
| path: &Path, | ||
| package: Option<&Path>, | ||
| allowed_modules: &[String], | ||
| target_version: PythonVersion, | ||
| ) -> Option<Diagnostic> { | ||
| if !path | ||
| .extension() | ||
| .is_some_and(|ext| ext == "py" || ext == "pyi") | ||
| { | ||
| return None; | ||
| } | ||
|
|
||
| if let Some(package) = package { | ||
| let module_name = if is_module_file(path) { | ||
| package.file_name() | ||
| } else { | ||
| path.file_stem() | ||
| } | ||
| .unwrap() | ||
| .to_string_lossy(); | ||
|
|
||
| if is_known_standard_library(target_version.minor(), &module_name) | ||
| && allowed_modules | ||
| .iter() | ||
| .all(|allowed_module| allowed_module != &module_name) | ||
| { | ||
| return Some(Diagnostic::new( | ||
| BuiltinModuleShadowing { | ||
| name: module_name.to_string(), | ||
| }, | ||
| TextRange::default(), | ||
| )); | ||
| } | ||
| } | ||
| None | ||
| } |
Oops, something went wrong.