From 41df39982e8bed0aa03d678c23398bf8054d0f69 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 13 Apr 2024 15:54:47 +0100 Subject: [PATCH 1/9] Add a new method to check whether an `Expr` refers to a builtin symbol --- .../test/fixtures/flake8_pyi/PYI036.pyi | 7 +++++++ crates/ruff_python_semantic/src/model.rs | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI036.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI036.pyi index a49791aa1b0b8..3f58441c94f21 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI036.pyi +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI036.pyi @@ -73,3 +73,10 @@ class BadFive: class BadSix: def __exit__(self, typ, exc, tb, weird_extra_arg, extra_arg2 = None) -> None: ... # PYI036: Extra arg must have default async def __aexit__(self, typ, exc, tb, *, weird_extra_arg) -> None: ... # PYI036: kwargs must have default + + +def isolated_scope(): + from builtins import type as Type + + class ShouldNotError: + def __exit__(self, typ: Type[BaseException] | None, exc: BaseException | None, tb: TracebackType | None) -> None: ... diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index ffd407ad34996..eb1b7e6d21fe3 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -257,6 +257,23 @@ impl<'a> SemanticModel<'a> { .is_some_and(|binding| binding.kind.is_builtin()) } + /// Return `true` if `member` is a reference to `builtins.$target`, + /// i.e. either `object` (where `object` is not overridden in the global scope), + /// or `builtins.object` (where `builtins` is imported as a module at the top level) + pub fn match_builtin_expr(&self, expr: &Expr, symbol: &str) -> bool { + debug_assert!(!symbol.contains('.')); + if self.seen_module(Modules::BUILTINS) { + // slow path + self.resolve_qualified_name(expr) + .is_some_and(|qualified_name| { + matches!(qualified_name.segments(), ["builtins" | "", member] if *member == symbol) + }) + } else { + // fast(er) path + matches!(expr, Expr::Name(ast::ExprName {id, ..}) if id == symbol && self.is_builtin(symbol)) + } + } + /// Return `true` if `member` is an "available" symbol, i.e., a symbol that has not been bound /// in the current scope, or in any containing scope. pub fn is_available(&self, member: &str) -> bool { @@ -1138,6 +1155,7 @@ impl<'a> SemanticModel<'a> { pub fn add_module(&mut self, module: &str) { match module { "_typeshed" => self.seen.insert(Modules::TYPESHED), + "builtins" => self.seen.insert(Modules::BUILTINS), "collections" => self.seen.insert(Modules::COLLECTIONS), "dataclasses" => self.seen.insert(Modules::DATACLASSES), "datetime" => self.seen.insert(Modules::DATETIME), @@ -1708,6 +1726,7 @@ bitflags! { const TYPING_EXTENSIONS = 1 << 15; const TYPESHED = 1 << 16; const DATACLASSES = 1 << 17; + const BUILTINS = 1 << 18; } } From 096abddc3ad1a16d2f08e7d1538d5f1d95dffd44 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 13 Apr 2024 16:41:57 +0100 Subject: [PATCH 2/9] Use the new method across our linter rules --- .../test/fixtures/flake8_bugbear/B004.py | 9 ++++ .../test/fixtures/flake8_bugbear/B009_B010.py | 3 ++ .../test/fixtures/flake8_bugbear/B905.py | 4 ++ .../test/fixtures/flake8_pie/PIE808.py | 3 ++ .../test/fixtures/flake8_slots/SLOT001.py | 6 +++ .../test/fixtures/perflint/PERF101.py | 5 ++ .../test/fixtures/pycodestyle/E721.py | 5 ++ .../test/fixtures/pylint/bad_open_mode.py | 3 ++ .../test/fixtures/pylint/nan_comparison.py | 6 +++ .../test/fixtures/pylint/nested_min_max.py | 3 ++ .../test/fixtures/pyupgrade/UP004.py | 6 +++ .../resources/test/fixtures/ruff/RUF024.py | 2 + crates/ruff_linter/src/checkers/ast/mod.rs | 9 ++-- .../rules/open_sleep_or_subprocess_call.rs | 2 +- .../src/rules/flake8_bandit/helpers.rs | 4 +- .../rules/assert_raises_exception.rs | 4 +- .../rules/getattr_with_constant.rs | 8 +-- .../rules/setattr_with_constant.rs | 8 +-- .../rules/unreliable_callable_check.rs | 11 ++-- .../rules/zip_without_explicit_strict.rs | 27 +++++----- ...__flake8_bugbear__tests__B004_B004.py.snap | 29 +++++++++++ ...ke8_bugbear__tests__B009_B009_B010.py.snap | 18 +++++++ ...rules__flake8_bugbear__tests__B905.py.snap | 21 ++++++++ .../rules/locals_in_render_function.rs | 4 +- .../rules/logging_call.rs | 6 +-- .../rules/unnecessary_range_start.rs | 8 +-- ...__flake8_pie__tests__PIE808_PIE808.py.snap | 24 +++++++-- .../rules/flake8_print/rules/print_call.rs | 50 +++++++++---------- .../flake8_pyi/rules/exit_annotations.rs | 19 +------ .../rules/str_or_repr_defined_in_stub.rs | 8 +-- .../rules/unnecessary_type_union.rs | 17 ++----- .../flake8_simplify/rules/ast_bool_op.rs | 15 ++---- .../rules/open_file_with_context_handler.rs | 49 +++++++++--------- .../rules/no_slots_in_tuple_subclass.rs | 13 ++--- ...ake8_slots__tests__SLOT001_SLOT001.py.snap | 9 +++- .../perflint/rules/unnecessary_list_cast.rs | 6 +-- ...__perflint__tests__PERF101_PERF101.py.snap | 16 ++++++ .../pycodestyle/rules/type_comparison.rs | 20 ++------ ...les__pycodestyle__tests__E721_E721.py.snap | 9 +++- ...destyle__tests__preview__E721_E721.py.snap | 11 +++- .../pyflakes/rules/invalid_print_syntax.rs | 18 ++----- .../src/rules/pylint/rules/bad_open_mode.rs | 31 ++++++------ .../src/rules/pylint/rules/nan_comparison.rs | 10 +--- .../src/rules/pylint/rules/nested_min_max.rs | 14 ++---- .../rules/pylint/rules/non_slot_assignment.rs | 13 ++--- .../pylint/rules/property_with_parameters.rs | 8 ++- .../pylint/rules/repeated_isinstance_calls.rs | 5 +- .../rules/unnecessary_list_index_lookup.rs | 7 +-- ...int__tests__PLW0177_nan_comparison.py.snap | 8 +++ ...lint__tests__PLW1501_bad_open_mode.py.snap | 9 +++- ...int__tests__PLW3301_nested_min_max.py.snap | 18 +++++++ .../rules/pyupgrade/rules/os_error_alias.rs | 15 ++---- .../rules/pyupgrade/rules/replace_str_enum.rs | 2 +- .../pyupgrade/rules/timeout_error_alias.rs | 9 +--- .../pyupgrade/rules/type_of_primitive.rs | 13 ++--- .../rules/useless_object_inheritance.rs | 10 +--- ...er__rules__pyupgrade__tests__UP004.py.snap | 16 ++++++ .../rules/refurb/rules/print_empty_string.rs | 7 +-- .../ruff/rules/mutable_fromkeys_value.rs | 8 +-- ..._rules__ruff__tests__RUF024_RUF024.py.snap | 36 ++++++++++--- .../tryceratops/rules/raise_vanilla_args.rs | 5 +- .../tryceratops/rules/raise_vanilla_class.rs | 15 +++--- .../tryceratops/rules/raise_within_try.rs | 2 +- .../rules/type_check_without_type_error.rs | 2 +- 64 files changed, 426 insertions(+), 335 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py index 6a6e8c86e637f..e1972a7bd9ca5 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py @@ -6,6 +6,15 @@ def this_is_a_bug(): print("Ooh, callable! Or is it?") +def still_a_bug(): + import builtins + o = object() + if builtins.hasattr(o, "__call__"): + print("B U G") + if builtins.getattr(o, "__call__", False): + print("B U G") + + def this_is_fine(): o = object() if callable(o): diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B009_B010.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B009_B010.py index a889e7ef2f95c..c47538b55e1c2 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B009_B010.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B009_B010.py @@ -64,3 +64,6 @@ # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1739800901 getattr(self. registration.registry, '__name__') + +import builtins +builtins.getattr(foo, "bar") diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B905.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B905.py index 4d94cabbceea6..031773b92edf3 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B905.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B905.py @@ -23,3 +23,7 @@ # Errors (limited iterators). zip([1, 2, 3], repeat(1, 1)) zip([1, 2, 3], repeat(1, times=4)) + +import builtins +# Still an error even though it uses the qualified name +builtins.zip([1, 2, 3]) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE808.py b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE808.py index 6765a20137f6b..2ba1a545e6dba 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE808.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE808.py @@ -1,6 +1,9 @@ # PIE808 range(0, 10) +import builtins +builtins.range(0, 10) + # OK range(x, 10) range(-15, 10) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_slots/SLOT001.py b/crates/ruff_linter/resources/test/fixtures/flake8_slots/SLOT001.py index d0533df9d7b64..03966e90ea777 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_slots/SLOT001.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_slots/SLOT001.py @@ -19,3 +19,9 @@ class Bad(Tuple[str, int, float]): # SLOT001 class Good(Tuple[str, int, float]): # OK __slots__ = ("foo",) + + +import builtins + +class AlsoBad(builtins.tuple[int, int]): # SLOT001 + pass diff --git a/crates/ruff_linter/resources/test/fixtures/perflint/PERF101.py b/crates/ruff_linter/resources/test/fixtures/perflint/PERF101.py index e624930ff2e04..b9ef94dbc3ea1 100644 --- a/crates/ruff_linter/resources/test/fixtures/perflint/PERF101.py +++ b/crates/ruff_linter/resources/test/fixtures/perflint/PERF101.py @@ -80,3 +80,8 @@ for i in list(foo_list): # OK if True: del foo_list[i + 1] + +import builtins + +for i in builtins.list(nested_tuple): # PERF101 + pass diff --git a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E721.py b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E721.py index 9ce183b6adad8..ae467ccf02348 100644 --- a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E721.py +++ b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E721.py @@ -138,3 +138,8 @@ def type(): #: E721 dtype == float + +import builtins + +if builtins.type(res) == memoryview: # E721 + pass diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/bad_open_mode.py b/crates/ruff_linter/resources/test/fixtures/pylint/bad_open_mode.py index 3ede40b308cea..e6e02ae1da089 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/bad_open_mode.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/bad_open_mode.py @@ -32,3 +32,6 @@ pathlib.Path(NAME).open("rwx") # [bad-open-mode] pathlib.Path(NAME).open(mode="rwx") # [bad-open-mode] pathlib.Path(NAME).open("rwx", encoding="utf-8") # [bad-open-mode] + +import builtins +builtins.open(NAME, "Ua", encoding="utf-8") diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/nan_comparison.py b/crates/ruff_linter/resources/test/fixtures/pylint/nan_comparison.py index be3b5d7f14f84..e9ef56c1304f5 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/nan_comparison.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/nan_comparison.py @@ -47,6 +47,12 @@ if y == npy_nan: pass +import builtins + +# PLW0117 +if x == builtins.float("nan"): + pass + # OK if math.isnan(x): pass diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/nested_min_max.py b/crates/ruff_linter/resources/test/fixtures/pylint/nested_min_max.py index 4857a873ba8af..682d32ef017e4 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/nested_min_max.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/nested_min_max.py @@ -39,3 +39,6 @@ # Starred argument should be copied as it is. max(1, max(*a)) + +import builtins +builtins.min(1, min(2, 3)) diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP004.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP004.py index 723733f9385ed..c86d29ff22316 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP004.py +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP004.py @@ -152,3 +152,9 @@ class A(object): class B(object): ... + + +import builtins + +class Unusual(builtins.object): + ... diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF024.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF024.py index 83875f4714d6a..3be5e56fc41c4 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF024.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF024.py @@ -12,6 +12,8 @@ dict.fromkeys(pierogi_fillings, set()) dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) dict.fromkeys(pierogi_fillings, dict()) +import builtins +builtins.dict.fromkeys(pierogi_fillings, dict()) # Okay. dict.fromkeys(pierogi_fillings) diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 86b4f54b6782f..aa58f24972c04 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -786,13 +786,13 @@ impl<'a> Visitor<'a> for Checker<'a> { for type_ in extract_handled_exceptions(handlers) { if let Some(qualified_name) = self.semantic.resolve_qualified_name(type_) { match qualified_name.segments() { - ["", "NameError"] => { + ["" | "builtins", "NameError"] => { handled_exceptions |= Exceptions::NAME_ERROR; } - ["", "ModuleNotFoundError"] => { + ["" | "builtins", "ModuleNotFoundError"] => { handled_exceptions |= Exceptions::MODULE_NOT_FOUND_ERROR; } - ["", "ImportError"] => { + ["" | "builtins", "ImportError"] => { handled_exceptions |= Exceptions::IMPORT_ERROR; } _ => {} @@ -1125,7 +1125,8 @@ impl<'a> Visitor<'a> for Checker<'a> { ] ) { Some(typing::Callable::MypyExtension) - } else if matches!(qualified_name.segments(), ["", "bool"]) { + } else if matches!(qualified_name.segments(), ["" | "builtins", "bool"]) + { Some(typing::Callable::Bool) } else { None diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/open_sleep_or_subprocess_call.rs b/crates/ruff_linter/src/rules/flake8_async/rules/open_sleep_or_subprocess_call.rs index f85383e6468fd..389a39d370a4a 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/open_sleep_or_subprocess_call.rs +++ b/crates/ruff_linter/src/rules/flake8_async/rules/open_sleep_or_subprocess_call.rs @@ -63,7 +63,7 @@ fn is_open_sleep_or_subprocess_call(func: &Expr, semantic: &SemanticModel) -> bo .is_some_and(|qualified_name| { matches!( qualified_name.segments(), - ["", "open"] + ["" | "builtins", "open"] | ["time", "sleep"] | [ "subprocess", diff --git a/crates/ruff_linter/src/rules/flake8_bandit/helpers.rs b/crates/ruff_linter/src/rules/flake8_bandit/helpers.rs index 53c7e2c039433..8db1deeff49f8 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/helpers.rs @@ -28,7 +28,7 @@ pub(super) fn is_untyped_exception(type_: Option<&Expr>, semantic: &SemanticMode .is_some_and(|qualified_name| { matches!( qualified_name.segments(), - ["", "Exception" | "BaseException"] + ["" | "builtins", "Exception" | "BaseException"] ) }) }) @@ -38,7 +38,7 @@ pub(super) fn is_untyped_exception(type_: Option<&Expr>, semantic: &SemanticMode .is_some_and(|qualified_name| { matches!( qualified_name.segments(), - ["", "Exception" | "BaseException"] + ["" | "builtins", "Exception" | "BaseException"] ) }) } diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs index ee2595ae283ea..200fddc654fb5 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs @@ -100,8 +100,8 @@ pub(crate) fn assert_raises_exception(checker: &mut Checker, items: &[WithItem]) .semantic() .resolve_qualified_name(arg) .and_then(|qualified_name| match qualified_name.segments() { - ["", "Exception"] => Some(ExceptionKind::Exception), - ["", "BaseException"] => Some(ExceptionKind::BaseException), + ["" | "builtins", "Exception"] => Some(ExceptionKind::Exception), + ["" | "builtins", "BaseException"] => Some(ExceptionKind::BaseException), _ => None, }) else { diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs index 9ef089807f6e2..cc6d6e7bc5522 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs @@ -54,12 +54,9 @@ pub(crate) fn getattr_with_constant( func: &Expr, args: &[Expr], ) { - let Expr::Name(ast::ExprName { id, .. }) = func else { + if !checker.semantic().match_builtin_expr(func, "getattr") { return; }; - if id != "getattr" { - return; - } let [obj, arg] = args else { return; }; @@ -75,9 +72,6 @@ pub(crate) fn getattr_with_constant( if is_mangled_private(value.to_str()) { return; } - if !checker.semantic().is_builtin("getattr") { - return; - } let mut diagnostic = Diagnostic::new(GetAttrWithConstant, expr.range()); diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs index e31d0cdc1a781..10dff69637117 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs @@ -68,10 +68,7 @@ pub(crate) fn setattr_with_constant( func: &Expr, args: &[Expr], ) { - let Expr::Name(ast::ExprName { id, .. }) = func else { - return; - }; - if id != "setattr" { + if !checker.semantic().match_builtin_expr(func, "setattr") { return; } let [obj, name, value] = args else { @@ -89,9 +86,6 @@ pub(crate) fn setattr_with_constant( if is_mangled_private(name.to_str()) { return; } - if !checker.semantic().is_builtin("setattr") { - return; - } // We can only replace a `setattr` call (which is an `Expr`) with an assignment // (which is a `Stmt`) if the `Expr` is already being used as a `Stmt` diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs index a66671048a41d..5419cc6e44456 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs @@ -1,7 +1,6 @@ -use ruff_python_ast::{self as ast, Expr}; - use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Expr}; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -58,12 +57,12 @@ pub(crate) fn unreliable_callable_check( func: &Expr, args: &[Expr], ) { - let Expr::Name(ast::ExprName { id, .. }) = func else { + let Some(qualified_name) = checker.semantic().resolve_qualified_name(func) else { return; }; - if !matches!(id.as_str(), "hasattr" | "getattr") { + let ["" | "builtins", function @ ("hasattr" | "getattr")] = qualified_name.segments() else { return; - } + }; let [obj, attr, ..] = args else { return; }; @@ -75,7 +74,7 @@ pub(crate) fn unreliable_callable_check( } let mut diagnostic = Diagnostic::new(UnreliableCallableCheck, expr.range()); - if id == "hasattr" { + if *function == "hasattr" { if checker.semantic().is_builtin("callable") { diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( format!("callable({})", checker.locator().slice(obj)), diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs index f2514b7d42d70..21833151d3db1 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs @@ -52,18 +52,16 @@ impl AlwaysFixableViolation for ZipWithoutExplicitStrict { /// B905 pub(crate) fn zip_without_explicit_strict(checker: &mut Checker, call: &ast::ExprCall) { - if let Expr::Name(ast::ExprName { id, .. }) = call.func.as_ref() { - if id == "zip" - && checker.semantic().is_builtin("zip") - && call.arguments.find_keyword("strict").is_none() - && !call - .arguments - .args - .iter() - .any(|arg| is_infinite_iterator(arg, checker.semantic())) - { - let mut diagnostic = Diagnostic::new(ZipWithoutExplicitStrict, call.range()); - diagnostic.set_fix(Fix::applicable_edit( + if checker.semantic().match_builtin_expr(&call.func, "zip") + && call.arguments.find_keyword("strict").is_none() + && !call + .arguments + .args + .iter() + .any(|arg| is_infinite_iterator(arg, checker.semantic())) + { + checker.diagnostics.push( + Diagnostic::new(ZipWithoutExplicitStrict, call.range()).with_fix(Fix::applicable_edit( add_argument( "strict=False", &call.arguments, @@ -81,9 +79,8 @@ pub(crate) fn zip_without_explicit_strict(checker: &mut Checker, call: &ast::Exp } else { Applicability::Safe }, - )); - checker.diagnostics.push(diagnostic); - } + )), + ); } } diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap index 69af59e452834..eb301b1c86492 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap @@ -31,4 +31,33 @@ B004.py:5:8: B004 Using `hasattr(x, "__call__")` to test if x is callable is unr | = help: Replace with `callable()` +B004.py:12:8: B004 [*] Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results. + | +10 | import builtins +11 | o = object() +12 | if builtins.hasattr(o, "__call__"): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B004 +13 | print("B U G") +14 | if builtins.getattr(o, "__call__", False): + | + = help: Replace with `callable()` +ℹ Safe fix +9 9 | def still_a_bug(): +10 10 | import builtins +11 11 | o = object() +12 |- if builtins.hasattr(o, "__call__"): + 12 |+ if callable(o): +13 13 | print("B U G") +14 14 | if builtins.getattr(o, "__call__", False): +15 15 | print("B U G") + +B004.py:14:8: B004 Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results. + | +12 | if builtins.hasattr(o, "__call__"): +13 | print("B U G") +14 | if builtins.getattr(o, "__call__", False): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B004 +15 | print("B U G") + | + = help: Replace with `callable()` diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B009_B009_B010.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B009_B009_B010.py.snap index 6e6b51e423dd8..fc00bb4ef9792 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B009_B009_B010.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B009_B009_B010.py.snap @@ -342,6 +342,8 @@ B009_B010.py:65:1: B009 [*] Do not call `getattr` with a constant attribute valu 65 | / getattr(self. 66 | | registration.registry, '__name__') | |_____________________________________^ B009 +67 | +68 | import builtins | = help: Replace `getattr` with attribute access @@ -353,5 +355,21 @@ B009_B010.py:65:1: B009 [*] Do not call `getattr` with a constant attribute valu 66 |- registration.registry, '__name__') 65 |+(self. 66 |+ registration.registry).__name__ +67 67 | +68 68 | import builtins +69 69 | builtins.getattr(foo, "bar") +B009_B010.py:69:1: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access. + | +68 | import builtins +69 | builtins.getattr(foo, "bar") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B009 + | + = help: Replace `getattr` with attribute access +ℹ Safe fix +66 66 | registration.registry, '__name__') +67 67 | +68 68 | import builtins +69 |-builtins.getattr(foo, "bar") + 69 |+foo.bar diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B905.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B905.py.snap index 6e51121edc9f4..2e0c24059003d 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B905.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B905.py.snap @@ -162,6 +162,8 @@ B905.py:24:1: B905 [*] `zip()` without an explicit `strict=` parameter 24 |-zip([1, 2, 3], repeat(1, 1)) 24 |+zip([1, 2, 3], repeat(1, 1), strict=False) 25 25 | zip([1, 2, 3], repeat(1, times=4)) +26 26 | +27 27 | import builtins B905.py:25:1: B905 [*] `zip()` without an explicit `strict=` parameter | @@ -169,6 +171,8 @@ B905.py:25:1: B905 [*] `zip()` without an explicit `strict=` parameter 24 | zip([1, 2, 3], repeat(1, 1)) 25 | zip([1, 2, 3], repeat(1, times=4)) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B905 +26 | +27 | import builtins | = help: Add explicit `strict=False` @@ -178,5 +182,22 @@ B905.py:25:1: B905 [*] `zip()` without an explicit `strict=` parameter 24 24 | zip([1, 2, 3], repeat(1, 1)) 25 |-zip([1, 2, 3], repeat(1, times=4)) 25 |+zip([1, 2, 3], repeat(1, times=4), strict=False) +26 26 | +27 27 | import builtins +28 28 | # Still an error even though it uses the qualified name +B905.py:29:1: B905 [*] `zip()` without an explicit `strict=` parameter + | +27 | import builtins +28 | # Still an error even though it uses the qualified name +29 | builtins.zip([1, 2, 3]) + | ^^^^^^^^^^^^^^^^^^^^^^^ B905 + | + = help: Add explicit `strict=False` +ℹ Safe fix +26 26 | +27 27 | import builtins +28 28 | # Still an error even though it uses the qualified name +29 |-builtins.zip([1, 2, 3]) + 29 |+builtins.zip([1, 2, 3], strict=False) diff --git a/crates/ruff_linter/src/rules/flake8_django/rules/locals_in_render_function.rs b/crates/ruff_linter/src/rules/flake8_django/rules/locals_in_render_function.rs index 9785a2cb9426a..2b60a5c2ec35d 100644 --- a/crates/ruff_linter/src/rules/flake8_django/rules/locals_in_render_function.rs +++ b/crates/ruff_linter/src/rules/flake8_django/rules/locals_in_render_function.rs @@ -73,7 +73,5 @@ fn is_locals_call(expr: &Expr, semantic: &SemanticModel) -> bool { let Expr::Call(ast::ExprCall { func, .. }) = expr else { return false; }; - semantic - .resolve_qualified_name(func) - .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", "locals"])) + semantic.match_builtin_expr(func, "locals") } diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs b/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs index 124759b1f6d59..2f6801f9f922e 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs +++ b/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs @@ -108,11 +108,7 @@ fn check_log_record_attr_clash(checker: &mut Checker, extra: &Keyword) { arguments: Arguments { keywords, .. }, .. }) => { - if checker - .semantic() - .resolve_qualified_name(func) - .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", "dict"])) - { + if checker.semantic().match_builtin_expr(func, "dict") { for keyword in keywords.iter() { if let Some(attr) = &keyword.arg { if is_reserved_attr(attr) { diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_range_start.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_range_start.rs index e158b75ebe555..b2d6903ed7d3e 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_range_start.rs +++ b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_range_start.rs @@ -44,13 +44,7 @@ impl AlwaysFixableViolation for UnnecessaryRangeStart { /// PIE808 pub(crate) fn unnecessary_range_start(checker: &mut Checker, call: &ast::ExprCall) { // Verify that the call is to the `range` builtin. - let Expr::Name(ast::ExprName { id, .. }) = call.func.as_ref() else { - return; - }; - if id != "range" { - return; - }; - if !checker.semantic().is_builtin("range") { + if !checker.semantic().match_builtin_expr(&call.func, "range") { return; }; diff --git a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE808_PIE808.py.snap b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE808_PIE808.py.snap index f51359beb7600..91c5932b028c7 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE808_PIE808.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE808_PIE808.py.snap @@ -7,7 +7,7 @@ PIE808.py:2:7: PIE808 [*] Unnecessary `start` argument in `range` 2 | range(0, 10) | ^ PIE808 3 | -4 | # OK +4 | import builtins | = help: Remove `start` argument @@ -16,7 +16,25 @@ PIE808.py:2:7: PIE808 [*] Unnecessary `start` argument in `range` 2 |-range(0, 10) 2 |+range(10) 3 3 | -4 4 | # OK -5 5 | range(x, 10) +4 4 | import builtins +5 5 | builtins.range(0, 10) +PIE808.py:5:16: PIE808 [*] Unnecessary `start` argument in `range` + | +4 | import builtins +5 | builtins.range(0, 10) + | ^ PIE808 +6 | +7 | # OK + | + = help: Remove `start` argument +ℹ Safe fix +2 2 | range(0, 10) +3 3 | +4 4 | import builtins +5 |-builtins.range(0, 10) + 5 |+builtins.range(10) +6 6 | +7 7 | # OK +8 8 | range(x, 10) diff --git a/crates/ruff_linter/src/rules/flake8_print/rules/print_call.rs b/crates/ruff_linter/src/rules/flake8_print/rules/print_call.rs index 6f7bf3ccb131f..50cfcf825500c 100644 --- a/crates/ruff_linter/src/rules/flake8_print/rules/print_call.rs +++ b/crates/ruff_linter/src/rules/flake8_print/rules/print_call.rs @@ -97,37 +97,32 @@ impl Violation for PPrint { /// T201, T203 pub(crate) fn print_call(checker: &mut Checker, call: &ast::ExprCall) { - let mut diagnostic = { - let qualified_name = checker.semantic().resolve_qualified_name(&call.func); - if qualified_name - .as_ref() - .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", "print"])) - { + let semantic = checker.semantic(); + + let Some(qualified_name) = semantic.resolve_qualified_name(&call.func) else { + return; + }; + + let mut diagnostic = match qualified_name.segments() { + ["" | "builtins", "print"] => { // If the print call has a `file=` argument (that isn't `None`, `"sys.stdout"`, // or `"sys.stderr"`), don't trigger T201. if let Some(keyword) = call.arguments.find_keyword("file") { if !keyword.value.is_none_literal_expr() { - if checker - .semantic() - .resolve_qualified_name(&keyword.value) - .map_or(true, |qualified_name| { - qualified_name.segments() != ["sys", "stdout"] - && qualified_name.segments() != ["sys", "stderr"] - }) - { + if semantic.resolve_qualified_name(&keyword.value).map_or( + true, + |qualified_name| { + !matches!(qualified_name.segments(), ["sys", "stdout" | "stderr"]) + }, + ) { return; } } } Diagnostic::new(Print, call.func.range()) - } else if qualified_name - .as_ref() - .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["pprint", "pprint"])) - { - Diagnostic::new(PPrint, call.func.range()) - } else { - return; } + ["pprint", "pprint"] => Diagnostic::new(PPrint, call.func.range()), + _ => return, }; if !checker.enabled(diagnostic.kind.rule()) { @@ -135,13 +130,14 @@ pub(crate) fn print_call(checker: &mut Checker, call: &ast::ExprCall) { } // Remove the `print`, if it's a standalone statement. - if checker.semantic().current_expression_parent().is_none() { - let statement = checker.semantic().current_statement(); - let parent = checker.semantic().current_statement_parent(); + if semantic.current_expression_parent().is_none() { + let statement = semantic.current_statement(); + let parent = semantic.current_statement_parent(); let edit = delete_stmt(statement, parent, checker.locator(), checker.indexer()); - diagnostic.set_fix(Fix::unsafe_edit(edit).isolate(Checker::isolation( - checker.semantic().current_statement_parent_id(), - ))); + diagnostic.set_fix( + Fix::unsafe_edit(edit) + .isolate(Checker::isolation(semantic.current_statement_parent_id())), + ); } checker.diagnostics.push(diagnostic); diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs index 3d27a44853f7f..393f8d234e29b 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs @@ -324,15 +324,7 @@ fn is_object_or_unused(expr: &Expr, semantic: &SemanticModel) -> bool { /// Return `true` if the [`Expr`] is `BaseException`. fn is_base_exception(expr: &Expr, semantic: &SemanticModel) -> bool { - semantic - .resolve_qualified_name(expr) - .as_ref() - .is_some_and(|qualified_name| { - matches!( - qualified_name.segments(), - ["" | "builtins", "BaseException"] - ) - }) + semantic.match_builtin_expr(expr, "BaseException") } /// Return `true` if the [`Expr`] is the `types.TracebackType` type. @@ -351,14 +343,7 @@ fn is_base_exception_type(expr: &Expr, semantic: &SemanticModel) -> bool { return false; }; - if semantic.match_typing_expr(value, "Type") - || semantic - .resolve_qualified_name(value) - .as_ref() - .is_some_and(|qualified_name| { - matches!(qualified_name.segments(), ["" | "builtins", "type"]) - }) - { + if semantic.match_typing_expr(value, "Type") || semantic.match_builtin_expr(value, "type") { is_base_exception(slice, semantic) } else { false diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/str_or_repr_defined_in_stub.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/str_or_repr_defined_in_stub.rs index d8a3f3d3e9dd0..051e702606247 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/str_or_repr_defined_in_stub.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/str_or_repr_defined_in_stub.rs @@ -78,13 +78,7 @@ pub(crate) fn str_or_repr_defined_in_stub(checker: &mut Checker, stmt: &Stmt) { return; } - if checker - .semantic() - .resolve_qualified_name(returns) - .map_or(true, |qualified_name| { - !matches!(qualified_name.segments(), ["" | "builtins", "str"]) - }) - { + if !checker.semantic().match_builtin_expr(returns, "str") { return; } diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs index 4f1ba2be98553..fe9c6779ebc3f 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs @@ -71,25 +71,18 @@ pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr) let mut type_exprs = Vec::new(); let mut other_exprs = Vec::new(); - let mut collect_type_exprs = |expr: &'a Expr, _parent: &'a Expr| { - let subscript = expr.as_subscript_expr(); - - if subscript.is_none() { - other_exprs.push(expr); - } else { - let unwrapped = subscript.unwrap(); + let mut collect_type_exprs = |expr: &'a Expr, _parent: &'a Expr| match expr { + Expr::Subscript(subscript) => { if checker .semantic() - .resolve_qualified_name(unwrapped.value.as_ref()) - .is_some_and(|qualified_name| { - matches!(qualified_name.segments(), ["" | "builtins", "type"]) - }) + .match_builtin_expr(&subscript.value, "type") { - type_exprs.push(unwrapped.slice.as_ref()); + type_exprs.push(&*subscript.slice); } else { other_exprs.push(expr); } } + _ => other_exprs.push(expr), }; traverse_union(&mut collect_type_exprs, checker.semantic(), union); diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/ast_bool_op.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/ast_bool_op.rs index 04ed8b05a5dbd..6be5de41dfab9 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/rules/ast_bool_op.rs +++ b/crates/ruff_linter/src/rules/flake8_simplify/rules/ast_bool_op.rs @@ -301,7 +301,7 @@ pub(crate) fn is_same_expr<'a>(a: &'a Expr, b: &'a Expr) -> Option<&'a str> { /// If `call` is an `isinstance()` call, return its target. fn isinstance_target<'a>(call: &'a Expr, semantic: &'a SemanticModel) -> Option<&'a Expr> { // Verify that this is an `isinstance` call. - let Expr::Call(ast::ExprCall { + let ast::ExprCall { func, arguments: Arguments { @@ -310,23 +310,14 @@ fn isinstance_target<'a>(call: &'a Expr, semantic: &'a SemanticModel) -> Option< range: _, }, range: _, - }) = &call - else { - return None; - }; + } = call.as_call_expr()?; if args.len() != 2 { return None; } if !keywords.is_empty() { return None; } - let Expr::Name(ast::ExprName { id: func_name, .. }) = func.as_ref() else { - return None; - }; - if func_name != "isinstance" { - return None; - } - if !semantic.is_builtin("isinstance") { + if !semantic.match_builtin_expr(func, "isinstance") { return None; } diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs index 148ed9187bad0..a847fe52e9a72 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs +++ b/crates/ruff_linter/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs @@ -113,26 +113,25 @@ fn match_exit_stack(semantic: &SemanticModel) -> bool { } /// Return `true` if `func` is the builtin `open` or `pathlib.Path(...).open`. -fn is_open(checker: &mut Checker, func: &Expr) -> bool { - match func { - // pathlib.Path(...).open() - Expr::Attribute(ast::ExprAttribute { attr, value, .. }) if attr.as_str() == "open" => { - match value.as_ref() { - Expr::Call(ast::ExprCall { func, .. }) => checker - .semantic() - .resolve_qualified_name(func) - .is_some_and(|qualified_name| { - matches!(qualified_name.segments(), ["pathlib", "Path"]) - }), - _ => false, - } - } - // open(...) - Expr::Name(ast::ExprName { id, .. }) => { - id.as_str() == "open" && checker.semantic().is_builtin("open") - } - _ => false, +fn is_open(semantic: &SemanticModel, func: &Expr) -> bool { + // open(...) + if semantic.match_builtin_expr(func, "open") { + return true; } + + // pathlib.Path(...).open() + let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func else { + return false; + }; + if attr != "open" { + return false; + } + let Expr::Call(ast::ExprCall { func, .. }) = &**value else { + return false; + }; + semantic + .resolve_qualified_name(func) + .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["pathlib", "Path"])) } /// Return `true` if the current expression is followed by a `close` call. @@ -161,27 +160,29 @@ fn is_closed(semantic: &SemanticModel) -> bool { /// SIM115 pub(crate) fn open_file_with_context_handler(checker: &mut Checker, func: &Expr) { - if !is_open(checker, func) { + let semantic = checker.semantic(); + + if !is_open(semantic, func) { return; } // Ex) `open("foo.txt").close()` - if is_closed(checker.semantic()) { + if is_closed(semantic) { return; } // Ex) `with open("foo.txt") as f: ...` - if checker.semantic().current_statement().is_with_stmt() { + if semantic.current_statement().is_with_stmt() { return; } // Ex) `with contextlib.ExitStack() as exit_stack: ...` - if match_exit_stack(checker.semantic()) { + if match_exit_stack(semantic) { return; } // Ex) `with contextlib.AsyncExitStack() as exit_stack: ...` - if match_async_exit_stack(checker.semantic()) { + if match_async_exit_stack(semantic) { return; } diff --git a/crates/ruff_linter/src/rules/flake8_slots/rules/no_slots_in_tuple_subclass.rs b/crates/ruff_linter/src/rules/flake8_slots/rules/no_slots_in_tuple_subclass.rs index 0c0c1462685ae..45eca0949acc0 100644 --- a/crates/ruff_linter/src/rules/flake8_slots/rules/no_slots_in_tuple_subclass.rs +++ b/crates/ruff_linter/src/rules/flake8_slots/rules/no_slots_in_tuple_subclass.rs @@ -55,16 +55,11 @@ pub(crate) fn no_slots_in_tuple_subclass(checker: &mut Checker, stmt: &Stmt, cla return; }; + let semantic = checker.semantic(); + if bases.iter().any(|base| { - checker - .semantic() - .resolve_qualified_name(map_subscript(base)) - .is_some_and(|qualified_name| { - matches!(qualified_name.segments(), ["" | "builtins", "tuple"]) - || checker - .semantic() - .match_typing_qualified_name(&qualified_name, "Tuple") - }) + let base = map_subscript(base); + semantic.match_builtin_expr(base, "tuple") || semantic.match_typing_expr(base, "Tuple") }) { if !has_slots(&class.body) { checker diff --git a/crates/ruff_linter/src/rules/flake8_slots/snapshots/ruff_linter__rules__flake8_slots__tests__SLOT001_SLOT001.py.snap b/crates/ruff_linter/src/rules/flake8_slots/snapshots/ruff_linter__rules__flake8_slots__tests__SLOT001_SLOT001.py.snap index 7aa7763e2a8cc..78d730a75e481 100644 --- a/crates/ruff_linter/src/rules/flake8_slots/snapshots/ruff_linter__rules__flake8_slots__tests__SLOT001_SLOT001.py.snap +++ b/crates/ruff_linter/src/rules/flake8_slots/snapshots/ruff_linter__rules__flake8_slots__tests__SLOT001_SLOT001.py.snap @@ -22,4 +22,11 @@ SLOT001.py:16:7: SLOT001 Subclasses of `tuple` should define `__slots__` 17 | pass | - +SLOT001.py:26:7: SLOT001 Subclasses of `tuple` should define `__slots__` + | +24 | import builtins +25 | +26 | class AlsoBad(builtins.tuple[int, int]): # SLOT001 + | ^^^^^^^ SLOT001 +27 | pass + | diff --git a/crates/ruff_linter/src/rules/perflint/rules/unnecessary_list_cast.rs b/crates/ruff_linter/src/rules/perflint/rules/unnecessary_list_cast.rs index 6cbcbc47937d2..d3fae55e3d566 100644 --- a/crates/ruff_linter/src/rules/perflint/rules/unnecessary_list_cast.rs +++ b/crates/ruff_linter/src/rules/perflint/rules/unnecessary_list_cast.rs @@ -69,11 +69,7 @@ pub(crate) fn unnecessary_list_cast(checker: &mut Checker, iter: &Expr, body: &[ return; }; - let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else { - return; - }; - - if !(id == "list" && checker.semantic().is_builtin("list")) { + if !checker.semantic().match_builtin_expr(func, "list") { return; } diff --git a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF101_PERF101.py.snap b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF101_PERF101.py.snap index d41a00b33eac7..bf33ecd3e3457 100644 --- a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF101_PERF101.py.snap +++ b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF101_PERF101.py.snap @@ -221,4 +221,20 @@ PERF101.py:69:10: PERF101 [*] Do not cast an iterable to `list` before iterating 71 71 | 72 72 | for i in list(foo_list): # OK +PERF101.py:86:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it + | +84 | import builtins +85 | +86 | for i in builtins.list(nested_tuple): # PERF101 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ PERF101 +87 | pass + | + = help: Remove `list()` cast +ℹ Safe fix +83 83 | +84 84 | import builtins +85 85 | +86 |-for i in builtins.list(nested_tuple): # PERF101 + 86 |+for i in nested_tuple: # PERF101 +87 87 | pass diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/type_comparison.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/type_comparison.rs index 2b7d46745103e..afe9624bfc164 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/type_comparison.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/type_comparison.rs @@ -76,11 +76,7 @@ fn deprecated_type_comparison(checker: &mut Checker, compare: &ast::ExprCompare) continue; }; - let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else { - continue; - }; - - if !(id == "type" && checker.semantic().is_builtin("type")) { + if !checker.semantic().match_builtin_expr(func, "type") { continue; } @@ -90,11 +86,7 @@ fn deprecated_type_comparison(checker: &mut Checker, compare: &ast::ExprCompare) func, arguments, .. }) => { // Ex) `type(obj) is type(1)` - let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else { - continue; - }; - - if id == "type" && checker.semantic().is_builtin("type") { + if checker.semantic().match_builtin_expr(func, "type") { // Allow comparison for types which are not obvious. if arguments .args @@ -189,13 +181,9 @@ fn is_type(expr: &Expr, semantic: &SemanticModel) -> bool { func, arguments, .. }) => { // Ex) `type(obj) == type(1)` - let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else { + if !semantic.match_builtin_expr(func, "type") { return false; - }; - - if !(id == "type" && semantic.is_builtin("type")) { - return false; - }; + } // Allow comparison for types which are not obvious. arguments diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E721_E721.py.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E721_E721.py.snap index 415b71d592217..a0592c9af8da7 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E721_E721.py.snap +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E721_E721.py.snap @@ -167,4 +167,11 @@ E721.py:117:12: E721 Do not compare types, use `isinstance()` 118 | ... | - +E721.py:144:4: E721 Do not compare types, use `isinstance()` + | +142 | import builtins +143 | +144 | if builtins.type(res) == memoryview: # E721 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E721 +145 | pass + | diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__preview__E721_E721.py.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__preview__E721_E721.py.snap index fc1d13521fdae..749cc427ed7ac 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__preview__E721_E721.py.snap +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__preview__E721_E721.py.snap @@ -134,6 +134,15 @@ E721.py:140:1: E721 Use `is` and `is not` for type comparisons, or `isinstance() 139 | #: E721 140 | dtype == float | ^^^^^^^^^^^^^^ E721 +141 | +142 | import builtins | - +E721.py:144:4: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks + | +142 | import builtins +143 | +144 | if builtins.type(res) == memoryview: # E721 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E721 +145 | pass + | diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/invalid_print_syntax.rs b/crates/ruff_linter/src/rules/pyflakes/rules/invalid_print_syntax.rs index 74acdca403167..767eb11c8b686 100644 --- a/crates/ruff_linter/src/rules/pyflakes/rules/invalid_print_syntax.rs +++ b/crates/ruff_linter/src/rules/pyflakes/rules/invalid_print_syntax.rs @@ -1,7 +1,6 @@ -use ruff_python_ast::{self as ast, Expr}; - use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::Expr; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -58,16 +57,9 @@ impl Violation for InvalidPrintSyntax { /// F633 pub(crate) fn invalid_print_syntax(checker: &mut Checker, left: &Expr) { - let Expr::Name(ast::ExprName { id, .. }) = &left else { - return; - }; - if id != "print" { - return; + if checker.semantic().match_builtin_expr(left, "print") { + checker + .diagnostics + .push(Diagnostic::new(InvalidPrintSyntax, left.range())); } - if !checker.semantic().is_builtin("print") { - return; - }; - checker - .diagnostics - .push(Diagnostic::new(InvalidPrintSyntax, left.range())); } diff --git a/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs b/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs index d60bb37deb7a7..5750cddf78969 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs @@ -85,23 +85,20 @@ enum Kind { /// If a function is a call to `open`, returns the kind of `open` call. fn is_open(func: &Expr, semantic: &SemanticModel) -> Option { - match func { - // Ex) `pathlib.Path(...).open(...)` - Expr::Attribute(ast::ExprAttribute { attr, value, .. }) if attr.as_str() == "open" => { - match value.as_ref() { - Expr::Call(ast::ExprCall { func, .. }) => semantic - .resolve_qualified_name(func) - .is_some_and(|qualified_name| { - matches!(qualified_name.segments(), ["pathlib", "Path"]) - }) - .then_some(Kind::Pathlib), - _ => None, - } - } - // Ex) `open(...)` - Expr::Name(ast::ExprName { id, .. }) => { - (id.as_str() == "open" && semantic.is_builtin("open")).then_some(Kind::Builtin) - } + // Ex) `open(...)` + if semantic.match_builtin_expr(func, "open") { + return Some(Kind::Builtin); + } + + // Ex) `pathlib.Path(...).open(...)` + let ast::ExprAttribute { attr, value, .. } = func.as_attribute_expr()?; + if attr != "open" { + return None; + } + let ast::ExprCall { func, .. } = value.as_call_expr()?; + let qualified_name = semantic.resolve_qualified_name(func)?; + match qualified_name.segments() { + ["pathlib", "Path"] => Some(Kind::Pathlib), _ => None, } } diff --git a/crates/ruff_linter/src/rules/pylint/rules/nan_comparison.rs b/crates/ruff_linter/src/rules/pylint/rules/nan_comparison.rs index a095adb76ec4f..da646b4dc88b8 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/nan_comparison.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/nan_comparison.rs @@ -100,14 +100,10 @@ fn is_nan_float(expr: &Expr, semantic: &SemanticModel) -> bool { return false; }; - let Expr::Name(ast::ExprName { id, .. }) = call.func.as_ref() else { + if !semantic.match_builtin_expr(&call.func, "float") { return false; }; - if id.as_str() != "float" { - return false; - } - if !call.arguments.keywords.is_empty() { return false; } @@ -127,9 +123,5 @@ fn is_nan_float(expr: &Expr, semantic: &SemanticModel) -> bool { return false; } - if !semantic.is_builtin("float") { - return false; - } - true } diff --git a/crates/ruff_linter/src/rules/pylint/rules/nested_min_max.rs b/crates/ruff_linter/src/rules/pylint/rules/nested_min_max.rs index be336eadae5c9..00000c0fd7c04 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/nested_min_max.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/nested_min_max.rs @@ -68,15 +68,11 @@ impl MinMax { if !keywords.is_empty() { return None; } - let Expr::Name(ast::ExprName { id, .. }) = func else { - return None; - }; - if id.as_str() == "min" && semantic.is_builtin("min") { - Some(MinMax::Min) - } else if id.as_str() == "max" && semantic.is_builtin("max") { - Some(MinMax::Max) - } else { - None + let qualified_name = semantic.resolve_qualified_name(func)?; + match qualified_name.segments() { + ["" | "builtins", "min"] => Some(MinMax::Min), + ["" | "builtins", "max"] => Some(MinMax::Max), + _ => None, } } } diff --git a/crates/ruff_linter/src/rules/pylint/rules/non_slot_assignment.rs b/crates/ruff_linter/src/rules/pylint/rules/non_slot_assignment.rs index bd2bf426bf2c3..27f3d4362b933 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/non_slot_assignment.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/non_slot_assignment.rs @@ -61,14 +61,15 @@ impl Violation for NonSlotAssignment { /// E0237 pub(crate) fn non_slot_assignment(checker: &mut Checker, class_def: &ast::StmtClassDef) { + let semantic = checker.semantic(); + // If the class inherits from another class (aside from `object`), then it's possible that // the parent class defines the relevant `__slots__`. - if !class_def.bases().iter().all(|base| { - checker - .semantic() - .resolve_qualified_name(base) - .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", "object"])) - }) { + if !class_def + .bases() + .iter() + .all(|base| semantic.match_builtin_expr(base, "object")) + { return; } diff --git a/crates/ruff_linter/src/rules/pylint/rules/property_with_parameters.rs b/crates/ruff_linter/src/rules/pylint/rules/property_with_parameters.rs index 42a7db35d28b9..01661edc6f7da 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/property_with_parameters.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/property_with_parameters.rs @@ -1,8 +1,6 @@ -use ruff_python_ast::{self as ast, Decorator, Expr, Parameters, Stmt}; - use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::identifier::Identifier; +use ruff_python_ast::{identifier::Identifier, Decorator, Parameters, Stmt}; use crate::checkers::ast::Checker; @@ -53,9 +51,10 @@ pub(crate) fn property_with_parameters( decorator_list: &[Decorator], parameters: &Parameters, ) { + let semantic = checker.semantic(); if !decorator_list .iter() - .any(|decorator| matches!(&decorator.expression, Expr::Name(ast::ExprName { id, .. }) if id == "property")) + .any(|decorator| semantic.match_builtin_expr(&decorator.expression, "property")) { return; } @@ -66,7 +65,6 @@ pub(crate) fn property_with_parameters( .chain(¶meters.kwonlyargs) .count() > 1 - && checker.semantic().is_builtin("property") { checker .diagnostics diff --git a/crates/ruff_linter/src/rules/pylint/rules/repeated_isinstance_calls.rs b/crates/ruff_linter/src/rules/pylint/rules/repeated_isinstance_calls.rs index 4ede23df5ca3d..b337c8c695a49 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/repeated_isinstance_calls.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/repeated_isinstance_calls.rs @@ -92,15 +92,12 @@ pub(crate) fn repeated_isinstance_calls( else { continue; }; - if !matches!(func.as_ref(), Expr::Name(ast::ExprName { id, .. }) if id == "isinstance") { + if !checker.semantic().match_builtin_expr(func, "isinstance") { continue; } let [obj, types] = &args[..] else { continue; }; - if !checker.semantic().is_builtin("isinstance") { - return; - } let (num_calls, matches) = obj_to_types .entry(obj.into()) .or_insert_with(|| (0, FxHashSet::default())); diff --git a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs index ee319ef621f88..62cb12b889c1f 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs @@ -125,12 +125,7 @@ fn enumerate_items<'a>( } = call_expr.as_call_expr()?; // Check that the function is the `enumerate` builtin. - if !semantic - .resolve_qualified_name(func.as_ref()) - .is_some_and(|qualified_name| { - matches!(qualified_name.segments(), ["builtins" | "", "enumerate"]) - }) - { + if !semantic.match_builtin_expr(func, "enumerate") { return None; } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0177_nan_comparison.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0177_nan_comparison.py.snap index 5630820c7aabb..a4f659c5668e6 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0177_nan_comparison.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0177_nan_comparison.py.snap @@ -80,3 +80,11 @@ nan_comparison.py:47:9: PLW0177 Comparing against a NaN value; use `np.isnan` in | ^^^^^^^ PLW0177 48 | pass | + +nan_comparison.py:53:9: PLW0177 Comparing against a NaN value; use `math.isnan` instead + | +52 | # PLW0117 +53 | if x == builtins.float("nan"): + | ^^^^^^^^^^^^^^^^^^^^^ PLW0177 +54 | pass + | diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1501_bad_open_mode.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1501_bad_open_mode.py.snap index 6e24d45dd749d..b029ad5b015d6 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1501_bad_open_mode.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1501_bad_open_mode.py.snap @@ -106,6 +106,13 @@ bad_open_mode.py:34:25: PLW1501 `rwx` is not a valid mode for `open` 33 | pathlib.Path(NAME).open(mode="rwx") # [bad-open-mode] 34 | pathlib.Path(NAME).open("rwx", encoding="utf-8") # [bad-open-mode] | ^^^^^ PLW1501 +35 | +36 | import builtins | - +bad_open_mode.py:37:21: PLW1501 `Ua` is not a valid mode for `open` + | +36 | import builtins +37 | builtins.open(NAME, "Ua", encoding="utf-8") + | ^^^^ PLW1501 + | diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW3301_nested_min_max.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW3301_nested_min_max.py.snap index 399812b7d634b..c30473aea7e73 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW3301_nested_min_max.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW3301_nested_min_max.py.snap @@ -283,6 +283,8 @@ nested_min_max.py:41:1: PLW3301 [*] Nested `max` calls can be flattened 40 | # Starred argument should be copied as it is. 41 | max(1, max(*a)) | ^^^^^^^^^^^^^^^ PLW3301 +42 | +43 | import builtins | = help: Flatten nested `max` calls @@ -292,5 +294,21 @@ nested_min_max.py:41:1: PLW3301 [*] Nested `max` calls can be flattened 40 40 | # Starred argument should be copied as it is. 41 |-max(1, max(*a)) 41 |+max(1, *a) +42 42 | +43 43 | import builtins +44 44 | builtins.min(1, min(2, 3)) +nested_min_max.py:44:1: PLW3301 [*] Nested `min` calls can be flattened + | +43 | import builtins +44 | builtins.min(1, min(2, 3)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW3301 + | + = help: Flatten nested `min` calls +ℹ Unsafe fix +41 41 | max(1, max(*a)) +42 42 | +43 43 | import builtins +44 |-builtins.min(1, min(2, 3)) + 44 |+builtins.min(1, 2, 3) diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs index c7a0567f06ac4..cb76f504f5d2f 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs @@ -61,19 +61,14 @@ fn is_alias(expr: &Expr, semantic: &SemanticModel) -> bool { .is_some_and(|qualified_name| { matches!( qualified_name.segments(), - ["", "EnvironmentError" | "IOError" | "WindowsError"] - | ["mmap" | "select" | "socket" | "os", "error"] + [ + "" | "builtins", + "EnvironmentError" | "IOError" | "WindowsError" + ] | ["mmap" | "select" | "socket" | "os", "error"] ) }) } -/// Return `true` if an [`Expr`] is `OSError`. -fn is_os_error(expr: &Expr, semantic: &SemanticModel) -> bool { - semantic - .resolve_qualified_name(expr) - .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", "OSError"])) -} - /// Create a [`Diagnostic`] for a single target, like an [`Expr::Name`]. fn atom_diagnostic(checker: &mut Checker, target: &Expr) { let mut diagnostic = Diagnostic::new( @@ -112,7 +107,7 @@ fn tuple_diagnostic(checker: &mut Checker, tuple: &ast::ExprTuple, aliases: &[&E if tuple .elts .iter() - .all(|elt| !is_os_error(elt, checker.semantic())) + .all(|elt| !checker.semantic().match_builtin_expr(elt, "OSError")) { let node = ast::ExprName { id: "OSError".into(), diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/replace_str_enum.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/replace_str_enum.rs index 4717ce2a5347f..5fc112369102a 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/replace_str_enum.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/replace_str_enum.rs @@ -111,7 +111,7 @@ pub(crate) fn replace_str_enum(checker: &mut Checker, class_def: &ast::StmtClass for base in arguments.args.iter() { if let Some(qualified_name) = checker.semantic().resolve_qualified_name(base) { match qualified_name.segments() { - ["", "str"] => inherits_str = true, + ["" | "builtins", "str"] => inherits_str = true, ["enum", "Enum"] => inherits_enum = true, _ => {} } diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs index b3591f26d78a6..548b06a535070 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs @@ -81,13 +81,6 @@ fn is_alias(expr: &Expr, semantic: &SemanticModel, target_version: PythonVersion }) } -/// Return `true` if an [`Expr`] is `TimeoutError`. -fn is_timeout_error(expr: &Expr, semantic: &SemanticModel) -> bool { - semantic - .resolve_qualified_name(expr) - .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", "TimeoutError"])) -} - /// Create a [`Diagnostic`] for a single target, like an [`Expr::Name`]. fn atom_diagnostic(checker: &mut Checker, target: &Expr) { let mut diagnostic = Diagnostic::new( @@ -126,7 +119,7 @@ fn tuple_diagnostic(checker: &mut Checker, tuple: &ast::ExprTuple, aliases: &[&E if tuple .elts .iter() - .all(|elt| !is_timeout_error(elt, checker.semantic())) + .all(|elt| !checker.semantic().match_builtin_expr(elt, "TimeoutError")) { let node = ast::ExprName { id: "TimeoutError".into(), diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/type_of_primitive.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/type_of_primitive.rs index c2fc297eaf9ca..844ac38d6d038 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/type_of_primitive.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/type_of_primitive.rs @@ -58,19 +58,16 @@ pub(crate) fn type_of_primitive(checker: &mut Checker, expr: &Expr, func: &Expr, let [arg] = args else { return; }; - if !checker - .semantic() - .resolve_qualified_name(func) - .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", "type"])) - { - return; - } let Some(primitive) = Primitive::from_expr(arg) else { return; }; + let semantic = checker.semantic(); + if !semantic.match_builtin_expr(func, "type") { + return; + } let mut diagnostic = Diagnostic::new(TypeOfPrimitive { primitive }, expr.range()); let builtin = primitive.builtin(); - if checker.semantic().is_builtin(&builtin) { + if semantic.is_builtin(&builtin) { diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( pad(primitive.builtin(), expr.range(), checker.locator()), expr.range(), diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/useless_object_inheritance.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/useless_object_inheritance.rs index 470cdd911ed86..755e8c468adb7 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/useless_object_inheritance.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/useless_object_inheritance.rs @@ -1,6 +1,6 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::{self as ast, Expr}; +use ruff_python_ast as ast; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -51,13 +51,7 @@ pub(crate) fn useless_object_inheritance(checker: &mut Checker, class_def: &ast: }; for base in arguments.args.iter() { - let Expr::Name(ast::ExprName { id, .. }) = base else { - continue; - }; - if id != "object" { - continue; - } - if !checker.semantic().is_builtin("object") { + if !checker.semantic().match_builtin_expr(base, "object") { continue; } diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP004.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP004.py.snap index ce91d7a5de6c7..0c4b87c5d9dbf 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP004.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP004.py.snap @@ -490,4 +490,20 @@ UP004.py:146:9: UP004 [*] Class `A` inherits from `object` 148 148 | 149 149 | +UP004.py:159:15: UP004 [*] Class `Unusual` inherits from `object` + | +157 | import builtins +158 | +159 | class Unusual(builtins.object): + | ^^^^^^^^^^^^^^^ UP004 +160 | ... + | + = help: Remove `object` inheritance +ℹ Safe fix +156 156 | +157 157 | import builtins +158 158 | +159 |-class Unusual(builtins.object): + 159 |+class Unusual: +160 160 | ... diff --git a/crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs b/crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs index 6bc52c14e5484..095d3331fbc40 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs @@ -70,12 +70,7 @@ impl Violation for PrintEmptyString { /// FURB105 pub(crate) fn print_empty_string(checker: &mut Checker, call: &ast::ExprCall) { - if !checker - .semantic() - .resolve_qualified_name(&call.func) - .as_ref() - .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", "print"])) - { + if !checker.semantic().match_builtin_expr(&call.func, "print") { return; } diff --git a/crates/ruff_linter/src/rules/ruff/rules/mutable_fromkeys_value.rs b/crates/ruff_linter/src/rules/ruff/rules/mutable_fromkeys_value.rs index 6cac3d994e429..b139d38407f2b 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mutable_fromkeys_value.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mutable_fromkeys_value.rs @@ -73,13 +73,7 @@ pub(crate) fn mutable_fromkeys_value(checker: &mut Checker, call: &ast::ExprCall if attr != "fromkeys" { return; } - let Some(name_expr) = value.as_name_expr() else { - return; - }; - if name_expr.id != "dict" { - return; - } - if !checker.semantic().is_builtin("dict") { + if !checker.semantic().match_builtin_expr(value, "dict") { return; } diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF024_RUF024.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF024_RUF024.py.snap index 0de5a384ad227..f083f1920fa04 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF024_RUF024.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF024_RUF024.py.snap @@ -82,7 +82,7 @@ RUF024.py:12:1: RUF024 [*] Do not pass mutable objects as values to `dict.fromke 12 |+{key: set() for key in pierogi_fillings} 13 13 | dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) 14 14 | dict.fromkeys(pierogi_fillings, dict()) -15 15 | +15 15 | import builtins RUF024.py:13:1: RUF024 [*] Do not pass mutable objects as values to `dict.fromkeys` | @@ -91,6 +91,7 @@ RUF024.py:13:1: RUF024 [*] Do not pass mutable objects as values to `dict.fromke 13 | dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF024 14 | dict.fromkeys(pierogi_fillings, dict()) +15 | import builtins | = help: Replace with comprehension @@ -101,8 +102,8 @@ RUF024.py:13:1: RUF024 [*] Do not pass mutable objects as values to `dict.fromke 13 |-dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) 13 |+{key: {"pre": "populated!"} for key in pierogi_fillings} 14 14 | dict.fromkeys(pierogi_fillings, dict()) -15 15 | -16 16 | # Okay. +15 15 | import builtins +16 16 | builtins.dict.fromkeys(pierogi_fillings, dict()) RUF024.py:14:1: RUF024 [*] Do not pass mutable objects as values to `dict.fromkeys` | @@ -110,8 +111,8 @@ RUF024.py:14:1: RUF024 [*] Do not pass mutable objects as values to `dict.fromke 13 | dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) 14 | dict.fromkeys(pierogi_fillings, dict()) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF024 -15 | -16 | # Okay. +15 | import builtins +16 | builtins.dict.fromkeys(pierogi_fillings, dict()) | = help: Replace with comprehension @@ -121,8 +122,27 @@ RUF024.py:14:1: RUF024 [*] Do not pass mutable objects as values to `dict.fromke 13 13 | dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) 14 |-dict.fromkeys(pierogi_fillings, dict()) 14 |+{key: dict() for key in pierogi_fillings} -15 15 | -16 16 | # Okay. -17 17 | dict.fromkeys(pierogi_fillings) +15 15 | import builtins +16 16 | builtins.dict.fromkeys(pierogi_fillings, dict()) +17 17 | +RUF024.py:16:1: RUF024 [*] Do not pass mutable objects as values to `dict.fromkeys` + | +14 | dict.fromkeys(pierogi_fillings, dict()) +15 | import builtins +16 | builtins.dict.fromkeys(pierogi_fillings, dict()) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF024 +17 | +18 | # Okay. + | + = help: Replace with comprehension +ℹ Unsafe fix +13 13 | dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) +14 14 | dict.fromkeys(pierogi_fillings, dict()) +15 15 | import builtins +16 |-builtins.dict.fromkeys(pierogi_fillings, dict()) + 16 |+{key: dict() for key in pierogi_fillings} +17 17 | +18 18 | # Okay. +19 19 | dict.fromkeys(pierogi_fillings) diff --git a/crates/ruff_linter/src/rules/tryceratops/rules/raise_vanilla_args.rs b/crates/ruff_linter/src/rules/tryceratops/rules/raise_vanilla_args.rs index 92d09b15a97d8..0d56ca090cd85 100644 --- a/crates/ruff_linter/src/rules/tryceratops/rules/raise_vanilla_args.rs +++ b/crates/ruff_linter/src/rules/tryceratops/rules/raise_vanilla_args.rs @@ -72,10 +72,7 @@ pub(crate) fn raise_vanilla_args(checker: &mut Checker, expr: &Expr) { // `NotImplementedError`. if checker .semantic() - .resolve_qualified_name(func) - .is_some_and(|qualified_name| { - matches!(qualified_name.segments(), ["", "NotImplementedError"]) - }) + .match_builtin_expr(func, "NotImplementedError") { return; } diff --git a/crates/ruff_linter/src/rules/tryceratops/rules/raise_vanilla_class.rs b/crates/ruff_linter/src/rules/tryceratops/rules/raise_vanilla_class.rs index bacdeeb12ef49..4a6a20f3c6bc4 100644 --- a/crates/ruff_linter/src/rules/tryceratops/rules/raise_vanilla_class.rs +++ b/crates/ruff_linter/src/rules/tryceratops/rules/raise_vanilla_class.rs @@ -63,15 +63,12 @@ impl Violation for RaiseVanillaClass { /// TRY002 pub(crate) fn raise_vanilla_class(checker: &mut Checker, expr: &Expr) { - if checker - .semantic() - .resolve_qualified_name(if let Expr::Call(ast::ExprCall { func, .. }) = expr { - func - } else { - expr - }) - .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", "Exception"])) - { + let node = if let Expr::Call(ast::ExprCall { func, .. }) = expr { + func + } else { + expr + }; + if checker.semantic().match_builtin_expr(node, "Exception") { checker .diagnostics .push(Diagnostic::new(RaiseVanillaClass, expr.range())); diff --git a/crates/ruff_linter/src/rules/tryceratops/rules/raise_within_try.rs b/crates/ruff_linter/src/rules/tryceratops/rules/raise_within_try.rs index ca769add40ad1..c4d1c88c88764 100644 --- a/crates/ruff_linter/src/rules/tryceratops/rules/raise_within_try.rs +++ b/crates/ruff_linter/src/rules/tryceratops/rules/raise_within_try.rs @@ -115,7 +115,7 @@ pub(crate) fn raise_within_try(checker: &mut Checker, body: &[Stmt], handlers: & .is_some_and(|qualified_name| { matches!( qualified_name.segments(), - ["", "Exception" | "BaseException"] + ["" | "builtins", "Exception" | "BaseException"] ) }) }) diff --git a/crates/ruff_linter/src/rules/tryceratops/rules/type_check_without_type_error.rs b/crates/ruff_linter/src/rules/tryceratops/rules/type_check_without_type_error.rs index a982a24a3ee9a..370e4ab698ad5 100644 --- a/crates/ruff_linter/src/rules/tryceratops/rules/type_check_without_type_error.rs +++ b/crates/ruff_linter/src/rules/tryceratops/rules/type_check_without_type_error.rs @@ -81,7 +81,7 @@ fn check_type_check_call(checker: &mut Checker, call: &Expr) -> bool { .is_some_and(|qualified_name| { matches!( qualified_name.segments(), - ["", "isinstance" | "issubclass" | "callable"] + ["" | "builtins", "isinstance" | "issubclass" | "callable"] ) }) } From 87fca4e42e93547e7c903b18c52c5c39784244f4 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 13 Apr 2024 16:44:57 +0100 Subject: [PATCH 3/9] Use the new semantic-model method in `refurb` rules --- .../resources/test/fixtures/refurb/FURB103.py | 15 ++++++++++ .../flake8_pyi/rules/exit_annotations.rs | 11 +++---- .../ruff_linter/src/rules/refurb/helpers.rs | 8 ++--- .../rules/refurb/rules/int_on_sliced_str.rs | 8 +---- .../rules/refurb/rules/list_reverse_copy.rs | 10 ++----- .../src/rules/refurb/rules/read_whole_file.rs | 2 +- .../refurb/rules/unnecessary_enumerate.rs | 16 ++++------ .../refurb/rules/unnecessary_from_float.rs | 30 +++++++------------ .../rules/verbose_decimal_constructor.rs | 9 +----- .../rules/refurb/rules/write_whole_file.rs | 2 +- ...es__refurb__tests__FURB103_FURB103.py.snap | 16 ++++++++++ 11 files changed, 61 insertions(+), 66 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB103.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB103.py index 2ba42f4ad823c..b6d8e1d034322 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB103.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB103.py @@ -59,6 +59,21 @@ def bar(x): f.write(foobar) +import builtins + + +# FURB103 +with builtins.open("file.txt", "w", newline="\r\n") as f: + f.write(foobar) + + +from builtins import open as o + + +# FURB103 +with o("file.txt", "w", newline="\r\n") as f: + f.write(foobar) + # Non-errors. with open("file.txt", errors="ignore", mode="wb") as f: diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs index 393f8d234e29b..f4679f0a84721 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs @@ -213,7 +213,9 @@ fn check_positional_args( let validations: [(ErrorKind, AnnotationValidator); 3] = [ (ErrorKind::FirstArgBadAnnotation, is_base_exception_type), - (ErrorKind::SecondArgBadAnnotation, is_base_exception), + (ErrorKind::SecondArgBadAnnotation, |expr, semantic| { + semantic.match_builtin_expr(expr, "BaseException") + }), (ErrorKind::ThirdArgBadAnnotation, is_traceback_type), ]; @@ -322,11 +324,6 @@ fn is_object_or_unused(expr: &Expr, semantic: &SemanticModel) -> bool { }) } -/// Return `true` if the [`Expr`] is `BaseException`. -fn is_base_exception(expr: &Expr, semantic: &SemanticModel) -> bool { - semantic.match_builtin_expr(expr, "BaseException") -} - /// Return `true` if the [`Expr`] is the `types.TracebackType` type. fn is_traceback_type(expr: &Expr, semantic: &SemanticModel) -> bool { semantic @@ -344,7 +341,7 @@ fn is_base_exception_type(expr: &Expr, semantic: &SemanticModel) -> bool { }; if semantic.match_typing_expr(value, "Type") || semantic.match_builtin_expr(value, "type") { - is_base_exception(slice, semantic) + semantic.match_builtin_expr(slice, "BaseException") } else { false } diff --git a/crates/ruff_linter/src/rules/refurb/helpers.rs b/crates/ruff_linter/src/rules/refurb/helpers.rs index 83ef1e5706cc6..97823d2032d25 100644 --- a/crates/ruff_linter/src/rules/refurb/helpers.rs +++ b/crates/ruff_linter/src/rules/refurb/helpers.rs @@ -137,10 +137,6 @@ fn find_file_open<'a>( .. } = item.context_expr.as_call_expr()?; - if func.as_name_expr()?.id != "open" { - return None; - } - let var = item.optional_vars.as_deref()?.as_name_expr()?; // Ignore calls with `*args` and `**kwargs`. In the exact case of `open(*filename, mode="w")`, @@ -152,6 +148,10 @@ fn find_file_open<'a>( return None; } + if !semantic.match_builtin_expr(func, "open") { + return None; + } + // Match positional arguments, get filename and mode. let (filename, pos_mode) = match_open_args(args)?; diff --git a/crates/ruff_linter/src/rules/refurb/rules/int_on_sliced_str.rs b/crates/ruff_linter/src/rules/refurb/rules/int_on_sliced_str.rs index 33af2c5ae1d9b..a27e969381987 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/int_on_sliced_str.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/int_on_sliced_str.rs @@ -66,13 +66,7 @@ impl AlwaysFixableViolation for IntOnSlicedStr { pub(crate) fn int_on_sliced_str(checker: &mut Checker, call: &ExprCall) { // Verify that the function is `int`. - let Expr::Name(name) = call.func.as_ref() else { - return; - }; - if name.id.as_str() != "int" { - return; - } - if !checker.semantic().is_builtin("int") { + if !checker.semantic().match_builtin_expr(&call.func, "int") { return; } diff --git a/crates/ruff_linter/src/rules/refurb/rules/list_reverse_copy.rs b/crates/ruff_linter/src/rules/refurb/rules/list_reverse_copy.rs index 7a6c8224821fd..2f1d5ab53388f 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/list_reverse_copy.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/list_reverse_copy.rs @@ -141,17 +141,11 @@ fn extract_name_from_reversed<'a>( return None; }; - let arg = func - .as_name_expr() - .is_some_and(|name| name.id == "reversed") - .then(|| arg.as_name_expr()) - .flatten()?; - - if !semantic.is_builtin("reversed") { + if !semantic.match_builtin_expr(func, "reversed") { return None; } - Some(arg) + arg.as_name_expr() } /// Given a slice expression, returns the inner argument if it's a reversed slice. diff --git a/crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs b/crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs index 50fdd85aa8556..e3e39cb022b5e 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs @@ -53,7 +53,7 @@ impl Violation for ReadWholeFile { /// FURB101 pub(crate) fn read_whole_file(checker: &mut Checker, with: &ast::StmtWith) { // `async` check here is more of a precaution. - if with.is_async || !checker.semantic().is_builtin("open") { + if with.is_async { return; } diff --git a/crates/ruff_linter/src/rules/refurb/rules/unnecessary_enumerate.rs b/crates/ruff_linter/src/rules/refurb/rules/unnecessary_enumerate.rs index 15eb34b7c851f..c2cd5b6ee9c9c 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/unnecessary_enumerate.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/unnecessary_enumerate.rs @@ -102,22 +102,16 @@ pub(crate) fn unnecessary_enumerate(checker: &mut Checker, stmt_for: &ast::StmtF return; }; - // Check that the function is the `enumerate` builtin. - let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else { - return; - }; - if id != "enumerate" { - return; - }; - if !checker.semantic().is_builtin("enumerate") { - return; - }; - // Get the first argument, which is the sequence to iterate over. let Some(Expr::Name(sequence)) = arguments.args.first() else { return; }; + // Check that the function is the `enumerate` builtin. + if !checker.semantic().match_builtin_expr(func, "enumerate") { + return; + } + // Check if the index and value are used. match ( checker.semantic().is_unused(index), diff --git a/crates/ruff_linter/src/rules/refurb/rules/unnecessary_from_float.rs b/crates/ruff_linter/src/rules/refurb/rules/unnecessary_from_float.rs index 8994ce81ac3a4..f37f6f25a473f 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/unnecessary_from_float.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/unnecessary_from_float.rs @@ -71,20 +71,19 @@ pub(crate) fn unnecessary_from_float(checker: &mut Checker, call: &ExprCall) { _ => return, }; + let semantic = checker.semantic(); + // The value must be either `decimal.Decimal` or `fractions.Fraction`. - let Some(constructor) = - checker - .semantic() - .resolve_qualified_name(value) - .and_then(|qualified_name| match qualified_name.segments() { - ["decimal", "Decimal"] => Some(Constructor::Decimal), - ["fractions", "Fraction"] => Some(Constructor::Fraction), - _ => None, - }) - else { + let Some(qualified_name) = semantic.resolve_qualified_name(value) else { return; }; + let constructor = match qualified_name.segments() { + ["decimal", "Decimal"] => Constructor::Decimal, + ["fractions", "Fraction"] => Constructor::Fraction, + _ => return, + }; + // `Decimal.from_decimal` doesn't exist. if matches!( (method_name, constructor), @@ -131,14 +130,6 @@ pub(crate) fn unnecessary_from_float(checker: &mut Checker, call: &ExprCall) { break 'short_circuit; }; - // Must be a call to the `float` builtin. - let Some(func_name) = func.as_name_expr() else { - break 'short_circuit; - }; - if func_name.id != "float" { - break 'short_circuit; - }; - // Must have exactly one argument, which is a string literal. if arguments.keywords.len() != 0 { break 'short_circuit; @@ -156,7 +147,8 @@ pub(crate) fn unnecessary_from_float(checker: &mut Checker, call: &ExprCall) { break 'short_circuit; } - if !checker.semantic().is_builtin("float") { + // Must be a call to the `float` builtin. + if !semantic.match_builtin_expr(func, "float") { break 'short_circuit; }; diff --git a/crates/ruff_linter/src/rules/refurb/rules/verbose_decimal_constructor.rs b/crates/ruff_linter/src/rules/refurb/rules/verbose_decimal_constructor.rs index 831b07f86df83..952e396070c5f 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/verbose_decimal_constructor.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/verbose_decimal_constructor.rs @@ -116,10 +116,7 @@ pub(crate) fn verbose_decimal_constructor(checker: &mut Checker, call: &ast::Exp func, arguments, .. }) => { // Must be a call to the `float` builtin. - let Some(func_name) = func.as_name_expr() else { - return; - }; - if func_name.id != "float" { + if !checker.semantic().match_builtin_expr(func, "float") { return; }; @@ -140,10 +137,6 @@ pub(crate) fn verbose_decimal_constructor(checker: &mut Checker, call: &ast::Exp return; } - if !checker.semantic().is_builtin("float") { - return; - }; - let replacement = checker.locator().slice(float).to_string(); let mut diagnostic = Diagnostic::new( VerboseDecimalConstructor { diff --git a/crates/ruff_linter/src/rules/refurb/rules/write_whole_file.rs b/crates/ruff_linter/src/rules/refurb/rules/write_whole_file.rs index 84265317f83d1..9898d1c8e0f41 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/write_whole_file.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/write_whole_file.rs @@ -54,7 +54,7 @@ impl Violation for WriteWholeFile { /// FURB103 pub(crate) fn write_whole_file(checker: &mut Checker, with: &ast::StmtWith) { // `async` check here is more of a precaution. - if with.is_async || !checker.semantic().is_builtin("open") { + if with.is_async { return; } diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB103_FURB103.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB103_FURB103.py.snap index e919362cda06b..7a25e911648aa 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB103_FURB103.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB103_FURB103.py.snap @@ -92,3 +92,19 @@ FURB103.py:58:6: FURB103 `open` and `write` should be replaced by `Path("file.tx | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB103 59 | f.write(foobar) | + +FURB103.py:66:6: FURB103 `open` and `write` should be replaced by `Path("file.txt").write_text(foobar, newline="\r\n")` + | +65 | # FURB103 +66 | with builtins.open("file.txt", "w", newline="\r\n") as f: + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB103 +67 | f.write(foobar) + | + +FURB103.py:74:6: FURB103 `open` and `write` should be replaced by `Path("file.txt").write_text(foobar, newline="\r\n")` + | +73 | # FURB103 +74 | with o("file.txt", "w", newline="\r\n") as f: + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB103 +75 | f.write(foobar) + | From 7167702d54378abd461ab78f7e541dd47983c7e0 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 13 Apr 2024 16:45:17 +0100 Subject: [PATCH 4/9] Add a new method to improve our ability to autofix when builtins are overridden --- crates/ruff_linter/src/importer/mod.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/crates/ruff_linter/src/importer/mod.rs b/crates/ruff_linter/src/importer/mod.rs index 6dce7a35a107b..c2ef4df975c73 100644 --- a/crates/ruff_linter/src/importer/mod.rs +++ b/crates/ruff_linter/src/importer/mod.rs @@ -229,6 +229,31 @@ impl<'a> Importer<'a> { .map_or_else(|| self.import_symbol(symbol, at, None, semantic), Ok) } + /// For a given builtin symbol, determine whether an [`Edit`] is necessary to make the symbol + /// available in the current scope. For example, if `zip` has been overridden in the relevant + /// scope, the `builtins` module will need to be imported in order for a `Fix` to reference + /// `zip`; but otherwise, that won't be necessary. + /// + /// Returns a two-item tuple. The first item is either `Some(Edit)` (indicating) that an + /// edit is necessary to make the symbol available, or `None`, indicating that the symbol has + /// not been overridden in the current scope. The second item in the tuple is the bound name + /// of the symbol. + /// + /// Attempts to reuse existing imports when possible. + pub(crate) fn get_or_import_builtin_symbol( + &self, + symbol: &str, + at: TextSize, + semantic: &SemanticModel, + ) -> Result<(Option, String), ResolutionError> { + if semantic.is_builtin(symbol) { + return Ok((None, symbol.to_string())); + } + let (import_edit, binding) = + self.get_or_import_symbol(&ImportRequest::import("builtins", symbol), at, semantic)?; + Ok((Some(import_edit), binding)) + } + /// Return the [`ImportedName`] to for existing symbol, if it's present in the given [`SemanticModel`]. fn find_symbol( symbol: &ImportRequest, From 7cbd0a6300c1de2bf9161494bf1d16f3c232598a Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 13 Apr 2024 16:58:04 +0100 Subject: [PATCH 5/9] Use the new method for autofixes across various lint rules --- .../test/fixtures/flake8_bugbear/B004.py | 10 ++++++++ .../resources/test/fixtures/pyflakes/F901.py | 5 ++++ .../rules/unreliable_callable_check.rs | 16 ++++++++---- ...__flake8_bugbear__tests__B004_B004.py.snap | 25 +++++++++++++++++++ .../flake8_pyi/rules/any_eq_ne_annotation.rs | 21 ++++++++++------ .../flake8_pyi/rules/exit_annotations.rs | 15 ++++++----- .../pyflakes/rules/raise_not_implemented.rs | 17 ++++++++----- ..._rules__pyflakes__tests__F901_F901.py.snap | 22 ++++++++++++++++ .../src/rules/pyupgrade/rules/open_alias.rs | 17 ++++++++----- .../rules/pyupgrade/rules/os_error_alias.rs | 17 ++++++++----- .../pyupgrade/rules/timeout_error_alias.rs | 17 ++++++++----- .../pyupgrade/rules/typing_text_str_alias.rs | 17 ++++++++----- ...er__rules__pyupgrade__tests__UP020.py.snap | 12 +++++++-- 13 files changed, 160 insertions(+), 51 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py index e1972a7bd9ca5..83590228a6673 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py @@ -15,6 +15,16 @@ def still_a_bug(): print("B U G") +def trickier_fix_for_this_one(): + o = object() + + def callable(x): + return True + + if hasattr(o, "__call__"): + print("STILL a bug!") + + def this_is_fine(): o = object() if callable(o): diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F901.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F901.py index 990c5d73af3f0..3df4f24800d9c 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyflakes/F901.py +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F901.py @@ -4,3 +4,8 @@ def f() -> None: def g() -> None: raise NotImplemented + + +def h() -> None: + NotImplementedError = "foo" + raise NotImplemented diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs index 5419cc6e44456..751ba59bdd0d8 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs @@ -75,12 +75,18 @@ pub(crate) fn unreliable_callable_check( let mut diagnostic = Diagnostic::new(UnreliableCallableCheck, expr.range()); if *function == "hasattr" { - if checker.semantic().is_builtin("callable") { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - format!("callable({})", checker.locator().slice(obj)), + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_builtin_symbol( + "callable", + expr.start(), + checker.semantic(), + )?; + let binding_edit = Edit::range_replacement( + format!("{binding}({})", checker.locator().slice(obj)), expr.range(), - ))); - } + ); + Ok(Fix::safe_edits(binding_edit, import_edit)) + }); } checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap index eb301b1c86492..da4177d024b9f 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap @@ -61,3 +61,28 @@ B004.py:14:8: B004 Using `hasattr(x, "__call__")` to test if x is callable is un 15 | print("B U G") | = help: Replace with `callable()` + +B004.py:24:8: B004 [*] Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results. + | +22 | return True +23 | +24 | if hasattr(o, "__call__"): + | ^^^^^^^^^^^^^^^^^^^^^^ B004 +25 | print("STILL a bug!") + | + = help: Replace with `callable()` + +ℹ Safe fix + 1 |+import builtins +1 2 | def this_is_a_bug(): +2 3 | o = object() +3 4 | if hasattr(o, "__call__"): +-------------------------------------------------------------------------------- +21 22 | def callable(x): +22 23 | return True +23 24 | +24 |- if hasattr(o, "__call__"): + 25 |+ if builtins.callable(o): +25 26 | print("STILL a bug!") +26 27 | +27 28 | diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs index ebc52fcfc4d56..3a8e5f2d05ef5 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs @@ -72,11 +72,13 @@ pub(crate) fn any_eq_ne_annotation(checker: &mut Checker, name: &str, parameters return; }; - if !checker.semantic().current_scope().kind.is_class() { + let semantic = checker.semantic(); + + if !semantic.current_scope().kind.is_class() { return; } - if checker.semantic().match_typing_expr(annotation, "Any") { + if semantic.match_typing_expr(annotation, "Any") { let mut diagnostic = Diagnostic::new( AnyEqNeAnnotation { method_name: name.to_string(), @@ -84,12 +86,15 @@ pub(crate) fn any_eq_ne_annotation(checker: &mut Checker, name: &str, parameters annotation.range(), ); // Ex) `def __eq__(self, obj: Any): ...` - if checker.semantic().is_builtin("object") { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - "object".to_string(), - annotation.range(), - ))); - } + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_builtin_symbol( + "object", + annotation.start(), + semantic, + )?; + let binding_edit = Edit::range_replacement(binding, annotation.range()); + Ok(Fix::safe_edits(binding_edit, import_edit)) + }); checker.diagnostics.push(diagnostic); } } diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs index f4679f0a84721..c7b18b3d8eb47 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs @@ -181,12 +181,15 @@ fn check_short_args_list(checker: &mut Checker, parameters: &Parameters, func_ki annotation.range(), ); - if checker.semantic().is_builtin("object") { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - "object".to_string(), - annotation.range(), - ))); - } + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_builtin_symbol( + "object", + annotation.start(), + checker.semantic(), + )?; + let binding_edit = Edit::range_replacement(binding, annotation.range()); + Ok(Fix::safe_edits(binding_edit, import_edit)) + }); checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/raise_not_implemented.rs b/crates/ruff_linter/src/rules/pyflakes/rules/raise_not_implemented.rs index 3a48fe902f77c..a8c061958a07f 100644 --- a/crates/ruff_linter/src/rules/pyflakes/rules/raise_not_implemented.rs +++ b/crates/ruff_linter/src/rules/pyflakes/rules/raise_not_implemented.rs @@ -75,11 +75,16 @@ pub(crate) fn raise_not_implemented(checker: &mut Checker, expr: &Expr) { return; }; let mut diagnostic = Diagnostic::new(RaiseNotImplemented, expr.range()); - if checker.semantic().is_builtin("NotImplementedError") { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - "NotImplementedError".to_string(), - expr.range(), - ))); - } + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_builtin_symbol( + "NotImplementedError", + expr.start(), + checker.semantic(), + )?; + Ok(Fix::safe_edits( + Edit::range_replacement(binding, expr.range()), + import_edit, + )) + }); checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F901_F901.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F901_F901.py.snap index 59889e1176e0c..33261837366b8 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F901_F901.py.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F901_F901.py.snap @@ -31,5 +31,27 @@ F901.py:6:11: F901 [*] `raise NotImplemented` should be `raise NotImplementedErr 5 5 | def g() -> None: 6 |- raise NotImplemented 6 |+ raise NotImplementedError +7 7 | +8 8 | +9 9 | def h() -> None: +F901.py:11:11: F901 [*] `raise NotImplemented` should be `raise NotImplementedError` + | + 9 | def h() -> None: +10 | NotImplementedError = "foo" +11 | raise NotImplemented + | ^^^^^^^^^^^^^^ F901 + | + = help: Use `raise NotImplementedError` +ℹ Safe fix + 1 |+import builtins +1 2 | def f() -> None: +2 3 | raise NotImplemented() +3 4 | +-------------------------------------------------------------------------------- +8 9 | +9 10 | def h() -> None: +10 11 | NotImplementedError = "foo" +11 |- raise NotImplemented + 12 |+ raise builtins.NotImplementedError diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/open_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/open_alias.rs index 685ddfcaaa22e..eb8e0bbb0a6cb 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/open_alias.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/open_alias.rs @@ -53,12 +53,17 @@ pub(crate) fn open_alias(checker: &mut Checker, expr: &Expr, func: &Expr) { .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["io", "open"])) { let mut diagnostic = Diagnostic::new(OpenAlias, expr.range()); - if checker.semantic().is_builtin("open") { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - "open".to_string(), - func.range(), - ))); - } + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_builtin_symbol( + "open", + expr.start(), + checker.semantic(), + )?; + Ok(Fix::safe_edits( + Edit::range_replacement(binding, func.range()), + import_edit, + )) + }); checker.diagnostics.push(diagnostic); } } diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs index cb76f504f5d2f..8047a3bde9d0c 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs @@ -77,12 +77,17 @@ fn atom_diagnostic(checker: &mut Checker, target: &Expr) { }, target.range(), ); - if checker.semantic().is_builtin("OSError") { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - "OSError".to_string(), - target.range(), - ))); - } + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_builtin_symbol( + "OSError", + target.start(), + checker.semantic(), + )?; + Ok(Fix::safe_edits( + Edit::range_replacement(binding, target.range()), + import_edit, + )) + }); checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs index 548b06a535070..8f126d59c55f1 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs @@ -89,12 +89,17 @@ fn atom_diagnostic(checker: &mut Checker, target: &Expr) { }, target.range(), ); - if checker.semantic().is_builtin("TimeoutError") { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - "TimeoutError".to_string(), - target.range(), - ))); - } + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_builtin_symbol( + "TimeoutError", + target.start(), + checker.semantic(), + )?; + Ok(Fix::safe_edits( + Edit::range_replacement(binding, target.range()), + import_edit, + )) + }); checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/typing_text_str_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/typing_text_str_alias.rs index a186e8b3d0e23..ffb432c458e61 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/typing_text_str_alias.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/typing_text_str_alias.rs @@ -57,12 +57,17 @@ pub(crate) fn typing_text_str_alias(checker: &mut Checker, expr: &Expr) { .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["typing", "Text"])) { let mut diagnostic = Diagnostic::new(TypingTextStrAlias, expr.range()); - if checker.semantic().is_builtin("str") { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - "str".to_string(), - expr.range(), - ))); - } + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_builtin_symbol( + "str", + expr.start(), + checker.semantic(), + )?; + Ok(Fix::safe_edits( + Edit::range_replacement(binding, expr.range()), + import_edit, + )) + }); checker.diagnostics.push(diagnostic); } } diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP020.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP020.py.snap index f6469321fcb92..d8d5ed3948468 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP020.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP020.py.snap @@ -20,7 +20,7 @@ UP020.py:3:6: UP020 [*] Use builtin `open` 5 5 | 6 6 | from io import open -UP020.py:8:6: UP020 Use builtin `open` +UP020.py:8:6: UP020 [*] Use builtin `open` | 6 | from io import open 7 | @@ -30,4 +30,12 @@ UP020.py:8:6: UP020 Use builtin `open` | = help: Replace with builtin `open` - +ℹ Safe fix +4 4 | print(f.read()) +5 5 | +6 6 | from io import open + 7 |+import builtins +7 8 | +8 |-with open("f.txt") as f: + 9 |+with builtins.open("f.txt") as f: +9 10 | print(f.read()) From 04ad9d05aa7ed3bfad189eca27ea8339d29a8ecb Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 15 Apr 2024 09:43:57 +0100 Subject: [PATCH 6/9] Move `match_builtin_expr` calls lower down all the rules --- .../rules/getattr_with_constant.rs | 6 ++--- .../rules/setattr_with_constant.rs | 6 ++--- .../rules/unreliable_callable_check.rs | 12 ++++----- .../rules/zip_without_explicit_strict.rs | 6 +++-- .../rules/unnecessary_range_start.rs | 10 +++---- .../rules/unnecessary_type_union.rs | 27 ++++++++----------- .../rules/open_file_with_context_handler.rs | 7 +++-- .../pycodestyle/rules/type_comparison.rs | 24 +++++++++-------- .../src/rules/pylint/rules/bad_open_mode.rs | 6 +++-- .../src/rules/pylint/rules/nan_comparison.rs | 21 +++++++-------- .../pylint/rules/repeated_isinstance_calls.rs | 6 ++--- .../rules/unnecessary_list_index_lookup.rs | 10 +++---- .../rules/pyupgrade/rules/os_error_alias.rs | 5 ++-- .../pyupgrade/rules/timeout_error_alias.rs | 5 ++-- .../ruff/rules/mutable_fromkeys_value.rs | 5 ++-- 15 files changed, 80 insertions(+), 76 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs index cc6d6e7bc5522..85ff36723e110 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs @@ -54,9 +54,6 @@ pub(crate) fn getattr_with_constant( func: &Expr, args: &[Expr], ) { - if !checker.semantic().match_builtin_expr(func, "getattr") { - return; - }; let [obj, arg] = args else { return; }; @@ -72,6 +69,9 @@ pub(crate) fn getattr_with_constant( if is_mangled_private(value.to_str()) { return; } + if !checker.semantic().match_builtin_expr(func, "getattr") { + return; + } let mut diagnostic = Diagnostic::new(GetAttrWithConstant, expr.range()); diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs index 10dff69637117..6f0b386c7c040 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs @@ -68,9 +68,6 @@ pub(crate) fn setattr_with_constant( func: &Expr, args: &[Expr], ) { - if !checker.semantic().match_builtin_expr(func, "setattr") { - return; - } let [obj, name, value] = args else { return; }; @@ -86,6 +83,9 @@ pub(crate) fn setattr_with_constant( if is_mangled_private(name.to_str()) { return; } + if !checker.semantic().match_builtin_expr(func, "setattr") { + return; + } // We can only replace a `setattr` call (which is an `Expr`) with an assignment // (which is a `Stmt`) if the `Expr` is already being used as a `Stmt` diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs index 751ba59bdd0d8..f1c006f7b16b5 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs @@ -57,12 +57,6 @@ pub(crate) fn unreliable_callable_check( func: &Expr, args: &[Expr], ) { - let Some(qualified_name) = checker.semantic().resolve_qualified_name(func) else { - return; - }; - let ["" | "builtins", function @ ("hasattr" | "getattr")] = qualified_name.segments() else { - return; - }; let [obj, attr, ..] = args else { return; }; @@ -72,6 +66,12 @@ pub(crate) fn unreliable_callable_check( if value != "__call__" { return; } + let Some(qualified_name) = checker.semantic().resolve_qualified_name(func) else { + return; + }; + let ["" | "builtins", function @ ("hasattr" | "getattr")] = qualified_name.segments() else { + return; + }; let mut diagnostic = Diagnostic::new(UnreliableCallableCheck, expr.range()); if *function == "hasattr" { diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs index 21833151d3db1..fd9d34f494e44 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs @@ -52,13 +52,15 @@ impl AlwaysFixableViolation for ZipWithoutExplicitStrict { /// B905 pub(crate) fn zip_without_explicit_strict(checker: &mut Checker, call: &ast::ExprCall) { - if checker.semantic().match_builtin_expr(&call.func, "zip") + let semantic = checker.semantic(); + + if semantic.match_builtin_expr(&call.func, "zip") && call.arguments.find_keyword("strict").is_none() && !call .arguments .args .iter() - .any(|arg| is_infinite_iterator(arg, checker.semantic())) + .any(|arg| is_infinite_iterator(arg, semantic)) { checker.diagnostics.push( Diagnostic::new(ZipWithoutExplicitStrict, call.range()).with_fix(Fix::applicable_edit( diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_range_start.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_range_start.rs index b2d6903ed7d3e..86dfaa673d7bb 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_range_start.rs +++ b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_range_start.rs @@ -43,11 +43,6 @@ impl AlwaysFixableViolation for UnnecessaryRangeStart { /// PIE808 pub(crate) fn unnecessary_range_start(checker: &mut Checker, call: &ast::ExprCall) { - // Verify that the call is to the `range` builtin. - if !checker.semantic().match_builtin_expr(&call.func, "range") { - return; - }; - // `range` doesn't accept keyword arguments. if !call.arguments.keywords.is_empty() { return; @@ -70,6 +65,11 @@ pub(crate) fn unnecessary_range_start(checker: &mut Checker, call: &ast::ExprCal return; }; + // Verify that the call is to the `range` builtin. + if !checker.semantic().match_builtin_expr(&call.func, "range") { + return; + }; + let mut diagnostic = Diagnostic::new(UnnecessaryRangeStart, start.range()); diagnostic.try_set_fix(|| { remove_argument( diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs index fe9c6779ebc3f..86e121d603cea 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs @@ -53,31 +53,26 @@ impl Violation for UnnecessaryTypeUnion { /// PYI055 pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr) { + let semantic = checker.semantic(); + // The `|` operator isn't always safe to allow to runtime-evaluated annotations. - if checker.semantic().execution_context().is_runtime() { + if semantic.execution_context().is_runtime() { return; } // Check if `union` is a PEP604 union (e.g. `float | int`) or a `typing.Union[float, int]` let subscript = union.as_subscript_expr(); - if subscript.is_some_and(|subscript| { - !checker - .semantic() - .match_typing_expr(&subscript.value, "Union") - }) { + if subscript.is_some_and(|subscript| !semantic.match_typing_expr(&subscript.value, "Union")) { return; } - let mut type_exprs = Vec::new(); - let mut other_exprs = Vec::new(); + let mut type_exprs: Vec<&Expr> = Vec::new(); + let mut other_exprs: Vec<&Expr> = Vec::new(); let mut collect_type_exprs = |expr: &'a Expr, _parent: &'a Expr| match expr { - Expr::Subscript(subscript) => { - if checker - .semantic() - .match_builtin_expr(&subscript.value, "type") - { - type_exprs.push(&*subscript.slice); + Expr::Subscript(ast::ExprSubscript { slice, value, .. }) => { + if semantic.match_builtin_expr(value, "type") { + type_exprs.push(slice); } else { other_exprs.push(expr); } @@ -85,7 +80,7 @@ pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr) _ => other_exprs.push(expr), }; - traverse_union(&mut collect_type_exprs, checker.semantic(), union); + traverse_union(&mut collect_type_exprs, semantic, union); if type_exprs.len() > 1 { let type_members: Vec = type_exprs @@ -102,7 +97,7 @@ pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr) union.range(), ); - if checker.semantic().is_builtin("type") { + if semantic.is_builtin("type") { let content = if let Some(subscript) = subscript { let types = &Expr::Subscript(ast::ExprSubscript { value: Box::new(Expr::Name(ast::ExprName { diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs index a847fe52e9a72..3ae0034fcff81 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs +++ b/crates/ruff_linter/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs @@ -126,11 +126,14 @@ fn is_open(semantic: &SemanticModel, func: &Expr) -> bool { if attr != "open" { return false; } - let Expr::Call(ast::ExprCall { func, .. }) = &**value else { + let Expr::Call(ast::ExprCall { + func: value_func, .. + }) = &**value + else { return false; }; semantic - .resolve_qualified_name(func) + .resolve_qualified_name(value_func) .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["pathlib", "Path"])) } diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/type_comparison.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/type_comparison.rs index afe9624bfc164..1c0b1d68fbe97 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/type_comparison.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/type_comparison.rs @@ -76,7 +76,9 @@ fn deprecated_type_comparison(checker: &mut Checker, compare: &ast::ExprCompare) continue; }; - if !checker.semantic().match_builtin_expr(func, "type") { + let semantic = checker.semantic(); + + if !semantic.match_builtin_expr(func, "type") { continue; } @@ -86,7 +88,7 @@ fn deprecated_type_comparison(checker: &mut Checker, compare: &ast::ExprCompare) func, arguments, .. }) => { // Ex) `type(obj) is type(1)` - if checker.semantic().match_builtin_expr(func, "type") { + if semantic.match_builtin_expr(func, "type") { // Allow comparison for types which are not obvious. if arguments .args @@ -104,8 +106,7 @@ fn deprecated_type_comparison(checker: &mut Checker, compare: &ast::ExprCompare) } Expr::Attribute(ast::ExprAttribute { value, .. }) => { // Ex) `type(obj) is types.NoneType` - if checker - .semantic() + if semantic .resolve_qualified_name(value.as_ref()) .is_some_and(|qualified_name| { matches!(qualified_name.segments(), ["types", ..]) @@ -133,7 +134,7 @@ fn deprecated_type_comparison(checker: &mut Checker, compare: &ast::ExprCompare) | "dict" | "set" | "memoryview" - ) && checker.semantic().is_builtin(id) + ) && semantic.is_builtin(id) { checker.diagnostics.push(Diagnostic::new( TypeComparison { @@ -180,16 +181,17 @@ fn is_type(expr: &Expr, semantic: &SemanticModel) -> bool { Expr::Call(ast::ExprCall { func, arguments, .. }) => { - // Ex) `type(obj) == type(1)` - if !semantic.match_builtin_expr(func, "type") { - return false; - } - // Allow comparison for types which are not obvious. - arguments + if !arguments .args .first() .is_some_and(|arg| !arg.is_name_expr() && !arg.is_none_literal_expr()) + { + return false; + } + + // Ex) `type(obj) == type(1)` + semantic.match_builtin_expr(func, "type") } Expr::Name(ast::ExprName { id, .. }) => { // Ex) `type(obj) == int` diff --git a/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs b/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs index 5750cddf78969..89fd517977fa9 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs @@ -95,8 +95,10 @@ fn is_open(func: &Expr, semantic: &SemanticModel) -> Option { if attr != "open" { return None; } - let ast::ExprCall { func, .. } = value.as_call_expr()?; - let qualified_name = semantic.resolve_qualified_name(func)?; + let ast::ExprCall { + func: value_func, .. + } = value.as_call_expr()?; + let qualified_name = semantic.resolve_qualified_name(value_func)?; match qualified_name.segments() { ["pathlib", "Path"] => Some(Kind::Pathlib), _ => None, diff --git a/crates/ruff_linter/src/rules/pylint/rules/nan_comparison.rs b/crates/ruff_linter/src/rules/pylint/rules/nan_comparison.rs index da646b4dc88b8..fdc3347e2de64 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/nan_comparison.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/nan_comparison.rs @@ -96,23 +96,20 @@ impl std::fmt::Display for Nan { /// Returns `true` if the expression is a call to `float("NaN")`. fn is_nan_float(expr: &Expr, semantic: &SemanticModel) -> bool { - let Expr::Call(call) = expr else { + let Expr::Call(ast::ExprCall { + func, + arguments: ast::Arguments { args, keywords, .. }, + .. + }) = expr + else { return false; }; - if !semantic.match_builtin_expr(&call.func, "float") { - return false; - }; - - if !call.arguments.keywords.is_empty() { + if !keywords.is_empty() { return false; } - let [arg] = call.arguments.args.as_ref() else { - return false; - }; - - let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = arg else { + let [Expr::StringLiteral(ast::ExprStringLiteral { value, .. })] = &**args else { return false; }; @@ -123,5 +120,5 @@ fn is_nan_float(expr: &Expr, semantic: &SemanticModel) -> bool { return false; } - true + semantic.match_builtin_expr(func, "float") } diff --git a/crates/ruff_linter/src/rules/pylint/rules/repeated_isinstance_calls.rs b/crates/ruff_linter/src/rules/pylint/rules/repeated_isinstance_calls.rs index b337c8c695a49..5c7613357cfba 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/repeated_isinstance_calls.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/repeated_isinstance_calls.rs @@ -92,12 +92,12 @@ pub(crate) fn repeated_isinstance_calls( else { continue; }; - if !checker.semantic().match_builtin_expr(func, "isinstance") { - continue; - } let [obj, types] = &args[..] else { continue; }; + if !checker.semantic().match_builtin_expr(func, "isinstance") { + continue; + } let (num_calls, matches) = obj_to_types .entry(obj.into()) .or_insert_with(|| (0, FxHashSet::default())); diff --git a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs index 62cb12b889c1f..64e966e515959 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs @@ -124,11 +124,6 @@ fn enumerate_items<'a>( func, arguments, .. } = call_expr.as_call_expr()?; - // Check that the function is the `enumerate` builtin. - if !semantic.match_builtin_expr(func, "enumerate") { - return None; - } - let Expr::Tuple(ast::ExprTuple { elts, .. }) = tuple_expr else { return None; }; @@ -156,6 +151,11 @@ fn enumerate_items<'a>( return None; }; + // Check that the function is the `enumerate` builtin. + if !semantic.match_builtin_expr(func, "enumerate") { + return None; + } + Some((sequence, index_name, value_name)) } diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs index 8047a3bde9d0c..e3d34b5c7afb1 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs @@ -94,7 +94,8 @@ fn atom_diagnostic(checker: &mut Checker, target: &Expr) { /// Create a [`Diagnostic`] for a tuple of expressions. fn tuple_diagnostic(checker: &mut Checker, tuple: &ast::ExprTuple, aliases: &[&Expr]) { let mut diagnostic = Diagnostic::new(OSErrorAlias { name: None }, tuple.range()); - if checker.semantic().is_builtin("OSError") { + let semantic = checker.semantic(); + if semantic.is_builtin("OSError") { // Filter out any `OSErrors` aliases. let mut remaining: Vec = tuple .elts @@ -112,7 +113,7 @@ fn tuple_diagnostic(checker: &mut Checker, tuple: &ast::ExprTuple, aliases: &[&E if tuple .elts .iter() - .all(|elt| !checker.semantic().match_builtin_expr(elt, "OSError")) + .all(|elt| !semantic.match_builtin_expr(elt, "OSError")) { let node = ast::ExprName { id: "OSError".into(), diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs index 8f126d59c55f1..683824f1b4973 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs @@ -106,7 +106,8 @@ fn atom_diagnostic(checker: &mut Checker, target: &Expr) { /// Create a [`Diagnostic`] for a tuple of expressions. fn tuple_diagnostic(checker: &mut Checker, tuple: &ast::ExprTuple, aliases: &[&Expr]) { let mut diagnostic = Diagnostic::new(TimeoutErrorAlias { name: None }, tuple.range()); - if checker.semantic().is_builtin("TimeoutError") { + let semantic = checker.semantic(); + if semantic.is_builtin("TimeoutError") { // Filter out any `TimeoutErrors` aliases. let mut remaining: Vec = tuple .elts @@ -124,7 +125,7 @@ fn tuple_diagnostic(checker: &mut Checker, tuple: &ast::ExprTuple, aliases: &[&E if tuple .elts .iter() - .all(|elt| !checker.semantic().match_builtin_expr(elt, "TimeoutError")) + .all(|elt| !semantic.match_builtin_expr(elt, "TimeoutError")) { let node = ast::ExprName { id: "TimeoutError".into(), diff --git a/crates/ruff_linter/src/rules/ruff/rules/mutable_fromkeys_value.rs b/crates/ruff_linter/src/rules/ruff/rules/mutable_fromkeys_value.rs index b139d38407f2b..6cea64454b89f 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mutable_fromkeys_value.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mutable_fromkeys_value.rs @@ -73,7 +73,8 @@ pub(crate) fn mutable_fromkeys_value(checker: &mut Checker, call: &ast::ExprCall if attr != "fromkeys" { return; } - if !checker.semantic().match_builtin_expr(value, "dict") { + let semantic = checker.semantic(); + if !semantic.match_builtin_expr(value, "dict") { return; } @@ -81,7 +82,7 @@ pub(crate) fn mutable_fromkeys_value(checker: &mut Checker, call: &ast::ExprCall let [keys, value] = &*call.arguments.args else { return; }; - if !is_mutable_expr(value, checker.semantic()) { + if !is_mutable_expr(value, semantic) { return; } From f6cb958e38d15c97e164d6ed1852863a198196ea Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 15 Apr 2024 12:26:48 +0100 Subject: [PATCH 7/9] Improve other areas of the semantic model also --- .../resources/test/fixtures/refurb/FURB129.py | 16 +++++++++ ...es__refurb__tests__FURB129_FURB129.py.snap | 36 +++++++++++++++++++ .../src/analyze/function_type.rs | 6 ++-- .../src/analyze/typing.rs | 2 +- .../src/analyze/visibility.rs | 18 ++++------ 5 files changed, 63 insertions(+), 15 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py index afa58e9798af6..51a3cf73a575e 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB129.py @@ -41,6 +41,22 @@ def func(): pass +import builtins + + +with builtins.open("FURB129.py") as f: + for line in f.readlines(): + pass + + +from builtins import open as o + + +with o("FURB129.py") as f: + for line in f.readlines(): + pass + + # False positives def func(f): for _line in f.readlines(): diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap index e24fa164edfcb..655f8d69591d4 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB129_FURB129.py.snap @@ -204,4 +204,40 @@ FURB129.py:38:22: FURB129 [*] Instead of calling `readlines()`, iterate over fil 40 40 | for _line in bar.readlines(): 41 41 | pass +FURB129.py:48:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly + | +47 | with builtins.open("FURB129.py") as f: +48 | for line in f.readlines(): + | ^^^^^^^^^^^^^ FURB129 +49 | pass + | + = help: Remove `readlines()` + +ℹ Unsafe fix +45 45 | +46 46 | +47 47 | with builtins.open("FURB129.py") as f: +48 |- for line in f.readlines(): + 48 |+ for line in f: +49 49 | pass +50 50 | +51 51 | + +FURB129.py:56:17: FURB129 [*] Instead of calling `readlines()`, iterate over file object directly + | +55 | with o("FURB129.py") as f: +56 | for line in f.readlines(): + | ^^^^^^^^^^^^^ FURB129 +57 | pass + | + = help: Remove `readlines()` +ℹ Unsafe fix +53 53 | +54 54 | +55 55 | with o("FURB129.py") as f: +56 |- for line in f.readlines(): + 56 |+ for line in f: +57 57 | pass +58 58 | +59 59 | diff --git a/crates/ruff_python_semantic/src/analyze/function_type.rs b/crates/ruff_python_semantic/src/analyze/function_type.rs index ff6d3ea560c57..e249c45a64956 100644 --- a/crates/ruff_python_semantic/src/analyze/function_type.rs +++ b/crates/ruff_python_semantic/src/analyze/function_type.rs @@ -37,7 +37,7 @@ pub fn classify( semantic .resolve_qualified_name(map_callable(expr)) .is_some_and( |qualified_name| { - matches!(qualified_name.segments(), ["", "type"] | ["abc", "ABCMeta"]) + matches!(qualified_name.segments(), ["" | "builtins", "type"] | ["abc", "ABCMeta"]) }) }) || decorator_list.iter().any(|decorator| is_class_method(decorator, semantic, classmethod_decorators)) @@ -63,7 +63,7 @@ fn is_static_method( .is_some_and(|qualified_name| { matches!( qualified_name.segments(), - ["", "staticmethod"] | ["abc", "abstractstaticmethod"] + ["" | "builtins", "staticmethod"] | ["abc", "abstractstaticmethod"] ) || staticmethod_decorators .iter() .any(|decorator| qualified_name == QualifiedName::from_dotted_name(decorator)) @@ -103,7 +103,7 @@ fn is_class_method( .is_some_and(|qualified_name| { matches!( qualified_name.segments(), - ["", "classmethod"] | ["abc", "abstractclassmethod"] + ["" | "builtins", "classmethod"] | ["abc", "abstractclassmethod"] ) || classmethod_decorators .iter() .any(|decorator| qualified_name == QualifiedName::from_dotted_name(decorator)) diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index ea8d9b3659672..ce82294fbe900 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -686,7 +686,7 @@ impl TypeChecker for IoBaseChecker { .is_some_and(|qualified_name| { matches!( qualified_name.segments(), - ["io", "open" | "open_code"] | ["os" | "", "open"] + ["io", "open" | "open_code"] | ["os" | "" | "builtins", "open"] ) }) } diff --git a/crates/ruff_python_semantic/src/analyze/visibility.rs b/crates/ruff_python_semantic/src/analyze/visibility.rs index 99e107f3d8a3b..816e7678fadec 100644 --- a/crates/ruff_python_semantic/src/analyze/visibility.rs +++ b/crates/ruff_python_semantic/src/analyze/visibility.rs @@ -15,20 +15,16 @@ pub enum Visibility { /// Returns `true` if a function is a "static method". pub fn is_staticmethod(decorator_list: &[Decorator], semantic: &SemanticModel) -> bool { - decorator_list.iter().any(|decorator| { - semantic - .resolve_qualified_name(map_callable(&decorator.expression)) - .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", "staticmethod"])) - }) + decorator_list + .iter() + .any(|decorator| semantic.match_builtin_expr(&decorator.expression, "staticmethod")) } /// Returns `true` if a function is a "class method". pub fn is_classmethod(decorator_list: &[Decorator], semantic: &SemanticModel) -> bool { - decorator_list.iter().any(|decorator| { - semantic - .resolve_qualified_name(map_callable(&decorator.expression)) - .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", "classmethod"])) - }) + decorator_list + .iter() + .any(|decorator| semantic.match_builtin_expr(&decorator.expression, "classmethod")) } /// Returns `true` if a function definition is an `@overload`. @@ -79,7 +75,7 @@ pub fn is_property( .is_some_and(|qualified_name| { matches!( qualified_name.segments(), - ["", "property"] | ["functools", "cached_property"] + ["" | "builtins", "property"] | ["functools", "cached_property"] ) || extra_properties .iter() .any(|extra_property| extra_property.segments() == qualified_name.segments()) From 4567626dddd9e979efc0dd2c820b18efeb93799a Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 15 Apr 2024 12:46:44 +0100 Subject: [PATCH 8/9] Equivalent fixes to `ruff_python_stdlib` also --- crates/ruff_python_stdlib/src/typing.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/ruff_python_stdlib/src/typing.rs b/crates/ruff_python_stdlib/src/typing.rs index a917dfa1ca2ca..bfd8f7f5c04b0 100644 --- a/crates/ruff_python_stdlib/src/typing.rs +++ b/crates/ruff_python_stdlib/src/typing.rs @@ -5,12 +5,13 @@ pub fn is_standard_library_generic(qualified_name: &[&str]) -> bool { matches!( qualified_name, - ["", "dict" | "frozenset" | "list" | "set" | "tuple" | "type"] - | [ - "collections" | "typing" | "typing_extensions", - "ChainMap" | "Counter" - ] - | ["collections" | "typing", "OrderedDict"] + [ + "" | "builtins", + "dict" | "frozenset" | "list" | "set" | "tuple" | "type" + ] | [ + "collections" | "typing" | "typing_extensions", + "ChainMap" | "Counter" + ] | ["collections" | "typing", "OrderedDict"] | ["collections", "defaultdict" | "deque"] | [ "collections", @@ -247,7 +248,7 @@ pub fn is_immutable_non_generic_type(qualified_name: &[&str]) -> bool { pub fn is_immutable_generic_type(qualified_name: &[&str]) -> bool { matches!( qualified_name, - ["", "tuple"] + ["" | "builtins", "tuple"] | [ "collections", "abc", @@ -285,7 +286,7 @@ pub fn is_immutable_generic_type(qualified_name: &[&str]) -> bool { pub fn is_mutable_return_type(qualified_name: &[&str]) -> bool { matches!( qualified_name, - ["", "dict" | "list" | "set"] + ["" | "builtins", "dict" | "list" | "set"] | [ "collections", "Counter" | "OrderedDict" | "defaultdict" | "deque" From e441e77830c88def6aa1f0e450fe81224343bd89 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 15 Apr 2024 13:56:02 +0100 Subject: [PATCH 9/9] Reimplement the high-level abstraction based on a lower-level one --- crates/ruff_linter/src/checkers/ast/mod.rs | 14 ++---- .../src/rules/flake8_bandit/helpers.rs | 18 ++------ .../rules/assert_raises_exception.rs | 22 ++++----- .../rules/unreliable_callable_check.rs | 8 ++-- .../rules/redundant_numeric_union.rs | 16 +++---- .../src/rules/pylint/rules/nested_min_max.rs | 7 ++- .../src/rules/refurb/rules/bit_count.rs | 14 ++---- .../tryceratops/rules/raise_within_try.rs | 9 +--- .../rules/type_check_without_type_error.rs | 29 +++++------- crates/ruff_python_ast/src/name.rs | 10 +++- crates/ruff_python_semantic/src/model.rs | 46 ++++++++++++++----- 11 files changed, 96 insertions(+), 97 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index aa58f24972c04..8cf3cd09a1ba9 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -784,17 +784,13 @@ impl<'a> Visitor<'a> for Checker<'a> { }) => { let mut handled_exceptions = Exceptions::empty(); for type_ in extract_handled_exceptions(handlers) { - if let Some(qualified_name) = self.semantic.resolve_qualified_name(type_) { - match qualified_name.segments() { - ["" | "builtins", "NameError"] => { - handled_exceptions |= Exceptions::NAME_ERROR; - } - ["" | "builtins", "ModuleNotFoundError"] => { + if let Some(builtins_name) = self.semantic.resolve_builtin_symbol(type_) { + match builtins_name { + "NameError" => handled_exceptions |= Exceptions::NAME_ERROR, + "ModuleNotFoundError" => { handled_exceptions |= Exceptions::MODULE_NOT_FOUND_ERROR; } - ["" | "builtins", "ImportError"] => { - handled_exceptions |= Exceptions::IMPORT_ERROR; - } + "ImportError" => handled_exceptions |= Exceptions::IMPORT_ERROR, _ => {} } } diff --git a/crates/ruff_linter/src/rules/flake8_bandit/helpers.rs b/crates/ruff_linter/src/rules/flake8_bandit/helpers.rs index 8db1deeff49f8..9f287804ff68d 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/helpers.rs @@ -24,23 +24,13 @@ pub(super) fn is_untyped_exception(type_: Option<&Expr>, semantic: &SemanticMode if let Expr::Tuple(ast::ExprTuple { elts, .. }) = &type_ { elts.iter().any(|type_| { semantic - .resolve_qualified_name(type_) - .is_some_and(|qualified_name| { - matches!( - qualified_name.segments(), - ["" | "builtins", "Exception" | "BaseException"] - ) - }) + .resolve_builtin_symbol(type_) + .is_some_and(|builtin| matches!(builtin, "Exception" | "BaseException")) }) } else { semantic - .resolve_qualified_name(type_) - .is_some_and(|qualified_name| { - matches!( - qualified_name.segments(), - ["" | "builtins", "Exception" | "BaseException"] - ) - }) + .resolve_builtin_symbol(type_) + .is_some_and(|builtin| matches!(builtin, "Exception" | "BaseException")) } }) } diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs index 200fddc654fb5..18c84a84c19e2 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_raises_exception.rs @@ -95,24 +95,22 @@ pub(crate) fn assert_raises_exception(checker: &mut Checker, items: &[WithItem]) return; }; - let Some(exception) = - checker - .semantic() - .resolve_qualified_name(arg) - .and_then(|qualified_name| match qualified_name.segments() { - ["" | "builtins", "Exception"] => Some(ExceptionKind::Exception), - ["" | "builtins", "BaseException"] => Some(ExceptionKind::BaseException), - _ => None, - }) - else { + let semantic = checker.semantic(); + + let Some(builtin_symbol) = semantic.resolve_builtin_symbol(arg) else { return; }; + let exception = match builtin_symbol { + "Exception" => ExceptionKind::Exception, + "BaseException" => ExceptionKind::BaseException, + _ => return, + }; + let assertion = if matches!(func.as_ref(), Expr::Attribute(ast::ExprAttribute { attr, .. }) if attr == "assertRaises") { AssertionKind::AssertRaises - } else if checker - .semantic() + } else if semantic .resolve_qualified_name(func) .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["pytest", "raises"])) && arguments.find_keyword("match").is_none() diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs index f1c006f7b16b5..d076946330a96 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs @@ -66,15 +66,15 @@ pub(crate) fn unreliable_callable_check( if value != "__call__" { return; } - let Some(qualified_name) = checker.semantic().resolve_qualified_name(func) else { + let Some(builtins_function) = checker.semantic().resolve_builtin_symbol(func) else { return; }; - let ["" | "builtins", function @ ("hasattr" | "getattr")] = qualified_name.segments() else { + if !matches!(builtins_function, "hasattr" | "getattr") { return; - }; + } let mut diagnostic = Diagnostic::new(UnreliableCallableCheck, expr.range()); - if *function == "hasattr" { + if builtins_function == "hasattr" { diagnostic.try_set_fix(|| { let (import_edit, binding) = checker.importer().get_or_import_builtin_symbol( "callable", diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs index 746fd8c1fe723..b8426eae856d9 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs @@ -96,20 +96,20 @@ fn check_annotation(checker: &mut Checker, annotation: &Expr) { let mut has_complex = false; let mut has_int = false; - let mut func = |expr: &Expr, _parent: &Expr| { - let Some(qualified_name) = checker.semantic().resolve_qualified_name(expr) else { + let mut find_numeric_type = |expr: &Expr, _parent: &Expr| { + let Some(builtin_type) = checker.semantic().resolve_builtin_symbol(expr) else { return; }; - match qualified_name.segments() { - ["" | "builtins", "int"] => has_int = true, - ["" | "builtins", "float"] => has_float = true, - ["" | "builtins", "complex"] => has_complex = true, - _ => (), + match builtin_type { + "int" => has_int = true, + "float" => has_float = true, + "complex" => has_complex = true, + _ => {} } }; - traverse_union(&mut func, checker.semantic(), annotation); + traverse_union(&mut find_numeric_type, checker.semantic(), annotation); if has_complex { if has_float { diff --git a/crates/ruff_linter/src/rules/pylint/rules/nested_min_max.rs b/crates/ruff_linter/src/rules/pylint/rules/nested_min_max.rs index 00000c0fd7c04..b2840e2d0740b 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/nested_min_max.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/nested_min_max.rs @@ -68,10 +68,9 @@ impl MinMax { if !keywords.is_empty() { return None; } - let qualified_name = semantic.resolve_qualified_name(func)?; - match qualified_name.segments() { - ["" | "builtins", "min"] => Some(MinMax::Min), - ["" | "builtins", "max"] => Some(MinMax::Max), + match semantic.resolve_builtin_symbol(func)? { + "min" => Some(Self::Min), + "max" => Some(Self::Max), _ => None, } } diff --git a/crates/ruff_linter/src/rules/refurb/rules/bit_count.rs b/crates/ruff_linter/src/rules/refurb/rules/bit_count.rs index c8f2e56fcaa4e..35f85b3c56077 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/bit_count.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/bit_count.rs @@ -97,15 +97,6 @@ pub(crate) fn bit_count(checker: &mut Checker, call: &ExprCall) { return; }; - // Ensure that we're performing a `bin(...)`. - if !checker - .semantic() - .resolve_qualified_name(func) - .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["" | "builtins", "bin"])) - { - return; - } - if !arguments.keywords.is_empty() { return; }; @@ -113,6 +104,11 @@ pub(crate) fn bit_count(checker: &mut Checker, call: &ExprCall) { return; }; + // Ensure that we're performing a `bin(...)`. + if !checker.semantic().match_builtin_expr(func, "bin") { + return; + } + // Extract, e.g., `x` in `bin(x)`. let literal_text = checker.locator().slice(arg); diff --git a/crates/ruff_linter/src/rules/tryceratops/rules/raise_within_try.rs b/crates/ruff_linter/src/rules/tryceratops/rules/raise_within_try.rs index c4d1c88c88764..c578468b28af0 100644 --- a/crates/ruff_linter/src/rules/tryceratops/rules/raise_within_try.rs +++ b/crates/ruff_linter/src/rules/tryceratops/rules/raise_within_try.rs @@ -111,13 +111,8 @@ pub(crate) fn raise_within_try(checker: &mut Checker, body: &[Stmt], handlers: & || handled_exceptions.iter().any(|expr| { checker .semantic() - .resolve_qualified_name(expr) - .is_some_and(|qualified_name| { - matches!( - qualified_name.segments(), - ["" | "builtins", "Exception" | "BaseException"] - ) - }) + .resolve_builtin_symbol(expr) + .is_some_and(|builtin| matches!(builtin, "Exception" | "BaseException")) }) { checker diff --git a/crates/ruff_linter/src/rules/tryceratops/rules/type_check_without_type_error.rs b/crates/ruff_linter/src/rules/tryceratops/rules/type_check_without_type_error.rs index 370e4ab698ad5..002fac596cb4c 100644 --- a/crates/ruff_linter/src/rules/tryceratops/rules/type_check_without_type_error.rs +++ b/crates/ruff_linter/src/rules/tryceratops/rules/type_check_without_type_error.rs @@ -74,26 +74,20 @@ fn has_control_flow(stmt: &Stmt) -> bool { } /// Returns `true` if an [`Expr`] is a call to check types. -fn check_type_check_call(checker: &mut Checker, call: &Expr) -> bool { - checker - .semantic() - .resolve_qualified_name(call) - .is_some_and(|qualified_name| { - matches!( - qualified_name.segments(), - ["" | "builtins", "isinstance" | "issubclass" | "callable"] - ) - }) +fn check_type_check_call(semantic: &SemanticModel, call: &Expr) -> bool { + semantic + .resolve_builtin_symbol(call) + .is_some_and(|builtin| matches!(builtin, "isinstance" | "issubclass" | "callable")) } /// Returns `true` if an [`Expr`] is a test to check types (e.g. via isinstance) -fn check_type_check_test(checker: &mut Checker, test: &Expr) -> bool { +fn check_type_check_test(semantic: &SemanticModel, test: &Expr) -> bool { match test { Expr::BoolOp(ast::ExprBoolOp { values, .. }) => values .iter() - .all(|expr| check_type_check_test(checker, expr)), - Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) => check_type_check_test(checker, operand), - Expr::Call(ast::ExprCall { func, .. }) => check_type_check_call(checker, func), + .all(|expr| check_type_check_test(semantic, expr)), + Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) => check_type_check_test(semantic, operand), + Expr::Call(ast::ExprCall { func, .. }) => check_type_check_call(semantic, func), _ => false, } } @@ -161,14 +155,15 @@ pub(crate) fn type_check_without_type_error( elif_else_clauses, .. } = stmt_if; + if let Some(Stmt::If(ast::StmtIf { test, .. })) = parent { - if !check_type_check_test(checker, test) { + if !check_type_check_test(checker.semantic(), test) { return; } } // Only consider the body when the `if` condition is all type-related - if !check_type_check_test(checker, test) { + if !check_type_check_test(checker.semantic(), test) { return; } check_body(checker, body); @@ -176,7 +171,7 @@ pub(crate) fn type_check_without_type_error( for clause in elif_else_clauses { if let Some(test) = &clause.test { // If there are any `elif`, they must all also be type-related - if !check_type_check_test(checker, test) { + if !check_type_check_test(checker.semantic(), test) { return; } } diff --git a/crates/ruff_python_ast/src/name.rs b/crates/ruff_python_ast/src/name.rs index 225a8e749067a..9796a7eb16fd8 100644 --- a/crates/ruff_python_ast/src/name.rs +++ b/crates/ruff_python_ast/src/name.rs @@ -47,9 +47,15 @@ impl<'a> QualifiedName<'a> { self.0.as_slice() } - /// If the first segment is empty, the `CallPath` is that of a builtin. + /// If the first segment is empty, the `CallPath` represents a "builtin binding". + /// + /// A builtin binding is the binding that a symbol has if it was part of Python's + /// global scope without any imports taking place. However, if builtin members are + /// accessed explicitly via the `builtins` module, they will not have a + /// "builtin binding", so this method will return `false`. + /// /// Ex) `["", "bool"]` -> `"bool"` - pub fn is_builtin(&self) -> bool { + fn is_builtin(&self) -> bool { matches!(self.segments(), ["", ..]) } diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index eb1b7e6d21fe3..bb8e8973b8cea 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -251,27 +251,51 @@ impl<'a> SemanticModel<'a> { } /// Return `true` if `member` is bound as a builtin. + /// + /// Note that a "builtin binding" does *not* include explicit lookups via the `builtins` + /// module, e.g. `import builtins; builtins.open`. It *only* includes the bindings + /// that are pre-populated in Python's global scope before any imports have taken place. pub fn is_builtin(&self, member: &str) -> bool { self.lookup_symbol(member) .map(|binding_id| &self.bindings[binding_id]) .is_some_and(|binding| binding.kind.is_builtin()) } - /// Return `true` if `member` is a reference to `builtins.$target`, + /// If `expr` is a reference to a builtins symbol, + /// return the name of that symbol. Else, return `None`. + /// + /// This method returns `true` both for "builtin bindings" + /// (present even without any imports, e.g. `open()`), and for explicit lookups + /// via the `builtins` module (e.g. `import builtins; builtins.open()`). + pub fn resolve_builtin_symbol<'expr>(&'a self, expr: &'expr Expr) -> Option<&'a str> + where + 'expr: 'a, + { + // Fast path: we only need to worry about name expressions + if !self.seen_module(Modules::BUILTINS) { + let name = &expr.as_name_expr()?.id; + return if self.is_builtin(name) { + Some(name) + } else { + None + }; + } + + // Slow path: we have to consider names and attributes + let qualified_name = self.resolve_qualified_name(expr)?; + match qualified_name.segments() { + ["" | "builtins", name] => Some(*name), + _ => None, + } + } + + /// Return `true` if `expr` is a reference to `builtins.$target`, /// i.e. either `object` (where `object` is not overridden in the global scope), /// or `builtins.object` (where `builtins` is imported as a module at the top level) pub fn match_builtin_expr(&self, expr: &Expr, symbol: &str) -> bool { debug_assert!(!symbol.contains('.')); - if self.seen_module(Modules::BUILTINS) { - // slow path - self.resolve_qualified_name(expr) - .is_some_and(|qualified_name| { - matches!(qualified_name.segments(), ["builtins" | "", member] if *member == symbol) - }) - } else { - // fast(er) path - matches!(expr, Expr::Name(ast::ExprName {id, ..}) if id == symbol && self.is_builtin(symbol)) - } + self.resolve_builtin_symbol(expr) + .is_some_and(|name| name == symbol) } /// Return `true` if `member` is an "available" symbol, i.e., a symbol that has not been bound