Enable attribute lookups via semantic model (#5536)

## 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.
This commit is contained in:
Charlie Marsh 2023-07-05 15:19:14 -04:00 committed by GitHub
parent 9478454b96
commit 9e1039f823
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 89 additions and 31 deletions

View file

@ -29,6 +29,26 @@ raise TypeError(
# 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()

View file

@ -1781,7 +1781,6 @@ where
match stmt {
Stmt::FunctionDef(ast::StmtFunctionDef {
body,
name,
args,
decorator_list,
returns,
@ -1789,7 +1788,6 @@ where
})
| Stmt::AsyncFunctionDef(ast::StmtAsyncFunctionDef {
body,
name,
args,
decorator_list,
returns,
@ -1848,13 +1846,6 @@ where
};
}
self.add_binding(
name,
stmt.identifier(),
BindingKind::FunctionDefinition,
BindingFlags::empty(),
);
let definition = docstrings::extraction::extract_definition(
ExtractionTarget::Function,
stmt,
@ -2064,19 +2055,28 @@ where
// Post-visit.
match stmt {
Stmt::FunctionDef(_) | Stmt::AsyncFunctionDef(_) => {
self.deferred.scopes.push(self.semantic.scope_id);
self.semantic.pop_scope();
self.semantic.pop_definition();
}
Stmt::ClassDef(ast::StmtClassDef { name, .. }) => {
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::ClassDefinition,
BindingKind::FunctionDefinition(scope_id),
BindingFlags::empty(),
);
}
Stmt::ClassDef(ast::StmtClassDef { 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::ClassDefinition(scope_id),
BindingFlags::empty(),
);
}
@ -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(

View file

@ -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))

View file

@ -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;
@ -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);

View file

@ -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
@ -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

View file

@ -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(_)
)
}
@ -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

View file

@ -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() {
@ -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,