[flake8-pyi] Significantly improve accuracy of `PYI019` if preview mode is enabled by AlexWaygood · Pull Request #15888 · astral-sh/ruff

AlexWaygood · 2025-02-02T22:51:29Z

(This PR is stacked on top of #15885; review that first.)

Summary

The logic PYI019 uses to detect TypeVars that could be replaced with Self is flawed in several significant ways:

The rule only looks at methods that have return-type annotations, but there are lots of methods that don't have return-type annotations that could still have their custom TypeVars replaced with Self.
The rule assumes that if the first parameter has the same annotation as the return-type annotation in an instance method, the method will be using a TypeVar in its annotations. This heuristic is surprisingly accurate, but it's still a somewhat unprincipled heuristic. It means that the rule flags code like this as "using TypeVars that could be replaced with Self, when that's not true at all:
```
class _NotATypeVar: ...

class Foo:
    def m(self: _NotATypeVar) -> _NotATypeVar: ...  # PYI019 flagged here
```
Following @InSyncWithFoo's excellent work in [flake8-pyi] Fix more complex cases (PYI019) #15821 to move this rule to be a bindings-based rule, there's now no longer any need to use heuristics like this; we can do much better.

Unfortunately, fixing these issues expands the scope of the rule (it will start flagging methods that it did not previously flag). This PR therefore applies these fixes only if preview mode is enabled. Otherwise, we continue to use the old heuristics that we always have.

Because the rule in preview mode can now apply to methods that do not have return-type annotations, we can no longer use the range of the return-type annotation as the range of the diagnostic. This PR therefore also changes the behaviour in preview mode so that we use the range of the "function header" for the diagnostic range (starting from the end of the function name, and ending either at the return-type annotation end (if there is a return-type annotation) or at the end of the function's parameters.

Test Plan

New snippets added to existing fixtures
A new snapshot has been added to show what the behaviour is when the rule runs on .py files with preview mode enabled. (We already have a snapshot for .pyi files with stable mode, .py files with stable mode, and .pyi files with preview mode.)

github-actions · 2025-02-02T22:58:11Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+30 -0 violations, +0 -0 fixes in 7 projects; 48 projects unchanged)

apache/superset (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ superset/sql/parse.py:167:21: PYI019 Use `Self` instead of custom TypeVar `TBaseSQLStatement`

pandas-dev/pandas (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ pandas/_libs/tslibs/timedeltas.pyi:97:16: PYI019 Use `Self` instead of custom TypeVar `_S`

python/typeshed (+13 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select E,F,FA,I,PYI,RUF,UP,W

+ stdlib/_typeshed/__init__.pyi:328:16: PYI019 Use `Self` instead of custom TypeVar `Self`
+ stdlib/_typeshed/__init__.pyi:330:24: PYI019 Use `Self` instead of custom TypeVar `Self`
+ stdlib/typing.pyi:960:15: PYI019 [*] Use `Self` instead of custom TypeVar `_T`
+ stdlib/typing_extensions.pyi:239:15: PYI019 Use `Self` instead of custom TypeVar `_T`
+ stubs/PyYAML/yaml/representer.pyi:28:24: PYI019 [*] Use `Self` instead of custom TypeVar `_R`
+ stubs/PyYAML/yaml/representer.pyi:30:30: PYI019 [*] Use `Self` instead of custom TypeVar `_R`
+ stubs/protobuf/google/protobuf/internal/containers.pyi:36:18: PYI019 [*] Use `Self` instead of custom TypeVar `_M`
+ stubs/protobuf/google/protobuf/internal/containers.pyi:52:18: PYI019 [*] Use `Self` instead of custom TypeVar `_M`
+ stubs/protobuf/google/protobuf/internal/containers.pyi:76:18: PYI019 [*] Use `Self` instead of custom TypeVar `_M`
+ stubs/protobuf/google/protobuf/internal/containers.pyi:99:18: PYI019 [*] Use `Self` instead of custom TypeVar `_M`
+ stubs/protobuf/google/protobuf/message.pyi:30:21: PYI019 [*] Use `Self` instead of custom TypeVar `_M`
+ stubs/protobuf/google/protobuf/message.pyi:31:23: PYI019 [*] Use `Self` instead of custom TypeVar `_M`
+ stubs/protobuf/google/protobuf/message.pyi:34:19: PYI019 [*] Use `Self` instead of custom TypeVar `_M`

rotki/rotki (+10 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ rotkehlchen/db/filtering.py:1138:13: PYI019 Use `Self` instead of custom TypeVar `T_EthSTakingFilterQ`
+ rotkehlchen/db/filtering.py:373:15: PYI019 Use `Self` instead of custom TypeVar `T_FilterQ`
+ rotkehlchen/db/filtering.py:866:13: PYI019 Use `Self` instead of custom TypeVar `T_HistoryBaseEntryFilterQ`
+ rotkehlchen/history/events/structures/base.py:372:39: PYI019 Use `Self` instead of custom TypeVar `T`
+ rotkehlchen/utils/mixins/enums.py:101:20: PYI019 Use `Self` instead of custom TypeVar `S`
+ rotkehlchen/utils/mixins/enums.py:126:20: PYI019 Use `Self` instead of custom TypeVar `S`
+ rotkehlchen/utils/mixins/enums.py:12:16: PYI019 Use `Self` instead of custom TypeVar `T`
+ rotkehlchen/utils/mixins/enums.py:151:28: PYI019 Use `Self` instead of custom TypeVar `D`
+ rotkehlchen/utils/mixins/enums.py:172:28: PYI019 Use `Self` instead of custom TypeVar `D`
+ rotkehlchen/utils/mixins/enums.py:77:20: PYI019 Use `Self` instead of custom TypeVar `S`

zulip/zulip (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ zerver/data_import/slack.py:109:18: PYI019 Use `Self` instead of custom TypeVar `SlackBotEmailT`
+ zerver/lib/thumbnail.py:55:20: PYI019 Use `Self` instead of custom TypeVar `T`

python-trio/trio (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ src/trio/testing/_fake_net.py:103:29: PYI019 Use `Self` instead of custom TypeVar `T_UDPEndpoint`

astropy/astropy (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ astropy/cosmology/_src/core.py:485:26: PYI019 Use `Self` instead of custom TypeVar `_FlatCosmoT`
+ astropy/cosmology/_src/flrw/base.py:1705:16: PYI019 Use `Self` instead of custom TypeVar `_FlatFLRWMixinT`

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
PYI019	30	30	0	0	0

AlexWaygood · 2025-02-02T23:11:44Z

ruff-ecosystem results

These all look like true positives from what I can see 🥳

MichaReiser · 2025-02-03T15:06:41Z

This is another case where diagnsotics with multiple ranges were really nice CC: @BurntSushi

MichaReiser

This is excellent. Thank you

MichaReiser · 2025-02-03T15:13:20Z

crates/ruff_linter/src/rules/flake8_pyi/rules/custom_type_var_for_self.rs

+        .unwrap_or_else(|| parameters.end());
+
+    let (method, diagnostic_range) =
+        match (function_kind, checker.settings.preview, returns.as_deref()) {


This one will be hard to find because it won't show up when searching for preview.is_enabled or preview.is_disabled (which I normally use).

It makes me wonder if we should start doing something similar to the formatter where we have a preview.rs file and each preview feature defines a is_feature_name_enabled(&settings). It makes it very easy to see all current preview behaviors and promoting it is as easy as removing the function -> Rust will tell you exactly where you have to make changes.

However, this will likely mean that you'd have to change this match to inline the preview check into the match arms. Although I'd kind of prefer this anyway because it reduces the number of lines that need changing when promoting the feature (the match header remains unchanged).

match (function_kind, returns.as_deref() { (FunctionType::Function | FunctionType::StaticMethod, _) => return None, (FunctionType::ClassMethod, returns) => { if is_more_precise_custom_type_var_for_self_enabled(settings) { ... } else if let Some(returns) = returns { ... }

I've switched to use preview.is_enabled(). It's not pretty... but I'm consoling myself that we'll be able to get rid of it soonish 😆

Oh no, I hoped you start using the preview feature functions... I'll think of you when I have to promote the behavior change 🤣

Ohh sorry lol. You can give this one to me when the time comes 🤣

crates/ruff_linter/src/rules/flake8_pyi/rules/custom_type_var_for_self.rs

…mode is enabled

…tom-type-var-for-self`(`PYI019`) (#16607) ## Summary This PR stabilizes several preview-only behaviours for `custom-typevar-for-self` (`PYI019`). Namely: - A new, more accurate technique is now employed for detecting custom TypeVars that are replaceable with `Self`. See #15888 for details. - The range of the diagnostic is now the full function header rather than just the return annotation. (Previously, the rule only applied to methods with return annotations, but this is no longer true due to the changes in the first bullet point.) - The fix is now available even when preview mode is not enabled. ## Test Plan - Existing snapshots that do not have preview mode enabled are updated - Preview-specific snapshots are removed - I'll check the ecosystem report on this PR to verify everything's as expected

AlexWaygood added rule Implementing or modifying a lint rule preview Related to preview mode features labels Feb 2, 2025

AlexWaygood mentioned this pull request Feb 2, 2025

[flake8-pyi] Make PYI019 autofixable for .py files in preview mode as well as stubs #15889

Merged

Base automatically changed from alex/pyi019-message to main February 3, 2025 14:24

This comment was marked as outdated.

Sign in to view

AlexWaygood force-pushed the alex/pyi019-heuristics branch from e82e76b to 4e2dc61 Compare February 3, 2025 14:57

This comment was marked as resolved.

Sign in to view

AlexWaygood force-pushed the alex/pyi019-heuristics branch from 4e2dc61 to a73295b Compare February 3, 2025 15:19

MichaReiser approved these changes Feb 3, 2025

View reviewed changes

[flake8-pyi] Significantly improve accuracy of PYI019 if preview …

0e243a3

…mode is enabled

AlexWaygood force-pushed the alex/pyi019-heuristics branch from a73295b to 0e243a3 Compare February 3, 2025 15:39

AlexWaygood enabled auto-merge (squash) February 3, 2025 15:42

AlexWaygood merged commit 62075af into main Feb 3, 2025
21 checks passed

AlexWaygood deleted the alex/pyi019-heuristics branch February 3, 2025 15:45

AlexWaygood mentioned this pull request Feb 4, 2025

Make Binding::range() point to the range of a type parameter's name, not the full type parameter #15935

Merged

BrewTestBot mentioned this pull request Feb 6, 2025

ruff 0.9.5 Homebrew/homebrew-core#206789

Merged

AlexWaygood mentioned this pull request Mar 10, 2025

[ruff-0.10] [flake8-pyi] Stabilize preview-mode behaviours for custom-type-var-for-self(PYI019) #16607

Merged

AlexWaygood mentioned this pull request Apr 4, 2025

Use new diagnostic functionality #17203

Open

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

[flake8-pyi] Significantly improve accuracy of `PYI019` if preview mode is enabled#15888

[flake8-pyi] Significantly improve accuracy of `PYI019` if preview mode is enabled#15888
AlexWaygood merged 1 commit intomainfrom
alex/pyi019-heuristics

AlexWaygood commented Feb 2, 2025

Uh oh!

github-actions bot commented Feb 2, 2025 •

edited

Loading

Uh oh!

AlexWaygood commented Feb 2, 2025

Uh oh!

This comment was marked as outdated.

This comment was marked as resolved.

MichaReiser commented Feb 3, 2025

Uh oh!

MichaReiser left a comment

Uh oh!

MichaReiser Feb 3, 2025

Uh oh!

AlexWaygood Feb 3, 2025

Uh oh!

MichaReiser Feb 3, 2025

Uh oh!

AlexWaygood Feb 3, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

AlexWaygood commented Feb 2, 2025

Summary

Test Plan

Uh oh!

github-actions bot commented Feb 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

AlexWaygood commented Feb 2, 2025

Uh oh!

This comment was marked as outdated.

This comment was marked as resolved.

MichaReiser commented Feb 3, 2025

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

MichaReiser Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Feb 2, 2025 •

edited

Loading

`ruff-ecosystem` results