diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI056.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI056.py new file mode 100644 index 0000000000..830c708320 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI056.py @@ -0,0 +1,12 @@ +__all__ = ["A", "B", "C"] + +# Errors +__all__.append("D") +__all__.extend(["E", "Foo"]) +__all__.remove("A") + +# OK +__all__ += ["D"] +foo = ["Hello"] +foo.append("World") +foo.bar.append("World") diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI056.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI056.pyi new file mode 100644 index 0000000000..830c708320 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI056.pyi @@ -0,0 +1,12 @@ +__all__ = ["A", "B", "C"] + +# Errors +__all__.append("D") +__all__.extend(["E", "Foo"]) +__all__.remove("A") + +# OK +__all__ += ["D"] +foo = ["Hello"] +foo.append("World") +foo.bar.append("World") diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index a3c6aa040e..6d5380fe44 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -3037,6 +3037,9 @@ where if self.enabled(Rule::DjangoLocalsInRenderFunction) { flake8_django::rules::locals_in_render_function(self, func, args, keywords); } + if self.is_stub && self.enabled(Rule::UnsupportedMethodCallOnAll) { + flake8_pyi::rules::unsupported_method_call_on_all(self, func); + } } Expr::Dict(ast::ExprDict { keys, diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 38f97cf148..745489e464 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -652,6 +652,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Pyi, "052") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnannotatedAssignmentInStub), (Flake8Pyi, "054") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::NumericLiteralTooLong), (Flake8Pyi, "053") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::StringOrBytesTooLong), + (Flake8Pyi, "056") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnsupportedMethodCallOnAll), // flake8-pytest-style (Flake8PytestStyle, "001") => (RuleGroup::Unspecified, rules::flake8_pytest_style::rules::PytestFixtureIncorrectParenthesesStyle), diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index 537226ebe9..c43a023ae0 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -91,6 +91,8 @@ mod tests { #[test_case(Rule::WrongTupleLengthVersionComparison, Path::new("PYI005.pyi"))] #[test_case(Rule::TypeAliasWithoutAnnotation, Path::new("PYI026.py"))] #[test_case(Rule::TypeAliasWithoutAnnotation, Path::new("PYI026.pyi"))] + #[test_case(Rule::UnsupportedMethodCallOnAll, Path::new("PYI056.py"))] + #[test_case(Rule::UnsupportedMethodCallOnAll, Path::new("PYI056.pyi"))] 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/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index c0bca38fc2..4b8d229df9 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -28,6 +28,7 @@ pub(crate) use unaliased_collections_abc_set_import::*; pub(crate) use unnecessary_literal_union::*; pub(crate) use unrecognized_platform::*; pub(crate) use unrecognized_version_info::*; +pub(crate) use unsupported_method_call_on_all::*; mod any_eq_ne_annotation; mod bad_version_info_comparison; @@ -59,3 +60,4 @@ mod unaliased_collections_abc_set_import; mod unnecessary_literal_union; mod unrecognized_platform; mod unrecognized_version_info; +mod unsupported_method_call_on_all; diff --git a/crates/ruff/src/rules/flake8_pyi/rules/unsupported_method_call_on_all.rs b/crates/ruff/src/rules/flake8_pyi/rules/unsupported_method_call_on_all.rs new file mode 100644 index 0000000000..f7d24371a2 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/rules/unsupported_method_call_on_all.rs @@ -0,0 +1,65 @@ +use rustpython_parser::ast::{self, Expr, Ranged}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks that `append`, `extend` and `remove` methods are not called on +/// `__all__`. +/// +/// ## Why is this bad? +/// Different type checkers have varying levels of support for calling these +/// methods on `__all__`. Instead, use the `+=` operator to add items to +/// `__all__`, which is known to be supported by all major type checkers. +/// +/// ## Example +/// ```python +/// __all__ = ["A"] +/// __all__.append("B") +/// ``` +/// +/// Use instead: +/// ```python +/// __all__ = ["A"] +/// __all__ += "B" +/// ``` +#[violation] +pub struct UnsupportedMethodCallOnAll { + name: String, +} + +impl Violation for UnsupportedMethodCallOnAll { + #[derive_message_formats] + fn message(&self) -> String { + let UnsupportedMethodCallOnAll { name } = self; + format!("Calling `.{name}()` on `__all__` may not be supported by all type checkers (use `+=` instead)") + } +} + +/// PYI056 +pub(crate) fn unsupported_method_call_on_all(checker: &mut Checker, func: &Expr) { + let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func else { + return; + }; + let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else { + return; + }; + if id.as_str() != "__all__" { + return; + } + if !is_unsupported_method(attr.as_str()) { + return; + } + checker.diagnostics.push(Diagnostic::new( + UnsupportedMethodCallOnAll { + name: attr.to_string(), + }, + func.range(), + )); +} + +fn is_unsupported_method(name: &str) -> bool { + matches!(name, "append" | "extend" | "remove") +} diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI056_PYI056.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI056_PYI056.py.snap new file mode 100644 index 0000000000..d1aa2e9116 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI056_PYI056.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- + diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI056_PYI056.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI056_PYI056.pyi.snap new file mode 100644 index 0000000000..79c5f952ec --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI056_PYI056.pyi.snap @@ -0,0 +1,32 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- +PYI056.pyi:4:1: PYI056 Calling `.append()` on `__all__` may not be supported by all type checkers (use `+=` instead) + | +3 | # Errors +4 | __all__.append("D") + | ^^^^^^^^^^^^^^ PYI056 +5 | __all__.extend(["E", "Foo"]) +6 | __all__.remove("A") + | + +PYI056.pyi:5:1: PYI056 Calling `.extend()` on `__all__` may not be supported by all type checkers (use `+=` instead) + | +3 | # Errors +4 | __all__.append("D") +5 | __all__.extend(["E", "Foo"]) + | ^^^^^^^^^^^^^^ PYI056 +6 | __all__.remove("A") + | + +PYI056.pyi:6:1: PYI056 Calling `.remove()` on `__all__` may not be supported by all type checkers (use `+=` instead) + | +4 | __all__.append("D") +5 | __all__.extend(["E", "Foo"]) +6 | __all__.remove("A") + | ^^^^^^^^^^^^^^ PYI056 +7 | +8 | # OK + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 66a99e5a56..20530484bf 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2380,6 +2380,7 @@ "PYI052", "PYI053", "PYI054", + "PYI056", "Q", "Q0", "Q00",