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.

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

[flake8-pyi] Improve autofix for nested and mixed type unions unnecessary-type-union (PYI055) #14272

Merged
merged 3 commits into from
Nov 12, 2024

Conversation

sbrugman
Copy link
Contributor

Summary

This PR improves the fix for PYI055 to be able to handle nested and mixed type unions.

It also marks the fix as unsafe when comments are present.

Test Plan

Copy link
Contributor

github-actions bot commented Nov 11, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@AlexWaygood AlexWaygood added rule Implementing or modifying a lint rule fixes Related to suggested fixes for violations labels Nov 11, 2024
@sbrugman sbrugman marked this pull request as ready for review November 11, 2024 17:03
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.

Nice.

The same as for other fixes. I think we should not offer the fix if the range contains comments instead of marking the fix as unsafe.

)));
}
// Mark [`Fix`] as unsafe when comments are in range.
let applicability = if checker.comment_ranges().intersects(union.range()) {
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I looked through all the intersects usages and I only found one exception where we set Applicability::Unsafe if the node contains comments. All other usages don't offer a fix instead. I think we should do the same here.

Could you take a look if we should use has_comments instead?

Copy link
Contributor Author

@sbrugman sbrugman Nov 12, 2024

Choose a reason for hiding this comment

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

has_comments is just wrapping intersects that includes leading and trailing content.

For type annotations I cannot come up with a case where the fix is changing the leading/trailing content:

   a: ( # comment 1
-    type[int] | type[float] ) = 3
+    type[int | float] ) =  3

I'm not absolutely sure though.

@charliermarsh charliermarsh merged commit bd30701 into astral-sh:main Nov 12, 2024
20 checks passed
@sbrugman sbrugman deleted the pyi055-improve-autofix branch November 12, 2024 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixes Related to suggested fixes for violations rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants