Skip to content

Commit

Permalink
Respect scoping rules when identifying builtins (#6468)
Browse files Browse the repository at this point in the history
## 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 #6466.

## Test Plan

`cargo test`
  • Loading branch information
charliermarsh authored Aug 10, 2023
1 parent dc3275f commit 6706ae4
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 2 deletions.
27 changes: 27 additions & 0 deletions crates/ruff/resources/test/fixtures/pycodestyle/E721.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
...
Original file line number Diff line number Diff line change
Expand Up @@ -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 | ...
|


6 changes: 4 additions & 2 deletions crates/ruff_python_semantic/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

Expand Down

0 comments on commit 6706ae4

Please sign in to comment.