From 8529d79a7005f2d0e6c2eeb3958b9ff53b7d4c88 Mon Sep 17 00:00:00 2001 From: Bhuminjay Soni Date: Fri, 14 Nov 2025 23:45:34 +0530 Subject: [PATCH] [ty] name is parameter and global is a syntax error (#21312) Co-authored-by: Alex Waygood Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com> --- crates/ruff_linter/src/checkers/ast/mod.rs | 24 ++-- .../ruff_python_parser/src/semantic_errors.rs | 2 + .../diagnostics/semantic_syntax_errors.md | 38 ++++++ ..._name_is_parameter_an…_(99bae53daf67ae6e).snap | 117 ++++++++++++++++++ .../src/semantic_index/builder.rs | 22 +++- .../src/semantic_index/symbol.rs | 9 ++ 6 files changed, 194 insertions(+), 18 deletions(-) create mode 100644 crates/ty_python_semantic/resources/mdtest/snapshots/semantic_syntax_erro…_-_Semantic_syntax_erro…_-_name_is_parameter_an…_(99bae53daf67ae6e).snap diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index f3315b3b47..5f7f459ba9 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -860,23 +860,17 @@ impl SemanticSyntaxContext for Checker<'_> { } fn is_bound_parameter(&self, name: &str) -> bool { - for scope in self.semantic.current_scopes() { - match scope.kind { - ScopeKind::Class(_) => return false, - ScopeKind::Function(ast::StmtFunctionDef { parameters, .. }) - | ScopeKind::Lambda(ast::ExprLambda { - parameters: Some(parameters), - .. - }) => return parameters.includes(name), - ScopeKind::Lambda(_) - | ScopeKind::Generator { .. } - | ScopeKind::Module - | ScopeKind::Type - | ScopeKind::DunderClassCell => {} + match self.semantic.current_scope().kind { + ScopeKind::Function(ast::StmtFunctionDef { parameters, .. }) => { + parameters.includes(name) } + ScopeKind::Class(_) + | ScopeKind::Lambda(_) + | ScopeKind::Generator { .. } + | ScopeKind::Module + | ScopeKind::Type + | ScopeKind::DunderClassCell => false, } - - false } } diff --git a/crates/ruff_python_parser/src/semantic_errors.rs b/crates/ruff_python_parser/src/semantic_errors.rs index 1577f80cb5..a9bc9c9101 100644 --- a/crates/ruff_python_parser/src/semantic_errors.rs +++ b/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; } diff --git a/crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md b/crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md index e3c830a1e8..f1c043478b 100644 --- a/crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md +++ b/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 + + + +```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 +``` diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/semantic_syntax_erro…_-_Semantic_syntax_erro…_-_name_is_parameter_an…_(99bae53daf67ae6e).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/semantic_syntax_erro…_-_Semantic_syntax_erro…_-_name_is_parameter_an…_(99bae53daf67ae6e).snap new file mode 100644 index 0000000000..21c00c8b7e --- /dev/null +++ b/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): + | + +``` diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index 17dd2c6c2e..519600ed9d 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/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)) } } diff --git a/crates/ty_python_semantic/src/semantic_index/symbol.rs b/crates/ty_python_semantic/src/semantic_index/symbol.rs index 400165485e..8aea606f59 100644 --- a/crates/ty_python_semantic/src/semantic_index/symbol.rs +++ b/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); }