Skip to content

Commit

Permalink
Enable attribute lookups via semantic model (#5536)
Browse files Browse the repository at this point in the history
## Summary

This PR enables us to resolve attribute accesses within files, at least
for static and class methods. For example, we can now detect that this
is a function access (and avoid a false-positive):

```python
class Class:
    @staticmethod
    def error():
        return ValueError("Something")


# OK
raise Class.error()
```

Closes #5487.

Closes #5416.
  • Loading branch information
charliermarsh authored Jul 5, 2023
1 parent 9478454 commit 9e1039f
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 27 deletions.
20 changes: 20 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_raise/RSE102.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,26 @@
# Hello, world!
)

# OK
raise AssertionError

# OK
raise AttributeError("test message")


def return_error():
return ValueError("Something")


# OK
raise return_error()


class Class:
@staticmethod
def error():
return ValueError("Something")


# OK
raise Class.error()
28 changes: 14 additions & 14 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1781,15 +1781,13 @@ where
match stmt {
Stmt::FunctionDef(ast::StmtFunctionDef {
body,
name,
args,
decorator_list,
returns,
..
})
| Stmt::AsyncFunctionDef(ast::StmtAsyncFunctionDef {
body,
name,
args,
decorator_list,
returns,
Expand Down Expand Up @@ -1848,13 +1846,6 @@ where
};
}

self.add_binding(
name,
stmt.identifier(),
BindingKind::FunctionDefinition,
BindingFlags::empty(),
);

let definition = docstrings::extraction::extract_definition(
ExtractionTarget::Function,
stmt,
Expand Down Expand Up @@ -2064,19 +2055,28 @@ where

// Post-visit.
match stmt {
Stmt::FunctionDef(_) | Stmt::AsyncFunctionDef(_) => {
self.deferred.scopes.push(self.semantic.scope_id);
Stmt::FunctionDef(ast::StmtFunctionDef { name, .. })
| Stmt::AsyncFunctionDef(ast::StmtAsyncFunctionDef { name, .. }) => {
let scope_id = self.semantic.scope_id;
self.deferred.scopes.push(scope_id);
self.semantic.pop_scope();
self.semantic.pop_definition();
self.add_binding(
name,
stmt.identifier(),
BindingKind::FunctionDefinition(scope_id),
BindingFlags::empty(),
);
}
Stmt::ClassDef(ast::StmtClassDef { name, .. }) => {
self.deferred.scopes.push(self.semantic.scope_id);
let scope_id = self.semantic.scope_id;
self.deferred.scopes.push(scope_id);
self.semantic.pop_scope();
self.semantic.pop_definition();
self.add_binding(
name,
stmt.identifier(),
BindingKind::ClassDefinition,
BindingKind::ClassDefinition(scope_id),
BindingFlags::empty(),
);
}
Expand Down Expand Up @@ -3887,7 +3887,7 @@ where
}

// Store the existing binding, if any.
let existing_id = self.semantic.lookup(name);
let existing_id = self.semantic.lookup_symbol(name);

// Add the bound exception name to the scope.
let binding_id = self.add_binding(
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/renamer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,8 @@ impl Renamer {
| BindingKind::LoopVar
| BindingKind::Global
| BindingKind::Nonlocal(_)
| BindingKind::ClassDefinition
| BindingKind::FunctionDefinition
| BindingKind::ClassDefinition(_)
| BindingKind::FunctionDefinition(_)
| BindingKind::Deletion
| BindingKind::UnboundException(_) => {
Some(Edit::range_replacement(target.to_string(), binding.range))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use rustpython_parser::ast::{self, Expr, Ranged};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};

use ruff_python_ast::helpers::match_parens;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -53,6 +54,17 @@ pub(crate) fn unnecessary_paren_on_raise_exception(checker: &mut Checker, expr:
}) = expr
{
if args.is_empty() && keywords.is_empty() {
// `raise func()` still requires parentheses; only `raise Class()` does not.
if checker
.semantic()
.lookup_attribute(func)
.map_or(false, |id| {
checker.semantic().binding(id).kind.is_function_definition()
})
{
return;
}

let range = match_parens(func.end(), checker.locator)
.expect("Expected call to include parentheses");
let mut diagnostic = Diagnostic::new(UnnecessaryParenOnRaiseException, range);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ RSE102.py:28:16: RSE102 [*] Unnecessary parentheses on raised exception
30 | | )
| |_^ RSE102
31 |
32 | raise AssertionError
32 | # OK
|
= help: Remove unnecessary parentheses

Expand All @@ -131,7 +131,7 @@ RSE102.py:28:16: RSE102 [*] Unnecessary parentheses on raised exception
30 |-)
28 |+raise TypeError
31 29 |
32 30 | raise AssertionError
33 31 |
32 30 | # OK
33 31 | raise AssertionError


14 changes: 7 additions & 7 deletions crates/ruff_python_semantic/src/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,11 @@ impl<'a> Binding<'a> {
}
matches!(
existing.kind,
BindingKind::ClassDefinition
| BindingKind::FunctionDefinition
| BindingKind::Import(..)
| BindingKind::FromImport(..)
| BindingKind::SubmoduleImport(..)
BindingKind::ClassDefinition(_)
| BindingKind::FunctionDefinition(_)
| BindingKind::Import(_)
| BindingKind::FromImport(_)
| BindingKind::SubmoduleImport(_)
)
}

Expand Down Expand Up @@ -372,14 +372,14 @@ pub enum BindingKind<'a> {
/// class Foo:
/// ...
/// ```
ClassDefinition,
ClassDefinition(ScopeId),

/// A binding for a function, like `foo` in:
/// ```python
/// def foo():
/// ...
/// ```
FunctionDefinition,
FunctionDefinition(ScopeId),

/// A binding for an `__all__` export, like `__all__` in:
/// ```python
Expand Down
28 changes: 27 additions & 1 deletion crates/ruff_python_semantic/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ impl<'a> SemanticModel<'a> {

/// Lookup a symbol in the current scope. This is a carbon copy of [`Self::resolve_read`], but
/// doesn't add any read references to the resolved symbol.
pub fn lookup(&mut self, symbol: &str) -> Option<BindingId> {
pub fn lookup_symbol(&self, symbol: &str) -> Option<BindingId> {
if self.in_forward_reference() {
if let Some(binding_id) = self.scopes.global().get(symbol) {
if !self.bindings[binding_id].is_unbound() {
Expand Down Expand Up @@ -456,6 +456,32 @@ impl<'a> SemanticModel<'a> {
None
}

/// Lookup a qualified attribute in the current scope.
///
/// For example, given `["Class", "method"`], resolve the `BindingKind::ClassDefinition`
/// associated with `Class`, then the `BindingKind::FunctionDefinition` associated with
/// `Class#method`.
pub fn lookup_attribute(&'a self, value: &'a Expr) -> Option<BindingId> {
let call_path = collect_call_path(value)?;

// Find the symbol in the current scope.
let (symbol, attribute) = call_path.split_first()?;
let mut binding_id = self.lookup_symbol(symbol)?;

// Recursively resolve class attributes, e.g., `foo.bar.baz` in.
let mut tail = attribute;
while let Some((symbol, rest)) = tail.split_first() {
// Find the next symbol in the class scope.
let BindingKind::ClassDefinition(scope_id) = self.binding(binding_id).kind else {
return None;
};
binding_id = self.scopes[scope_id].get(symbol)?;
tail = rest;
}

Some(binding_id)
}

/// Given a `BindingId`, return the `BindingId` of the submodule import that it aliases.
fn resolve_submodule(
&self,
Expand Down

0 comments on commit 9e1039f

Please sign in to comment.