Python/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff synced 2026-01-20 21:10:48 -05:00
Code Issues Packages Projects Releases Wiki Activity

Add help: subdiagnostics for several Ruff rules that can sometimes appear to disagree with ty (#22331)

Browse Source
This commit is contained in:
Alex Waygood
2026-01-02 22:10:39 +00:00
committed by GitHub
parent 74978cfff2
commit d0f841bff2
14 changed files with 133 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs
View File

@@ -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",
);
}
}

8
crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_default_value_positional_argument.rs
View File

@@ -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);
}
}
}

9
crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_type_hint_positional_argument.rs
View File

@@ -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);
}
}

1
crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT001_FBT.py.snap
View File

@@ -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

8
crates/ruff_linter/src/rules/pep8_naming/rules/invalid_function_name.rs
View File

@@ -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",
);
}
}

5
crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N802_N802.py.snap
View File

@@ -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

1
crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N802_N802.py.snap
View File

@@ -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

6
crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs
View File

@@ -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",
);
}
}

16
crates/ruff_linter/src/rules/pylint/rules/too_many_arguments.rs
View File

@@ -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",
);
}
}

16
crates/ruff_linter/src/rules/pylint/rules/too_many_positional_arguments.rs
View File

@@ -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",
);
}
}

4
crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0913_too_many_arguments.py.snap
View File

@@ -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

2
crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0917_too_many_positional_arguments.py.snap
View File

@@ -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

10
crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR6301_no_self_use.py.snap
View File

@@ -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

29
crates/ruff_python_semantic/src/analyze/function_type.rs
View File

@@ -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(