Skip to content

Conversation

@asukaminato0721
Copy link
Contributor

@asukaminato0721 asukaminato0721 commented Jan 7, 2026

Summary

Fixes #1987
Fixes #2047

Implemented safe handling for parser-synthesized targets so keyword assignments no longer panic.

treat all empty identifiers as synthesized so later passes skip binding logic.

route synthesized lvalues and empty Identifiers through anonymous bindings, and short-circuit bind_single_name_assign when parser recovery produces an empty target, ensuring we still analyze RHS expressions without touching static scopes.

Test Plan

added regression test covering async = 1 and documented Ruff’s diagnostic wording.

@meta-cla meta-cla bot added the cla signed label Jan 7, 2026
@asukaminato0721 asukaminato0721 marked this pull request as ready for review January 7, 2026 12:18
Copilot AI review requested due to automatic review settings January 7, 2026 12:18
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a panic that occurred when the parser synthesized empty identifiers during error recovery (e.g., when reserved keywords like async are used as assignment targets). The fix routes these synthesized targets through anonymous bindings, allowing the RHS expressions to still be analyzed for diagnostics without attempting to bind non-existent names to the static scope.

Key changes:

  • Modified the detection logic for synthesized names to check only for empty strings, not both empty strings and empty ranges
  • Added early-return handling in bind_single_name_assign to route empty identifiers through anonymous bindings
  • Updated bind_target_name to use the synthesized name check for determining whether to create anonymous or definition-based bindings
  • Added a regression test for keyword assignment errors

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
pyrefly/lib/test/semantic_syntax_errors.rs Adds regression test for keyword assignment (async = 1) to ensure parse errors are reported without panicking
pyrefly/lib/binding/target.rs Implements safe handling for synthesized empty identifiers in both bind_target_name and bind_single_name_assign functions by routing them through anonymous bindings
crates/pyrefly_python/src/ast.rs Simplifies is_synthesized_empty_name to check only for empty identifier strings, making it more robust for parser error recovery

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

This comment has been minimized.

@asukaminato0721 asukaminato0721 marked this pull request as draft January 7, 2026 12:52
@github-actions

This comment has been minimized.

@asukaminato0721 asukaminato0721 marked this pull request as ready for review January 7, 2026 14:55
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link

github-actions bot commented Jan 9, 2026

Diff from mypy_primer, showing the effect of this PR on open source code:

pandas (https://github.com/pandas-dev/pandas)
- ::error file=pandas/core/indexing.py,line=1470,col=9,endLine=1470,endColumn=28,title=Pyrefly bad-override::Class member `_LocIndexer._convert_to_indexer` overrides parent class `_LocationIndexer` in an inconsistent manner%0A  `_LocIndexer._convert_to_indexer` has type `BoundMethod[_LocIndexer, (self: _LocIndexer, key: Unknown, axis: int) -> ndarray[tuple[Any, ...], dtype[signedinteger[_NBitIntP]]] | ndarray[tuple[Any, ...], dtype[Any]] | dict[str, int | integer[Any]] | dict[str, tuple[Unknown, ...]] | dict[str, Unknown] | Unknown]`, which is not assignable to `BoundMethod[_LocIndexer, (self: _LocIndexer, key: Unknown, axis: int) -> Never]`, the type of `_LocationIndexer._convert_to_indexer`
+ ::error file=pandas/core/indexing.py,line=1470,col=9,endLine=1470,endColumn=28,title=Pyrefly bad-override::Class member `_LocIndexer._convert_to_indexer` overrides parent class `_LocationIndexer` in an inconsistent manner%0A  `_LocIndexer._convert_to_indexer` has type `BoundMethod[_LocIndexer, (self: _LocIndexer, key: Unknown, axis: int) -> int | ndarray[tuple[int], dtype[Any]] | ndarray[tuple[Any, ...], dtype[signedinteger[_NBitIntP]]] | ndarray[tuple[Any, ...], dtype[Any]] | slice[int, int, Any] | slice[Any, Any, Any] | dict[str, int | integer[Any]] | dict[str, tuple[Unknown, ...]] | dict[str, Unknown] | Unknown]`, which is not assignable to `BoundMethod[_LocIndexer, (self: _LocIndexer, key: Unknown, axis: int) -> Never]`, the type of `_LocationIndexer._convert_to_indexer`

@meta-codesync
Copy link

meta-codesync bot commented Jan 9, 2026

@migeed-z has imported this pull request. If you are a Meta employee, you can view this in D90401781.

Copy link
Contributor

@stroxler stroxler left a comment

Choose a reason for hiding this comment

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

Review automatically exported from Phabricator review in Meta.

@meta-codesync
Copy link

meta-codesync bot commented Jan 9, 2026

@migeed-z merged this pull request in 1624807.

@asukaminato0721 asukaminato0721 deleted the 1987 branch January 10, 2026 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pyrefly crashes on black package panic: Name not found in static scope of module __unknown__

4 participants