Implement PYI030: Unnecessary literal union (#5570)

Implements PYI030 as part of
https://github.com/astral-sh/ruff/issues/848

> Union expressions should never have more than one Literal member, as
Literal[1] | Literal[2] is semantically identical to Literal[1, 2].

Note we differ slightly from the flake8-pyi implementation:

- We detect cases where there are parentheses or nested unions
- We detect cases with mixed `Union` and `|` syntax
- We use the same error message for all violations; flake8-pyi has two
different messages
- We retain the user's quoting style when displaying string literals;
flake8-pyi uses single quotes
- We warn on duplicates of the same literal `Literal[1] | Literal[1]`
This commit is contained in:
Zanie 2023-07-07 11:43:10 -05:00 committed by GitHub
parent 60d318ddcf
commit bb7303f867
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 504 additions and 9 deletions

View File

@ -0,0 +1,24 @@
from typing import Literal
# Shouldn't emit for any cases in the non-stub file for compatibility with flake8-pyi.
# Note that this rule could be applied here in the future.
field1: Literal[1] # OK
field2: Literal[1] | Literal[2] # OK
def func1(arg1: Literal[1] | Literal[2]): # OK
print(arg1)
def func2() -> Literal[1] | Literal[2]: # OK
return "my Literal[1]ing"
field3: Literal[1] | Literal[2] | str # OK
field4: str | Literal[1] | Literal[2] # OK
field5: Literal[1] | str | Literal[2] # OK
field6: Literal[1] | bool | Literal[2] | str # OK
field7 = Literal[1] | Literal[2] # OK
field8: Literal[1] | (Literal[2] | str) # OK
field9: Literal[1] | (Literal[2] | str) # OK
field10: (Literal[1] | str) | Literal[2] # OK
field11: dict[Literal[1] | Literal[2], str] # OK

View File

@ -0,0 +1,86 @@
import typing
import typing_extensions
from typing import Literal
# Shouldn't affect non-union field types.
field1: Literal[1] # OK
# Should emit for duplicate field types.
field2: Literal[1] | Literal[2] # Error
# Should emit for union types in arguments.
def func1(arg1: Literal[1] | Literal[2]): # Error
print(arg1)
# Should emit for unions in return types.
def func2() -> Literal[1] | Literal[2]: # Error
return "my Literal[1]ing"
# Should emit in longer unions, even if not directly adjacent.
field3: Literal[1] | Literal[2] | str # Error
field4: str | Literal[1] | Literal[2] # Error
field5: Literal[1] | str | Literal[2] # Error
field6: Literal[1] | bool | Literal[2] | str # Error
# Should emit for non-type unions.
field7 = Literal[1] | Literal[2] # Error
# Should emit for parenthesized unions.
field8: Literal[1] | (Literal[2] | str) # Error
# Should handle user parentheses when fixing.
field9: Literal[1] | (Literal[2] | str) # Error
field10: (Literal[1] | str) | Literal[2] # Error
# Should emit for union in generic parent type.
field11: dict[Literal[1] | Literal[2], str] # Error
# Should emit for unions with more than two cases
field12: Literal[1] | Literal[2] | Literal[3] # Error
field13: Literal[1] | Literal[2] | Literal[3] | Literal[4] # Error
# Should emit for unions with more than two cases, even if not directly adjacent
field14: Literal[1] | Literal[2] | str | Literal[3] # Error
# Should emit for unions with mixed literal internal types
field15: Literal[1] | Literal["foo"] | Literal[True] # Error
# Shouldn't emit for duplicate field types with same value; covered by Y016
field16: Literal[1] | Literal[1] # OK
# Shouldn't emit if in new parent type
field17: Literal[1] | dict[Literal[2], str] # OK
# Shouldn't emit if not in a union parent
field18: dict[Literal[1], Literal[2]] # OK
# Should respect name of literal type used
field19: typing.Literal[1] | typing.Literal[2] # Error
# Should emit in cases with newlines
field20: typing.Union[
Literal[
1 # test
],
Literal[2],
] # Error, newline and comment will not be emitted in message
# Should handle multiple unions with multiple members
field21: Literal[1, 2] | Literal[3, 4] # Error
# Should emit in cases with `typing.Union` instead of `|`
field22: typing.Union[Literal[1], Literal[2]] # Error
# Should emit in cases with `typing_extensions.Literal`
field23: typing_extensions.Literal[1] | typing_extensions.Literal[2] # Error
# Should emit in cases with nested `typing.Union`
field24: typing.Union[Literal[1], typing.Union[Literal[2], str]] # Error
# Should emit in cases with mixed `typing.Union` and `|`
field25: typing.Union[Literal[1], Literal[2] | str] # Error
# Should emit only once in cases with multiple nested `typing.Union`
field24: typing.Union[Literal[1], typing.Union[Literal[2], typing.Union[Literal[3], Literal[4]]]] # Error

View File

@ -2189,6 +2189,22 @@ where
}
}
// Ex) Union[...]
if self.enabled(Rule::UnnecessaryLiteralUnion) {
let mut check = true;
// Avoid duplicate checks if the parent is an `Union[...]`
if let Some(Expr::Subscript(ast::ExprSubscript { value, .. })) =
self.semantic.expr_grandparent()
{
check = !self.semantic.match_typing_expr(value, "Union");
}
if check {
flake8_pyi::rules::unnecessary_literal_union(self, expr);
}
}
if self.semantic.match_typing_expr(value, "Literal") {
self.semantic.flags |= SemanticModelFlags::LITERAL;
}
@ -3136,18 +3152,24 @@ where
if self.is_stub {
if self.enabled(Rule::DuplicateUnionMember)
&& self.semantic.in_type_definition()
&& self.semantic.expr_parent().map_or(true, |parent| {
!matches!(
parent,
Expr::BinOp(ast::ExprBinOp {
op: Operator::BitOr,
..
})
)
})
// Avoid duplicate checks if the parent is an `|`
&& !matches!(
self.semantic.expr_parent(),
Some(Expr::BinOp(ast::ExprBinOp { op: Operator::BitOr, ..}))
)
{
flake8_pyi::rules::duplicate_union_member(self, expr);
}
if self.enabled(Rule::UnnecessaryLiteralUnion)
// Avoid duplicate checks if the parent is an `|`
&& !matches!(
self.semantic.expr_parent(),
Some(Expr::BinOp(ast::ExprBinOp { op: Operator::BitOr, ..}))
)
{
flake8_pyi::rules::unnecessary_literal_union(self, expr);
}
}
}
Expr::UnaryOp(ast::ExprUnaryOp {

View File

@ -630,6 +630,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Pyi, "024") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::CollectionsNamedTuple),
(Flake8Pyi, "025") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnaliasedCollectionsAbcSetImport),
(Flake8Pyi, "029") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::StrOrReprDefinedInStub),
(Flake8Pyi, "030") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnnecessaryLiteralUnion),
(Flake8Pyi, "032") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::AnyEqNeAnnotation),
(Flake8Pyi, "033") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::TypeCommentInStub),
(Flake8Pyi, "034") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::NonSelfReturnType),

