[flake8-pyi] Implement redundant-none-literal (PYI061) (#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 <Alex.Waygood@Gmail.com>
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
Simon Brugman 2024-11-16 19:22:51 +01:00 committed by GitHub
parent 4a2310b595
commit 78210b198b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 618 additions and 2 deletions

View file

@ -0,0 +1,60 @@
from typing import Literal
def func1(arg1: Literal[None]):
...
def func2(arg1: Literal[None] | int):
...
def func3() -> Literal[None]:
...
def func4(arg1: Literal[int, None, float]):
...
def func5(arg1: Literal[None, None]):
...
def func6(arg1: Literal[
"hello",
None # Comment 1
, "world"
]):
...
def func7(arg1: Literal[
None # Comment 1
]):
...
# OK
def good_func(arg1: Literal[int] | None):
...
# From flake8-pyi
Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None"
Literal[True, None] # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
###
# The following rules here are slightly subtle,
# but make sense when it comes to giving the best suggestions to users of flake8-pyi.
###
# If Y061 and Y062 both apply, but all the duplicate members are None,
# only emit Y061...
Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
# ... but if Y061 and Y062 both apply
# and there are no None members in the Literal[] slice,
# only emit Y062:
Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True"

View file

@ -0,0 +1,37 @@
from typing import Literal
def func1(arg1: Literal[None]): ...
def func2(arg1: Literal[None] | int): ...
def func3() -> Literal[None]: ...
def func4(arg1: Literal[int, None, float]): ...
def func5(arg1: Literal[None, None]): ...
def func6(arg1: Literal[
"hello",
None # Comment 1
, "world"
]): ...
def func7(arg1: Literal[
None # Comment 1
]): ...
# OK
def good_func(arg1: Literal[int] | None): ...
# From flake8-pyi
Literal[None] # PYI061 None inside "Literal[]" expression. Replace with "None"
Literal[True, None] # PYI061 None inside "Literal[]" expression. Replace with "Literal[True] | None"

View file

@ -104,9 +104,14 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
}
// Ex) Literal[...]
if checker.enabled(Rule::DuplicateLiteralMember) {
if checker.any_enabled(&[Rule::DuplicateLiteralMember, Rule::RedundantNoneLiteral]) {
if !checker.semantic.in_nested_literal() {
flake8_pyi::rules::duplicate_literal_member(checker, expr);
if checker.enabled(Rule::DuplicateLiteralMember) {
flake8_pyi::rules::duplicate_literal_member(checker, expr);
}
if checker.enabled(Rule::RedundantNoneLiteral) {
flake8_pyi::rules::redundant_none_literal(checker, expr);
}
}
}

View file

@ -786,6 +786,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Pyi, "058") => (RuleGroup::Stable, rules::flake8_pyi::rules::GeneratorReturnFromIterMethod),
(Flake8Pyi, "057") => (RuleGroup::Stable, rules::flake8_pyi::rules::ByteStringUsage),
(Flake8Pyi, "059") => (RuleGroup::Preview, rules::flake8_pyi::rules::GenericNotLastBaseClass),
(Flake8Pyi, "061") => (RuleGroup::Preview, rules::flake8_pyi::rules::RedundantNoneLiteral),
(Flake8Pyi, "062") => (RuleGroup::Stable, rules::flake8_pyi::rules::DuplicateLiteralMember),
(Flake8Pyi, "063") => (RuleGroup::Preview, rules::flake8_pyi::rules::PrePep570PositionalArgument),
(Flake8Pyi, "064") => (RuleGroup::Preview, rules::flake8_pyi::rules::RedundantFinalLiteral),

View file

