Skip to content

Commit

Permalink
Avoid A003 violations for explicitly overridden methods (#6076)
Browse files Browse the repository at this point in the history
## Summary

If a method is annotated with `@typing_extensions.override`, we should
avoid flagging A003 on it. This isn't part of the standard library yet,
but it's used to explicitly mark methods as overrides.
  • Loading branch information
charliermarsh authored Jul 25, 2023
1 parent f5c69c1 commit 9171bd4
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 3 deletions.
12 changes: 12 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_builtins/A003.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,15 @@ def filter(self, record: LogRecord) -> bool:

def str(self) -> None:
...


from typing_extensions import override


class MyClass:
@override
def str(self):
pass

def int(self):
pass
3 changes: 2 additions & 1 deletion crates/ruff/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,10 +332,11 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
}
if let ScopeKind::Class(class_def) = checker.semantic.scope().kind {
if checker.enabled(Rule::BuiltinAttributeShadowing) {
flake8_builtins::rules::builtin_attribute_shadowing(
flake8_builtins::rules::builtin_method_shadowing(
checker,
class_def,
name,
decorator_list,
name.range(),
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use ruff_text_size::TextRange;
use rustpython_parser::ast;
use rustpython_parser::ast::Decorator;

use ruff_diagnostics::Diagnostic;
use ruff_diagnostics::Violation;
Expand All @@ -10,7 +11,8 @@ use crate::checkers::ast::Checker;
use crate::rules::flake8_builtins::helpers::shadows_builtin;

/// ## What it does
/// Checks for any class attributes that use the same name as a builtin.
/// Checks for any class attributes or methods that use the same name as a
/// builtin.
///
/// ## Why is this bad?
/// Reusing a builtin name for the name of an attribute increases the
Expand All @@ -20,7 +22,9 @@ use crate::rules::flake8_builtins::helpers::shadows_builtin;
///
/// Builtins can be marked as exceptions to this rule via the
/// [`flake8-builtins.builtins-ignorelist`] configuration option, or
/// converted to the appropriate dunder method.
/// converted to the appropriate dunder method. Methods decorated with
/// `@typing.override` or `@typing_extensions.override` are also
/// ignored.
///
/// ## Example
/// ```python
Expand Down Expand Up @@ -81,12 +85,39 @@ pub(crate) fn builtin_attribute_shadowing(
return;
}

checker.diagnostics.push(Diagnostic::new(
BuiltinAttributeShadowing {
name: name.to_string(),
},
range,
));
}
}

/// A003
pub(crate) fn builtin_method_shadowing(
checker: &mut Checker,
class_def: &ast::StmtClassDef,
name: &str,
decorator_list: &[Decorator],
range: TextRange,
) {
if shadows_builtin(name, &checker.settings.flake8_builtins.builtins_ignorelist) {
// Ignore some standard-library methods. Ideally, we'd ignore all overridden methods, since
// those should be flagged on the superclass, but that's more difficult.
if is_standard_library_override(name, class_def, checker.semantic()) {
return;
}

// Ignore explicit overrides.
if decorator_list.iter().any(|decorator| {
checker
.semantic()
.match_typing_expr(&decorator.expression, "override")
}) {
return;
}

checker.diagnostics.push(Diagnostic::new(
BuiltinAttributeShadowing {
name: name.to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,13 @@ A003.py:40:9: A003 Class attribute `str` is shadowing a Python builtin
41 | ...
|

A003.py:52:9: A003 Class attribute `int` is shadowing a Python builtin
|
50 | pass
51 |
52 | def int(self):
| ^^^ A003
53 | pass
|


Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,13 @@ A003.py:40:9: A003 Class attribute `str` is shadowing a Python builtin
41 | ...
|

A003.py:52:9: A003 Class attribute `int` is shadowing a Python builtin
|
50 | pass
51 |
52 | def int(self):
| ^^^ A003
53 | pass
|


0 comments on commit 9171bd4

Please sign in to comment.