Skip to content

Commit

Permalink
Add W0601
Browse files Browse the repository at this point in the history
  • Loading branch information
tibor-reiss committed Apr 7, 2024
1 parent 6050bab commit 181ccaa
Show file tree
Hide file tree
Showing 8 changed files with 202 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# pylint: disable=invalid-name, import-outside-toplevel, too-few-public-methods, unused-import
# pylint: disable=missing-module-docstring, missing-function-docstring, missing-class-docstring
# pylint: disable=global-at-module-level, global-statement, global-variable-not-assigned
CONSTANT = 1


def FUNC():
pass


class CLASS:
pass


# BAD
def global_variable_undefined():
global SOMEVAR # [global-variable-undefined]
SOMEVAR = 2


# OK
def global_constant():
global CONSTANT
print(CONSTANT)


def global_with_import():
global sys
import sys


def global_with_import_from():
global namedtuple
from collections import namedtuple


def override_func():
global FUNC

def FUNC():
pass

FUNC()


def override_class():
global CLASS

class CLASS():
pass

CLASS()
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
pycodestyle::rules::ambiguous_variable_name(name, name.range())
}));
}
if checker.enabled(Rule::GlobalVariableUndefined) {
pylint::rules::global_variable_undefined(checker, stmt);
}
}
Stmt::Nonlocal(nonlocal @ ast::StmtNonlocal { names, range: _ }) => {
if checker.enabled(Rule::AmbiguousVariableName) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "W0211") => (RuleGroup::Preview, rules::pylint::rules::BadStaticmethodArgument),
(Pylint, "W0245") => (RuleGroup::Preview, rules::pylint::rules::SuperWithoutBrackets),
(Pylint, "W0406") => (RuleGroup::Stable, rules::pylint::rules::ImportSelf),
(Pylint, "W0601") => (RuleGroup::Stable, rules::pylint::rules::GlobalVariableUndefined),
(Pylint, "W0602") => (RuleGroup::Stable, rules::pylint::rules::GlobalVariableNotAssigned),
(Pylint, "W0603") => (RuleGroup::Stable, rules::pylint::rules::GlobalStatement),
(Pylint, "W0604") => (RuleGroup::Preview, rules::pylint::rules::GlobalAtModuleLevel),
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ mod tests {
Rule::GlobalVariableNotAssigned,
Path::new("global_variable_not_assigned.py")
)]
#[test_case(
Rule::GlobalVariableUndefined,
Path::new("global_variable_undefined.py")
)]
#[test_case(Rule::ImportOutsideTopLevel, Path::new("import_outside_top_level.py"))]
#[test_case(
Rule::ImportPrivateName,
Expand Down
128 changes: 128 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/global_variable_undefined.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr, Stmt, StmtGlobal};
use ruff_python_semantic::{BindingKind, ScopeKind};

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

/// ## What it does
/// Checks that all `global` variables are indeed defined on module level
///
/// ## Why is this bad?
/// If the module level declaration is missing, then either if was
/// forgotten or the `global` can be omitted.
///
/// ## Example
/// ```python
/// def foo():
/// global var # [global-variable-undefined]
/// var = 10
/// print(var)
/// ```
///
/// Use instead:
/// ```python
/// var = 1
///
///
/// def foo():
/// global var
/// var = 10
/// print(var)
/// ```
#[violation]
pub struct GlobalVariableUndefined {
name: String,
}

impl Violation for GlobalVariableUndefined {
#[derive_message_formats]
fn message(&self) -> String {
let GlobalVariableUndefined { name } = self;
format!("Global variable `{name}` is undefined at the module")
}
}

/// PLW0601
pub(crate) fn global_variable_undefined(checker: &mut Checker, stmt: &Stmt) {
if checker.semantic().current_scope().kind.is_module() {
return;
}

let Stmt::Global(StmtGlobal { names, range }) = stmt else {
return;
};
let Some(module_scope) = checker
.semantic()
.current_scopes()
.find(|scope| scope.kind.is_module())
else {
return;
};
let imported_names = get_imports(checker);
let mut undefined_names = vec![];

for name in names {
// Skip if imported names
if imported_names.contains(&name.as_str()) {
continue;
}
// Skip if module level class or function definition
let Some(binding_id) = module_scope.get(name) else {
continue;
};
let binding = checker.semantic().binding(binding_id);
if matches!(
binding.kind,
BindingKind::ClassDefinition(_) | BindingKind::FunctionDefinition(_)
) {
continue;
}
// Skip if module level definition
let Some(node_id) = binding.source else {
continue;
};
let node = checker.semantic().node(node_id);
if let Some(Expr::Name(ast::ExprName { .. })) = node.as_expression() {
continue;
};

undefined_names.push(name);
}

for name in undefined_names {
checker.diagnostics.push(Diagnostic::new(
GlobalVariableUndefined {
name: name.to_string(),
},
*range,
));
}
}

fn get_imports<'a>(checker: &'a Checker) -> Vec<&'a str> {
// Get all names imported in the current scope
let Some(fs) = checker
.semantic()
.current_scopes()
.find(|scope| scope.kind.is_function())
else {
return vec![];
};
let ScopeKind::Function(ast::StmtFunctionDef{ body, .. }) = fs.kind else {
return vec![];
};
let mut import_names = vec![];
for stmt in body {
match stmt {
Stmt::Import(ast::StmtImport { names, .. })
| Stmt::ImportFrom(ast::StmtImportFrom { names, .. }) => {
for name in names {
import_names.push(name.name.as_str());
}
}
_ => (),
}
}
import_names
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub(crate) use eq_without_hash::*;
pub(crate) use global_at_module_level::*;
pub(crate) use global_statement::*;
pub(crate) use global_variable_not_assigned::*;
pub(crate) use global_variable_undefined::*;
pub(crate) use if_stmt_min_max::*;
pub(crate) use import_outside_top_level::*;
pub(crate) use import_private_name::*;
Expand Down Expand Up @@ -117,6 +118,7 @@ mod eq_without_hash;
mod global_at_module_level;
mod global_statement;
mod global_variable_not_assigned;
mod global_variable_undefined;
mod if_stmt_min_max;
mod import_outside_top_level;
mod import_private_name;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
global_variable_undefined.py:17:5: PLW0601 Global variable `SOMEVAR` is undefined at the module
|
15 | # BAD
16 | def global_variable_undefined():
17 | global SOMEVAR # [global-variable-undefined]
| ^^^^^^^^^^^^^^ PLW0601
18 | SOMEVAR = 2
|
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.

0 comments on commit 181ccaa

Please sign in to comment.