From 3dd78e711e3ae1464c6b288acd91d28685b3bb9b Mon Sep 17 00:00:00 2001 From: Bhuminjay Soni Date: Tue, 21 Oct 2025 22:21:16 +0530 Subject: [PATCH] [syntax-errors] Name is parameter and global (#20426) ## Summary This PR implements a new semantic syntax error where name is parameter & global. ## Test Plan I have written inline test as directed in #17412 --------- Signed-off-by: 11happy Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com> --- crates/ruff_linter/src/checkers/ast/mod.rs | 21 +++++++ crates/ruff_linter/src/linter.rs | 36 ++++++++++++ ...GlobalParameter_global_parameter_3.10.snap | 56 +++++++++++++++++++ .../ruff_python_parser/src/semantic_errors.rs | 23 ++++++++ crates/ruff_python_parser/tests/fixtures.rs | 4 ++ .../src/semantic_index/builder.rs | 3 + 6 files changed, 143 insertions(+) create mode 100644 crates/ruff_linter/src/snapshots/ruff_linter__linter__tests__semantic_syntax_error_GlobalParameter_global_parameter_3.10.snap diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index c6d4a5bf3d..7750d29f34 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -725,6 +725,7 @@ impl SemanticSyntaxContext for Checker<'_> { | SemanticSyntaxErrorKind::WriteToDebug(_) | SemanticSyntaxErrorKind::DifferentMatchPatternBindings | SemanticSyntaxErrorKind::InvalidExpression(..) + | SemanticSyntaxErrorKind::GlobalParameter(_) | SemanticSyntaxErrorKind::DuplicateMatchKey(_) | SemanticSyntaxErrorKind::DuplicateMatchClassAttribute(_) | SemanticSyntaxErrorKind::InvalidStarExpression @@ -846,6 +847,26 @@ impl SemanticSyntaxContext for Checker<'_> { } false } + + 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 => {} + } + } + + false + } } impl<'a> Visitor<'a> for Checker<'a> { diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 1386704920..3503a69c25 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -1040,6 +1040,42 @@ mod tests { PythonVersion::PY310, "DuplicateMatchKey" )] + #[test_case( + "global_parameter", + " + def f(a): + global a + + def g(a): + if True: + global a + + def h(a): + def inner(): + global a + + def i(a): + try: + global a + except Exception: + pass + + def f(a): + a = 1 + global a + + def f(a): + a = 1 + a = 2 + global a + + def f(a): + class Inner: + global a # ok + ", + PythonVersion::PY310, + "GlobalParameter" + )] #[test_case( "duplicate_match_class_attribute", " diff --git a/crates/ruff_linter/src/snapshots/ruff_linter__linter__tests__semantic_syntax_error_GlobalParameter_global_parameter_3.10.snap b/crates/ruff_linter/src/snapshots/ruff_linter__linter__tests__semantic_syntax_error_GlobalParameter_global_parameter_3.10.snap new file mode 100644 index 0000000000..c1c7fbd378 --- /dev/null +++ b/crates/ruff_linter/src/snapshots/ruff_linter__linter__tests__semantic_syntax_error_GlobalParameter_global_parameter_3.10.snap @@ -0,0 +1,56 @@ +--- +source: crates/ruff_linter/src/linter.rs +--- +invalid-syntax: name `a` is parameter and global + --> :3:12 + | +2 | def f(a): +3 | global a + | ^ +4 | +5 | def g(a): + | + +invalid-syntax: name `a` is parameter and global + --> :7:16 + | +5 | def g(a): +6 | if True: +7 | global a + | ^ +8 | +9 | def h(a): + | + +invalid-syntax: name `a` is parameter and global + --> :15:16 + | +13 | def i(a): +14 | try: +15 | global a + | ^ +16 | except Exception: +17 | pass + | + +invalid-syntax: name `a` is parameter and global + --> :21:12 + | +19 | def f(a): +20 | a = 1 +21 | global a + | ^ +22 | +23 | def f(a): + | + +invalid-syntax: name `a` is parameter and global + --> :26:12 + | +24 | a = 1 +25 | a = 2 +26 | global a + | ^ +27 | +28 | def f(a): + | diff --git a/crates/ruff_python_parser/src/semantic_errors.rs b/crates/ruff_python_parser/src/semantic_errors.rs index ac57426915..aba6e5ed7c 100644 --- a/crates/ruff_python_parser/src/semantic_errors.rs +++ b/crates/ruff_python_parser/src/semantic_errors.rs @@ -133,6 +133,17 @@ impl SemanticSyntaxChecker { } Self::duplicate_parameter_name(parameters, ctx); } + Stmt::Global(ast::StmtGlobal { names, .. }) => { + for name in names { + if ctx.is_bound_parameter(name) { + Self::add_error( + ctx, + SemanticSyntaxErrorKind::GlobalParameter(name.to_string()), + name.range, + ); + } + } + } Stmt::ClassDef(ast::StmtClassDef { type_params, .. }) | Stmt::TypeAlias(ast::StmtTypeAlias { type_params, .. }) => { if let Some(type_params) = type_params { @@ -1137,6 +1148,9 @@ impl Display for SemanticSyntaxError { } SemanticSyntaxErrorKind::BreakOutsideLoop => f.write_str("`break` outside loop"), SemanticSyntaxErrorKind::ContinueOutsideLoop => f.write_str("`continue` outside loop"), + SemanticSyntaxErrorKind::GlobalParameter(name) => { + write!(f, "name `{name}` is parameter and global") + } SemanticSyntaxErrorKind::DifferentMatchPatternBindings => { write!(f, "alternative patterns bind different names") } @@ -1520,6 +1534,13 @@ pub enum SemanticSyntaxErrorKind { /// Represents the use of a `continue` statement outside of a loop. ContinueOutsideLoop, + /// Represents a function parameter that is also declared as `global`. + /// + /// Declaring a parameter as `global` is invalid, since parameters are already + /// bound in the local scope of the function. Using `global` on them introduces + /// ambiguity and will result in a `SyntaxError`. + GlobalParameter(String), + /// Represents the use of alternative patterns in a `match` statement that bind different names. /// /// Python requires all alternatives in an OR pattern (`|`) to bind the same set of names. @@ -2054,6 +2075,8 @@ pub trait SemanticSyntaxContext { fn report_semantic_error(&self, error: SemanticSyntaxError); fn in_loop_context(&self) -> bool; + + fn is_bound_parameter(&self, name: &str) -> bool; } /// Modified version of [`std::str::EscapeDefault`] that does not escape single or double quotes. diff --git a/crates/ruff_python_parser/tests/fixtures.rs b/crates/ruff_python_parser/tests/fixtures.rs index 9837d9d873..c646fe525b 100644 --- a/crates/ruff_python_parser/tests/fixtures.rs +++ b/crates/ruff_python_parser/tests/fixtures.rs @@ -575,6 +575,10 @@ impl SemanticSyntaxContext for SemanticSyntaxCheckerVisitor<'_> { fn in_loop_context(&self) -> bool { true } + + fn is_bound_parameter(&self, _name: &str) -> bool { + false + } } impl Visitor<'_> for SemanticSyntaxCheckerVisitor<'_> { diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index 5bf8cbb3e7..8107f9c122 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -2789,6 +2789,9 @@ 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 + } } #[derive(Copy, Clone, Debug, PartialEq)]