@ -124,6 +124,8 @@ mod tests {
#[test_case(Rule::UnusedPrivateTypedDict, Path::new("PYI049.pyi"))]
#[test_case(Rule::WrongTupleLengthVersionComparison, Path::new("PYI005.py"))]
#[test_case(Rule::WrongTupleLengthVersionComparison, Path::new("PYI005.pyi"))]
#[test_case(Rule::RedundantNoneLiteral, Path::new("PYI061.py"))]
#[test_case(Rule::RedundantNoneLiteral, Path::new("PYI061.pyi"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(

View file

@ -27,6 +27,7 @@ pub(crate) use prefix_type_params::*;
pub(crate) use quoted_annotation_in_stub::*;
pub(crate) use redundant_final_literal::*;
pub(crate) use redundant_literal_union::*;
pub(crate) use redundant_none_literal::*;
pub(crate) use redundant_numeric_union::*;
pub(crate) use simple_defaults::*;
pub(crate) use str_or_repr_defined_in_stub::*;
@ -69,6 +70,7 @@ mod prefix_type_params;
mod quoted_annotation_in_stub;
mod redundant_final_literal;
mod redundant_literal_union;
mod redundant_none_literal;
mod redundant_numeric_union;
mod simple_defaults;
mod str_or_repr_defined_in_stub;

View file

@ -0,0 +1,112 @@
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Expr, ExprNoneLiteral};
use ruff_python_semantic::analyze::typing::traverse_literal;
use ruff_text_size::Ranged;
use smallvec::SmallVec;
use crate::checkers::ast::Checker;
/// ## What it does
/// Checks for redundant `Literal[None]` annotations.
///
/// ## Why is this bad?
/// While `Literal[None]` is a valid type annotation, it is semantically equivalent to `None`.
/// Prefer `None` over `Literal[None]` for both consistency and readability.
///
/// ## Example
/// ```python
/// from typing import Literal
///
/// Literal[None]
/// Literal[1, 2, 3, "foo", 5, None]
/// ```
///
/// Use instead:
/// ```python
/// from typing import Literal
///
/// None
/// Literal[1, 2, 3, "foo", 5] | None
/// ```
///
/// ## References
/// - [Typing documentation: Legal parameters for `Literal` at type check time](https://typing.readthedocs.io/en/latest/spec/literal.html#legal-parameters-for-literal-at-type-check-time)
#[violation]
pub struct RedundantNoneLiteral {
other_literal_elements_seen: bool,
}
impl Violation for RedundantNoneLiteral {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
#[derive_message_formats]
fn message(&self) -> String {
if self.other_literal_elements_seen {
"`Literal[None, ...]` can be replaced with `Literal[...] | None`".to_string()
} else {
"`Literal[None]` can be replaced with `None`".to_string()
}
}
fn fix_title(&self) -> Option<String> {
Some(if self.other_literal_elements_seen {
"Replace with `Literal[...] | None`".to_string()
} else {
"Replace with `None`".to_string()
})
}
}
/// RUF037
pub(crate) fn redundant_none_literal<'a>(checker: &mut Checker, literal_expr: &'a Expr) {
if !checker.semantic().seen_typing() {
return;
}
let mut none_exprs: SmallVec<[&ExprNoneLiteral; 1]> = SmallVec::new();
let mut other_literal_elements_seen = false;
let mut find_none = |expr: &'a Expr, _parent: &'a Expr| {
if let Expr::NoneLiteral(none_expr) = expr {
none_exprs.push(none_expr);
} else {
other_literal_elements_seen = true;
}
};
traverse_literal(&mut find_none, checker.semantic(), literal_expr);
if none_exprs.is_empty() {
return;
}
// Provide a [`Fix`] when the complete `Literal` can be replaced. Applying the fix
// can leave an unused import to be fixed by the `unused-import` rule.
let fix = if other_literal_elements_seen {
None
} else {
Some(Fix::applicable_edit(
Edit::range_replacement("None".to_string(), literal_expr.range()),
if checker.comment_ranges().intersects(literal_expr.range()) {
Applicability::Unsafe
} else {
Applicability::Safe
},
))
};
for none_expr in none_exprs {
let mut diagnostic = Diagnostic::new(
RedundantNoneLiteral {
other_literal_elements_seen,
},
none_expr.range(),
);
if let Some(ref fix) = fix {
diagnostic.set_fix(fix.clone());
}
checker.diagnostics.push(diagnostic);
}
}

View file

