From 4e1b289a67cf4570f1e4258c5559da8cc2af8a50 Mon Sep 17 00:00:00 2001 From: Calum Young <32770960+calumy@users.noreply.github.com> Date: Tue, 27 Aug 2024 15:02:14 +0100 Subject: [PATCH] Disable E741 in stub files (#13119) Co-authored-by: Alex Waygood --- .../test/fixtures/pycodestyle/E741.pyi | 75 +++++++ .../checkers/ast/analyze/except_handler.rs | 10 +- .../src/checkers/ast/analyze/expression.rs | 6 +- .../src/checkers/ast/analyze/parameter.rs | 10 +- .../src/checkers/ast/analyze/statement.rs | 12 +- .../ruff_linter/src/rules/pycodestyle/mod.rs | 5 + .../rules/ambiguous_variable_name.rs | 19 +- ...es__pycodestyle__tests__E741_E741.pyi.snap | 211 ++++++++++++++++++ ...estyle__tests__preview__E741_E741.pyi.snap | 4 + 9 files changed, 326 insertions(+), 26 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/pycodestyle/E741.pyi create mode 100644 crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E741_E741.pyi.snap create mode 100644 crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__preview__E741_E741.pyi.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E741.pyi b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E741.pyi new file mode 100644 index 0000000000..a238b578e4 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E741.pyi @@ -0,0 +1,75 @@ +from contextlib import contextmanager + +l = 0 +I = 0 +O = 0 +l: int = 0 + +a, l = 0, 1 +[a, l] = 0, 1 +a, *l = 0, 1, 2 +a = l = 0 + +o = 0 +i = 0 + +for l in range(3): + pass + + +for a, l in zip(range(3), range(3)): + pass + + +def f1(): + global l + l = 0 + + +def f2(): + l = 0 + + def f3(): + nonlocal l + l = 1 + + f3() + return l + + +def f4(l, /, I): + return l, I, O + + +def f5(l=0, *, I=1): + return l, I + + +def f6(*l, **I): + return l, I + + +@contextmanager +def ctx1(): + yield 0 + + +with ctx1() as l: + pass + + +@contextmanager +def ctx2(): + yield 0, 1 + + +with ctx2() as (a, l): + pass + +try: + pass +except ValueError as l: + pass + +if (l := 5) > 0: + pass diff --git a/crates/ruff_linter/src/checkers/ast/analyze/except_handler.rs b/crates/ruff_linter/src/checkers/ast/analyze/except_handler.rs index 2e39f0f965..0c3d6372ab 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/except_handler.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/except_handler.rs @@ -70,11 +70,11 @@ pub(crate) fn except_handler(except_handler: &ExceptHandler, checker: &mut Check } if let Some(name) = name { if checker.enabled(Rule::AmbiguousVariableName) { - if let Some(diagnostic) = - pycodestyle::rules::ambiguous_variable_name(name.as_str(), name.range()) - { - checker.diagnostics.push(diagnostic); - } + pycodestyle::rules::ambiguous_variable_name( + checker, + name.as_str(), + name.range(), + ); } if checker.enabled(Rule::BuiltinVariableShadowing) { flake8_builtins::rules::builtin_variable_shadowing(checker, name, name.range()); diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 2445d819b9..57a64933b4 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -259,11 +259,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { } } if checker.enabled(Rule::AmbiguousVariableName) { - if let Some(diagnostic) = - pycodestyle::rules::ambiguous_variable_name(id, expr.range()) - { - checker.diagnostics.push(diagnostic); - } + pycodestyle::rules::ambiguous_variable_name(checker, id, expr.range()); } if !checker.semantic.current_scope().kind.is_class() { if checker.enabled(Rule::BuiltinVariableShadowing) { diff --git a/crates/ruff_linter/src/checkers/ast/analyze/parameter.rs b/crates/ruff_linter/src/checkers/ast/analyze/parameter.rs index 0cedd98098..6627956a33 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/parameter.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/parameter.rs @@ -8,11 +8,11 @@ use crate::rules::{flake8_builtins, pep8_naming, pycodestyle}; /// Run lint rules over a [`Parameter`] syntax node. pub(crate) fn parameter(parameter: &Parameter, checker: &mut Checker) { if checker.enabled(Rule::AmbiguousVariableName) { - if let Some(diagnostic) = - pycodestyle::rules::ambiguous_variable_name(¶meter.name, parameter.name.range()) - { - checker.diagnostics.push(diagnostic); - } + pycodestyle::rules::ambiguous_variable_name( + checker, + ¶meter.name, + parameter.name.range(), + ); } if checker.enabled(Rule::InvalidArgumentName) { if let Some(diagnostic) = pep8_naming::rules::invalid_argument_name( diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 7cbd4db520..cc3b772d88 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -24,16 +24,16 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { pylint::rules::global_at_module_level(checker, stmt); } if checker.enabled(Rule::AmbiguousVariableName) { - checker.diagnostics.extend(names.iter().filter_map(|name| { - pycodestyle::rules::ambiguous_variable_name(name, name.range()) - })); + for name in names { + pycodestyle::rules::ambiguous_variable_name(checker, name, name.range()); + } } } Stmt::Nonlocal(nonlocal @ ast::StmtNonlocal { names, range: _ }) => { if checker.enabled(Rule::AmbiguousVariableName) { - checker.diagnostics.extend(names.iter().filter_map(|name| { - pycodestyle::rules::ambiguous_variable_name(name, name.range()) - })); + for name in names { + pycodestyle::rules::ambiguous_variable_name(checker, name, name.range()); + } } if checker.enabled(Rule::NonlocalWithoutBinding) { if !checker.semantic.scope_id.is_global() { diff --git a/crates/ruff_linter/src/rules/pycodestyle/mod.rs b/crates/ruff_linter/src/rules/pycodestyle/mod.rs index 4c1900f625..9850b51d89 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/mod.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/mod.rs @@ -26,6 +26,9 @@ mod tests { #[test_case(Rule::AmbiguousClassName, Path::new("E742.py"))] #[test_case(Rule::AmbiguousFunctionName, Path::new("E743.py"))] #[test_case(Rule::AmbiguousVariableName, Path::new("E741.py"))] + // E741 has different behaviour for `.pyi` files in preview mode; + // this test case checks it still has the old behaviour in stable mode + #[test_case(Rule::AmbiguousVariableName, Path::new("E741.pyi"))] #[test_case(Rule::LambdaAssignment, Path::new("E731.py"))] #[test_case(Rule::BareExcept, Path::new("E722.py"))] #[test_case(Rule::BlankLineWithWhitespace, Path::new("W29.py"))] @@ -75,6 +78,8 @@ mod tests { #[test_case(Rule::TooManyNewlinesAtEndOfFile, Path::new("W391_2.py"))] #[test_case(Rule::TooManyNewlinesAtEndOfFile, Path::new("W391_3.py"))] #[test_case(Rule::TooManyNewlinesAtEndOfFile, Path::new("W391_4.py"))] + // E741 has different behaviour for `.pyi` files in preview mode + #[test_case(Rule::AmbiguousVariableName, Path::new("E741.pyi"))] fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!( "preview__{}_{}", diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/ambiguous_variable_name.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/ambiguous_variable_name.rs index 4abeb9444e..1ad5e9de82 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/ambiguous_variable_name.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/ambiguous_variable_name.rs @@ -3,6 +3,7 @@ use ruff_text_size::TextRange; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use crate::checkers::ast::Checker; use crate::rules::pycodestyle::helpers::is_ambiguous_name; /// ## What it does @@ -25,6 +26,13 @@ use crate::rules::pycodestyle::helpers::is_ambiguous_name; /// o = 123 /// i = 42 /// ``` +/// +/// ## Preview mode behavior for stub files +/// In [preview] mode, this rule is automatically disabled for all stub files +/// (files with `.pyi` extensions). The rule has little relevance for authors +/// of stubs: a well-written stub should aim to faithfully represent the +/// interface of the equivalent .py file as it exists at runtime, including any +/// ambiguously named variables in the runtime module. #[violation] pub struct AmbiguousVariableName(pub String); @@ -38,13 +46,14 @@ impl Violation for AmbiguousVariableName { } /// E741 -pub(crate) fn ambiguous_variable_name(name: &str, range: TextRange) -> Option { +pub(crate) fn ambiguous_variable_name(checker: &mut Checker, name: &str, range: TextRange) { + if checker.settings.preview.is_enabled() && checker.source_type.is_stub() { + return; + } if is_ambiguous_name(name) { - Some(Diagnostic::new( + checker.diagnostics.push(Diagnostic::new( AmbiguousVariableName(name.to_string()), range, - )) - } else { - None + )); } } diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E741_E741.pyi.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E741_E741.pyi.snap new file mode 100644 index 0000000000..79854e35dd --- /dev/null +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E741_E741.pyi.snap @@ -0,0 +1,211 @@ +--- +source: crates/ruff_linter/src/rules/pycodestyle/mod.rs +--- +E741.pyi:3:1: E741 Ambiguous variable name: `l` + | +1 | from contextlib import contextmanager +2 | +3 | l = 0 + | ^ E741 +4 | I = 0 +5 | O = 0 + | + +E741.pyi:4:1: E741 Ambiguous variable name: `I` + | +3 | l = 0 +4 | I = 0 + | ^ E741 +5 | O = 0 +6 | l: int = 0 + | + +E741.pyi:5:1: E741 Ambiguous variable name: `O` + | +3 | l = 0 +4 | I = 0 +5 | O = 0 + | ^ E741 +6 | l: int = 0 + | + +E741.pyi:6:1: E741 Ambiguous variable name: `l` + | +4 | I = 0 +5 | O = 0 +6 | l: int = 0 + | ^ E741 +7 | +8 | a, l = 0, 1 + | + +E741.pyi:8:4: E741 Ambiguous variable name: `l` + | + 6 | l: int = 0 + 7 | + 8 | a, l = 0, 1 + | ^ E741 + 9 | [a, l] = 0, 1 +10 | a, *l = 0, 1, 2 + | + +E741.pyi:9:5: E741 Ambiguous variable name: `l` + | + 8 | a, l = 0, 1 + 9 | [a, l] = 0, 1 + | ^ E741 +10 | a, *l = 0, 1, 2 +11 | a = l = 0 + | + +E741.pyi:10:5: E741 Ambiguous variable name: `l` + | + 8 | a, l = 0, 1 + 9 | [a, l] = 0, 1 +10 | a, *l = 0, 1, 2 + | ^ E741 +11 | a = l = 0 + | + +E741.pyi:11:5: E741 Ambiguous variable name: `l` + | + 9 | [a, l] = 0, 1 +10 | a, *l = 0, 1, 2 +11 | a = l = 0 + | ^ E741 +12 | +13 | o = 0 + | + +E741.pyi:16:5: E741 Ambiguous variable name: `l` + | +14 | i = 0 +15 | +16 | for l in range(3): + | ^ E741 +17 | pass + | + +E741.pyi:20:8: E741 Ambiguous variable name: `l` + | +20 | for a, l in zip(range(3), range(3)): + | ^ E741 +21 | pass + | + +E741.pyi:25:12: E741 Ambiguous variable name: `l` + | +24 | def f1(): +25 | global l + | ^ E741 +26 | l = 0 + | + +E741.pyi:26:5: E741 Ambiguous variable name: `l` + | +24 | def f1(): +25 | global l +26 | l = 0 + | ^ E741 + | + +E741.pyi:30:5: E741 Ambiguous variable name: `l` + | +29 | def f2(): +30 | l = 0 + | ^ E741 +31 | +32 | def f3(): + | + +E741.pyi:33:18: E741 Ambiguous variable name: `l` + | +32 | def f3(): +33 | nonlocal l + | ^ E741 +34 | l = 1 + | + +E741.pyi:34:9: E741 Ambiguous variable name: `l` + | +32 | def f3(): +33 | nonlocal l +34 | l = 1 + | ^ E741 +35 | +36 | f3() + | + +E741.pyi:40:8: E741 Ambiguous variable name: `l` + | +40 | def f4(l, /, I): + | ^ E741 +41 | return l, I, O + | + +E741.pyi:40:14: E741 Ambiguous variable name: `I` + | +40 | def f4(l, /, I): + | ^ E741 +41 | return l, I, O + | + +E741.pyi:44:8: E741 Ambiguous variable name: `l` + | +44 | def f5(l=0, *, I=1): + | ^ E741 +45 | return l, I + | + +E741.pyi:44:16: E741 Ambiguous variable name: `I` + | +44 | def f5(l=0, *, I=1): + | ^ E741 +45 | return l, I + | + +E741.pyi:48:9: E741 Ambiguous variable name: `l` + | +48 | def f6(*l, **I): + | ^ E741 +49 | return l, I + | + +E741.pyi:48:14: E741 Ambiguous variable name: `I` + | +48 | def f6(*l, **I): + | ^ E741 +49 | return l, I + | + +E741.pyi:57:16: E741 Ambiguous variable name: `l` + | +57 | with ctx1() as l: + | ^ E741 +58 | pass + | + +E741.pyi:66:20: E741 Ambiguous variable name: `l` + | +66 | with ctx2() as (a, l): + | ^ E741 +67 | pass + | + +E741.pyi:71:22: E741 Ambiguous variable name: `l` + | +69 | try: +70 | pass +71 | except ValueError as l: + | ^ E741 +72 | pass + | + +E741.pyi:74:5: E741 Ambiguous variable name: `l` + | +72 | pass +73 | +74 | if (l := 5) > 0: + | ^ E741 +75 | pass + | diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__preview__E741_E741.pyi.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__preview__E741_E741.pyi.snap new file mode 100644 index 0000000000..6dcc4546f1 --- /dev/null +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__preview__E741_E741.pyi.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/pycodestyle/mod.rs +--- +