Skip to content

Commit

Permalink
[pylint] - implement global-at-module-level (W0604) (#8058)
Browse files Browse the repository at this point in the history
## Summary

Implements
[`global-at-module-level`/`W0604`](https://pylint.pycqa.org/en/latest/user_guide/messages/warning/global-at-module-level.html)

See #970

## Test Plan

`cargo test` and manually
  • Loading branch information
diceroll123 authored Oct 19, 2023
1 parent a85ed30 commit 693f957
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
global price # W0604

price = 25

if True:
global X # W0604

def no_error():
global price
price = 30
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 @@ -21,6 +21,9 @@ use crate::settings::types::PythonVersion;
pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
match stmt {
Stmt::Global(ast::StmtGlobal { names, range: _ }) => {
if checker.enabled(Rule::GlobalAtModuleLevel) {
pylint::rules::global_at_module_level(checker, stmt);
}
if checker.enabled(Rule::AmbiguousVariableName) {
checker.diagnostics.extend(names.iter().filter_map(|name| {
pycodestyle::rules::ambiguous_variable_name(name, name.range())
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 @@ -264,6 +264,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "W0131") => (RuleGroup::Stable, rules::pylint::rules::NamedExprWithoutContext),
(Pylint, "W0406") => (RuleGroup::Stable, rules::pylint::rules::ImportSelf),
(Pylint, "W0602") => (RuleGroup::Stable, rules::pylint::rules::GlobalVariableNotAssigned),
(Pylint, "W0604") => (RuleGroup::Preview, rules::pylint::rules::GlobalAtModuleLevel),
(Pylint, "W0603") => (RuleGroup::Stable, rules::pylint::rules::GlobalStatement),
(Pylint, "W0711") => (RuleGroup::Stable, rules::pylint::rules::BinaryOpException),
(Pylint, "W1508") => (RuleGroup::Stable, rules::pylint::rules::InvalidEnvvarDefault),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ mod tests {
#[test_case(Rule::NoSelfUse, Path::new("no_self_use.py"))]
#[test_case(Rule::MisplacedBareRaise, Path::new("misplaced_bare_raise.py"))]
#[test_case(Rule::LiteralMembership, Path::new("literal_membership.py"))]
#[test_case(Rule::GlobalAtModuleLevel, Path::new("global_at_module_level.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::Stmt;
use ruff_text_size::Ranged;

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

/// ## What it does
/// Checks for uses of the `global` keyword at the module level.
///
/// ## Why is this bad?
/// The `global` keyword is used within functions to indicate that a name
/// refers to a global variable, rather than a local variable.
///
/// At the module level, all names are global by default, so the `global`
/// keyword is redundant.
#[violation]
pub struct GlobalAtModuleLevel;

impl Violation for GlobalAtModuleLevel {
#[derive_message_formats]
fn message(&self) -> String {
format!("`global` at module level is redundant")
}
}

/// PLW0604
pub(crate) fn global_at_module_level(checker: &mut Checker, stmt: &Stmt) {
if checker.semantic().current_scope().kind.is_module() {
checker
.diagnostics
.push(Diagnostic::new(GlobalAtModuleLevel, stmt.range()));
}
}
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 @@ -14,6 +14,7 @@ pub(crate) use comparison_with_itself::*;
pub(crate) use continue_in_finally::*;
pub(crate) use duplicate_bases::*;
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 import_self::*;
Expand Down Expand Up @@ -78,6 +79,7 @@ mod comparison_with_itself;
mod continue_in_finally;
mod duplicate_bases;
mod eq_without_hash;
mod global_at_module_level;
mod global_statement;
mod global_variable_not_assigned;
mod import_self;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
global_at_module_level.py:1:1: PLW0604 `global` at module level is redundant
|
1 | global price # W0604
| ^^^^^^^^^^^^ PLW0604
2 |
3 | price = 25
|

global_at_module_level.py:6:5: PLW0604 `global` at module level is redundant
|
5 | if True:
6 | global X # W0604
| ^^^^^^^^ PLW0604
7 |
8 | def no_error():
|


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 693f957

Please sign in to comment.