@ -0,0 +1,243 @@
---
source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs
---
PYI061.py:4:25: PYI061 [*] `Literal[None]` can be replaced with `None`
|
4 | def func1(arg1: Literal[None]):
| ^^^^ PYI061
5 | ...
|
= help: Replace with `None`
Safe fix
1 1 | from typing import Literal
2 2 |
3 3 |
4 |-def func1(arg1: Literal[None]):
4 |+def func1(arg1: None):
5 5 | ...
6 6 |
7 7 |
PYI061.py:8:25: PYI061 [*] `Literal[None]` can be replaced with `None`
|
8 | def func2(arg1: Literal[None] | int):
| ^^^^ PYI061
9 | ...
|
= help: Replace with `None`
Safe fix
5 5 | ...
6 6 |
7 7 |
8 |-def func2(arg1: Literal[None] | int):
8 |+def func2(arg1: None | int):
9 9 | ...
10 10 |
11 11 |
PYI061.py:12:24: PYI061 [*] `Literal[None]` can be replaced with `None`
|
12 | def func3() -> Literal[None]:
| ^^^^ PYI061
13 | ...
|
= help: Replace with `None`
Safe fix
9 9 | ...
10 10 |
11 11 |
12 |-def func3() -> Literal[None]:
12 |+def func3() -> None:
13 13 | ...
14 14 |
15 15 |
PYI061.py:16:30: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
16 | def func4(arg1: Literal[int, None, float]):
| ^^^^ PYI061
17 | ...
|
= help: Replace with `Literal[...] | None`
PYI061.py:20:25: PYI061 [*] `Literal[None]` can be replaced with `None`
|
20 | def func5(arg1: Literal[None, None]):
| ^^^^ PYI061
21 | ...
|
= help: Replace with `None`
Safe fix
17 17 | ...
18 18 |
19 19 |
20 |-def func5(arg1: Literal[None, None]):
20 |+def func5(arg1: None):
21 21 | ...
22 22 |
23 23 |
PYI061.py:20:31: PYI061 [*] `Literal[None]` can be replaced with `None`
|
20 | def func5(arg1: Literal[None, None]):
| ^^^^ PYI061
21 | ...
|
= help: Replace with `None`
Safe fix
17 17 | ...
18 18 |
19 19 |
20 |-def func5(arg1: Literal[None, None]):
20 |+def func5(arg1: None):
21 21 | ...
22 22 |
23 23 |
PYI061.py:26:5: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
24 | def func6(arg1: Literal[
25 | "hello",
26 | None # Comment 1
| ^^^^ PYI061
27 | , "world"
28 | ]):
|
= help: Replace with `Literal[...] | None`
PYI061.py:33:5: PYI061 [*] `Literal[None]` can be replaced with `None`
|
32 | def func7(arg1: Literal[
33 | None # Comment 1
| ^^^^ PYI061
34 | ]):
35 | ...
|
= help: Replace with `None`
Unsafe fix
29 29 | ...
30 30 |
31 31 |
32 |-def func7(arg1: Literal[
33 |- None # Comment 1
34 |- ]):
32 |+def func7(arg1: None):
35 33 | ...
36 34 |
37 35 |
PYI061.py:44:9: PYI061 [*] `Literal[None]` can be replaced with `None`
|
43 | # From flake8-pyi
44 | Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None"
| ^^^^ PYI061
45 | Literal[True, None] # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
|
= help: Replace with `None`
Safe fix
41 41 |
42 42 |
43 43 | # From flake8-pyi
44 |-Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None"
44 |+None # Y061 None inside "Literal[]" expression. Replace with "None"
45 45 | Literal[True, None] # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
46 46 |
47 47 | ###
PYI061.py:45:15: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
43 | # From flake8-pyi
44 | Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None"
45 | Literal[True, None] # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
| ^^^^ PYI061
46 |
47 | ###
|
= help: Replace with `Literal[...] | None`
PYI061.py:54:9: PYI061 [*] `Literal[None]` can be replaced with `None`
|
52 | # If Y061 and Y062 both apply, but all the duplicate members are None,
53 | # only emit Y061...
54 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
| ^^^^ PYI061
55 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
|
= help: Replace with `None`
Safe fix
51 51 |
52 52 | # If Y061 and Y062 both apply, but all the duplicate members are None,
53 53 | # only emit Y061...
54 |-Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
54 |+None # Y061 None inside "Literal[]" expression. Replace with "None"
55 55 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
56 56 |
57 57 | # ... but if Y061 and Y062 both apply
PYI061.py:54:15: PYI061 [*] `Literal[None]` can be replaced with `None`
|
52 | # If Y061 and Y062 both apply, but all the duplicate members are None,
53 | # only emit Y061...
54 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
| ^^^^ PYI061
55 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
|
= help: Replace with `None`
Safe fix
51 51 |
52 52 | # If Y061 and Y062 both apply, but all the duplicate members are None,
53 53 | # only emit Y061...
54 |-Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
54 |+None # Y061 None inside "Literal[]" expression. Replace with "None"
55 55 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
56 56 |
57 57 | # ... but if Y061 and Y062 both apply
PYI061.py:55:12: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
53 | # only emit Y061...
54 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
55 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
| ^^^^ PYI061
56 |
57 | # ... but if Y061 and Y062 both apply
|
= help: Replace with `Literal[...] | None`
PYI061.py:55:25: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
53 | # only emit Y061...
54 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
55 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
| ^^^^ PYI061
56 |
57 | # ... but if Y061 and Y062 both apply
|
= help: Replace with `Literal[...] | None`
PYI061.py:60:9: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
58 | # and there are no None members in the Literal[] slice,
59 | # only emit Y062:
60 | Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True"
| ^^^^ PYI061
|
= help: Replace with `Literal[...] | None`
PYI061.py:60:21: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
58 | # and there are no None members in the Literal[] slice,
59 | # only emit Y062:
60 | Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True"
| ^^^^ PYI061
|
= help: Replace with `Literal[...] | None`

