Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pylint] Implement nonlocal-and-global (E115) #10407

Merged
merged 4 commits into from Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,67 @@
# Positive cases

counter = 0


def count():
global counter
nonlocal counter
counter += 1


def count():
counter = 0

def count(counter_type):
if counter_type == "nonlocal":
nonlocal counter
counter += 1
else:
global counter
counter += 1


def count():
counter = 0

def count_twice():
for i in range(2):
nonlocal counter
counter += 1
global counter


def count():
nonlocal counter
global counter
counter += 1


# Negative cases

counter = 0


def count():
global counter
counter += 1


def count():
counter = 0

def count_local():
nonlocal counter
counter += 1


def count():
counter = 0

def count_local():
nonlocal counter
counter += 1

def count_global():
global counter
counter += 1
5 changes: 4 additions & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Expand Up @@ -30,7 +30,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
}));
}
}
Stmt::Nonlocal(ast::StmtNonlocal { names, range: _ }) => {
Stmt::Nonlocal(nonlocal @ ast::StmtNonlocal { names, range: _ }) => {
if checker.enabled(Rule::AmbiguousVariableName) {
checker.diagnostics.extend(names.iter().filter_map(|name| {
pycodestyle::rules::ambiguous_variable_name(name, name.range())
Expand All @@ -50,6 +50,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
}
}
}
if checker.enabled(Rule::NonlocalAndGlobal) {
pylint::rules::nonlocal_and_global(checker, nonlocal);
}
}
Stmt::Break(_) => {
if checker.enabled(Rule::BreakOutsideLoop) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Expand Up @@ -234,6 +234,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "C3002") => (RuleGroup::Stable, rules::pylint::rules::UnnecessaryDirectLambdaCall),
(Pylint, "E0100") => (RuleGroup::Stable, rules::pylint::rules::YieldInInit),
(Pylint, "E0101") => (RuleGroup::Stable, rules::pylint::rules::ReturnInInit),
(Pylint, "E0115") => (RuleGroup::Preview, rules::pylint::rules::NonlocalAndGlobal),
(Pylint, "E0116") => (RuleGroup::Stable, rules::pylint::rules::ContinueInFinally),
(Pylint, "E0117") => (RuleGroup::Stable, rules::pylint::rules::NonlocalWithoutBinding),
(Pylint, "E0118") => (RuleGroup::Stable, rules::pylint::rules::LoadBeforeGlobalDeclaration),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Expand Up @@ -92,6 +92,7 @@ mod tests {
Rule::NamedExprWithoutContext,
Path::new("named_expr_without_context.py")
)]
#[test_case(Rule::NonlocalAndGlobal, Path::new("nonlocal_and_global.py"))]
#[test_case(Rule::NonlocalWithoutBinding, Path::new("nonlocal_without_binding.py"))]
#[test_case(Rule::NonSlotAssignment, Path::new("non_slot_assignment.py"))]
#[test_case(Rule::PropertyWithParameters, Path::new("property_with_parameters.py"))]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Expand Up @@ -44,6 +44,7 @@ pub(crate) use no_self_use::*;
pub(crate) use non_ascii_module_import::*;
pub(crate) use non_ascii_name::*;
pub(crate) use non_slot_assignment::*;
pub(crate) use nonlocal_and_global::*;
pub(crate) use nonlocal_without_binding::*;
pub(crate) use potential_index_error::*;
pub(crate) use property_with_parameters::*;
Expand Down Expand Up @@ -134,6 +135,7 @@ mod no_self_use;
mod non_ascii_module_import;
mod non_ascii_name;
mod non_slot_assignment;
mod nonlocal_and_global;
mod nonlocal_without_binding;
mod potential_index_error;
mod property_with_parameters;
Expand Down
70 changes: 70 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/nonlocal_and_global.rs
@@ -0,0 +1,70 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for variables which are both declared as both `nonlocal` and
/// `global`.
///
/// ## Why is this bad?
/// A `nonlocal` variable is a variable that is defined in the nearest
/// enclosing scope, but not in the global scope, while a `global` variable is
/// a variable that is defined in the global scope.
///
/// Declaring a variable as both `nonlocal` and `global` is contradictory and
/// will raise a `SyntaxError`.
///
/// ## Example
/// ```python
/// counter = 0
///
///
/// def increment():
/// global counter
/// nonlocal counter
/// counter += 1
/// ```
///
/// Use instead:
/// ```python
/// counter = 0
///
///
/// def increment():
/// global counter
/// counter += 1
/// ```
///
/// ## References
/// - [Python documentation: The `global` statement](https://docs.python.org/3/reference/simple_stmts.html#the-global-statement)
/// - [Python documentation: The `nonlocal` statement](https://docs.python.org/3/reference/simple_stmts.html#nonlocal)
#[violation]
pub struct NonlocalAndGlobal {
pub(crate) name: String,
}

impl Violation for NonlocalAndGlobal {
#[derive_message_formats]
fn message(&self) -> String {
let NonlocalAndGlobal { name } = self;
format!("Name `{name}` is both `nonlocal` and `global`")
}
}

/// E115
pub(crate) fn nonlocal_and_global(checker: &mut Checker, nonlocal: &ast::StmtNonlocal) {
// Determine whether any of the newly declared `nonlocal` variables are already declared as
// `global`.
for name in &nonlocal.names {
if let Some(global) = checker.semantic().global(name) {
checker.diagnostics.push(Diagnostic::new(
NonlocalAndGlobal {
name: name.to_string(),
},
global,
));
}
}
}
@@ -0,0 +1,37 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
nonlocal_and_global.py:7:12: PLE0115 Name `counter` is both `nonlocal` and `global`
|
6 | def count():
7 | global counter
| ^^^^^^^ PLE0115
8 | nonlocal counter
9 | counter += 1
|

nonlocal_and_global.py:20:20: PLE0115 Name `counter` is both `nonlocal` and `global`
|
18 | counter += 1
19 | else:
20 | global counter
| ^^^^^^^ PLE0115
21 | counter += 1
|

nonlocal_and_global.py:31:16: PLE0115 Name `counter` is both `nonlocal` and `global`
|
29 | nonlocal counter
30 | counter += 1
31 | global counter
| ^^^^^^^ PLE0115
|

nonlocal_and_global.py:36:12: PLE0115 Name `counter` is both `nonlocal` and `global`
|
34 | def count():
35 | nonlocal counter
36 | global counter
| ^^^^^^^ PLE0115
37 | counter += 1
|
1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.