diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs index b91bdad4a2..de0f27f23d 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs @@ -1,6 +1,8 @@ +use ruff_db::diagnostic::Diagnostic; use ruff_python_ast::name::QualifiedName; use ruff_python_ast::{self as ast, Expr}; use ruff_python_semantic::SemanticModel; +use ruff_python_semantic::analyze::function_type::is_subject_to_liskov_substitution_principle; use crate::checkers::ast::Checker; use crate::settings::LinterSettings; @@ -191,3 +193,27 @@ pub(super) fn allow_boolean_trap(call: &ast::ExprCall, checker: &Checker) -> boo false } + +pub(super) fn add_liskov_substitution_principle_help( + diagnostic: &mut Diagnostic, + function_name: &str, + decorator_list: &[ast::Decorator], + checker: &Checker, +) { + let semantic = checker.semantic(); + let parent_scope = semantic.current_scope(); + let pep8_settings = &checker.settings().pep8_naming; + if is_subject_to_liskov_substitution_principle( + function_name, + decorator_list, + parent_scope, + semantic, + &pep8_settings.classmethod_decorators, + &pep8_settings.staticmethod_decorators, + ) { + diagnostic.help( + "Consider adding `@typing.override` if changing the function signature \ + would violate the Liskov Substitution Principle", + ); + } +} diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_default_value_positional_argument.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_default_value_positional_argument.rs index cfe817a259..d20c770445 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_default_value_positional_argument.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_default_value_positional_argument.rs @@ -6,7 +6,9 @@ use ruff_python_semantic::analyze::visibility; use crate::Violation; use crate::checkers::ast::Checker; -use crate::rules::flake8_boolean_trap::helpers::is_allowed_func_def; +use crate::rules::flake8_boolean_trap::helpers::{ + add_liskov_substitution_principle_help, is_allowed_func_def, +}; /// ## What it does /// Checks for the use of boolean positional arguments in function definitions, @@ -139,7 +141,9 @@ pub(crate) fn boolean_default_value_positional_argument( return; } - checker.report_diagnostic(BooleanDefaultValuePositionalArgument, param.identifier()); + let mut diagnostic = checker + .report_diagnostic(BooleanDefaultValuePositionalArgument, param.identifier()); + add_liskov_substitution_principle_help(&mut diagnostic, name, decorator_list, checker); } } } diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_type_hint_positional_argument.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_type_hint_positional_argument.rs index 05a87a28c4..dd3363fee6 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_type_hint_positional_argument.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_type_hint_positional_argument.rs @@ -7,7 +7,9 @@ use ruff_python_semantic::analyze::visibility; use crate::Violation; use crate::checkers::ast::Checker; -use crate::rules::flake8_boolean_trap::helpers::is_allowed_func_def; +use crate::rules::flake8_boolean_trap::helpers::{ + add_liskov_substitution_principle_help, is_allowed_func_def, +}; /// ## What it does /// Checks for the use of boolean positional arguments in function definitions, @@ -149,7 +151,10 @@ pub(crate) fn boolean_type_hint_positional_argument( return; } - checker.report_diagnostic(BooleanTypeHintPositionalArgument, parameter.identifier()); + let mut diagnostic = + checker.report_diagnostic(BooleanTypeHintPositionalArgument, parameter.identifier()); + + add_liskov_substitution_principle_help(&mut diagnostic, name, decorator_list, checker); } } diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT001_FBT.py.snap b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT001_FBT.py.snap index 2ab5c9ac07..de000a9ef2 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT001_FBT.py.snap +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT001_FBT.py.snap @@ -97,6 +97,7 @@ FBT001 Boolean-typed positional argument in function definition | ^^^^^ 91 | pass | +help: Consider adding `@typing.override` if changing the function signature would violate the Liskov Substitution Principle FBT001 Boolean-typed positional argument in function definition --> FBT.py:100:10 diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_function_name.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_function_name.rs index d7fbd985dd..6757d77ef7 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_function_name.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_function_name.rs @@ -130,10 +130,16 @@ pub(crate) fn invalid_function_name( return; } - checker.report_diagnostic( + let mut diagnostic = checker.report_diagnostic( InvalidFunctionName { name: name.to_string(), }, stmt.identifier(), ); + if parent_class.is_some() { + diagnostic.help( + "Consider adding `@typing.override` if this method \ + overrides a method from a superclass", + ); + } } diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N802_N802.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N802_N802.py.snap index a377df9606..f04e6de4d5 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N802_N802.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N802_N802.py.snap @@ -42,6 +42,7 @@ N802 Function name `testTest` should be lowercase | ^^^^^^^^ 41 | assert True | +help: Consider adding `@typing.override` if this method overrides a method from a superclass N802 Function name `bad_Name` should be lowercase --> N802.py:65:9 @@ -52,6 +53,7 @@ N802 Function name `bad_Name` should be lowercase | ^^^^^^^^ 66 | pass | +help: Consider adding `@typing.override` if this method overrides a method from a superclass N802 Function name `dont_GET` should be lowercase --> N802.py:84:9 @@ -62,6 +64,7 @@ N802 Function name `dont_GET` should be lowercase | ^^^^^^^^ 85 | pass | +help: Consider adding `@typing.override` if this method overrides a method from a superclass N802 Function name `dont_OPTIONS` should be lowercase --> N802.py:95:9 @@ -72,6 +75,7 @@ N802 Function name `dont_OPTIONS` should be lowercase | ^^^^^^^^^^^^ 96 | pass | +help: Consider adding `@typing.override` if this method overrides a method from a superclass N802 Function name `dont_OPTIONS` should be lowercase --> N802.py:106:9 @@ -82,3 +86,4 @@ N802 Function name `dont_OPTIONS` should be lowercase | ^^^^^^^^^^^^ 107 | pass | +help: Consider adding `@typing.override` if this method overrides a method from a superclass diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N802_N802.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N802_N802.py.snap index 9e74ba98da..a972592931 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N802_N802.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N802_N802.py.snap @@ -20,3 +20,4 @@ N802 Function name `stillBad` should be lowercase | ^^^^^^^^ 14 | return super().tearDown() | +help: Consider adding `@typing.override` if this method overrides a method from a superclass diff --git a/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs b/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs index e814fb502c..69d630910d 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs @@ -146,11 +146,15 @@ pub(crate) fn no_self_use(checker: &Checker, scope_id: ScopeId, scope: &Scope) { .map(|binding_id| semantic.binding(binding_id)) .is_some_and(|binding| binding.kind.is_argument() && binding.is_unused()) { - checker.report_diagnostic( + let mut diagnostic = checker.report_diagnostic( NoSelfUse { method_name: name.to_string(), }, func.identifier(), ); + diagnostic.help( + "Consider adding `@typing.override` if this method overrides \ + a method from a superclass", + ); } } diff --git a/crates/ruff_linter/src/rules/pylint/rules/too_many_arguments.rs b/crates/ruff_linter/src/rules/pylint/rules/too_many_arguments.rs index 0fb6b4f04e..51ef4c706d 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/too_many_arguments.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/too_many_arguments.rs @@ -1,6 +1,7 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast as ast; use ruff_python_ast::identifier::Identifier; +use ruff_python_semantic::analyze::function_type::is_subject_to_liskov_substitution_principle; use ruff_python_semantic::analyze::{function_type, visibility}; use crate::Violation; @@ -121,11 +122,24 @@ pub(crate) fn too_many_arguments(checker: &Checker, function_def: &ast::StmtFunc return; } - checker.report_diagnostic( + let mut diagnostic = checker.report_diagnostic( TooManyArguments { c_args: num_arguments, max_args: checker.settings().pylint.max_args, }, function_def.identifier(), ); + if is_subject_to_liskov_substitution_principle( + &function_def.name, + &function_def.decorator_list, + semantic.current_scope(), + semantic, + &checker.settings().pep8_naming.classmethod_decorators, + &checker.settings().pep8_naming.staticmethod_decorators, + ) { + diagnostic.help( + "Consider adding `@typing.override` if changing the function signature \ + would violate the Liskov Substitution Principle", + ); + } } diff --git a/crates/ruff_linter/src/rules/pylint/rules/too_many_positional_arguments.rs b/crates/ruff_linter/src/rules/pylint/rules/too_many_positional_arguments.rs index db661c68b7..2134fe6a3f 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/too_many_positional_arguments.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/too_many_positional_arguments.rs @@ -1,5 +1,6 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::{self as ast, identifier::Identifier}; +use ruff_python_semantic::analyze::function_type::is_subject_to_liskov_substitution_principle; use ruff_python_semantic::analyze::{function_type, visibility}; use crate::Violation; @@ -125,11 +126,24 @@ pub(crate) fn too_many_positional_arguments( return; } - checker.report_diagnostic( + let mut diagnostic = checker.report_diagnostic( TooManyPositionalArguments { c_pos: num_positional_args, max_pos: checker.settings().pylint.max_positional_args, }, function_def.identifier(), ); + if is_subject_to_liskov_substitution_principle( + &function_def.name, + &function_def.decorator_list, + semantic.current_scope(), + semantic, + &checker.settings().pep8_naming.classmethod_decorators, + &checker.settings().pep8_naming.staticmethod_decorators, + ) { + diagnostic.help( + "Consider adding `@typing.override` if changing the function signature \ + would violate the Liskov Substitution Principle", + ); + } } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0913_too_many_arguments.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0913_too_many_arguments.py.snap index 60f069c1ef..8612e48fbb 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0913_too_many_arguments.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0913_too_many_arguments.py.snap @@ -49,6 +49,7 @@ PLR0913 Too many arguments in function definition (8 > 5) | ^ 52 | pass | +help: Consider adding `@typing.override` if changing the function signature would violate the Liskov Substitution Principle PLR0913 Too many arguments in function definition (8 > 5) --> too_many_arguments.py:58:9 @@ -58,6 +59,7 @@ PLR0913 Too many arguments in function definition (8 > 5) | ^ 59 | pass | +help: Consider adding `@typing.override` if changing the function signature would violate the Liskov Substitution Principle PLR0913 Too many arguments in function definition (8 > 5) --> too_many_arguments.py:66:9 @@ -67,6 +69,7 @@ PLR0913 Too many arguments in function definition (8 > 5) | ^ 67 | pass | +help: Consider adding `@typing.override` if changing the function signature would violate the Liskov Substitution Principle PLR0913 Too many arguments in function definition (6 > 5) --> too_many_arguments.py:70:9 @@ -76,3 +79,4 @@ PLR0913 Too many arguments in function definition (6 > 5) | ^ 71 | pass | +help: Consider adding `@typing.override` if changing the function signature would violate the Liskov Substitution Principle diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0917_too_many_positional_arguments.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0917_too_many_positional_arguments.py.snap index c46bd0b617..a2822eeb6c 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0917_too_many_positional_arguments.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0917_too_many_positional_arguments.py.snap @@ -34,6 +34,7 @@ PLR0917 Too many positional arguments (6/5) | ^ 44 | pass | +help: Consider adding `@typing.override` if changing the function signature would violate the Liskov Substitution Principle PLR0917 Too many positional arguments (6/5) --> too_many_positional_arguments.py:47:9 @@ -43,3 +44,4 @@ PLR0917 Too many positional arguments (6/5) | ^ 48 | pass | +help: Consider adding `@typing.override` if changing the function signature would violate the Liskov Substitution Principle diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR6301_no_self_use.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR6301_no_self_use.py.snap index 5e82421071..031b62e273 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR6301_no_self_use.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR6301_no_self_use.py.snap @@ -9,6 +9,7 @@ PLR6301 Method `developer_greeting` could be a function, class method, or static | ^^^^^^^^^^^^^^^^^^ 8 | print(f"Greetings {name}!") | +help: Consider adding `@typing.override` if this method overrides a method from a superclass PLR6301 Method `greeting_1` could be a function, class method, or static method --> no_self_use.py:10:9 @@ -19,6 +20,7 @@ PLR6301 Method `greeting_1` could be a function, class method, or static method | ^^^^^^^^^^ 11 | print("Hello!") | +help: Consider adding `@typing.override` if this method overrides a method from a superclass PLR6301 Method `greeting_2` could be a function, class method, or static method --> no_self_use.py:13:9 @@ -29,6 +31,7 @@ PLR6301 Method `greeting_2` could be a function, class method, or static method | ^^^^^^^^^^ 14 | print("Hi!") | +help: Consider adding `@typing.override` if this method overrides a method from a superclass PLR6301 Method `validate_y` could be a function, class method, or static method --> no_self_use.py:103:9 @@ -39,6 +42,7 @@ PLR6301 Method `validate_y` could be a function, class method, or static method 104 | if value <= 0: 105 | raise ValueError("y must be a positive integer") | +help: Consider adding `@typing.override` if this method overrides a method from a superclass PLR6301 Method `non_simple_assignment` could be a function, class method, or static method --> no_self_use.py:128:9 @@ -50,6 +54,7 @@ PLR6301 Method `non_simple_assignment` could be a function, class method, or sta 129 | msg = foo = "" 130 | raise NotImplementedError(msg) | +help: Consider adding `@typing.override` if this method overrides a method from a superclass PLR6301 Method `non_simple_assignment_2` could be a function, class method, or static method --> no_self_use.py:132:9 @@ -61,6 +66,7 @@ PLR6301 Method `non_simple_assignment_2` could be a function, class method, or s 133 | msg[0] = "" 134 | raise NotImplementedError(msg) | +help: Consider adding `@typing.override` if this method overrides a method from a superclass PLR6301 Method `unused_message` could be a function, class method, or static method --> no_self_use.py:136:9 @@ -72,6 +78,7 @@ PLR6301 Method `unused_message` could be a function, class method, or static met 137 | msg = "" 138 | raise NotImplementedError("") | +help: Consider adding `@typing.override` if this method overrides a method from a superclass PLR6301 Method `unused_message_2` could be a function, class method, or static method --> no_self_use.py:140:9 @@ -83,6 +90,7 @@ PLR6301 Method `unused_message_2` could be a function, class method, or static m 141 | msg = "" 142 | raise NotImplementedError(x) | +help: Consider adding `@typing.override` if this method overrides a method from a superclass PLR6301 Method `developer_greeting` could be a function, class method, or static method --> no_self_use.py:145:9 @@ -92,6 +100,7 @@ PLR6301 Method `developer_greeting` could be a function, class method, or static | ^^^^^^^^^^^^^^^^^^ 146 | print(t"Greetings {name}!") | +help: Consider adding `@typing.override` if this method overrides a method from a superclass PLR6301 Method `tstring` could be a function, class method, or static method --> no_self_use.py:151:9 @@ -103,3 +112,4 @@ PLR6301 Method `tstring` could be a function, class method, or static method 152 | msg = t"{x}" 153 | raise NotImplementedError(msg) | +help: Consider adding `@typing.override` if this method overrides a method from a superclass diff --git a/crates/ruff_python_semantic/src/analyze/function_type.rs b/crates/ruff_python_semantic/src/analyze/function_type.rs index 5949b71c88..3449ce5fc0 100644 --- a/crates/ruff_python_semantic/src/analyze/function_type.rs +++ b/crates/ruff_python_semantic/src/analyze/function_type.rs @@ -47,6 +47,35 @@ pub fn classify( } } +/// Return `true` if this function is subject to the Liskov Substitution Principle. +/// +/// Type checkers will check nearly all methods for compliance with the Liskov Substitution +/// Principle, but some methods are exempt. +pub fn is_subject_to_liskov_substitution_principle( + function_name: &str, + decorator_list: &[Decorator], + parent_scope: &Scope, + semantic: &SemanticModel, + classmethod_decorators: &[String], + staticmethod_decorators: &[String], +) -> bool { + let kind = classify( + function_name, + decorator_list, + parent_scope, + semantic, + classmethod_decorators, + staticmethod_decorators, + ); + + match (kind, function_name) { + (FunctionType::Function | FunctionType::NewMethod, _) => false, + (FunctionType::Method, "__init__" | "__post_init__" | "__replace__") => false, + (_, "__init_subclass__") => false, + (FunctionType::Method | FunctionType::ClassMethod | FunctionType::StaticMethod, _) => true, + } +} + /// Return `true` if a [`Decorator`] is indicative of a static method. /// Note: Implicit static methods like `__new__` are not considered. fn is_static_method(