[ty] name is parameter and global is a syntax error (#21312)

Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>

crates/ruff_linter/src/checkers/ast/mod.rs

@@ -860,24 +860,18 @@ impl SemanticSyntaxContext for Checker<'_> {
     }
 
     fn is_bound_parameter(&self, name: &str) -> bool {
-        for scope in self.semantic.current_scopes() {
+        match self.semantic.current_scope().kind {
-            match scope.kind {
+            ScopeKind::Function(ast::StmtFunctionDef { parameters, .. }) => {
-                ScopeKind::Class(_) => return false,
+                parameters.includes(name)
-                ScopeKind::Function(ast::StmtFunctionDef { parameters, .. })
+            }
-                | ScopeKind::Lambda(ast::ExprLambda {
+            ScopeKind::Class(_)
-                    parameters: Some(parameters),
+            | ScopeKind::Lambda(_)
-                    ..
-                }) => return parameters.includes(name),
-                ScopeKind::Lambda(_)
             | ScopeKind::Generator { .. }
             | ScopeKind::Module
             | ScopeKind::Type
-                | ScopeKind::DunderClassCell => {}
+            | ScopeKind::DunderClassCell => false,
         }
     }
-
-        false
-    }
 }
 
 impl<'a> Visitor<'a> for Checker<'a> {

crates/ruff_python_parser/src/semantic_errors.rs

@@ -2107,8 +2107,10 @@ pub trait SemanticSyntaxContext {
 
     fn report_semantic_error(&self, error: SemanticSyntaxError);
 
+    /// Returns `true` if the visitor is inside a `for` or `while` loop.
     fn in_loop_context(&self) -> bool;
 
+    /// Returns `true` if `name` is a bound parameter in the current function or lambda scope.
     fn is_bound_parameter(&self, name: &str) -> bool;
 }
 

crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md

