Skip to content

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

Merged
charliermarsh merged 3 commits intoastral-sh:mainfrom
sbrugman:pyi055-improve-autofix
Nov 12, 2024
Merged

[flake8-pyi] Improve autofix for nested and mixed type unions unnecessary-type-union (PYI055)#14272
charliermarsh merged 3 commits intoastral-sh:mainfrom
sbrugman:pyi055-improve-autofix

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

@github-actions
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
@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