[syntax-errors] Reimplement PLE0118 (#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>
This commit is contained in:
Brent Westbrook 2025-04-02 09:03:44 -04:00 committed by GitHub
parent d45593288f
commit d382065f8a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 62 additions and 27 deletions

View file

@ -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 {

View file

@ -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,6 +541,10 @@ 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 => {
@ -547,6 +552,21 @@ impl SemanticSyntaxContext for Checker<'_> {
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(_)

View file

@ -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(),
));
}
}
}