[syntax-errors] Reimplement PLE0118 (astral-sh#17135)

Summary
--

This PR reimplements
[load-before-global-declaration
(PLE0118)](https://docs.astral.sh/ruff/rules/load-before-global-declaration/)
as a semantic syntax error.

I added a `global` method to the `SemanticSyntaxContext` trait to make
this very easy, at least in ruff. Does red-knot have something similar?

If this approach will also work in red-knot, I think some of the other
PLE rules are also compile-time errors in CPython, PLE0117 in
particular. 0115 and 0116 also mention `SyntaxError`s in their docs, but
I haven't confirmed them in the REPL yet.

Test Plan
--

Existing linter tests for PLE0118. I think this actually can't be tested
very easily in an inline test because the `TestContext` doesn't have a
real way to track globals.

---------

Co-authored-by: Micha Reiser <micha@reiser.io>

Author: 2 people

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

@@ -337,9 +337,6 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
             if checker.enabled(Rule::UndocumentedWarn) {
                 flake8_logging::rules::undocumented_warn(checker, expr);
             }
-            if checker.enabled(Rule::LoadBeforeGlobalDeclaration) {
-                pylint::rules::load_before_global_declaration(checker, id, expr);
-            }
         }
         Expr::Attribute(attribute) => {
             if attribute.ctx == ExprContext::Load {

crates/ruff_linter/src/checkers/ast/mod.rs

@@ -67,6 +67,7 @@ use crate::noqa::NoqaMapping;
 use crate::package::PackageRoot;
 use crate::registry::Rule;
 use crate::rules::pyflakes::rules::LateFutureImport;
+use crate::rules::pylint::rules::LoadBeforeGlobalDeclaration;
 use crate::rules::{flake8_pyi, flake8_type_checking, pyflakes, pyupgrade};
 use crate::settings::{flags, LinterSettings};
 use crate::{docstrings, noqa, Locator};
@@ -540,13 +541,32 @@ impl SemanticSyntaxContext for Checker<'_> {
         self.target_version
     }
 
+    fn global(&self, name: &str) -> Option<TextRange> {
+        self.semantic.global(name)
+    }
+
     fn report_semantic_error(&self, error: SemanticSyntaxError) {
         match error.kind {
             SemanticSyntaxErrorKind::LateFutureImport => {
                 if self.settings.rules.enabled(Rule::LateFutureImport) {
                     self.report_diagnostic(Diagnostic::new(LateFutureImport, error.range));
                 }
             }
+            SemanticSyntaxErrorKind::LoadBeforeGlobalDeclaration { name, start } => {
+                if self
+                    .settings
+                    .rules
+                    .enabled(Rule::LoadBeforeGlobalDeclaration)
+                {
+                    self.report_diagnostic(Diagnostic::new(
+                        LoadBeforeGlobalDeclaration {
+                            name,
+                            row: self.compute_source_row(start),
+                        },
+                        error.range,
+                    ));
+                }
+            }
             SemanticSyntaxErrorKind::ReboundComprehensionVariable
             | SemanticSyntaxErrorKind::DuplicateTypeParameter
             | SemanticSyntaxErrorKind::MultipleCaseAssignment(_)

crates/ruff_linter/src/rules/pylint/rules/load_before_global_declaration.rs

@@ -1,11 +1,6 @@
-use ruff_python_ast::Expr;
-
-use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_diagnostics::Violation;
 use ruff_macros::{derive_message_formats, ViolationMetadata};
 use ruff_source_file::SourceRow;
-use ruff_text_size::Ranged;
-
-use crate::checkers::ast::Checker;
 
 /// ## What it does
 /// Checks for uses of names that are declared as `global` prior to the
@@ -42,8 +37,8 @@ use crate::checkers::ast::Checker;
 /// - [Python documentation: The `global` statement](https://docs.python.org/3/reference/simple_stmts.html#the-global-statement)
 #[derive(ViolationMetadata)]
 pub(crate) struct LoadBeforeGlobalDeclaration {
-    name: String,
-    row: SourceRow,
+    pub(crate) name: String,
+    pub(crate) row: SourceRow,
 }
 
 impl Violation for LoadBeforeGlobalDeclaration {
@@ -53,18 +48,3 @@ impl Violation for LoadBeforeGlobalDeclaration {
         format!("Name `{name}` is used prior to global declaration on {row}")
     }
 }
-
-/// PLE0118
-pub(crate) fn load_before_global_declaration(checker: &Checker, name: &str, expr: &Expr) {
-    if let Some(stmt) = checker.semantic().global(name) {
-        if expr.start() < stmt.start() {
-            checker.report_diagnostic(Diagnostic::new(
-                LoadBeforeGlobalDeclaration {
-                    name: name.to_string(),
-                    row: checker.compute_source_row(stmt.start()),
-                },
-                expr.range(),
-            ));
-        }
-    }
-}

crates/ruff_python_parser/src/semantic_errors.rs

@@ -12,7 +12,7 @@ use ruff_python_ast::{
     Expr, ExprContext, IrrefutablePatternKind, Pattern, PythonVersion, Stmt, StmtExpr,
     StmtImportFrom,
 };
-use ruff_text_size::{Ranged, TextRange};
+use ruff_text_size::{Ranged, TextRange, TextSize};
 use rustc_hash::FxHashSet;
 
 #[derive(Debug)]
@@ -397,6 +397,21 @@ impl SemanticSyntaxChecker {
                         _ => {}
                     };
                 }
+
+                // PLE0118
+                if let Some(stmt) = ctx.global(id) {
+                    let start = stmt.start();
+                    if expr.start() < start {
+                        Self::add_error(
+                            ctx,
+                            SemanticSyntaxErrorKind::LoadBeforeGlobalDeclaration {
+                                name: id.to_string(),
+                                start,
+                            },
+                            expr.range(),
+                        );
+                    }
+                }
             }
             Expr::Yield(ast::ExprYield {
                 value: Some(value), ..
@@ -499,6 +514,9 @@ impl Display for SemanticSyntaxError {
                     write!(f, "cannot delete `__debug__` on Python {python_version} (syntax was removed in 3.9)")
                 }
             },
+            SemanticSyntaxErrorKind::LoadBeforeGlobalDeclaration { name, start: _ } => {
+                write!(f, "name `{name}` is used prior to global declaration")
+            }
             SemanticSyntaxErrorKind::InvalidStarExpression => {
                 f.write_str("can't use starred expression here")
             }
@@ -616,6 +634,19 @@ pub enum SemanticSyntaxErrorKind {
     /// [BPO 45000]: https://github.com/python/cpython/issues/89163
     WriteToDebug(WriteToDebugKind),
 
+    /// Represents the use of a `global` variable before its `global` declaration.
+    ///
+    /// ## Examples
+    ///
+    /// ```python
+    /// counter = 1
+    /// def increment():
+    ///     print(f"Adding 1 to {counter}")
+    ///     global counter
+    ///     counter += 1
+    /// ```
+    LoadBeforeGlobalDeclaration { name: String, start: TextSize },
+
     /// Represents the use of a starred expression in an invalid location, such as a `return` or
     /// `yield` statement.
     ///
@@ -758,6 +789,9 @@ pub trait SemanticSyntaxContext {
     /// The target Python version for detecting backwards-incompatible syntax changes.
     fn python_version(&self) -> PythonVersion;
 
+    /// Return the [`TextRange`] at which a name is declared as `global` in the current scope.
+    fn global(&self, name: &str) -> Option<TextRange>;
+
     fn report_semantic_error(&self, error: SemanticSyntaxError);
 }
 

crates/ruff_python_parser/tests/fixtures.rs

@@ -488,4 +488,8 @@ impl SemanticSyntaxContext for TestContext {
     fn report_semantic_error(&self, error: SemanticSyntaxError) {
         self.diagnostics.borrow_mut().push(error);
     }
+
+    fn global(&self, _name: &str) -> Option<TextRange> {
+        None
+    }
 }