Skip to content

[ty] name is parameter and global is a syntax error #21312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Nov 14, 2025
Merged

[ty] name is parameter and global is a syntax error #21312

merged 6 commits into from
Nov 14, 2025

Conversation

@11happy
Copy link
Contributor

@11happy 11happy commented Nov 7, 2025

Summary

This PR implements is_bound_parameter for ty for syntax error name is parameter and global, this is a follow up to this #20426

Test Plan

mdtest and snapshots

CC

@11happy
ty: name is parameter and global
96a96ae 
Signed-off-by: 11happy <soni5happy@gmail.com>
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025 •
edited by astral-sh-bot bot
Loading

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@11happy 11happy mentioned this pull request Nov 7, 2025
Open
14 tasks
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025 •
edited by astral-sh-bot bot
Loading

mypy_primer results

No ecosystem changes detected ✅

No memory usage changes detected ✅

@11happy
update snapshot
3c991d2 
Signed-off-by: 11happy <soni5happy@gmail.com>
@11happy
test: update snapshot
127425c 
Signed-off-by: 11happy <soni5happy@gmail.com>
@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label Nov 7, 2025
@MichaReiser MichaReiser changed the title [ty]: name is parameter and global is a syntax error [ty] name is parameter and global is a syntax error Nov 9, 2025
MichaReiser
MichaReiser previously requested changes Nov 10, 2025
View reviewed changes
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

It seems some mdtests are failing. Would you mind taking a look at why they're failing?

@11happy
Copy link
Contributor Author

11happy commented Nov 10, 2025 

crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md:386 unexpected error: 12 [unresolved-global] "Invalid global declaration of `a`: `a` has no declarations or bindings in the global scope"

crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md:386 unexpected error: 12 [invalid-syntax] "name `a` is used prior to global declaration"

actually two more errors are also emitted along with the expected one , can we supress others ? as the changes in this PR are not related to other two errors ,

@ntBre
Copy link
Contributor

ntBre commented Nov 10, 2025

For the first error, I think you can just add a global a binding before the function definition. The second case looks interesting, I think it's basically a conflicting error with the error you're adding in this PR. I think we might want to suppress the used prior to global declaration error in this case so that we only report the more specific parameter and global error.

I checked how we handle this in ruff too, and it looks like we only emit the parameter and global syntax error, even if PLE0118 is also enabled.

@11happy
feat: add new method is_parameter for symbol
89dc04f 
Signed-off-by: 11happy <soni5happy@gmail.com>
@11happy
Copy link
Contributor Author

11happy commented Nov 12, 2025 •
edited
Loading

For the first error, I think you can just add a global a binding before the function definition. The second case looks interesting, I think it's basically a conflicting error with the error you're adding in this PR. I think we might want to suppress the used prior to global declaration error in this case so that we only report the more specific parameter and global error.

I checked how we handle this in ruff too, and it looks like we only emit the parameter and global syntax error, even if PLE0118 is also enabled.

what do you think of this approach I have added a new symbol flag _IS_PARAMETER ? is working as expected now

@11happy
test: update snapshot
ddcff38 
Signed-off-by: 11happy <soni5happy@gmail.com>
@ntBre
Copy link
Contributor

ntBre commented Nov 13, 2025

Nice! This does seem to work. My only hesitation is that I suspect there is an existing way to obtain this information without adding adding a new SymbolFlags variant. I poked around a bit and didn't find anything, but it's probably worth someone on the ty team taking a look if they get the chance.

Maybe @AlexWaygood and I can have a look tomorrow during our meeting :)

@carljm
Copy link
Contributor

carljm commented Nov 13, 2025

FWIW I think it's probably possible to do it another way, but this way looks pretty clean and straightforward, and IMO is very much in line with the intended use of SymbolFlags, so no objection from me.

AlexWaygood
AlexWaygood approved these changes Nov 14, 2025
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

thank you!

@AlexWaygood AlexWaygood dismissed MichaReiser’s stale review November 14, 2025 17:48

Requested changes were made

@astral-sh-bot
Copy link

astral-sh-bot bot commented Nov 14, 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.

@AlexWaygood AlexWaygood merged commit 8529d79 into astral-sh:main Nov 14, 2025
41 checks passed
dcreager added a commit that referenced this pull request Nov 14, 2025
@dcreager
Merge remote-tracking branch 'origin/main' into dcreager/deep-comparison
f83a5a3 
* origin/main: (59 commits)
  [ty] Improve diagnostic range for `non-subscriptable` diagnostics (#21461)
  [ty] Improve literal promotion heuristics (#21439)
  [ty] Further improve details around which expressions should be deferred in stub files (#21456)
  [ty] Improve generic class constructor inference (#21442)
  [ty] Propagate type context through conditional expressions (#21443)
  [ty] Suppress completions when introducing names with `as`
  [ty] Add panic-by-default await methods to `TestServer` (#21451)
  [ty] name is parameter and global is a syntax error (#21312)
  [ty] Fixup a few details around version-specific dataclass features (#21453)
  [ty] Support attribute-expression `TYPE_CHECKING` conditionals (#21449)
  [ty] Support stringified annotations in value-position `Annotated` instances (#21447)
  [ty] Type inference for genererator expressions (#21437)
  [ty] Make `__getattr__` available for `ModuleType` instances (#21450)
  [ty] Increase default receive timeout in tests to 10s (#21448)
  [ty] Add synthetic members to completions on dataclasses (#21446)
  [ty] Support legacy `typing` special forms in implicit type aliases (#21433)
  Bump 0.14.5 (#21435)
  [ty] Support `type[…]` and `Type[…]` in implicit type aliases (#21421)
  [ty] Respect notebook cell boundaries when adding an auto import (#21322)
  Update PyCharm setup instructions (#21409)
  ...
dcreager added a commit that referenced this pull request Nov 14, 2025
@dcreager
Merge branch 'dcreager/deep-comparison' into dcreager/coolable
94106a4 
* dcreager/deep-comparison: (64 commits)
  assuming
  SubtypingAssuming
  implies_subtype_of
  name tweak
  Apply suggestions from code review
  [ty] Improve diagnostic range for `non-subscriptable` diagnostics (#21461)
  [ty] Improve literal promotion heuristics (#21439)
  [ty] Further improve details around which expressions should be deferred in stub files (#21456)
  [ty] Improve generic class constructor inference (#21442)
  [ty] Propagate type context through conditional expressions (#21443)
  [ty] Suppress completions when introducing names with `as`
  [ty] Add panic-by-default await methods to `TestServer` (#21451)
  [ty] name is parameter and global is a syntax error (#21312)
  [ty] Fixup a few details around version-specific dataclass features (#21453)
  [ty] Support attribute-expression `TYPE_CHECKING` conditionals (#21449)
  [ty] Support stringified annotations in value-position `Annotated` instances (#21447)
  [ty] Type inference for genererator expressions (#21437)
  [ty] Make `__getattr__` available for `ModuleType` instances (#21450)
  [ty] Increase default receive timeout in tests to 10s (#21448)
  [ty] Add synthetic members to completions on dataclasses (#21446)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygood AlexWaygood AlexWaygood approved these changes

@MichaReiser MichaReiser MichaReiser left review comments

@dhruvmanila dhruvmanila Awaiting requested review from dhruvmanila dhruvmanila is a code owner

Assignees

No one assigned

Labels

ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@11happy @ntBre @carljm @MichaReiser @AlexWaygood