-
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 PYI051
#6215
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
a882131
save progress PYI051
LaBatata101 0c46a8b
Merge branch 'main' into PYI051
LaBatata101 28a595d
Fix duplicate union check
LaBatata101 8a8abf5
Implement PYI051
LaBatata101 1ab86ba
Merge branch 'main' into PYI051
LaBatata101 0b98920
Tweak logic
charliermarsh 8d3ab1c
Tweaks
charliermarsh 1419e53
Merge branch 'main' into PYI051
charliermarsh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import typing | ||
from typing import Literal, TypeAlias, Union | ||
|
||
A: str | Literal["foo"] | ||
B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] | ||
C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] | ||
D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] | ||
|
||
def func(x: complex | Literal[1J], y: Union[Literal[3.14], float]): ... | ||
|
||
# OK | ||
A: Literal["foo"] | ||
B: TypeAlias = Literal[b"bar", b"foo"] | ||
C: TypeAlias = typing.Union[Literal[5], Literal["foo"]] | ||
D: TypeAlias = Literal[b"str_bytes", 42] | ||
|
||
def func(x: Literal[1J], y: Literal[3.14]): ... |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import typing | ||
from typing import Literal, TypeAlias, Union | ||
|
||
A: str | Literal["foo"] | ||
B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] | ||
C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] | ||
D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] | ||
|
||
def func(x: complex | Literal[1J], y: Union[Literal[3.14], float]): ... | ||
|
||
# OK | ||
A: Literal["foo"] | ||
B: TypeAlias = Literal[b"bar", b"foo"] | ||
C: TypeAlias = typing.Union[Literal[5], Literal["foo"]] | ||
D: TypeAlias = Literal[b"str_bytes", 42] | ||
|
||
def func(x: Literal[1J], y: Literal[3.14]): ... |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
155 changes: 155 additions & 0 deletions
155
crates/ruff/src/rules/flake8_pyi/rules/redundant_literal_union.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,155 @@ | ||
use rustc_hash::FxHashSet; | ||
use std::fmt; | ||
|
||
use ast::{Constant, Ranged}; | ||
use ruff_diagnostics::{Diagnostic, Violation}; | ||
use ruff_macros::{derive_message_formats, violation}; | ||
use ruff_python_ast::{self as ast, Expr}; | ||
use ruff_python_semantic::SemanticModel; | ||
|
||
use crate::{checkers::ast::Checker, rules::flake8_pyi::helpers::traverse_union}; | ||
|
||
/// ## What it does | ||
/// Checks for the presence of redundant `Literal` types and builtin super | ||
/// types in an union. | ||
/// | ||
/// ## Why is this bad? | ||
/// The use of `Literal` types in a union with the builtin super type of one of | ||
/// its literal members is redundant, as the super type is strictly more | ||
/// general than the `Literal` type. | ||
/// | ||
/// For example, `Literal["A"] | str` is equivalent to `str`, and | ||
/// `Literal[1] | int` is equivalent to `int`, as `str` and `int` are the super | ||
/// types of `"A"` and `1` respectively. | ||
/// | ||
/// ## Example | ||
/// ```python | ||
/// from typing import Literal | ||
/// | ||
/// A: Literal["A"] | str | ||
/// ``` | ||
/// | ||
/// Use instead: | ||
/// ```python | ||
/// from typing import Literal | ||
/// | ||
/// A: Literal["A"] | ||
/// ``` | ||
#[violation] | ||
pub struct RedundantLiteralUnion { | ||
literal: String, | ||
builtin_type: ExprType, | ||
} | ||
|
||
impl Violation for RedundantLiteralUnion { | ||
#[derive_message_formats] | ||
fn message(&self) -> String { | ||
let RedundantLiteralUnion { | ||
literal, | ||
builtin_type, | ||
} = self; | ||
format!("`Literal[{literal}]` is redundant in a union with `{builtin_type}`",) | ||
} | ||
} | ||
|
||
/// PYI051 | ||
pub(crate) fn redundant_literal_union<'a>(checker: &mut Checker, union: &'a Expr) { | ||
let mut literal_exprs = Vec::new(); | ||
let mut builtin_types_in_union = FxHashSet::default(); | ||
|
||
// Adds a member to `literal_exprs` for each value in a `Literal`, and any builtin types | ||
// to `builtin_types_in_union`. | ||
let mut func = |expr: &'a Expr, _| { | ||
if let Expr::Subscript(ast::ExprSubscript { value, slice, .. }) = expr { | ||
if checker.semantic().match_typing_expr(value, "Literal") { | ||
if let Expr::Tuple(ast::ExprTuple { elts, .. }) = slice.as_ref() { | ||
literal_exprs.extend(elts.iter()); | ||
} else { | ||
literal_exprs.push(slice); | ||
} | ||
} | ||
return; | ||
} | ||
|
||
let Some(builtin_type) = match_builtin_type(expr, checker.semantic()) else { | ||
return; | ||
}; | ||
builtin_types_in_union.insert(builtin_type); | ||
}; | ||
|
||
traverse_union(&mut func, checker.semantic(), union, None); | ||
|
||
for literal_expr in literal_exprs { | ||
let Some(constant_type) = match_constant_type(literal_expr) else { | ||
continue; | ||
}; | ||
|
||
if builtin_types_in_union.contains(&constant_type) { | ||
checker.diagnostics.push(Diagnostic::new( | ||
RedundantLiteralUnion { | ||
literal: checker.locator().slice(literal_expr.range()).to_string(), | ||
builtin_type: constant_type, | ||
}, | ||
literal_expr.range(), | ||
)); | ||
} | ||
} | ||
} | ||
|
||
#[derive(Debug, PartialEq, Eq, Hash, Copy, Clone)] | ||
enum ExprType { | ||
Int, | ||
Str, | ||
Bool, | ||
Float, | ||
Bytes, | ||
Complex, | ||
} | ||
|
||
impl fmt::Display for ExprType { | ||
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { | ||
match self { | ||
Self::Int => fmt.write_str("int"), | ||
Self::Str => fmt.write_str("str"), | ||
Self::Bool => fmt.write_str("bool"), | ||
Self::Float => fmt.write_str("float"), | ||
Self::Bytes => fmt.write_str("bytes"), | ||
Self::Complex => fmt.write_str("complex"), | ||
} | ||
} | ||
} | ||
|
||
/// Return the [`ExprType`] of an [`Expr]` if it is a builtin type (e.g. `int`, `bool`, `float`, | ||
/// `str`, `bytes`, or `complex`). | ||
fn match_builtin_type(expr: &Expr, model: &SemanticModel) -> Option<ExprType> { | ||
let name = expr.as_name_expr()?; | ||
let result = match name.id.as_str() { | ||
"int" => ExprType::Int, | ||
"bool" => ExprType::Bool, | ||
"str" => ExprType::Str, | ||
"float" => ExprType::Float, | ||
"bytes" => ExprType::Bytes, | ||
"complex" => ExprType::Complex, | ||
_ => return None, | ||
}; | ||
if !model.is_builtin(name.id.as_str()) { | ||
return None; | ||
} | ||
Some(result) | ||
} | ||
|
||
/// Return the [`ExprType`] of an [`Expr]` if it is a constant (e.g., an `int`, like `1`, or a | ||
/// `bool`, like `True`). | ||
fn match_constant_type(expr: &Expr) -> Option<ExprType> { | ||
let constant = expr.as_constant_expr()?; | ||
let result = match constant.value { | ||
Constant::Bool(_) => ExprType::Bool, | ||
Constant::Str(_) => ExprType::Str, | ||
Constant::Bytes(_) => ExprType::Bytes, | ||
Constant::Int(_) => ExprType::Int, | ||
Constant::Float(_) => ExprType::Float, | ||
Constant::Complex { .. } => ExprType::Complex, | ||
_ => return None, | ||
}; | ||
Some(result) | ||
} |
4 changes: 4 additions & 0 deletions
4
...ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI051_PYI051.py.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
--- | ||
source: crates/ruff/src/rules/flake8_pyi/mod.rs | ||
--- | ||
|
90 changes: 90 additions & 0 deletions
90
...uff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI051_PYI051.pyi.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
--- | ||
source: crates/ruff/src/rules/flake8_pyi/mod.rs | ||
--- | ||
PYI051.pyi:4:18: PYI051 `Literal["foo"]` is redundant in a union with `str` | ||
| | ||
2 | from typing import Literal, TypeAlias, Union | ||
3 | | ||
4 | A: str | Literal["foo"] | ||
| ^^^^^ PYI051 | ||
5 | B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] | ||
6 | C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] | ||
| | ||
|
||
PYI051.pyi:5:37: PYI051 `Literal[b"bar"]` is redundant in a union with `bytes` | ||
| | ||
4 | A: str | Literal["foo"] | ||
5 | B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] | ||
| ^^^^^^ PYI051 | ||
6 | C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] | ||
7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] | ||
| | ||
|
||
PYI051.pyi:5:45: PYI051 `Literal[b"foo"]` is redundant in a union with `bytes` | ||
| | ||
4 | A: str | Literal["foo"] | ||
5 | B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] | ||
| ^^^^^^ PYI051 | ||
6 | C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] | ||
7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] | ||
| | ||
|
||
PYI051.pyi:6:37: PYI051 `Literal[5]` is redundant in a union with `int` | ||
| | ||
4 | A: str | Literal["foo"] | ||
5 | B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] | ||
6 | C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] | ||
| ^ PYI051 | ||
7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] | ||
| | ||
|
||
PYI051.pyi:6:67: PYI051 `Literal["foo"]` is redundant in a union with `str` | ||
| | ||
4 | A: str | Literal["foo"] | ||
5 | B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] | ||
6 | C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] | ||
| ^^^^^ PYI051 | ||
7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] | ||
| | ||
|
||
PYI051.pyi:7:37: PYI051 `Literal[b"str_bytes"]` is redundant in a union with `bytes` | ||
| | ||
5 | B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] | ||
6 | C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] | ||
7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] | ||
| ^^^^^^^^^^^^ PYI051 | ||
8 | | ||
9 | def func(x: complex | Literal[1J], y: Union[Literal[3.14], float]): ... | ||
| | ||
|
||
PYI051.pyi:7:51: PYI051 `Literal[42]` is redundant in a union with `int` | ||
| | ||
5 | B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] | ||
6 | C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] | ||
7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] | ||
| ^^ PYI051 | ||
8 | | ||
9 | def func(x: complex | Literal[1J], y: Union[Literal[3.14], float]): ... | ||
| | ||
|
||
PYI051.pyi:9:31: PYI051 `Literal[1J]` is redundant in a union with `complex` | ||
| | ||
7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] | ||
8 | | ||
9 | def func(x: complex | Literal[1J], y: Union[Literal[3.14], float]): ... | ||
| ^^ PYI051 | ||
10 | | ||
11 | # OK | ||
| | ||
|
||
PYI051.pyi:9:53: PYI051 `Literal[3.14]` is redundant in a union with `float` | ||
| | ||
7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] | ||
8 | | ||
9 | def func(x: complex | Literal[1J], y: Union[Literal[3.14], float]): ... | ||
| ^^^^ PYI051 | ||
10 | | ||
11 | # OK | ||
| | ||
|
||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@LaBatata101 - I think the issue is that we needed to treat these two cases as separate. Otherwise, we were flagging cases like
Literal[1, 2]
, since we'd detect that we haveint
in theUnion
(by way of visiting the1
and2
).