Skip to content
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

Visit PEP 764 inline TypedDicts' keys as non-type-expressions #15073

Merged
merged 3 commits into from
Dec 30, 2024
Merged

Visit PEP 764 inline TypedDicts' keys as non-type-expressions #15073

merged 3 commits into from
Dec 30, 2024
+122 −0

Conversation

InSyncWithFoo
Copy link
Contributor

Summary

Resolves #10812.

Test Plan

cargo nextest run and cargo insta test.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 20, 2024 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@InSyncWithFoo
Copy link
Contributor Author

The INLINE_TYPEDDICT bitflag was added "just in case". I'll remove it if it is not desired.

@dhruvmanila
Copy link
Member

The INLINE_TYPEDDICT bitflag was added "just in case". I'll remove it if it is not desired.

Yeah, we should remove it if it's not being used currently.

I'll also make a general recommendation to use typed_dict instead of typeddict as I'd consider them two separate words.

Apart from this, I'm not exactly sure what the scope of this should be as the PEP is still in draft so we don't know where all we need to change this. What about when the slice element is not a dictionary like the one in your test case? I don't see it being supported in the PEP yet. Should we only limit it to the following syntax (https://github.com/Viicos/peps/blob/inlined-typed-dictionaries/peps/pep-0764.rst#typing-specification-changes)?

<TypedDict> '[' '{' (string: ':' annotation_expression ',')* '}' ']'

@dhruvmanila dhruvmanila added the rule Implementing or modifying a lint rule label Dec 24, 2024
@InSyncWithFoo
Copy link
Contributor Author

InSyncWithFoo commented Dec 24, 2024 •
edited
Loading

@dhruvmanila When the expression is not a dictionary literal, TypedDict[expr] is invalid as a type expression. In this case, I think expr should be visited as a normal (non-type) expression, which is already the case with this PR.

This PR could be considered provisional support for the PEP. Pyright (and Basedpyright) already did so; parity by other tools could help the PEP gain wider recognition.

dhruvmanila
dhruvmanila approved these changes Dec 30, 2024
View reviewed changes
Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

Thanks!

@dhruvmanila dhruvmanila merged commit d4ee6ab into astral-sh:main Dec 30, 2024
21 checks passed
@InSyncWithFoo InSyncWithFoo deleted the inline-typeddicts branch December 30, 2024 09:48
dcreager added a commit that referenced this pull request Dec 30, 2024
@dcreager
Merge branch 'main' into dcreager/infer-binary
dffd23d 
* main:
  Add all PEP-585 names to UP006 rule (#5454)
  [`flake8-simplify`] More precise inference for dictionaries (`SIM300`) (#15164)
  `@no_type_check` support (#15122)
  Visit PEP 764 inline `TypedDict`s' keys as non-type-expressions (#15073)
  [red-knot] Add diagnostic for invalid unpacking (#15086)
  [`flake8-use-pathlib`] Catch redundant joins in `PTH201` and avoid syntax errors (#15177)
  Update Rust crate glob to v0.3.2 (#15185)
  Update astral-sh/setup-uv action to v5 (#15193)
  Update dependency mdformat-mkdocs to v4.1.1 (#15192)
  Update Rust crate serde_with to v3.12.0 (#15191)
  Update NPM Development dependencies (#15190)
  Update pre-commit hook rhysd/actionlint to v1.7.5 (#15189)
  Update Rust crate syn to v2.0.93 (#15188)
  Update Rust crate serde to v1.0.217 (#15187)
  Update Rust crate quote to v1.0.38 (#15186)
  Update Rust crate compact_str to v0.8.1 (#15184)
  [`flake8-type-checking`] Disable TC006 & TC007 in stub files (#15179)
  Test explicit shadowing involving `def`s (#15174)
  Fix typo in `NameImport.qualified_name` docstring (#15170)
  [airflow]: extend names moved from core to provider (AIR303) (#15159)
@BrewTestBot BrewTestBot mentioned this pull request Jan 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhruvmanila dhruvmanila dhruvmanila approved these changes

Assignees
No one assigned
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Milestone
No milestone
2 participants
@InSyncWithFoo @dhruvmanila