Add scope

astral-sh · Jul 5, 2023 · 8e8a334 · 8e8a334
1 parent 9a8e5f7
commit 8e8a334
Show file tree

Hide file tree

Showing 8 changed files with 256 additions and 27 deletions.
diff --git a/crates/ruff/resources/test/fixtures/flake8_raise/RSE102.py b/crates/ruff/resources/test/fixtures/flake8_raise/RSE102.py
@@ -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()
diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs
@@ -1778,15 +1778,13 @@ where
         match stmt {
             Stmt::FunctionDef(ast::StmtFunctionDef {
                 body,
-                name,
                 args,
                 decorator_list,
                 returns,
                 ..
             })
             | Stmt::AsyncFunctionDef(ast::StmtAsyncFunctionDef {
                 body,
-                name,
                 args,
                 decorator_list,
                 returns,
@@ -1845,13 +1843,6 @@ where
                     };
                 }
 
-                self.add_binding(
-                    name,
-                    stmt.identifier(),
-                    BindingKind::FunctionDefinition,
-                    BindingFlags::empty(),
-                );
-
                 let definition = docstrings::extraction::extract_definition(
                     ExtractionTarget::Function,
                     stmt,
@@ -2061,19 +2052,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(),
                 );
             }
@@ -3884,7 +3884,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(

diff --git a/crates/ruff/src/renamer.rs b/crates/ruff/src/renamer.rs
@@ -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))

diff --git a/crates/ruff/src/rules/flake8_raise/rules/unnecessary_paren_on_raise_exception.rs b/crates/ruff/src/rules/flake8_raise/rules/unnecessary_paren_on_raise_exception.rs
@@ -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);

diff --git a/...ots/ruff__rules__flake8_raise__tests__unnecessary-paren-on-raise-exception_RSE102.py.snap b/...ots/ruff__rules__flake8_raise__tests__unnecessary-paren-on-raise-exception_RSE102.py.snap
@@ -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,40 @@ 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
+
+RSE102.py:44:19: RSE102 [*] Unnecessary parentheses on raised exception
+   |
+43 | # OK
+44 | raise return_error()
+   |                   ^^ RSE102
+   |
+   = help: Remove unnecessary parentheses
+
+ℹ Fix
+41 41 | 
+42 42 | 
+43 43 | # OK
+44    |-raise return_error()
+   44 |+raise return_error
+45 45 | 
+46 46 | 
+47 47 | class Class:
+
+RSE102.py:54:18: RSE102 [*] Unnecessary parentheses on raised exception
+   |
+53 | # OK
+54 | raise Class.error()
+   |                  ^^ RSE102
+   |
+   = help: Remove unnecessary parentheses
+
+ℹ Fix
+51 51 | 
+52 52 | 
+53 53 | # OK
+54    |-raise Class.error()
+   54 |+raise Class.error
 
 
diff --git a/...ruff__rules__flake8_raise__tests__unnecessary-paren-on-raise-exception_RSE102.py.snap.new b/...ruff__rules__flake8_raise__tests__unnecessary-paren-on-raise-exception_RSE102.py.snap.new
@@ -0,0 +1,138 @@
+---
+source: crates/ruff/src/rules/flake8_raise/mod.rs
+assertion_line: 23
+---
+RSE102.py:5:21: RSE102 [*] Unnecessary parentheses on raised exception
+  |
+3 | except TypeError:
+4 |     # RSE102
+5 |     raise ValueError()
+  |                     ^^ RSE102
+6 | 
+7 | try:
+  |
+  = help: Remove unnecessary parentheses
+
+ℹ Fix
+2 2 |     y = 6 + "7"
+3 3 | except TypeError:
+4 4 |     # RSE102
+5   |-    raise ValueError()
+  5 |+    raise ValueError
+6 6 | 
+7 7 | try:
+8 8 |     x = 1 / 0
+
+RSE102.py:13:16: RSE102 [*] Unnecessary parentheses on raised exception
+   |
+12 | # RSE102
+13 | raise TypeError()
+   |                ^^ RSE102
+14 | 
+15 | # RSE102
+   |
+   = help: Remove unnecessary parentheses
+
+ℹ Fix
+10 10 |     raise
+11 11 | 
+12 12 | # RSE102
+13    |-raise TypeError()
+   13 |+raise TypeError
+14 14 | 
+15 15 | # RSE102
+16 16 | raise TypeError ()
+
+RSE102.py:16:17: RSE102 [*] Unnecessary parentheses on raised exception
+   |
+15 | # RSE102
+16 | raise TypeError ()
+   |                 ^^ RSE102
+17 | 
+18 | # RSE102
+   |
+   = help: Remove unnecessary parentheses
+
+ℹ Fix
+13 13 | raise TypeError()
+14 14 | 
+15 15 | # RSE102
+16    |-raise TypeError ()
+   16 |+raise TypeError
+17 17 | 
+18 18 | # RSE102
+19 19 | raise TypeError \
+
+RSE102.py:20:5: RSE102 [*] Unnecessary parentheses on raised exception
+   |
+18 | # RSE102
+19 | raise TypeError \
+20 |     ()
+   |     ^^ RSE102
+21 | 
+22 | # RSE102
+   |
+   = help: Remove unnecessary parentheses
+
+ℹ Fix
+16 16 | raise TypeError ()
+17 17 | 
+18 18 | # RSE102
+19    |-raise TypeError \
+20    |-    ()
+   19 |+raise TypeError
+21 20 | 
+22 21 | # RSE102
+23 22 | raise TypeError(
+
+RSE102.py:23:16: RSE102 [*] Unnecessary parentheses on raised exception
+   |
+22 |   # RSE102
+23 |   raise TypeError(
+   |  ________________^
+24 | | 
+25 | | )
+   | |_^ RSE102
+26 |   
+27 |   # RSE102
+   |
+   = help: Remove unnecessary parentheses
+
+ℹ Fix
+20 20 |     ()
+21 21 | 
+22 22 | # RSE102
+23    |-raise TypeError(
+24    |-
+25    |-)
+   23 |+raise TypeError
+26 24 | 
+27 25 | # RSE102
+28 26 | raise TypeError(
+
+RSE102.py:28:16: RSE102 [*] Unnecessary parentheses on raised exception
+   |
+27 |   # RSE102
+28 |   raise TypeError(
+   |  ________________^
+29 | |     # Hello, world!
+30 | | )
+   | |_^ RSE102
+31 |   
+32 |   # OK
+   |
+   = help: Remove unnecessary parentheses
+
+ℹ Fix
+25 25 | )
+26 26 | 
+27 27 | # RSE102
+28    |-raise TypeError(
+29    |-    # Hello, world!
+30    |-)
+   28 |+raise TypeError
+31 29 | 
+32 30 | # OK
+33 31 | raise AssertionError
+
+
diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs
@@ -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

diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs
@@ -412,9 +412,35 @@ impl<'a> SemanticModel<'a> {
         }
     }
 
+    /// 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)
+    }
+
     /// 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() {