Skip to content

Conversation

@11happy
Copy link
Contributor

@11happy 11happy commented Oct 22, 2025

Summary

This PR ports PLE0117 as a semantic syntax error.

Test Plan

Tests previously written

Signed-off-by: 11happy <soni5happy@gmail.com>
Signed-off-by: 11happy <soni5happy@gmail.com>
@11happy 11happy marked this pull request as ready for review November 2, 2025 03:15
@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2025

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2025

mypy_primer results

No ecosystem changes detected ✅
No memory usage changes detected ✅

Signed-off-by: 11happy <soni5happy@gmail.com>
Signed-off-by: 11happy <soni5happy@gmail.com>
@11happy
Copy link
Contributor Author

11happy commented Nov 2, 2025

@ntBre its ready for review : )

@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This looks good overall, I just had a few small suggestions.

Comment on lines 226 to 229

// test_ok nonlocal_declaration_at_module_level
// def _():
// nonlocal x
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably revert this change since it just moves the test down, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@ntBre ntBre added the internal An internal refactor or improvement label Nov 3, 2025
11happy and others added 7 commits November 5, 2025 17:18
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
Signed-off-by: 11happy <soni5happy@gmail.com>
Signed-off-by: 11happy <soni5happy@gmail.com>
Signed-off-by: 11happy <soni5happy@gmail.com>
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I just pushed a couple of commits removing the unrelated snapshot removal and updating the helper method's docstring. I also added a comment to the ty version of has_nonlocal_binding since we already check for this syntax error in ty here:

// There's no matching binding in an enclosing scope. This `nonlocal` statement is
// invalid.
if let Some(builder) = self
.context
.report_diagnostic(DiagnosticId::InvalidSyntax, Severity::Error)
{
builder
.into_diagnostic(format_args!("no binding for nonlocal `{name}` found"))
.annotate(Annotation::primary(self.context.span(*range)));
}

Thanks @AlexWaygood for pointing that out!

(Ideally we'd use the SemanticSyntaxErrorKind here too, but I didn't see a great way to do that immediately. We'd need another helper like InferContext::report_diagnostic)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@ntBre ntBre enabled auto-merge (squash) November 5, 2025 19:09
@ntBre ntBre merged commit cddc0fe into astral-sh:main Nov 5, 2025
40 checks passed
carljm added a commit to MatthewMckee4/ruff that referenced this pull request Nov 6, 2025
* main: (188 commits)
  [ty] Discover site-packages from the environment that ty is installed in (astral-sh#21286)
  [ty] Make special cases for `UnionType` slightly narrower (astral-sh#21276)
  Require ignore 0.4.24 in `Cargo.toml` (astral-sh#21292)
  [ty] Favour imported symbols over builtin symbols (astral-sh#21285)
  docs: revise Ruff setup instructions for Zed editor (astral-sh#20935)
  [ty] Update salsa (astral-sh#21281)
  [syntax-error]: no binding for nonlocal  PLE0117 as a semantic syntax error (astral-sh#21032)
  [ty] Constraining a typevar with itself (possibly via union or intersection) (astral-sh#21273)
  [`ruff`] Fix false positives on starred arguments (`RUF057`) (astral-sh#21256)
  [ty] Simplify unions containing multiple type variables during inference (astral-sh#21275)
  [ty] Add `ty_server::Db` trait (astral-sh#21241)
  [ty] Refactor `Range` to/from `TextRange` conversion as prep for notebook support (astral-sh#21230)
  [ty] Fix playground crash when file name includes path separator (astral-sh#21151)
  [`refurb`] Fix false negative for underscores before sign in `Decimal` constructor (`FURB157`) (astral-sh#21190)
  [ty] Allow values of type `None` in type expressions (astral-sh#21263)
  Run codspeed benchmarks with `profiling` profile (astral-sh#21261)
  [ty] Update expected diagnostic count in benchmarks (astral-sh#21269)
  Avoid extra parentheses for long `match` patterns with `as` captures (astral-sh#21176)
  [ty] Update salsa (astral-sh#21265)
  [ty] `dict` is not assignable to `TypedDict` (astral-sh#21238)
  ...
dcreager added a commit that referenced this pull request Nov 7, 2025
* origin/main:
  Remove duplicate preview tests for `FURB101` and `FURB103` (#21303)
  [ty] Add support for `Literal`s in implicit type aliases (#21296)
  [ty] Add missing `heap_size` to `variance_of` queries (#21318)
  [`pyupgrade`] Fix false positive on relative imports from local `.builtins` module (`UP029`) (#21309)
  [ty] Make range/position conversions fallible (#21297)
  Bump 0.14.4 (#21306)
  Fix main by using `infer_expression` (#21299)
  [ty] Understand legacy and PEP 695 `ParamSpec` (#21139)
  [ty] Discover site-packages from the environment that ty is installed in (#21286)
  [ty] Make special cases for `UnionType` slightly narrower (#21276)
  Require ignore 0.4.24 in `Cargo.toml` (#21292)
  [ty] Favour imported symbols over builtin symbols (#21285)
  docs: revise Ruff setup instructions for Zed editor (#20935)
  [ty] Update salsa (#21281)
  [syntax-error]: no binding for nonlocal  PLE0117 as a semantic syntax error (#21032)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal An internal refactor or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants