[refurb] Implement repeated-global (FURB154) (#11187)

Implement repeated_global (FURB154) lint.
See:
- https://github.com/astral-sh/ruff/issues/1348
- [original
lint](https://github.com/dosisod/refurb/blob/master/refurb/checks/builtin/simplify_global_and_nonlocal.py)

## Test Plan
cargo test

crates/ruff_linter/resources/test/fixtures/refurb/FURB154.py

@@ -0,0 +1,86 @@
+# Errors
+
+def f1():
+    global x
+    global y
+
+
+def f3():
+    global x
+    global y
+    global z
+
+
+def f4():
+    global x
+    global y
+    pass
+    global x
+    global y
+
+
+def f2():
+    x = y = z = 1
+
+    def inner():
+        nonlocal x
+        nonlocal y
+
+    def inner2():
+        nonlocal x
+        nonlocal y
+        nonlocal z
+
+    def inner3():
+        nonlocal x
+        nonlocal y
+        pass
+        nonlocal x
+        nonlocal y
+
+
+def f5():
+    w = x = y = z = 1
+
+    def inner():
+        global w
+        global x
+        nonlocal y
+        nonlocal z
+
+    def inner2():
+        global x
+        nonlocal y
+        nonlocal z
+
+
+def f6():
+    global x, y, z
+    global a, b, c
+    global d, e, f
+
+
+# Ok
+
+def fx():
+    x = y = 1
+
+    def inner():
+        global x
+        nonlocal y
+
+    def inner2():
+        nonlocal x
+        pass
+        nonlocal y
+
+
+def fy():
+    global x
+    pass
+    global y
+
+
+def fz():
+    pass
+    global x

crates/ruff_linter/src/checkers/ast/analyze/suite.rs

@@ -3,10 +3,14 @@ use ruff_python_ast::Stmt;
 use crate::checkers::ast::Checker;
 use crate::codes::Rule;
 use crate::rules::flake8_pie;
+use crate::rules::refurb;
 
 /// Run lint rules over a suite of [`Stmt`] syntax nodes.
 pub(crate) fn suite(suite: &[Stmt], checker: &mut Checker) {
     if checker.enabled(Rule::UnnecessaryPlaceholder) {
         flake8_pie::rules::unnecessary_placeholder(checker, suite);
     }
+    if checker.enabled(Rule::RepeatedGlobal) {
+        refurb::rules::repeated_global(checker, suite);
+    }
 }

crates/ruff_linter/src/codes.rs

@@ -1073,6 +1073,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy),
         (Refurb, "148") => (RuleGroup::Preview, rules::refurb::rules::UnnecessaryEnumerate),
         (Refurb, "152") => (RuleGroup::Preview, rules::refurb::rules::MathConstant),
+        (Refurb, "154") => (RuleGroup::Preview, rules::refurb::rules::RepeatedGlobal),
         (Refurb, "157") => (RuleGroup::Preview, rules::refurb::rules::VerboseDecimalConstructor),
         (Refurb, "161") => (RuleGroup::Preview, rules::refurb::rules::BitCount),
         (Refurb, "163") => (RuleGroup::Preview, rules::refurb::rules::RedundantLogBase),

crates/ruff_linter/src/rules/refurb/mod.rs

@@ -27,6 +27,7 @@ mod tests {
     #[test_case(Rule::SliceCopy, Path::new("FURB145.py"))]
     #[test_case(Rule::UnnecessaryEnumerate, Path::new("FURB148.py"))]
     #[test_case(Rule::MathConstant, Path::new("FURB152.py"))]
+    #[test_case(Rule::RepeatedGlobal, Path::new("FURB154.py"))]
     #[test_case(Rule::VerboseDecimalConstructor, Path::new("FURB157.py"))]
     #[test_case(Rule::UnnecessaryFromFloat, Path::new("FURB164.py"))]
     #[test_case(Rule::PrintEmptyString, Path::new("FURB105.py"))]

crates/ruff_linter/src/rules/refurb/rules/mod.rs

@@ -20,6 +20,7 @@ pub(crate) use regex_flag_alias::*;
 pub(crate) use reimplemented_operator::*;
 pub(crate) use reimplemented_starmap::*;
 pub(crate) use repeated_append::*;
+pub(crate) use repeated_global::*;
 pub(crate) use single_item_membership_test::*;
 pub(crate) use slice_copy::*;
 pub(crate) use sorted_min_max::*;
@@ -51,6 +52,7 @@ mod regex_flag_alias;
 mod reimplemented_operator;
 mod reimplemented_starmap;
 mod repeated_append;
+mod repeated_global;
 mod single_item_membership_test;
 mod slice_copy;
 mod sorted_min_max;

crates/ruff_linter/src/rules/refurb/rules/repeated_global.rs

@@ -0,0 +1,125 @@
+use itertools::Itertools;
+
+use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
+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 consecutive `global` (or `nonlocal`) statements.
+///
+/// ## Why is this bad?
+/// The `global` and `nonlocal` keywords accepts multiple comma-separated names.
+/// Instead of using multiple `global` (or `nonlocal`) statements for separate
+/// variables, you can use a single statement to declare multiple variables at
+/// once.
+///
+/// ## Example
+/// ```python
+/// def func():
+///     global x
+///     global y
+///
+///     print(x, y)
+/// ```
+///
+/// Use instead:
+/// ```python
+/// def func():
+///     global x, y
+///
+///     print(x, y)
+/// ```
+///
+/// ## 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#the-nonlocal-statement)
+#[violation]
+pub struct RepeatedGlobal {
+    global_kind: GlobalKind,
+}
+
+impl AlwaysFixableViolation for RepeatedGlobal {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Use of repeated consecutive `{}`", self.global_kind)
+    }
+
+    fn fix_title(&self) -> String {
+        format!("Merge `{}` statements", self.global_kind)
+    }
+}
+
+/// FURB154
+pub(crate) fn repeated_global(checker: &mut Checker, mut suite: &[Stmt]) {
+    while let Some(idx) = suite
+        .iter()
+        .position(|stmt| GlobalKind::from_stmt(stmt).is_some())
+    {
+        let global_kind = GlobalKind::from_stmt(&suite[idx]).unwrap();
+
+        suite = &suite[idx..];
+
+        // Collect until we see a non-`global` (or non-`nonlocal`) statement.
+        let (globals_sequence, next_suite) = suite.split_at(
+            suite
+                .iter()
+                .position(|stmt| GlobalKind::from_stmt(stmt) != Some(global_kind))
+                .unwrap_or(suite.len()),
+        );
+
+        // If there are at least two consecutive `global` (or `nonlocal`) statements, raise a
+        // diagnostic.
+        if let [first, .., last] = globals_sequence {
+            let range = first.range().cover(last.range());
+            checker.diagnostics.push(
+                Diagnostic::new(RepeatedGlobal { global_kind }, range).with_fix(Fix::safe_edit(
+                    Edit::range_replacement(
+                        format!(
+                            "{global_kind} {}",
+                            globals_sequence
+                                .iter()
+                                .flat_map(|stmt| match stmt {
+                                    Stmt::Global(stmt) => &stmt.names,
+                                    Stmt::Nonlocal(stmt) => &stmt.names,
+                                    _ => unreachable!(),
+                                })
+                                .map(|identifier| &identifier.id)
+                                .format(", ")
+                        ),
+                        range,
+                    ),
+                )),
+            );
+        }
+
+        suite = next_suite;
+    }
+}
+
+#[derive(Debug, PartialEq, Eq, Clone, Copy)]
+enum GlobalKind {
+    Global,
+    NonLocal,
+}
+
+impl GlobalKind {
+    fn from_stmt(stmt: &Stmt) -> Option<Self> {
+        match stmt {
+            Stmt::Global(_) => Some(GlobalKind::Global),
+            Stmt::Nonlocal(_) => Some(GlobalKind::NonLocal),
+            _ => None,
+        }
+    }
+}
+
+impl std::fmt::Display for GlobalKind {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            GlobalKind::Global => write!(f, "global"),
+            GlobalKind::NonLocal => write!(f, "nonlocal"),
+        }
+    }
+}

crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB154_FURB154.py.snap

@@ -0,0 +1,276 @@
+---
+source: crates/ruff_linter/src/rules/refurb/mod.rs
+---
+FURB154.py:4:5: FURB154 [*] Use of repeated consecutive `global`
+  |
+3 |   def f1():
+4 |       global x
+  |  _____^
+5 | |     global y
+  | |____________^ FURB154
+  |
+  = help: Merge `global` statements
+
+ℹ Safe fix
+1 1 | # Errors
+2 2 | 
+3 3 | def f1():
+4   |-    global x
+5   |-    global y
+  4 |+    global x, y
+6 5 | 
+7 6 | 
+8 7 | def f3():
+
+FURB154.py:9:5: FURB154 [*] Use of repeated consecutive `global`
+   |
+ 8 |   def f3():
+ 9 |       global x
+   |  _____^
+10 | |     global y
+11 | |     global z
+   | |____________^ FURB154
+   |
+   = help: Merge `global` statements
+
+ℹ Safe fix
+6  6  | 
+7  7  | 
+8  8  | def f3():
+9     |-    global x
+10    |-    global y
+11    |-    global z
+   9  |+    global x, y, z
+12 10 | 
+13 11 | 
+14 12 | def f4():
+
+FURB154.py:15:5: FURB154 [*] Use of repeated consecutive `global`
+   |
+14 |   def f4():
+15 |       global x
+   |  _____^
+16 | |     global y
+   | |____________^ FURB154
+17 |       pass
+18 |       global x
+   |
+   = help: Merge `global` statements
+
+ℹ Safe fix
+12 12 | 
+13 13 | 
+14 14 | def f4():
+15    |-    global x
+16    |-    global y
+   15 |+    global x, y
+17 16 |     pass
+18 17 |     global x
+19 18 |     global y
+
+FURB154.py:18:5: FURB154 [*] Use of repeated consecutive `global`
+   |
+16 |       global y
+17 |       pass
+18 |       global x
+   |  _____^
+19 | |     global y
+   | |____________^ FURB154
+   |
+   = help: Merge `global` statements
+
+ℹ Safe fix
+15 15 |     global x
+16 16 |     global y
+17 17 |     pass
+18    |-    global x
+19    |-    global y
+   18 |+    global x, y
+20 19 | 
+21 20 | 
+22 21 | def f2():
+
+FURB154.py:26:9: FURB154 [*] Use of repeated consecutive `nonlocal`
+   |
+25 |       def inner():
+26 |           nonlocal x
+   |  _________^
+27 | |         nonlocal y
+   | |__________________^ FURB154
+28 |   
+29 |       def inner2():
+   |
+   = help: Merge `nonlocal` statements
+
+ℹ Safe fix
+23 23 |     x = y = z = 1
+24 24 | 
+25 25 |     def inner():
+26    |-        nonlocal x
+27    |-        nonlocal y
+   26 |+        nonlocal x, y
+28 27 | 
+29 28 |     def inner2():
+30 29 |         nonlocal x
+
+FURB154.py:30:9: FURB154 [*] Use of repeated consecutive `nonlocal`
+   |
+29 |       def inner2():
+30 |           nonlocal x
+   |  _________^
+31 | |         nonlocal y
+32 | |         nonlocal z
+   | |__________________^ FURB154
+33 |   
+34 |       def inner3():
+   |
+   = help: Merge `nonlocal` statements
+
+ℹ Safe fix
+27 27 |         nonlocal y
+28 28 | 
+29 29 |     def inner2():
+30    |-        nonlocal x
+31    |-        nonlocal y
+32    |-        nonlocal z
+   30 |+        nonlocal x, y, z
+33 31 | 
+34 32 |     def inner3():
+35 33 |         nonlocal x
+
+FURB154.py:35:9: FURB154 [*] Use of repeated consecutive `nonlocal`
+   |
+34 |       def inner3():
+35 |           nonlocal x
+   |  _________^
+36 | |         nonlocal y
+   | |__________________^ FURB154
+37 |           pass
+38 |           nonlocal x
+   |
+   = help: Merge `nonlocal` statements
+
+ℹ Safe fix
+32 32 |         nonlocal z
+33 33 | 
+34 34 |     def inner3():
+35    |-        nonlocal x
+36    |-        nonlocal y
+   35 |+        nonlocal x, y
+37 36 |         pass
+38 37 |         nonlocal x
+39 38 |         nonlocal y
+
+FURB154.py:38:9: FURB154 [*] Use of repeated consecutive `nonlocal`
+   |
+36 |           nonlocal y
+37 |           pass
+38 |           nonlocal x
+   |  _________^
+39 | |         nonlocal y
+   | |__________________^ FURB154
+   |
+   = help: Merge `nonlocal` statements
+
+ℹ Safe fix
+35 35 |         nonlocal x
+36 36 |         nonlocal y
+37 37 |         pass
+38    |-        nonlocal x
+39    |-        nonlocal y
+   38 |+        nonlocal x, y
+40 39 | 
+41 40 | 
+42 41 | def f5():
+
+FURB154.py:46:9: FURB154 [*] Use of repeated consecutive `global`
+   |
+45 |       def inner():
+46 |           global w
+   |  _________^
+47 | |         global x
+   | |________________^ FURB154
+48 |           nonlocal y
+49 |           nonlocal z
+   |
+   = help: Merge `global` statements
+
+ℹ Safe fix
+43 43 |     w = x = y = z = 1
+44 44 | 
+45 45 |     def inner():
+46    |-        global w
+47    |-        global x
+   46 |+        global w, x
+48 47 |         nonlocal y
+49 48 |         nonlocal z
+50 49 | 
+
+FURB154.py:48:9: FURB154 [*] Use of repeated consecutive `nonlocal`
+   |
+46 |           global w
+47 |           global x
+48 |           nonlocal y
+   |  _________^
+49 | |         nonlocal z
+   | |__________________^ FURB154
+50 |   
+51 |       def inner2():
+   |
+   = help: Merge `nonlocal` statements
+
+ℹ Safe fix
+45 45 |     def inner():
+46 46 |         global w
+47 47 |         global x
+48    |-        nonlocal y
+49    |-        nonlocal z
+   48 |+        nonlocal y, z
+50 49 | 
+51 50 |     def inner2():
+52 51 |         global x
+
+FURB154.py:53:9: FURB154 [*] Use of repeated consecutive `nonlocal`
+   |
+51 |       def inner2():
+52 |           global x
+53 |           nonlocal y
+   |  _________^
+54 | |         nonlocal z
+   | |__________________^ FURB154
+   |
+   = help: Merge `nonlocal` statements
+
+ℹ Safe fix
+50 50 | 
+51 51 |     def inner2():
+52 52 |         global x
+53    |-        nonlocal y
+54    |-        nonlocal z
+   53 |+        nonlocal y, z
+55 54 | 
+56 55 | 
+57 56 | def f6():
+
+FURB154.py:58:5: FURB154 [*] Use of repeated consecutive `global`
+   |
+57 |   def f6():
+58 |       global x, y, z
+   |  _____^
+59 | |     global a, b, c
+60 | |     global d, e, f
+   | |__________________^ FURB154
+   |
+   = help: Merge `global` statements
+
+ℹ Safe fix
+55 55 | 
+56 56 | 
+57 57 | def f6():
+58    |-    global x, y, z
+59    |-    global a, b, c
+60    |-    global d, e, f
+   58 |+    global x, y, z, a, b, c, d, e, f
+61 59 | 
+62 60 | 
+63 61 | # Ok

ruff.schema.json

@@ -3067,6 +3067,7 @@
         "FURB148",
         "FURB15",
         "FURB152",
+        "FURB154",
         "FURB157",
         "FURB16",
         "FURB161",