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] Implement PYI064 #11325

Merged
merged 13 commits into from
May 28, 2024
Merged

Conversation

tusharsadhwani
Copy link
Contributor

@tusharsadhwani tusharsadhwani commented May 7, 2024

Summary

Implements Y064 from flake8-pyi and its autofix.

Test Plan

cargo test / cargo insta review

w1: Final[Literal[123]] # PYI064

# This should be fixable, but marked as unsafe
w2: Final[Literal[123]] = "random value" # PYI064
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think an unsafe autofix is appropriate here (specifically when the assignment value is different from the literal value).

Is autofixing it to w2: Final = 123 in unsafe mode fair?

Copy link
Member

Choose a reason for hiding this comment

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

ummmmmmm this kind of thing doesn't really make any sense, right? I think I'd lean towards not doing an autofix here -- "refuse the temptation to guess"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fair enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Copy link
Contributor

github-actions bot commented May 7, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.


// Discards tuples like `Literal[1, 2, 3]`
// and complex literals like `Literal[{1, 2}]`
if !matches!(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

can potentially be moved to ruff_python_ast/src/helpers.rs, as a is_literal_expr helper.

PlatformName = Literal["linux", "macos", "windows"]
PLATFORMS: Final[set[PlatformName]] = {"linux", "macos", "windows"} # No issue here

foo: Final[{1, 2, 3}] = {1, 2, 3} # No issue here
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now this mimics flake8-pyi behaviour, however I think raising the issue here should be fine?

@charliermarsh
Copy link
Member

@AlexWaygood -- will follow-up and merge this.

@charliermarsh charliermarsh added rule Implementing or modifying a lint rule preview Related to preview mode features labels May 28, 2024
@charliermarsh charliermarsh changed the title [flake8-pyi] Implement PYI064 [flake8-pyi] Implement PYI064 May 28, 2024
@charliermarsh charliermarsh enabled auto-merge (squash) May 28, 2024 23:52
@charliermarsh charliermarsh merged commit e0169d8 into astral-sh:main May 28, 2024
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Related to preview mode features rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants