mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-28 02:39:59 +00:00
[syntax-errors] Name is parameter and global (#20426)
## Summary <!-- What's the purpose of the change? What does it do, and why? --> This PR implements a new semantic syntax error where name is parameter & global. ## Test Plan <!-- How was it tested? --> I have written inline test as directed in #17412 --------- Signed-off-by: 11happy <soni5happy@gmail.com> Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
This commit is contained in:
parent
e1cada1ec3
commit
3dd78e711e
6 changed files with 143 additions and 0 deletions
|
|
@ -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> {
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
"
|
||||
|
|
|
|||
|
|
@ -0,0 +1,56 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/linter.rs
|
||||
---
|
||||
invalid-syntax: name `a` is parameter and global
|
||||
--> <filename>:3:12
|
||||
|
|
||||
2 | def f(a):
|
||||
3 | global a
|
||||
| ^
|
||||
4 |
|
||||
5 | def g(a):
|
||||
|
|
||||
|
||||
invalid-syntax: name `a` is parameter and global
|
||||
--> <filename>: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
|
||||
--> <filename>:15:16
|
||||
|
|
||||
13 | def i(a):
|
||||
14 | try:
|
||||
15 | global a
|
||||
| ^
|
||||
16 | except Exception:
|
||||
17 | pass
|
||||
|
|
||||
|
||||
invalid-syntax: name `a` is parameter and global
|
||||
--> <filename>: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
|
||||
--> <filename>:26:12
|
||||
|
|
||||
24 | a = 1
|
||||
25 | a = 2
|
||||
26 | global a
|
||||
| ^
|
||||
27 |
|
||||
28 | def f(a):
|
||||
|
|
||||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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<'_> {
|
||||
|
|
|
|||
|
|
@ -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)]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue