From 9171bd4c283a7d0e2c57259f0523cdf9fbae527d Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 25 Jul 2023 12:21:23 -0400 Subject: [PATCH] Avoid A003 violations for explicitly overridden methods (#6076) ## 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. --- .../test/fixtures/flake8_builtins/A003.py | 12 +++++++ .../src/checkers/ast/analyze/statement.rs | 3 +- .../rules/builtin_attribute_shadowing.rs | 35 +++++++++++++++++-- ..._flake8_builtins__tests__A003_A003.py.snap | 9 +++++ ...sts__A003_A003.py_builtins_ignorelist.snap | 9 +++++ 5 files changed, 65 insertions(+), 3 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_builtins/A003.py b/crates/ruff/resources/test/fixtures/flake8_builtins/A003.py index d2eaa3e393d4e..66d83c229d72f 100644 --- a/crates/ruff/resources/test/fixtures/flake8_builtins/A003.py +++ b/crates/ruff/resources/test/fixtures/flake8_builtins/A003.py @@ -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 diff --git a/crates/ruff/src/checkers/ast/analyze/statement.rs b/crates/ruff/src/checkers/ast/analyze/statement.rs index 5765aa8193742..958a9dcd1defd 100644 --- a/crates/ruff/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff/src/checkers/ast/analyze/statement.rs @@ -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(), ); } diff --git a/crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs b/crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs index aa686b2595aa0..353e92f5060c8 100644 --- a/crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs +++ b/crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs @@ -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; @@ -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 @@ -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 @@ -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(), diff --git a/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py.snap b/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py.snap index 3a96ef9104e50..5da7f00304905 100644 --- a/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py.snap +++ b/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py.snap @@ -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 + | + diff --git a/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py_builtins_ignorelist.snap b/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py_builtins_ignorelist.snap index ebdf4e0fdf422..7c86544588bad 100644 --- a/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py_builtins_ignorelist.snap +++ b/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py_builtins_ignorelist.snap @@ -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 + | +