View file

@ -0,0 +1,152 @@
---
source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs
---
PYI061.pyi:4:25: PYI061 [*] `Literal[None]` can be replaced with `None`
|
4 | def func1(arg1: Literal[None]): ...
| ^^^^ PYI061
|
= help: Replace with `None`
Safe fix
1 1 | from typing import Literal
2 2 |
3 3 |
4 |-def func1(arg1: Literal[None]): ...
4 |+def func1(arg1: None): ...
5 5 |
6 6 |
7 7 | def func2(arg1: Literal[None] | int): ...
PYI061.pyi:7:25: PYI061 [*] `Literal[None]` can be replaced with `None`
|
7 | def func2(arg1: Literal[None] | int): ...
| ^^^^ PYI061
|
= help: Replace with `None`
Safe fix
4 4 | def func1(arg1: Literal[None]): ...
5 5 |
6 6 |
7 |-def func2(arg1: Literal[None] | int): ...
7 |+def func2(arg1: None | int): ...
8 8 |
9 9 |
10 10 | def func3() -> Literal[None]: ...
PYI061.pyi:10:24: PYI061 [*] `Literal[None]` can be replaced with `None`
|
10 | def func3() -> Literal[None]: ...
| ^^^^ PYI061
|
= help: Replace with `None`
Safe fix
7 7 | def func2(arg1: Literal[None] | int): ...
8 8 |
9 9 |
10 |-def func3() -> Literal[None]: ...
10 |+def func3() -> None: ...
11 11 |
12 12 |
13 13 | def func4(arg1: Literal[int, None, float]): ...
PYI061.pyi:13:30: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
13 | def func4(arg1: Literal[int, None, float]): ...
| ^^^^ PYI061
|
= help: Replace with `Literal[...] | None`
PYI061.pyi:16:25: PYI061 [*] `Literal[None]` can be replaced with `None`
|
16 | def func5(arg1: Literal[None, None]): ...
| ^^^^ PYI061
|
= help: Replace with `None`
Safe fix
13 13 | def func4(arg1: Literal[int, None, float]): ...
14 14 |
15 15 |
16 |-def func5(arg1: Literal[None, None]): ...
16 |+def func5(arg1: None): ...
17 17 |
18 18 |
19 19 | def func6(arg1: Literal[
PYI061.pyi:16:31: PYI061 [*] `Literal[None]` can be replaced with `None`
|
16 | def func5(arg1: Literal[None, None]): ...
| ^^^^ PYI061
|
= help: Replace with `None`
Safe fix
13 13 | def func4(arg1: Literal[int, None, float]): ...
14 14 |
15 15 |
16 |-def func5(arg1: Literal[None, None]): ...
16 |+def func5(arg1: None): ...
17 17 |
18 18 |
19 19 | def func6(arg1: Literal[
PYI061.pyi:21:5: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
19 | def func6(arg1: Literal[
20 | "hello",
21 | None # Comment 1
| ^^^^ PYI061
22 | , "world"
23 | ]): ...
|
= help: Replace with `Literal[...] | None`
PYI061.pyi:27:5: PYI061 [*] `Literal[None]` can be replaced with `None`
|
26 | def func7(arg1: Literal[
27 | None # Comment 1
| ^^^^ PYI061
28 | ]): ...
|
= help: Replace with `None`
Unsafe fix
23 23 | ]): ...
24 24 |
25 25 |
26 |-def func7(arg1: Literal[
27 |- None # Comment 1
28 |-]): ...
26 |+def func7(arg1: None): ...
29 27 |
30 28 |
31 29 | # OK
PYI061.pyi:36:9: PYI061 [*] `Literal[None]` can be replaced with `None`
|
35 | # From flake8-pyi
36 | Literal[None] # PYI061 None inside "Literal[]" expression. Replace with "None"
| ^^^^ PYI061
37 | Literal[True, None] # PYI061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
|
= help: Replace with `None`
Safe fix
33 33 |
34 34 |
35 35 | # From flake8-pyi
36 |-Literal[None] # PYI061 None inside "Literal[]" expression. Replace with "None"
36 |+None # PYI061 None inside "Literal[]" expression. Replace with "None"
37 37 | Literal[True, None] # PYI061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
PYI061.pyi:37:15: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
35 | # From flake8-pyi
36 | Literal[None] # PYI061 None inside "Literal[]" expression. Replace with "None"
37 | Literal[True, None] # PYI061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
| ^^^^ PYI061
|
= help: Replace with `Literal[...] | None`