From 3fcc1402f63f438f25f8793e33708d64c88e808c Mon Sep 17 00:00:00 2001 From: Steve C Date: Tue, 2 Jan 2024 16:57:53 -0500 Subject: [PATCH] [pylint] - implement `super-without-brackets`/`W0245` (#9257) ## Summary Implement [`super-without-brackets`/`W0245`](https://pylint.readthedocs.io/en/latest/user_guide/messages/warning/super-without-brackets.html) See: #970 ## Test Plan `cargo test` --- .../fixtures/pylint/super_without_brackets.py | 33 +++++ .../src/checkers/ast/analyze/expression.rs | 3 + 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 + .../pylint/rules/super_without_brackets.rs | 116 ++++++++++++++++++ ...ts__PLW0245_super_without_brackets.py.snap | 24 ++++ ruff.schema.json | 3 + 8 files changed, 183 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/pylint/super_without_brackets.py create mode 100644 crates/ruff_linter/src/rules/pylint/rules/super_without_brackets.rs create mode 100644 crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0245_super_without_brackets.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/super_without_brackets.py b/crates/ruff_linter/resources/test/fixtures/pylint/super_without_brackets.py new file mode 100644 index 0000000000000..7f190e1dbe436 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/super_without_brackets.py @@ -0,0 +1,33 @@ +class Animal: + @staticmethod + def speak(): + return f"This animal says something." + + +class BadDog(Animal): + @staticmethod + def speak(): + original_speak = super.speak() # PLW0245 + return f"{original_speak} But as a dog, it barks!" + + +class GoodDog(Animal): + @staticmethod + def speak(): + original_speak = super().speak() # OK + return f"{original_speak} But as a dog, it barks!" + + +class FineDog(Animal): + @staticmethod + def speak(): + super = "super" + original_speak = super.speak() # OK + return f"{original_speak} But as a dog, it barks!" + + +def super_without_class() -> None: + super.blah() # OK + + +super.blah() # OK diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index e5bb598e73085..b5955034fd85d 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -438,6 +438,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { } } } + if checker.enabled(Rule::SuperWithoutBrackets) { + pylint::rules::super_without_brackets(checker, func); + } if checker.enabled(Rule::BitCount) { refurb::rules::bit_count(checker, call); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 75f4d29673462..d6e79da23a619 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -275,6 +275,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "W0127") => (RuleGroup::Stable, rules::pylint::rules::SelfAssigningVariable), (Pylint, "W0129") => (RuleGroup::Stable, rules::pylint::rules::AssertOnStringLiteral), (Pylint, "W0131") => (RuleGroup::Stable, rules::pylint::rules::NamedExprWithoutContext), + (Pylint, "W0245") => (RuleGroup::Preview, rules::pylint::rules::SuperWithoutBrackets), (Pylint, "W0406") => (RuleGroup::Stable, rules::pylint::rules::ImportSelf), (Pylint, "W0602") => (RuleGroup::Stable, rules::pylint::rules::GlobalVariableNotAssigned), (Pylint, "W0604") => (RuleGroup::Preview, rules::pylint::rules::GlobalAtModuleLevel), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index 02161bad780b7..66d3fe1c3d07d 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -162,6 +162,7 @@ mod tests { )] #[test_case(Rule::NoClassmethodDecorator, Path::new("no_method_decorator.py"))] #[test_case(Rule::NoStaticmethodDecorator, Path::new("no_method_decorator.py"))] + #[test_case(Rule::SuperWithoutBrackets, Path::new("super_without_brackets.py"))] #[test_case( Rule::UnnecessaryDictIndexLookup, Path::new("unnecessary_dict_index_lookup.py") diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 28c0a5c980794..07344e84a38dc 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -52,6 +52,7 @@ pub(crate) use self_assigning_variable::*; pub(crate) use single_string_slots::*; pub(crate) use subprocess_popen_preexec_fn::*; pub(crate) use subprocess_run_without_check::*; +pub(crate) use super_without_brackets::*; pub(crate) use sys_exit_alias::*; pub(crate) use too_many_arguments::*; pub(crate) use too_many_boolean_expressions::*; @@ -131,6 +132,7 @@ mod self_assigning_variable; mod single_string_slots; mod subprocess_popen_preexec_fn; mod subprocess_run_without_check; +mod super_without_brackets; mod sys_exit_alias; mod too_many_arguments; mod too_many_boolean_expressions; diff --git a/crates/ruff_linter/src/rules/pylint/rules/super_without_brackets.rs b/crates/ruff_linter/src/rules/pylint/rules/super_without_brackets.rs new file mode 100644 index 0000000000000..04b744c002db0 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/super_without_brackets.rs @@ -0,0 +1,116 @@ +use ruff_python_ast::{self as ast, Expr}; +use ruff_python_semantic::{analyze::function_type, ScopeKind}; + +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for `super` calls without parentheses. +/// +/// ## Why is this bad? +/// When `super` is used without parentheses, it is not an actual call, and +/// thus has no effect. +/// +/// ## Example +/// ```python +/// class Animal: +/// @staticmethod +/// def speak(): +/// return "This animal says something." +/// +/// +/// class Dog(Animal): +/// @staticmethod +/// def speak(): +/// original_speak = super.speak() +/// return f"{original_speak} But as a dog, it barks!" +/// ``` +/// +/// Use instead: +/// ```python +/// class Animal: +/// @staticmethod +/// def speak(): +/// return "This animal says something." +/// +/// +/// class Dog(Animal): +/// @staticmethod +/// def speak(): +/// original_speak = super().speak() +/// return f"{original_speak} But as a dog, it barks!" +/// ``` +#[violation] +pub struct SuperWithoutBrackets; + +impl AlwaysFixableViolation for SuperWithoutBrackets { + #[derive_message_formats] + fn message(&self) -> String { + format!("`super` call is missing parentheses") + } + + fn fix_title(&self) -> String { + "Add parentheses to `super` call".to_string() + } +} + +/// PLW0245 +pub(crate) fn super_without_brackets(checker: &mut Checker, func: &Expr) { + // The call must be to `super` (without parentheses). + let Expr::Attribute(ast::ExprAttribute { value, .. }) = func else { + return; + }; + + let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else { + return; + }; + + if id.as_str() != "super" { + return; + } + + if !checker.semantic().is_builtin(id.as_str()) { + return; + } + + let scope = checker.semantic().current_scope(); + + // The current scope _must_ be a function. + let ScopeKind::Function(function_def) = scope.kind else { + return; + }; + + let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else { + return; + }; + + // The function must be a method, class method, or static method. + let classification = function_type::classify( + &function_def.name, + &function_def.decorator_list, + parent, + checker.semantic(), + &checker.settings.pep8_naming.classmethod_decorators, + &checker.settings.pep8_naming.staticmethod_decorators, + ); + if !matches!( + classification, + function_type::FunctionType::Method { .. } + | function_type::FunctionType::ClassMethod { .. } + | function_type::FunctionType::StaticMethod { .. } + ) { + return; + } + + let mut diagnostic = Diagnostic::new(SuperWithoutBrackets, value.range()); + + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + "super()".to_string(), + value.range(), + ))); + + checker.diagnostics.push(diagnostic); +} diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0245_super_without_brackets.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0245_super_without_brackets.py.snap new file mode 100644 index 0000000000000..d2abe86131b9d --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0245_super_without_brackets.py.snap @@ -0,0 +1,24 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +super_without_brackets.py:10:26: PLW0245 [*] `super` call is missing parentheses + | + 8 | @staticmethod + 9 | def speak(): +10 | original_speak = super.speak() # PLW0245 + | ^^^^^ PLW0245 +11 | return f"{original_speak} But as a dog, it barks!" + | + = help: Add parentheses to `super` call + +ℹ Safe fix +7 7 | class BadDog(Animal): +8 8 | @staticmethod +9 9 | def speak(): +10 |- original_speak = super.speak() # PLW0245 + 10 |+ original_speak = super().speak() # PLW0245 +11 11 | return f"{original_speak} But as a dog, it barks!" +12 12 | +13 13 | + + diff --git a/ruff.schema.json b/ruff.schema.json index c50d2e69df672..74b1c0a6d64df 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3225,6 +3225,9 @@ "PLW0129", "PLW013", "PLW0131", + "PLW02", + "PLW024", + "PLW0245", "PLW04", "PLW040", "PLW0406",