From 6706ae48286bd1efd5d750d4d8103658e9cfe8cd Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 10 Aug 2023 10:20:09 -0400 Subject: [PATCH] Respect scoping rules when identifying builtins (#6468) ## Summary Our `is_builtin` check did a naive walk over the parent scopes; instead, it needs to (e.g.) skip symbols in a class scope if being called outside of the class scope itself. Closes https://github.com/astral-sh/ruff/issues/6466. ## Test Plan `cargo test` --- .../test/fixtures/pycodestyle/E721.py | 27 +++++++++++++++++++ ...les__pycodestyle__tests__E721_E721.py.snap | 18 +++++++++++++ crates/ruff_python_semantic/src/model.rs | 6 +++-- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pycodestyle/E721.py b/crates/ruff/resources/test/fixtures/pycodestyle/E721.py index 5f07803832eca..6eacc0cb0c9ab 100644 --- a/crates/ruff/resources/test/fixtures/pycodestyle/E721.py +++ b/crates/ruff/resources/test/fixtures/pycodestyle/E721.py @@ -61,3 +61,30 @@ #: E721 assert type(res) is int + + +class Foo: + def asdf(self, value: str | None): + #: E721 + if type(value) is str: + ... + + +class Foo: + def type(self): + pass + + def asdf(self, value: str | None): + #: E721 + if type(value) is str: + ... + + +class Foo: + def asdf(self, value: str | None): + def type(): + pass + + # Okay + if type(value) is str: + ... diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E721_E721.py.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E721_E721.py.snap index 197999938484d..50476a509094a 100644 --- a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E721_E721.py.snap +++ b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E721_E721.py.snap @@ -152,4 +152,22 @@ E721.py:63:8: E721 Do not compare types, use `isinstance()` | ^^^^^^^^^^^^^^^^ E721 | +E721.py:69:12: E721 Do not compare types, use `isinstance()` + | +67 | def asdf(self, value: str | None): +68 | #: E721 +69 | if type(value) is str: + | ^^^^^^^^^^^^^^^^^^ E721 +70 | ... + | + +E721.py:79:12: E721 Do not compare types, use `isinstance()` + | +77 | def asdf(self, value: str | None): +78 | #: E721 +79 | if type(value) is str: + | ^^^^^^^^^^^^^^^^^^ E721 +80 | ... + | + diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index b6e7faa3db5f4..0a97a0a638c4f 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -249,14 +249,16 @@ impl<'a> SemanticModel<'a> { /// Return `true` if `member` is bound as a builtin. pub fn is_builtin(&self, member: &str) -> bool { - self.find_binding(member) + self.lookup_symbol(member) + .map(|binding_id| &self.bindings[binding_id]) .is_some_and(|binding| binding.kind.is_builtin()) } /// 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 { - self.find_binding(member) + self.lookup_symbol(member) + .map(|binding_id| &self.bindings[binding_id]) .map_or(true, |binding| binding.kind.is_builtin()) }