-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 redundant-none-literal
(PYI061
)
#14316
[flake8-pyi
] Implement redundant-none-literal
(PYI061
)
#14316
Conversation
|
code | total | + violation | - violation | + fix | - fix |
---|---|---|---|---|---|
PYI061 | 14 | 14 | 0 | 0 | 0 |
Hi @sbrugman Thanks for another PR. It would help me review your PRs and avoid unnecessary work if you could fill out the Test plan. E.g. did you review the ecosystem changes already? If so, then I don't have to do so :) |
Is there some previous issue where we discussed this rule? I'm asking because the rule itself would be a better fit for |
@MichaReiser I've updated the test plan above, will do so for the other issues as well. You're right that these rules fit better in another category, but since there is no upstream rule I've placed them here. I'm ok moving the rules if preferred. |
It looks like this could actually be in flake8-pyi: |
ruff
] Implement redundant-none-literal
(RUF037
)flake8-pyi
] Implement redundant-none-literal
(PYI061
)
9f4cad0
to
581f0da
Compare
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.
LGTM, @AlexWaygood could you sign off the rule :)
if let Some(ref fix) = fix { | ||
diagnostic.set_fix(fix.clone()); | ||
} |
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.
I'm surprised that setting the same fix works 😆
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.
Thanks! A couple of nitpicks, but looks great overall
crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_none_literal.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_none_literal.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Alex Waygood <[email protected]>
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! Looks great, just a couple of nits about the test remaining
crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.pyi
Outdated
Show resolved
Hide resolved
crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.pyi
Outdated
Show resolved
Hide resolved
…sh#14316) ## Summary `Literal[None]` can be simplified into `None` in type annotations. Surprising to see that this is not that rare: - https://github.com/langchain-ai/langchain/blob/master/libs/langchain/langchain/chat_models/base.py#L54 - https://github.com/sqlalchemy/sqlalchemy/blob/main/lib/sqlalchemy/sql/annotation.py#L69 - https://github.com/jax-ml/jax/blob/main/jax/numpy/__init__.pyi#L961 - https://github.com/huggingface/huggingface_hub/blob/main/src/huggingface_hub/inference/_common.py#L179 ## Test Plan `cargo test` Reviewed all ecosystem results, and they are true positives. --------- Co-authored-by: Alex Waygood <[email protected]> Co-authored-by: Charlie Marsh <[email protected]>
Summary
Literal[None]
can be simplified intoNone
in type annotations.Surprising to see that this is not that rare:
Test Plan
cargo test
Reviewed all ecosystem results, and they are true positives.