Skip to content

Commit cddc0fe

Browse files
11happyntBreAlexWaygood
authored
[syntax-error]: no binding for nonlocal PLE0117 as a semantic syntax error (#21032)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary <!-- What's the purpose of the change? What does it do, and why? --> This PR ports PLE0117 as a semantic syntax error. ## Test Plan <!-- How was it tested? --> Tests previously written --------- Signed-off-by: 11happy <soni5happy@gmail.com> Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
1 parent eda85f3 commit cddc0fe

File tree

6 files changed

+44
-24
lines changed

6 files changed

+44
-24
lines changed

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
4343
pycodestyle::rules::ambiguous_variable_name(checker, name, name.range());
4444
}
4545
}
46-
if checker.is_rule_enabled(Rule::NonlocalWithoutBinding) {
47-
pylint::rules::nonlocal_without_binding(checker, nonlocal);
48-
}
4946
if checker.is_rule_enabled(Rule::NonlocalAndGlobal) {
5047
pylint::rules::nonlocal_and_global(checker, nonlocal);
5148
}

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ use crate::rules::pyflakes::rules::{
7373
UndefinedLocalWithNestedImportStarUsage, YieldOutsideFunction,
7474
};
7575
use crate::rules::pylint::rules::{
76-
AwaitOutsideAsync, LoadBeforeGlobalDeclaration, YieldFromInAsyncFunction,
76+
AwaitOutsideAsync, LoadBeforeGlobalDeclaration, NonlocalWithoutBinding,
77+
YieldFromInAsyncFunction,
7778
};
7879
use crate::rules::{flake8_pyi, flake8_type_checking, pyflakes, pyupgrade};
7980
use crate::settings::rule_table::RuleTable;
@@ -641,6 +642,10 @@ impl SemanticSyntaxContext for Checker<'_> {
641642
self.semantic.global(name)
642643
}
643644

645+
fn has_nonlocal_binding(&self, name: &str) -> bool {
646+
self.semantic.nonlocal(name).is_some()
647+
}
648+
644649
fn report_semantic_error(&self, error: SemanticSyntaxError) {
645650
match error.kind {
646651
SemanticSyntaxErrorKind::LateFutureImport => {
@@ -717,6 +722,12 @@ impl SemanticSyntaxContext for Checker<'_> {
717722
self.report_diagnostic(pyflakes::rules::ContinueOutsideLoop, error.range);
718723
}
719724
}
725+
SemanticSyntaxErrorKind::NonlocalWithoutBinding(name) => {
726+
// PLE0117
727+
if self.is_rule_enabled(Rule::NonlocalWithoutBinding) {
728+
self.report_diagnostic(NonlocalWithoutBinding { name }, error.range);
729+
}
730+
}
720731
SemanticSyntaxErrorKind::ReboundComprehensionVariable
721732
| SemanticSyntaxErrorKind::DuplicateTypeParameter
722733
| SemanticSyntaxErrorKind::MultipleCaseAssignment(_)

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

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
use ruff_macros::{ViolationMetadata, derive_message_formats};
2-
use ruff_python_ast as ast;
3-
use ruff_text_size::Ranged;
42

53
use crate::Violation;
6-
use crate::checkers::ast::Checker;
74

85
/// ## What it does
96
/// Checks for `nonlocal` names without bindings.
@@ -46,19 +43,3 @@ impl Violation for NonlocalWithoutBinding {
4643
format!("Nonlocal name `{name}` found without binding")
4744
}
4845
}
49-
50-
/// PLE0117
51-
pub(crate) fn nonlocal_without_binding(checker: &Checker, nonlocal: &ast::StmtNonlocal) {
52-
if !checker.semantic().scope_id.is_global() {
53-
for name in &nonlocal.names {
54-
if checker.semantic().nonlocal(name).is_none() {
55-
checker.report_diagnostic(
56-
NonlocalWithoutBinding {
57-
name: name.to_string(),
58-
},
59-
name.range(),
60-
);
61-
}
62-
}
63-
}
64-
}

crates/ruff_python_parser/src/semantic_errors.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ impl SemanticSyntaxChecker {
219219
AwaitOutsideAsyncFunctionKind::AsyncWith,
220220
);
221221
}
222-
Stmt::Nonlocal(ast::StmtNonlocal { range, .. }) => {
222+
Stmt::Nonlocal(ast::StmtNonlocal { names, range, .. }) => {
223223
// test_ok nonlocal_declaration_at_module_level
224224
// def _():
225225
// nonlocal x
@@ -234,6 +234,18 @@ impl SemanticSyntaxChecker {
234234
*range,
235235
);
236236
}
237+
238+
if !ctx.in_module_scope() {
239+
for name in names {
240+
if !ctx.has_nonlocal_binding(name) {
241+
Self::add_error(
242+
ctx,
243+
SemanticSyntaxErrorKind::NonlocalWithoutBinding(name.to_string()),
244+
name.range,
245+
);
246+
}
247+
}
248+
}
237249
}
238250
Stmt::Break(ast::StmtBreak { range, .. }) => {
239251
if !ctx.in_loop_context() {
@@ -1154,6 +1166,9 @@ impl Display for SemanticSyntaxError {
11541166
SemanticSyntaxErrorKind::DifferentMatchPatternBindings => {
11551167
write!(f, "alternative patterns bind different names")
11561168
}
1169+
SemanticSyntaxErrorKind::NonlocalWithoutBinding(name) => {
1170+
write!(f, "no binding for nonlocal `{name}` found")
1171+
}
11571172
}
11581173
}
11591174
}
@@ -1554,6 +1569,9 @@ pub enum SemanticSyntaxErrorKind {
15541569
/// ...
15551570
/// ```
15561571
DifferentMatchPatternBindings,
1572+
1573+
/// Represents a nonlocal statement for a name that has no binding in an enclosing scope.
1574+
NonlocalWithoutBinding(String),
15571575
}
15581576

15591577
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, get_size2::GetSize)]
@@ -2004,6 +2022,9 @@ pub trait SemanticSyntaxContext {
20042022
/// Return the [`TextRange`] at which a name is declared as `global` in the current scope.
20052023
fn global(&self, name: &str) -> Option<TextRange>;
20062024

2025+
/// Returns `true` if `name` has a binding in an enclosing scope.
2026+
fn has_nonlocal_binding(&self, name: &str) -> bool;
2027+
20072028
/// Returns `true` if the visitor is currently in an async context, i.e. an async function.
20082029
fn in_async_context(&self) -> bool;
20092030

crates/ruff_python_parser/tests/fixtures.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,10 @@ impl SemanticSyntaxContext for SemanticSyntaxCheckerVisitor<'_> {
527527
None
528528
}
529529

530+
fn has_nonlocal_binding(&self, _name: &str) -> bool {
531+
true
532+
}
533+
530534
fn in_async_context(&self) -> bool {
531535
if let Some(scope) = self.scopes.iter().next_back() {
532536
match scope {

crates/ty_python_semantic/src/semantic_index/builder.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2712,6 +2712,12 @@ impl SemanticSyntaxContext for SemanticIndexBuilder<'_, '_> {
27122712
None
27132713
}
27142714

2715+
// We handle the one syntax error that relies on this method (`NonlocalWithoutBinding`) directly
2716+
// in `TypeInferenceBuilder::infer_nonlocal_statement`, so this just returns `true`.
2717+
fn has_nonlocal_binding(&self, _name: &str) -> bool {
2718+
true
2719+
}
2720+
27152721
fn in_async_context(&self) -> bool {
27162722
for scope_info in self.scope_stack.iter().rev() {
27172723
let scope = &self.scopes[scope_info.file_scope_id];

0 commit comments

Comments
 (0)