From 4f2bf1bdaade105183aba73da7e4c50bc1d8ea49 Mon Sep 17 00:00:00 2001 From: Steve C Date: Wed, 18 Oct 2023 01:54:25 -0400 Subject: [PATCH 1/2] implement C2401 --- .../test/fixtures/pylint/non_ascii_name.py | 9 ++++ .../src/checkers/ast/analyze/statement.rs | 11 +++++ crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/pylint/mod.rs | 1 + .../ruff_linter/src/rules/pylint/rules/mod.rs | 2 + .../src/rules/pylint/rules/non_ascii_name.rs | 41 +++++++++++++++++++ ...int__tests__PLC2401_non_ascii_name.py.snap | 30 ++++++++++++++ ruff.schema.json | 4 ++ 8 files changed, 99 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/pylint/non_ascii_name.py create mode 100644 crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs create mode 100644 crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2401_non_ascii_name.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/non_ascii_name.py b/crates/ruff_linter/resources/test/fixtures/pylint/non_ascii_name.py new file mode 100644 index 0000000000000..6953587f0cca0 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/non_ascii_name.py @@ -0,0 +1,9 @@ +ápple_count: int = 1 # C2401 +ápple_count += 2 # C2401 +ápple_count = 3 # C2401 + +# this rule only works on assignment! +ápple_count == 3 # Ok + +# normal ascii +apple_count = 4 # Ok diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 3729f64ab1687..469fb0c45fa84 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1012,6 +1012,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { } } Stmt::AugAssign(ast::StmtAugAssign { target, .. }) => { + if checker.enabled(Rule::NonAsciiName) { + pylint::rules::non_ascii_name(checker, target); + } if checker.enabled(Rule::GlobalStatement) { if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() { pylint::rules::global_statement(checker, id); @@ -1314,6 +1317,11 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { } } Stmt::Assign(assign @ ast::StmtAssign { targets, value, .. }) => { + if checker.enabled(Rule::NonAsciiName) { + for target in targets { + pylint::rules::non_ascii_name(checker, target); + } + } if checker.enabled(Rule::LambdaAssignment) { if let [target] = &targets[..] { pycodestyle::rules::lambda_assignment(checker, target, value, None, stmt); @@ -1427,6 +1435,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { .. }, ) => { + if checker.enabled(Rule::NonAsciiName) { + pylint::rules::non_ascii_name(checker, target); + } if let Some(value) = value { if checker.enabled(Rule::LambdaAssignment) { pycodestyle::rules::lambda_assignment( diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index e5360cabfae85..9a19565df43c7 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -211,6 +211,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "C0205") => (RuleGroup::Stable, rules::pylint::rules::SingleStringSlots), (Pylint, "C0208") => (RuleGroup::Stable, rules::pylint::rules::IterationOverSet), (Pylint, "C0414") => (RuleGroup::Stable, rules::pylint::rules::UselessImportAlias), + (Pylint, "C2401") => (RuleGroup::Preview, rules::pylint::rules::NonAsciiName), #[allow(deprecated)] (Pylint, "C1901") => (RuleGroup::Nursery, rules::pylint::rules::CompareToEmptyString), (Pylint, "C3002") => (RuleGroup::Stable, rules::pylint::rules::UnnecessaryDirectLambdaCall), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index d1807b2695d11..13ce433f44ec5 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -139,6 +139,7 @@ mod tests { #[test_case(Rule::MisplacedBareRaise, Path::new("misplaced_bare_raise.py"))] #[test_case(Rule::LiteralMembership, Path::new("literal_membership.py"))] #[test_case(Rule::GlobalAtModuleLevel, Path::new("global_at_module_level.py"))] + #[test_case(Rule::NonAsciiName, Path::new("non_ascii_name.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 30ede8337a5ab..9c92cf2254ba9 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -34,6 +34,7 @@ pub(crate) use misplaced_bare_raise::*; pub(crate) use named_expr_without_context::*; pub(crate) use nested_min_max::*; pub(crate) use no_self_use::*; +pub(crate) use non_ascii_name::*; pub(crate) use nonlocal_without_binding::*; pub(crate) use property_with_parameters::*; pub(crate) use redefined_loop_name::*; @@ -99,6 +100,7 @@ mod misplaced_bare_raise; mod named_expr_without_context; mod nested_min_max; mod no_self_use; +mod non_ascii_name; mod nonlocal_without_binding; mod property_with_parameters; mod redefined_loop_name; diff --git a/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs b/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs new file mode 100644 index 0000000000000..9bcbfd4922a1d --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs @@ -0,0 +1,41 @@ +use ruff_python_ast::{self as ast, Expr}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for the use of non-ASCII characters in symbol names. +/// +/// ## Why is this bad? +/// Pylint discourages the use of non-ASCII characters in symbol names as +/// they can cause confusion and compatibility issues. +/// +/// ## References +/// - [PEP 672](https://peps.python.org/pep-0672/) +#[violation] +pub struct NonAsciiName; + +impl Violation for NonAsciiName { + #[derive_message_formats] + fn message(&self) -> String { + format!("Symbol name contains a non-ASCII character, consider renaming it.") + } +} + +/// PLC2401 +pub(crate) fn non_ascii_name(checker: &mut Checker, target: &Expr) { + let Expr::Name(ast::ExprName { id, .. }) = target else { + return; + }; + + if id.is_ascii() { + return; + } + + checker + .diagnostics + .push(Diagnostic::new(NonAsciiName, target.range())); +} diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2401_non_ascii_name.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2401_non_ascii_name.py.snap new file mode 100644 index 0000000000000..bdc2358e1d7c6 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2401_non_ascii_name.py.snap @@ -0,0 +1,30 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +non_ascii_name.py:1:1: PLC2401 Symbol name contains a non-ASCII character, consider renaming it. + | +1 | ápple_count: int = 1 # C2401 + | ^^^^^^^^^^^ PLC2401 +2 | ápple_count += 2 # C2401 +3 | ápple_count = 3 # C2401 + | + +non_ascii_name.py:2:1: PLC2401 Symbol name contains a non-ASCII character, consider renaming it. + | +1 | ápple_count: int = 1 # C2401 +2 | ápple_count += 2 # C2401 + | ^^^^^^^^^^^ PLC2401 +3 | ápple_count = 3 # C2401 + | + +non_ascii_name.py:3:1: PLC2401 Symbol name contains a non-ASCII character, consider renaming it. + | +1 | ápple_count: int = 1 # C2401 +2 | ápple_count += 2 # C2401 +3 | ápple_count = 3 # C2401 + | ^^^^^^^^^^^ PLC2401 +4 | +5 | # this rule only works on assignment! + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 40429ce33b1ba..c302f9558d338 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2949,6 +2949,10 @@ "PLC19", "PLC190", "PLC1901", + "PLC2", + "PLC24", + "PLC240", + "PLC2401", "PLC3", "PLC30", "PLC300", From f306aa5223b433906cd4a796c26380c9880c5f9b Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 20 Oct 2023 14:41:54 -0400 Subject: [PATCH 2/2] Extend to all bindings --- .../test/fixtures/pylint/non_ascii_name.py | 30 ++++- .../src/checkers/ast/analyze/bindings.rs | 6 + .../src/checkers/ast/analyze/statement.rs | 12 +- .../pylint/rules/non_ascii_module_import.rs | 7 +- .../src/rules/pylint/rules/non_ascii_name.rs | 113 +++++++++++++++--- ...int__tests__PLC2401_non_ascii_name.py.snap | 57 ++++++++- 6 files changed, 183 insertions(+), 42 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/non_ascii_name.py b/crates/ruff_linter/resources/test/fixtures/pylint/non_ascii_name.py index 6953587f0cca0..0487770133434 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/non_ascii_name.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/non_ascii_name.py @@ -2,8 +2,30 @@ ápple_count += 2 # C2401 ápple_count = 3 # C2401 -# this rule only works on assignment! -ápple_count == 3 # Ok +(ápple_count for ápple_count in y) -# normal ascii -apple_count = 4 # Ok + +def func(ápple_count): + global ápple_count + nonlocal ápple_count + + +def ápple_count(): + pass + + +match ápple_count: + case ápple_count: + pass + +ápple_count: int + +try: + 1/0 +except ápple_count: + pass + +# OK +print(ápple_count) +ápple_count == 3 +apple_count = 4 diff --git a/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs b/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs index e7a1f64baed75..0fbc85f5552fa 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs @@ -10,6 +10,7 @@ pub(crate) fn bindings(checker: &mut Checker) { if !checker.any_enabled(&[ Rule::InvalidAllFormat, Rule::InvalidAllObject, + Rule::NonAsciiName, Rule::UnaliasedCollectionsAbcSetImport, Rule::UnconventionalImportAlias, Rule::UnusedVariable, @@ -49,6 +50,11 @@ pub(crate) fn bindings(checker: &mut Checker) { checker.diagnostics.push(diagnostic); } } + if checker.enabled(Rule::NonAsciiName) { + if let Some(diagnostic) = pylint::rules::non_ascii_name(binding, checker.locator) { + checker.diagnostics.push(diagnostic); + } + } if checker.enabled(Rule::UnconventionalImportAlias) { if let Some(diagnostic) = flake8_import_conventions::rules::unconventional_import_alias( checker, diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index da21ec0ee79d8..29e5a72173440 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1020,9 +1020,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { } } Stmt::AugAssign(ast::StmtAugAssign { target, .. }) => { - if checker.enabled(Rule::NonAsciiName) { - pylint::rules::non_ascii_name(checker, target); - } if checker.enabled(Rule::GlobalStatement) { if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() { pylint::rules::global_statement(checker, id); @@ -1328,11 +1325,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { } } Stmt::Assign(assign @ ast::StmtAssign { targets, value, .. }) => { - if checker.enabled(Rule::NonAsciiName) { - for target in targets { - pylint::rules::non_ascii_name(checker, target); - } - } + checker.enabled(Rule::NonAsciiName); if checker.enabled(Rule::LambdaAssignment) { if let [target] = &targets[..] { pycodestyle::rules::lambda_assignment(checker, target, value, None, stmt); @@ -1446,9 +1439,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { .. }, ) => { - if checker.enabled(Rule::NonAsciiName) { - pylint::rules::non_ascii_name(checker, target); - } if let Some(value) = value { if checker.enabled(Rule::LambdaAssignment) { pycodestyle::rules::lambda_assignment( diff --git a/crates/ruff_linter/src/rules/pylint/rules/non_ascii_module_import.rs b/crates/ruff_linter/src/rules/pylint/rules/non_ascii_module_import.rs index f1fc168e16117..577a4b69f19f1 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/non_ascii_module_import.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/non_ascii_module_import.rs @@ -10,8 +10,8 @@ use crate::checkers::ast::Checker; /// Checks for the use of non-ASCII characters in import statements. /// /// ## Why is this bad? -/// Pylint discourages the use of non-ASCII characters in symbol names as -/// they can cause confusion and compatibility issues. +/// The use of non-ASCII characters in import statements can cause confusion +/// and compatibility issues (see: [PEP 672]). /// /// ## Example /// ```python @@ -28,8 +28,7 @@ use crate::checkers::ast::Checker; /// import bár as bar /// ``` /// -/// ## References -/// - [PEP 672](https://peps.python.org/pep-0672/) +/// [PEP 672]: https://peps.python.org/pep-0672/ #[violation] pub struct NonAsciiImportName { name: String, diff --git a/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs b/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs index 9bcbfd4922a1d..c3326b9f5f63c 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs @@ -1,41 +1,116 @@ -use ruff_python_ast::{self as ast, Expr}; +use std::fmt; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_semantic::{Binding, BindingKind}; +use ruff_source_file::Locator; use ruff_text_size::Ranged; -use crate::checkers::ast::Checker; - /// ## What it does -/// Checks for the use of non-ASCII characters in symbol names. +/// Checks for the use of non-ASCII characters in variable names. /// /// ## Why is this bad? -/// Pylint discourages the use of non-ASCII characters in symbol names as -/// they can cause confusion and compatibility issues. +/// The use of non-ASCII characters in variable names can cause confusion +/// and compatibility issues (see: [PEP 672]). +/// +/// ## Example +/// ```python +/// ápple_count: int +/// ``` +/// +/// Use instead: +/// ```python +/// apple_count: int +/// ``` /// -/// ## References -/// - [PEP 672](https://peps.python.org/pep-0672/) +/// [PEP 672]: https://peps.python.org/pep-0672/ #[violation] -pub struct NonAsciiName; +pub struct NonAsciiName { + name: String, + kind: Kind, +} impl Violation for NonAsciiName { #[derive_message_formats] fn message(&self) -> String { - format!("Symbol name contains a non-ASCII character, consider renaming it.") + let Self { name, kind } = self; + format!("{kind} name `{name}` contains a non-ASCII character, consider renaming it") } } /// PLC2401 -pub(crate) fn non_ascii_name(checker: &mut Checker, target: &Expr) { - let Expr::Name(ast::ExprName { id, .. }) = target else { - return; +pub(crate) fn non_ascii_name(binding: &Binding, locator: &Locator) -> Option { + let name = binding.name(locator); + if name.is_ascii() { + return None; + } + + let kind = match binding.kind { + BindingKind::Annotation => Kind::Annotation, + BindingKind::Argument => Kind::Argument, + BindingKind::NamedExprAssignment => Kind::NamedExprAssignment, + BindingKind::UnpackedAssignment => Kind::UnpackedAssignment, + BindingKind::Assignment => Kind::Assignment, + BindingKind::TypeParam => Kind::TypeParam, + BindingKind::LoopVar => Kind::LoopVar, + BindingKind::Global => Kind::Global, + BindingKind::Nonlocal(_) => Kind::Nonlocal, + BindingKind::ClassDefinition(_) => Kind::ClassDefinition, + BindingKind::FunctionDefinition(_) => Kind::FunctionDefinition, + BindingKind::BoundException => Kind::BoundException, + + BindingKind::Builtin + | BindingKind::Export(_) + | BindingKind::FutureImport + | BindingKind::Import(_) + | BindingKind::FromImport(_) + | BindingKind::SubmoduleImport(_) + | BindingKind::Deletion + | BindingKind::UnboundException(_) => { + return None; + } }; - if id.is_ascii() { - return; - } + Some(Diagnostic::new( + NonAsciiName { + name: name.to_string(), + kind, + }, + binding.range(), + )) +} - checker - .diagnostics - .push(Diagnostic::new(NonAsciiName, target.range())); +#[derive(Debug, PartialEq, Eq)] +enum Kind { + Annotation, + Argument, + NamedExprAssignment, + UnpackedAssignment, + Assignment, + TypeParam, + LoopVar, + Global, + Nonlocal, + ClassDefinition, + FunctionDefinition, + BoundException, +} + +impl fmt::Display for Kind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Kind::Annotation => f.write_str("Annotation"), + Kind::Argument => f.write_str("Argument"), + Kind::NamedExprAssignment => f.write_str("Variable"), + Kind::UnpackedAssignment => f.write_str("Variable"), + Kind::Assignment => f.write_str("Variable"), + Kind::TypeParam => f.write_str("Type parameter"), + Kind::LoopVar => f.write_str("Variable"), + Kind::Global => f.write_str("Global"), + Kind::Nonlocal => f.write_str("Nonlocal"), + Kind::ClassDefinition => f.write_str("Class"), + Kind::FunctionDefinition => f.write_str("Function"), + Kind::BoundException => f.write_str("Exception"), + } + } } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2401_non_ascii_name.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2401_non_ascii_name.py.snap index bdc2358e1d7c6..1ef5432f9575f 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2401_non_ascii_name.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2401_non_ascii_name.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/pylint/mod.rs --- -non_ascii_name.py:1:1: PLC2401 Symbol name contains a non-ASCII character, consider renaming it. +non_ascii_name.py:1:1: PLC2401 Variable name `ápple_count` contains a non-ASCII character, consider renaming it | 1 | ápple_count: int = 1 # C2401 | ^^^^^^^^^^^ PLC2401 @@ -9,7 +9,7 @@ non_ascii_name.py:1:1: PLC2401 Symbol name contains a non-ASCII character, consi 3 | ápple_count = 3 # C2401 | -non_ascii_name.py:2:1: PLC2401 Symbol name contains a non-ASCII character, consider renaming it. +non_ascii_name.py:2:1: PLC2401 Variable name `ápple_count` contains a non-ASCII character, consider renaming it | 1 | ápple_count: int = 1 # C2401 2 | ápple_count += 2 # C2401 @@ -17,14 +17,63 @@ non_ascii_name.py:2:1: PLC2401 Symbol name contains a non-ASCII character, consi 3 | ápple_count = 3 # C2401 | -non_ascii_name.py:3:1: PLC2401 Symbol name contains a non-ASCII character, consider renaming it. +non_ascii_name.py:3:1: PLC2401 Variable name `ápple_count` contains a non-ASCII character, consider renaming it | 1 | ápple_count: int = 1 # C2401 2 | ápple_count += 2 # C2401 3 | ápple_count = 3 # C2401 | ^^^^^^^^^^^ PLC2401 4 | -5 | # this rule only works on assignment! +5 | (ápple_count for ápple_count in y) | +non_ascii_name.py:5:18: PLC2401 Variable name `ápple_count` contains a non-ASCII character, consider renaming it + | +3 | ápple_count = 3 # C2401 +4 | +5 | (ápple_count for ápple_count in y) + | ^^^^^^^^^^^ PLC2401 + | + +non_ascii_name.py:8:10: PLC2401 Argument name `ápple_count` contains a non-ASCII character, consider renaming it + | + 8 | def func(ápple_count): + | ^^^^^^^^^^^ PLC2401 + 9 | global ápple_count +10 | nonlocal ápple_count + | + +non_ascii_name.py:9:12: PLC2401 Global name `ápple_count` contains a non-ASCII character, consider renaming it + | + 8 | def func(ápple_count): + 9 | global ápple_count + | ^^^^^^^^^^^ PLC2401 +10 | nonlocal ápple_count + | + +non_ascii_name.py:13:5: PLC2401 Function name `ápple_count` contains a non-ASCII character, consider renaming it + | +13 | def ápple_count(): + | ^^^^^^^^^^^ PLC2401 +14 | pass + | + +non_ascii_name.py:18:10: PLC2401 Variable name `ápple_count` contains a non-ASCII character, consider renaming it + | +17 | match ápple_count: +18 | case ápple_count: + | ^^^^^^^^^^^ PLC2401 +19 | pass + | + +non_ascii_name.py:21:1: PLC2401 Annotation name `ápple_count` contains a non-ASCII character, consider renaming it + | +19 | pass +20 | +21 | ápple_count: int + | ^^^^^^^^^^^ PLC2401 +22 | +23 | try: + | +