From e53652779da6d8aeade4368f9465a4358400acb7 Mon Sep 17 00:00:00 2001 From: Jonathan Plasse <13716151+JonathanPlasse@users.noreply.github.com> Date: Sun, 19 Feb 2023 15:21:33 +0100 Subject: [PATCH] Avoid raising `B027` violations in `.pyi` files (#3016) --- .../test/fixtures/flake8_bugbear/B027.pyi | 101 ++++++++++++++++++ crates/ruff/src/checkers/ast.rs | 46 ++++---- crates/ruff/src/resolver.rs | 5 + crates/ruff/src/rules/flake8_bugbear/mod.rs | 1 + ..._flake8_bugbear__tests__B027_B027.pyi.snap | 6 ++ crates/ruff/src/rules/isort/track.rs | 3 +- 6 files changed, 141 insertions(+), 21 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/flake8_bugbear/B027.pyi create mode 100644 crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B027_B027.pyi.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_bugbear/B027.pyi b/crates/ruff/resources/test/fixtures/flake8_bugbear/B027.pyi new file mode 100644 index 0000000000000..d7d563c36550e --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_bugbear/B027.pyi @@ -0,0 +1,101 @@ +""" +Should emit: +B027 - on lines 13, 16, 19, 23 +""" +import abc +from abc import ABC +from abc import abstractmethod, abstractproperty +from abc import abstractmethod as notabstract +from abc import abstractproperty as notabstract_property + + +class AbstractClass(ABC): + def empty_1(self): # error + ... + + def empty_2(self): # error + pass + + def empty_3(self): # error + """docstring""" + ... + + def empty_4(self): # error + """multiple ellipsis/pass""" + ... + pass + ... + pass + + @notabstract + def abstract_0(self): + ... + + @abstractmethod + def abstract_1(self): + ... + + @abstractmethod + def abstract_2(self): + pass + + @abc.abstractmethod + def abstract_3(self): + ... + + @abc.abstractproperty + def abstract_4(self): + ... + + @abstractproperty + def abstract_5(self): + ... + + @notabstract_property + def abstract_6(self): + ... + + def body_1(self): + print("foo") + ... + + def body_2(self): + self.body_1() + + +class NonAbstractClass: + def empty_1(self): # safe + ... + + def empty_2(self): # safe + pass + + +# ignore @overload, fixes issue #304 +# ignore overload with other imports, fixes #308 +import typing +import typing as t +import typing as anything +from typing import Union, overload + + +class AbstractClass(ABC): + @overload + def empty_1(self, foo: str): + ... + + @typing.overload + def empty_1(self, foo: int): + ... + + @t.overload + def empty_1(self, foo: list): + ... + + @anything.overload + def empty_1(self, foo: float): + ... + + @abstractmethod + def empty_1(self, foo: Union[str, int, list, float]): + ... diff --git a/crates/ruff/src/checkers/ast.rs b/crates/ruff/src/checkers/ast.rs index ac732166e80c7..717cee2c0951c 100644 --- a/crates/ruff/src/checkers/ast.rs +++ b/crates/ruff/src/checkers/ast.rs @@ -32,6 +32,7 @@ use crate::ast::visitor::{walk_excepthandler, Visitor}; use crate::ast::{branch_detection, cast, helpers, operations, typing, visitor}; use crate::docstrings::definition::{Definition, DefinitionKind, Docstring, Documentable}; use crate::registry::{Diagnostic, Rule}; +use crate::resolver::is_interface_definition_path; use crate::rules::{ flake8_2020, flake8_annotations, flake8_bandit, flake8_blind_except, flake8_boolean_trap, flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_datetimez, flake8_debugger, @@ -58,6 +59,7 @@ pub struct Checker<'a> { pub(crate) path: &'a Path, module_path: Option>, package: Option<&'a Path>, + is_interface_definition: bool, autofix: flags::Autofix, noqa: flags::Noqa, pub(crate) settings: &'a Settings, @@ -126,6 +128,7 @@ impl<'a> Checker<'a> { style: &'a Stylist, indexer: &'a Indexer, ) -> Checker<'a> { + let is_interface_definition = is_interface_definition_path(path); Checker { settings, noqa_line_for, @@ -134,6 +137,7 @@ impl<'a> Checker<'a> { path, package, module_path, + is_interface_definition, locator, stylist: style, indexer, @@ -173,7 +177,7 @@ impl<'a> Checker<'a> { in_type_checking_block: false, seen_import_boundary: false, futures_allowed: true, - annotations_future_enabled: path.extension().map_or(false, |ext| ext == "pyi"), + annotations_future_enabled: is_interface_definition, except_handlers: vec![], // Check-specific state. flake8_bugbear_seen: vec![], @@ -835,18 +839,20 @@ where flake8_bugbear::rules::useless_expression(self, body); } - if self - .settings - .rules - .enabled(&Rule::AbstractBaseClassWithoutAbstractMethod) - || self + if !self.is_interface_definition { + if self .settings .rules - .enabled(&Rule::EmptyMethodWithoutAbstractDecorator) - { - flake8_bugbear::rules::abstract_base_class( - self, stmt, name, bases, keywords, body, - ); + .enabled(&Rule::AbstractBaseClassWithoutAbstractMethod) + || self + .settings + .rules + .enabled(&Rule::EmptyMethodWithoutAbstractDecorator) + { + flake8_bugbear::rules::abstract_base_class( + self, stmt, name, bases, keywords, body, + ); + } } if self @@ -1769,8 +1775,8 @@ where } } - if self.settings.rules.enabled(&Rule::PrefixTypeParams) { - if self.path.extension().map_or(false, |ext| ext == "pyi") { + if self.is_interface_definition { + if self.settings.rules.enabled(&Rule::PrefixTypeParams) { flake8_pyi::rules::prefix_type_params(self, value, targets); } } @@ -3209,13 +3215,13 @@ where flake8_simplify::rules::yoda_conditions(self, expr, left, ops, comparators); } - if self - .settings - .rules - .enabled(&Rule::UnrecognizedPlatformCheck) - || self.settings.rules.enabled(&Rule::UnrecognizedPlatformName) - { - if self.path.extension().map_or(false, |ext| ext == "pyi") { + if self.is_interface_definition { + if self + .settings + .rules + .enabled(&Rule::UnrecognizedPlatformCheck) + || self.settings.rules.enabled(&Rule::UnrecognizedPlatformName) + { flake8_pyi::rules::unrecognized_platform( self, expr, diff --git a/crates/ruff/src/resolver.rs b/crates/ruff/src/resolver.rs index be8b4804ba105..2cba43043a07f 100644 --- a/crates/ruff/src/resolver.rs +++ b/crates/ruff/src/resolver.rs @@ -203,6 +203,11 @@ fn is_python_path(path: &Path) -> bool { .map_or(false, |ext| ext == "py" || ext == "pyi") } +/// Return `true` if the `Path` appears to be that of a Python interface definition file (`.pyi`). +pub fn is_interface_definition_path(path: &Path) -> bool { + path.extension().map_or(false, |ext| ext == "pyi") +} + /// Return `true` if the `Entry` appears to be that of a Python file. pub fn is_python_entry(entry: &DirEntry) -> bool { is_python_path(entry.path()) diff --git a/crates/ruff/src/rules/flake8_bugbear/mod.rs b/crates/ruff/src/rules/flake8_bugbear/mod.rs index 6b03217507c11..b28073fd79b83 100644 --- a/crates/ruff/src/rules/flake8_bugbear/mod.rs +++ b/crates/ruff/src/rules/flake8_bugbear/mod.rs @@ -40,6 +40,7 @@ mod tests { #[test_case(Rule::DuplicateTryBlockException, Path::new("B025.py"); "B025")] #[test_case(Rule::StarArgUnpackingAfterKeywordArg, Path::new("B026.py"); "B026")] #[test_case(Rule::EmptyMethodWithoutAbstractDecorator, Path::new("B027.py"); "B027")] + #[test_case(Rule::EmptyMethodWithoutAbstractDecorator, Path::new("B027.pyi"); "B027_pyi")] #[test_case(Rule::RaiseWithoutFromInsideExcept, Path::new("B904.py"); "B904")] #[test_case(Rule::ZipWithoutExplicitStrict, Path::new("B905.py"); "B905")] fn rules(rule_code: Rule, path: &Path) -> Result<()> { diff --git a/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B027_B027.pyi.snap b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B027_B027.pyi.snap new file mode 100644 index 0000000000000..28b468d5d892a --- /dev/null +++ b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B027_B027.pyi.snap @@ -0,0 +1,6 @@ +--- +source: crates/ruff/src/rules/flake8_bugbear/mod.rs +expression: diagnostics +--- +[] + diff --git a/crates/ruff/src/rules/isort/track.rs b/crates/ruff/src/rules/isort/track.rs index 1633c29d2b384..befdd3c92bac3 100644 --- a/crates/ruff/src/rules/isort/track.rs +++ b/crates/ruff/src/rules/isort/track.rs @@ -9,6 +9,7 @@ use rustpython_parser::ast::{ use super::helpers; use crate::ast::visitor::Visitor; use crate::directives::IsortDirectives; +use crate::resolver::is_interface_definition_path; use crate::source_code::Locator; #[derive(Debug)] @@ -39,7 +40,7 @@ impl<'a> ImportTracker<'a> { Self { locator, directives, - pyi: path.extension().map_or(false, |ext| ext == "pyi"), + pyi: is_interface_definition_path(path), blocks: vec![Block::default()], split_index: 0, nested: false,