View File

@ -52,6 +52,8 @@ mod tests {
#[test_case(Rule::UnassignedSpecialVariableInStub, Path::new("PYI035.pyi"))]
#[test_case(Rule::StrOrReprDefinedInStub, Path::new("PYI029.py"))]
#[test_case(Rule::StrOrReprDefinedInStub, Path::new("PYI029.pyi"))]
#[test_case(Rule::UnnecessaryLiteralUnion, Path::new("PYI030.py"))]
#[test_case(Rule::UnnecessaryLiteralUnion, Path::new("PYI030.pyi"))]
#[test_case(Rule::StubBodyMultipleStatements, Path::new("PYI048.py"))]
#[test_case(Rule::StubBodyMultipleStatements, Path::new("PYI048.pyi"))]
#[test_case(Rule::TSuffixedTypeAlias, Path::new("PYI043.py"))]

View File

@ -22,6 +22,7 @@ pub(crate) use stub_body_multiple_statements::*;
pub(crate) use type_alias_naming::*;
pub(crate) use type_comment_in_stub::*;
pub(crate) use unaliased_collections_abc_set_import::*;
pub(crate) use unnecessary_literal_union::*;
pub(crate) use unrecognized_platform::*;
pub(crate) use unrecognized_version_info::*;
@ -49,5 +50,6 @@ mod stub_body_multiple_statements;
mod type_alias_naming;
mod type_comment_in_stub;
mod unaliased_collections_abc_set_import;
mod unnecessary_literal_union;
mod unrecognized_platform;
mod unrecognized_version_info;

View File

@ -0,0 +1,120 @@
use ruff_python_semantic::SemanticModel;
use rustpython_parser::ast::{self, Expr, Operator, Ranged};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use smallvec::SmallVec;
use crate::checkers::ast::Checker;
/// ## What it does
/// Checks for the presence of multiple literal types in a union.
///
/// ## Why is this bad?
/// Literal types accept multiple arguments and it is clearer to specify them
/// as a single literal.
///
/// ## Example
/// ```python
/// from typing import Literal
///
/// field: Literal[1] | Literal[2]
/// ```
///
/// Use instead:
/// ```python
/// from typing import Literal
///
/// field: Literal[1, 2]
/// ```
#[violation]
pub struct UnnecessaryLiteralUnion {
members: Vec<String>,
}
impl Violation for UnnecessaryLiteralUnion {
#[derive_message_formats]
fn message(&self) -> String {
format!(
"Multiple literal members in a union. Use a single literal, e.g. `Literal[{}]`",
self.members.join(", ")
)
}
}
/// PYI030
pub(crate) fn unnecessary_literal_union<'a>(checker: &mut Checker, expr: &'a Expr) {
let mut literal_exprs = SmallVec::<[&Box<Expr>; 1]>::new();
// Adds a member to `literal_exprs` if it is a `Literal` annotation
let mut collect_literal_expr = |expr: &'a Expr| {
if let Expr::Subscript(ast::ExprSubscript { value, slice, .. }) = expr {
if checker.semantic().match_typing_expr(value, "Literal") {
literal_exprs.push(slice);
}
}
};
// Traverse the union, collect all literal members
traverse_union(&mut collect_literal_expr, expr, checker.semantic());
// Raise a violation if more than one
if literal_exprs.len() > 1 {
let diagnostic = Diagnostic::new(
UnnecessaryLiteralUnion {
members: literal_exprs
.into_iter()
.map(|literal_expr| checker.locator.slice(literal_expr.range()).to_string())
.collect(),
},
expr.range(),
);
checker.diagnostics.push(diagnostic);
}
}
/// Traverse a "union" type annotation, calling `func` on each expression in the union.
fn traverse_union<'a, F>(func: &mut F, expr: &'a Expr, semantic: &SemanticModel)
where
F: FnMut(&'a Expr),
{
// Ex) x | y
if let Expr::BinOp(ast::ExprBinOp {
op: Operator::BitOr,
left,
right,
range: _,
}) = expr
{
// The union data structure usually looks like this:
// a | b | c -> (a | b) | c
//
// However, parenthesized expressions can coerce it into any structure:
// a | (b | c)
//
// So we have to traverse both branches in order (left, then right), to report members
// in the order they appear in the source code.
// Traverse the left then right arms
traverse_union(func, left, semantic);
traverse_union(func, right, semantic);
return;
}
// Ex) Union[x, y]
if let Expr::Subscript(ast::ExprSubscript { value, slice, .. }) = expr {
if semantic.match_typing_expr(value, "Union") {
if let Expr::Tuple(ast::ExprTuple { elts, .. }) = slice.as_ref() {
// Traverse each element of the tuple within the union recursively to handle cases
// such as `Union[..., Union[...]]
elts.iter()
.for_each(|elt| traverse_union(func, elt, semantic));
return;
}
}
}
// Otherwise, call the function on expression
func(expr);
}

View File

@ -0,0 +1,4 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---

View File

@ -0,0 +1,233 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---
PYI030.pyi:9:9: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
8 | # Should emit for duplicate field types.
9 | field2: Literal[1] | Literal[2] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^ PYI030
10 |
11 | # Should emit for union types in arguments.
|
PYI030.pyi:12:17: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
11 | # Should emit for union types in arguments.
12 | def func1(arg1: Literal[1] | Literal[2]): # Error
| ^^^^^^^^^^^^^^^^^^^^^^^ PYI030
13 | print(arg1)
|
PYI030.pyi:17:16: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
16 | # Should emit for unions in return types.
17 | def func2() -> Literal[1] | Literal[2]: # Error
| ^^^^^^^^^^^^^^^^^^^^^^^ PYI030
18 | return "my Literal[1]ing"
|
PYI030.pyi:22:9: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
21 | # Should emit in longer unions, even if not directly adjacent.
22 | field3: Literal[1] | Literal[2] | str # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
23 | field4: str | Literal[1] | Literal[2] # Error
24 | field5: Literal[1] | str | Literal[2] # Error
|
PYI030.pyi:23:9: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
21 | # Should emit in longer unions, even if not directly adjacent.
22 | field3: Literal[1] | Literal[2] | str # Error
23 | field4: str | Literal[1] | Literal[2] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
24 | field5: Literal[1] | str | Literal[2] # Error
25 | field6: Literal[1] | bool | Literal[2] | str # Error
|
PYI030.pyi:24:9: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
22 | field3: Literal[1] | Literal[2] | str # Error
23 | field4: str | Literal[1] | Literal[2] # Error
24 | field5: Literal[1] | str | Literal[2] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
25 | field6: Literal[1] | bool | Literal[2] | str # Error
|
PYI030.pyi:25:9: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
23 | field4: str | Literal[1] | Literal[2] # Error
24 | field5: Literal[1] | str | Literal[2] # Error
25 | field6: Literal[1] | bool | Literal[2] | str # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
26 |
27 | # Should emit for non-type unions.
|
PYI030.pyi:28:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
27 | # Should emit for non-type unions.
28 | field7 = Literal[1] | Literal[2] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^ PYI030
29 |
30 | # Should emit for parenthesized unions.
|
PYI030.pyi:31:9: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
30 | # Should emit for parenthesized unions.
31 | field8: Literal[1] | (Literal[2] | str) # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
32 |
33 | # Should handle user parentheses when fixing.
|
PYI030.pyi:34:9: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
33 | # Should handle user parentheses when fixing.
34 | field9: Literal[1] | (Literal[2] | str) # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
35 | field10: (Literal[1] | str) | Literal[2] # Error
|
PYI030.pyi:35:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
33 | # Should handle user parentheses when fixing.
34 | field9: Literal[1] | (Literal[2] | str) # Error
35 | field10: (Literal[1] | str) | Literal[2] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
36 |
37 | # Should emit for union in generic parent type.
|
PYI030.pyi:38:15: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
37 | # Should emit for union in generic parent type.
38 | field11: dict[Literal[1] | Literal[2], str] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^ PYI030
39 |
40 | # Should emit for unions with more than two cases
|
PYI030.pyi:41:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2, 3]`
|
40 | # Should emit for unions with more than two cases
41 | field12: Literal[1] | Literal[2] | Literal[3] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
42 | field13: Literal[1] | Literal[2] | Literal[3] | Literal[4] # Error
|
PYI030.pyi:42:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2, 3, 4]`
|
40 | # Should emit for unions with more than two cases
41 | field12: Literal[1] | Literal[2] | Literal[3] # Error
42 | field13: Literal[1] | Literal[2] | Literal[3] | Literal[4] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
43 |
44 | # Should emit for unions with more than two cases, even if not directly adjacent
|
PYI030.pyi:45:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2, 3]`
|
44 | # Should emit for unions with more than two cases, even if not directly adjacent
45 | field14: Literal[1] | Literal[2] | str | Literal[3] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
46 |
47 | # Should emit for unions with mixed literal internal types
|
PYI030.pyi:48:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, "foo", True]`
|
47 | # Should emit for unions with mixed literal internal types
48 | field15: Literal[1] | Literal["foo"] | Literal[True] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
49 |
50 | # Shouldn't emit for duplicate field types with same value; covered by Y016
|
PYI030.pyi:51:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 1]`
|
50 | # Shouldn't emit for duplicate field types with same value; covered by Y016
51 | field16: Literal[1] | Literal[1] # OK
| ^^^^^^^^^^^^^^^^^^^^^^^ PYI030
52 |
53 | # Shouldn't emit if in new parent type
|
PYI030.pyi:60:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
59 | # Should respect name of literal type used
60 | field19: typing.Literal[1] | typing.Literal[2] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
61 |
62 | # Should emit in cases with newlines
|
PYI030.pyi:63:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
62 | # Should emit in cases with newlines
63 | field20: typing.Union[
| __________^
64 | | Literal[
65 | | 1 # test
66 | | ],
67 | | Literal[2],
68 | | ] # Error, newline and comment will not be emitted in message
| |_^ PYI030
69 |
70 | # Should handle multiple unions with multiple members
|
PYI030.pyi:71:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2, 3, 4]`
|
70 | # Should handle multiple unions with multiple members
71 | field21: Literal[1, 2] | Literal[3, 4] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
72 |
73 | # Should emit in cases with `typing.Union` instead of `|`
|
PYI030.pyi:74:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
73 | # Should emit in cases with `typing.Union` instead of `|`
74 | field22: typing.Union[Literal[1], Literal[2]] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
75 |
76 | # Should emit in cases with `typing_extensions.Literal`
|
PYI030.pyi:77:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
76 | # Should emit in cases with `typing_extensions.Literal`
77 | field23: typing_extensions.Literal[1] | typing_extensions.Literal[2] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
78 |
79 | # Should emit in cases with nested `typing.Union`
|
PYI030.pyi:80:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
79 | # Should emit in cases with nested `typing.Union`
80 | field24: typing.Union[Literal[1], typing.Union[Literal[2], str]] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
81 |
82 | # Should emit in cases with mixed `typing.Union` and `|`
|
PYI030.pyi:83:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
|
82 | # Should emit in cases with mixed `typing.Union` and `|`
83 | field25: typing.Union[Literal[1], Literal[2] | str] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
84 |
85 | # Should emit only once in cases with multiple nested `typing.Union`
|
PYI030.pyi:86:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2, 3, 4]`
|
85 | # Should emit only once in cases with multiple nested `typing.Union`
86 | field24: typing.Union[Literal[1], typing.Union[Literal[2], typing.Union[Literal[3], Literal[4]]]] # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI030
|

1
ruff.schema.json generated
View File

@ -2346,6 +2346,7 @@
"PYI025",
"PYI029",
"PYI03",
"PYI030",
"PYI032",
"PYI033",
"PYI034",