@@ -376,3 +376,41 @@ for x in range(42):
         break  # error: [invalid-syntax]
         continue  # error: [invalid-syntax]
 ```
+
+## name is parameter and global
+
+<!-- snapshot-diagnostics -->
+
+```py
+a = None
+
+def f(a):
+    global a  # error: [invalid-syntax]
+
+def g(a):
+    if True:
+        global a  # error: [invalid-syntax]
+
+def h(a):
+    def inner():
+        global a
+
+def i(a):
+    try:
+        global a  # error: [invalid-syntax]
+    except Exception:
+        pass
+
+def f(a):
+    a = 1
+    global a  # error: [invalid-syntax]
+
+def f(a):
+    a = 1
+    a = 2
+    global a  # error: [invalid-syntax]
+
+def f(a):
+    class Inner:
+        global a  # ok
+```

crates/ty_python_semantic/resources/mdtest/snapshots/semantic_syntax_erro…_-_Semantic_syntax_erro…_-_name_is_parameter_an…_(99bae53daf67ae6e).snap

@@ -0,0 +1,117 @@
+---
+source: crates/ty_test/src/lib.rs
+expression: snapshot
+---
+---
+mdtest name: semantic_syntax_errors.md - Semantic syntax error diagnostics - name is parameter and global
+mdtest path: crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md
+---
+
+# Python source files
+
+## mdtest_snippet.py
+
+```
+ 1 | a = None
+ 2 | 
+ 3 | def f(a):
+ 4 |     global a  # error: [invalid-syntax]
+ 5 | 
+ 6 | def g(a):
+ 7 |     if True:
+ 8 |         global a  # error: [invalid-syntax]
+ 9 | 
+10 | def h(a):
+11 |     def inner():
+12 |         global a
+13 | 
+14 | def i(a):
+15 |     try:
+16 |         global a  # error: [invalid-syntax]
+17 |     except Exception:
+18 |         pass
+19 | 
+20 | def f(a):
+21 |     a = 1
+22 |     global a  # error: [invalid-syntax]
+23 | 
+24 | def f(a):
+25 |     a = 1
+26 |     a = 2
+27 |     global a  # error: [invalid-syntax]
+28 | 
+29 | def f(a):
+30 |     class Inner:
+31 |         global a  # ok
+```
+
+# Diagnostics
+
+```
+error[invalid-syntax]: name `a` is parameter and global
+ --> src/mdtest_snippet.py:4:12
+  |
+3 | def f(a):
+4 |     global a  # error: [invalid-syntax]
+  |            ^
+5 |
+6 | def g(a):
+  |
+
+```
+
+```
+error[invalid-syntax]: name `a` is parameter and global
+  --> src/mdtest_snippet.py:8:16
+   |
+ 6 | def g(a):
+ 7 |     if True:
+ 8 |         global a  # error: [invalid-syntax]
+   |                ^
+ 9 |
+10 | def h(a):
+   |
+
+```
+
+```
+error[invalid-syntax]: name `a` is parameter and global
+  --> src/mdtest_snippet.py:16:16
+   |
+14 | def i(a):
+15 |     try:
+16 |         global a  # error: [invalid-syntax]
+   |                ^
+17 |     except Exception:
+18 |         pass
+   |
+
+```
+
+```
+error[invalid-syntax]: name `a` is parameter and global
+  --> src/mdtest_snippet.py:22:12
+   |
+20 | def f(a):
+21 |     a = 1
+22 |     global a  # error: [invalid-syntax]
+   |            ^
+23 |
+24 | def f(a):
+   |
+
+```
+
+```
+error[invalid-syntax]: name `a` is parameter and global
+  --> src/mdtest_snippet.py:27:12
+   |
+25 |     a = 1
+26 |     a = 2
+27 |     global a  # error: [invalid-syntax]
+   |            ^
+28 |
+29 | def f(a):
+   |
+
+```

crates/ty_python_semantic/src/semantic_index/builder.rs

@@ -1165,6 +1165,9 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
         }
         if let Some(vararg) = parameters.vararg.as_ref() {
             let symbol = self.add_symbol(vararg.name.id().clone());
+            self.current_place_table_mut()
+                .symbol_mut(symbol)
+                .mark_parameter();
             self.add_definition(
                 symbol.into(),
                 DefinitionNodeRef::VariadicPositionalParameter(vararg),
@@ -1172,6 +1175,9 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
         }
         if let Some(kwarg) = parameters.kwarg.as_ref() {
             let symbol = self.add_symbol(kwarg.name.id().clone());
+            self.current_place_table_mut()
+                .symbol_mut(symbol)
+                .mark_parameter();
             self.add_definition(
                 symbol.into(),
                 DefinitionNodeRef::VariadicKeywordParameter(kwarg),
@@ -1184,6 +1190,10 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
 
         let definition = self.add_definition(symbol.into(), parameter);
 
+        self.current_place_table_mut()
+            .symbol_mut(symbol)
+            .mark_parameter();
+
         // Insert a mapping from the inner Parameter node to the same definition. This
         // ensures that calling `HasType::inferred_type` on the inner parameter returns
         // a valid type (and doesn't panic)
@@ -2248,7 +2258,9 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
                     let symbol_id = self.add_symbol(name.id.clone());
                     let symbol = self.current_place_table().symbol(symbol_id);
                     // Check whether the variable has already been accessed in this scope.
-                    if symbol.is_bound() || symbol.is_declared() || symbol.is_used() {
+                    if (symbol.is_bound() || symbol.is_declared() || symbol.is_used())
+                        && !symbol.is_parameter()
+                    {
                         self.report_semantic_error(SemanticSyntaxError {
                             kind: SemanticSyntaxErrorKind::LoadBeforeGlobalDeclaration {
                                 name: name.to_string(),
@@ -2892,8 +2904,12 @@ impl SemanticSyntaxContext for SemanticIndexBuilder<'_, '_> {
     fn in_loop_context(&self) -> bool {
         self.current_scope_info().current_loop.is_some()
     }
-    fn is_bound_parameter(&self, _name: &str) -> bool {
+
-        false
+    fn is_bound_parameter(&self, name: &str) -> bool {
+        self.scopes[self.current_scope()]
+            .node()
+            .as_function()
+            .is_some_and(|func| func.node(self.module).parameters.includes(name))
     }
 }
 

crates/ty_python_semantic/src/semantic_index/symbol.rs

@@ -38,6 +38,7 @@ bitflags! {
         const MARKED_NONLOCAL       = 1 << 4;
         /// true if the symbol is assigned more than once, or if it is assigned even though it is already in use
         const IS_REASSIGNED         = 1 << 5;
+        const IS_PARAMETER          = 1 << 6;
     }
 }
 
@@ -116,6 +117,10 @@ impl Symbol {
         self.flags.contains(SymbolFlags::IS_REASSIGNED)
     }
 
+    pub(crate) fn is_parameter(&self) -> bool {
+        self.flags.contains(SymbolFlags::IS_PARAMETER)
+    }
+
     pub(super) fn mark_global(&mut self) {
         self.insert_flags(SymbolFlags::MARKED_GLOBAL);
     }
@@ -140,6 +145,10 @@ impl Symbol {
         self.insert_flags(SymbolFlags::IS_DECLARED);
     }
 
+    pub(super) fn mark_parameter(&mut self) {
+        self.insert_flags(SymbolFlags::IS_PARAMETER);
+    }
+
     fn insert_flags(&mut self, flags: SymbolFlags) {
         self.flags.insert(flags);
     }