-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Document when a rule was added #20859
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.
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
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
sharkdp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
As for the actual versions that we put in, I think I would probably prefer them to point to our first stable release, once that's out?
Reading something like "introduced in 0.0.1-alpha.17" in the documentation for a core ty rule might look more confusing than helpful?
Yes, I think we can consider doing this once we have a stable release (or we update all versions to 0.0.1 with the beta released). |
…able * dcreager/non-inferable-api: just the api parts [ty] Fix further issues in `super()` inference logic (#20843) [ty] Document when a rule was added (#20859) [ty] Treat `Callable` dunder members as bound method descriptors (#20860) [ty] Handle decorators which return unions of `Callable`s (#20858) Fix false negatives in `Truthiness::from_expr` for lambdas, generators, and f-strings (#20704) [ty] Rename Type unwrapping methods (#20857) Update `lint.flake8-type-checking.quoted-annotations` docs (#20765)
…nt-sets * dcreager/non-non-inferable: (174 commits) [ty] Add (unused) `inferable` parameter to type property methods (#20865) Run macos tests on macos (#20889) Remove `release` CI job (#20887) [ty] CI: Faster ecosystem analysis (#20886) Remove `strip` from release profile (#20885) [ty] Sync vendored typeshed stubs (#20876) [ty] Add some completion ranking improvements (#20807) Improved error recovery for unclosed strings (including f- and t-strings) (#20848) Enable lto=fat (#20863) [`pyupgrade`] Extend `UP019` to detect `typing_extensions.Text` (`UP019`) (#20825) [`flake8-bugbear`] Omit annotation in preview fix for `B006` (#20877) fix(docs): Fix typo in `RUF015` description (#20873) [ty] Improve and extend tests for instance attributes redeclared in subclasses (#20866) [ty] Ignore slow seeds as a temporary measure (#20870) use existing method Remove parentheses around multiple exception types on Python 3.14+ (#20768) Update Black tests (#20794) just the api parts [ty] Fix further issues in `super()` inference logic (#20843) [ty] Document when a rule was added (#20859) ...
Summary -- Inspired by #20859, this PR adds the version a rule was added, and the file and line where it was defined, to `ViolationMetadata`. The file and line just use the standard `file!` and `line!` macros, while the more interesting version field uses a new `violation_metadata` attribute parsed by our `ViolationMetadata` derive macro. I moved the commit modifying all of the rule files to the end, so it should be a lot easier to review by omitting that one. As a curiosity and a bit of a sanity check, I also plotted the rule numbers over time: <img width="640" height="480" alt="image" src="https://github.com/user-attachments/assets/75b0b5cc-3521-4d40-a395-8807e6f4925f" /> I think this looks pretty reasonable and avoids some of the artifacts the earlier versions of the script ran into, such as the `rule` sub-command not being available or `--explain` requiring a file argument. <details><summary>Script and summary data</summary> ```shell gawk --csv ' NR > 1 { split($2, a, ".") major = a[1]; minor = a[2]; micro = a[3] # sum the number of rules added per minor version versions[minor] += 1 } END { tot = 0 for (i = 0; i <= 14; i++) { tot += versions[i] print i, tot } } ' ruff_rules_metadata.csv > summary.dat ``` ``` 0 696 1 768 2 778 3 803 4 822 5 848 6 855 7 865 8 893 9 915 10 916 11 924 12 929 13 932 14 933 ``` </details> Test Plan -- I built and viewed the documentation locally, and it looks pretty good! <img width="1466" height="676" alt="image" src="https://github.com/user-attachments/assets/5e227df4-7294-4d12-bdaa-31cac4e9ad5c" /> The spacing seems a bit awkward following the `h1` at the top, so I'm wondering if this might look nicer as a footer in Ruff. The links work well too: - [v0.0.271](https://github.com/astral-sh/ruff/releases/tag/v0.0.271) - [Related issues](https://github.com/astral-sh/ruff/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20airflow-variable-name-task-id-mismatch) - [View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fruff_linter%2Fsrc%2Frules%2Fairflow%2Frules%2Ftask_variable_name.rs#L34) The last one even works on `main` now since it points to the `derive(ViolationMetadata)` line. In terms of binary size, this branch is a bit bigger than main with 38,654,520 bytes compared to 38,635,728 (+20 KB). I guess that's not _too_ much of an increase, but I wanted to check since we're generating a lot more code with macros. --------- Co-authored-by: GiGaGon <[email protected]>
Summary
Replace the placeholder
1.0.0version in the lint definitions with the version when they were first added to ty.Update the
rules.mdgeneration script to include the version in the documentation.Closes astral-sh/ty#828