-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[ruff
] Implement redundant-bool-literal
(RUF038
)
#14319
Changes from 2 commits
2c32d33
f432473
705d355
876ebee
a9cc844
4cf2d4e
38de877
0adaded
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
from typing import Literal | ||
|
||
|
||
def func1(arg1: Literal[True, False]): | ||
... | ||
|
||
|
||
def func2(arg1: Literal[True, False, True]): | ||
... | ||
|
||
|
||
def func3() -> Literal[True, False]: | ||
... | ||
|
||
|
||
def func4(arg1: Literal[True, False] | bool): | ||
... | ||
|
||
|
||
def func5(arg1: Literal[False, True]): | ||
... | ||
|
||
|
||
def func6(arg1: Literal[True, False, "hello", "world"]): | ||
... | ||
|
||
# ok | ||
def good_func1(arg1: bool): | ||
... | ||
|
||
|
||
def good_func2(arg1: Literal[True]): | ||
... |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
from typing import Literal | ||
|
||
|
||
def func1(arg1: Literal[True, False]): ... | ||
|
||
def func2(arg1: Literal[True, False, True]): ... | ||
|
||
def func3() -> Literal[True, False]: ... | ||
|
||
def func4(arg1: Literal[True, False] | bool): ... | ||
|
||
def func5(arg1: Literal[False, True]): ... | ||
|
||
def func6(arg1: Literal[True, False, "hello", "world"]): ... | ||
|
||
# ok | ||
def good_func1(arg1: bool): ... | ||
|
||
def good_func2(arg1: Literal[True]): ... | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,124 @@ | ||||||||||||||||||||||
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; | ||||||||||||||||||||||
use ruff_macros::{derive_message_formats, violation}; | ||||||||||||||||||||||
use ruff_python_ast::Expr; | ||||||||||||||||||||||
use ruff_python_semantic::analyze::typing::traverse_literal; | ||||||||||||||||||||||
use ruff_text_size::Ranged; | ||||||||||||||||||||||
|
||||||||||||||||||||||
use bitflags::bitflags; | ||||||||||||||||||||||
|
||||||||||||||||||||||
use crate::checkers::ast::Checker; | ||||||||||||||||||||||
|
||||||||||||||||||||||
/// ## What it does | ||||||||||||||||||||||
/// Checks for `Literal[True, False]` type annotations. | ||||||||||||||||||||||
/// | ||||||||||||||||||||||
/// ## Why is this bad? | ||||||||||||||||||||||
/// The `bool` type has exactly two constant instances: `True` and `False`. Writing | ||||||||||||||||||||||
/// `Literal[True, False]` in the context of type annotations could be replaced by a | ||||||||||||||||||||||
/// bare `bool` annotations. Static type checkers such as [mypy] treat them as | ||||||||||||||||||||||
/// equivalent. | ||||||||||||||||||||||
/// | ||||||||||||||||||||||
/// ## Example | ||||||||||||||||||||||
/// ```python | ||||||||||||||||||||||
/// Literal[True, False] | ||||||||||||||||||||||
/// Literal[True, False, "hello", "world"] | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
/// ``` | ||||||||||||||||||||||
/// | ||||||||||||||||||||||
/// Use instead: | ||||||||||||||||||||||
/// ```python | ||||||||||||||||||||||
/// bool | ||||||||||||||||||||||
/// Literal["hello", "world"] | bool | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
/// ``` | ||||||||||||||||||||||
/// | ||||||||||||||||||||||
/// ## Fix safety | ||||||||||||||||||||||
/// The fix for this rule is marked as unsafe: | ||||||||||||||||||||||
/// | ||||||||||||||||||||||
/// - The type annotation might be intentional when the type checker used does not | ||||||||||||||||||||||
/// treat `bool` as equivalent when overloading boolean arguments with `Literal[True]` | ||||||||||||||||||||||
/// and `Literal[False]`, e.g. see [#14764] and [#5421]. | ||||||||||||||||||||||
/// - `bool` is not strictly equivalent to `Literal[True, False]`, as `bool` is | ||||||||||||||||||||||
/// a subclass of `int`, and this rule might not apply if the type annotations are used | ||||||||||||||||||||||
/// in a numerical context. | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (arguably it's a type-checker bug if they don't understand that |
||||||||||||||||||||||
/// - The `Literal` might contain comments. | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
/// | ||||||||||||||||||||||
/// ## 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) | ||||||||||||||||||||||
/// - [Python documentation: Boolean type - `bool`](https://docs.python.org/3/library/stdtypes.html#boolean-type-bool) | ||||||||||||||||||||||
/// | ||||||||||||||||||||||
/// [mypy](https://github.com/python/mypy/blob/master/mypy/typeops.py#L985) | ||||||||||||||||||||||
/// [#14764](https://github.com/python/mypy/issues/14764) | ||||||||||||||||||||||
/// [#5421](https://github.com/microsoft/pyright/issues/5421) | ||||||||||||||||||||||
#[violation] | ||||||||||||||||||||||
pub struct RedundantBoolLiteral { | ||||||||||||||||||||||
seen_others: bool, | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
impl Violation for RedundantBoolLiteral { | ||||||||||||||||||||||
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; | ||||||||||||||||||||||
|
||||||||||||||||||||||
#[derive_message_formats] | ||||||||||||||||||||||
fn message(&self) -> String { | ||||||||||||||||||||||
if self.seen_others { | ||||||||||||||||||||||
"`Literal[True, False, ...]` can be replaced with `Literal[...] | bool`".to_string() | ||||||||||||||||||||||
} else { | ||||||||||||||||||||||
"`Literal[True, False]` can be replaced with a bare `bool`".to_string() | ||||||||||||||||||||||
} | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
fn fix_title(&self) -> Option<String> { | ||||||||||||||||||||||
Some("Replace with `bool`".to_string()) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
/// RUF038 | ||||||||||||||||||||||
pub(crate) fn redundant_bool_literal<'a>(checker: &mut Checker, literal_expr: &'a Expr) { | ||||||||||||||||||||||
if !checker.semantic().seen_typing() { | ||||||||||||||||||||||
return; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
let mut seen_expr = BooleanLiteral::empty(); | ||||||||||||||||||||||
|
||||||||||||||||||||||
let mut find_bools = |expr: &'a Expr, _parent: &'a Expr| { | ||||||||||||||||||||||
let expr_type = match expr { | ||||||||||||||||||||||
Expr::BooleanLiteral(boolean_expr) => { | ||||||||||||||||||||||
if boolean_expr.value { | ||||||||||||||||||||||
BooleanLiteral::TRUE | ||||||||||||||||||||||
} else { | ||||||||||||||||||||||
BooleanLiteral::FALSE | ||||||||||||||||||||||
} | ||||||||||||||||||||||
} | ||||||||||||||||||||||
_ => BooleanLiteral::OTHER, | ||||||||||||||||||||||
}; | ||||||||||||||||||||||
seen_expr.insert(expr_type); | ||||||||||||||||||||||
}; | ||||||||||||||||||||||
|
||||||||||||||||||||||
traverse_literal(&mut find_bools, checker.semantic(), literal_expr); | ||||||||||||||||||||||
|
||||||||||||||||||||||
if !seen_expr.contains(BooleanLiteral::TRUE | BooleanLiteral::FALSE) { | ||||||||||||||||||||||
return; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
let seen_others = seen_expr.contains(BooleanLiteral::OTHER); | ||||||||||||||||||||||
|
||||||||||||||||||||||
let mut diagnostic = | ||||||||||||||||||||||
Diagnostic::new(RedundantBoolLiteral { seen_others }, literal_expr.range()); | ||||||||||||||||||||||
|
||||||||||||||||||||||
// 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. | ||||||||||||||||||||||
if !seen_others { | ||||||||||||||||||||||
diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( | ||||||||||||||||||||||
"bool".to_string(), | ||||||||||||||||||||||
literal_expr.range(), | ||||||||||||||||||||||
))); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
checker.diagnostics.push(diagnostic); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
bitflags! { | ||||||||||||||||||||||
#[derive(Default, Debug)] | ||||||||||||||||||||||
struct BooleanLiteral: u8 { | ||||||||||||||||||||||
const TRUE = 1 << 0; | ||||||||||||||||||||||
const FALSE = 1 << 1; | ||||||||||||||||||||||
const OTHER = 1 << 2; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
--- | ||
source: crates/ruff_linter/src/rules/ruff/mod.rs | ||
--- | ||
RUF038.py:4:17: RUF038 [*] `Literal[True, False]` can be replaced with a bare `bool` | ||
| | ||
4 | def func1(arg1: Literal[True, False]): | ||
| ^^^^^^^^^^^^^^^^^^^^ RUF038 | ||
5 | ... | ||
| | ||
= help: Replace with `bool` | ||
|
||
ℹ Unsafe fix | ||
1 1 | from typing import Literal | ||
2 2 | | ||
3 3 | | ||
4 |-def func1(arg1: Literal[True, False]): | ||
4 |+def func1(arg1: bool): | ||
5 5 | ... | ||
6 6 | | ||
7 7 | | ||
|
||
RUF038.py:8:17: RUF038 [*] `Literal[True, False]` can be replaced with a bare `bool` | ||
| | ||
8 | def func2(arg1: Literal[True, False, True]): | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF038 | ||
9 | ... | ||
| | ||
= help: Replace with `bool` | ||
|
||
ℹ Unsafe fix | ||
5 5 | ... | ||
6 6 | | ||
7 7 | | ||
8 |-def func2(arg1: Literal[True, False, True]): | ||
8 |+def func2(arg1: bool): | ||
9 9 | ... | ||
10 10 | | ||
11 11 | | ||
|
||
RUF038.py:12:16: RUF038 [*] `Literal[True, False]` can be replaced with a bare `bool` | ||
| | ||
12 | def func3() -> Literal[True, False]: | ||
| ^^^^^^^^^^^^^^^^^^^^ RUF038 | ||
13 | ... | ||
| | ||
= help: Replace with `bool` | ||
|
||
ℹ Unsafe fix | ||
9 9 | ... | ||
10 10 | | ||
11 11 | | ||
12 |-def func3() -> Literal[True, False]: | ||
12 |+def func3() -> bool: | ||
13 13 | ... | ||
14 14 | | ||
15 15 | | ||
|
||
RUF038.py:16:17: RUF038 [*] `Literal[True, False]` can be replaced with a bare `bool` | ||
| | ||
16 | def func4(arg1: Literal[True, False] | bool): | ||
| ^^^^^^^^^^^^^^^^^^^^ RUF038 | ||
17 | ... | ||
| | ||
= help: Replace with `bool` | ||
|
||
ℹ Unsafe fix | ||
13 13 | ... | ||
14 14 | | ||
15 15 | | ||
16 |-def func4(arg1: Literal[True, False] | bool): | ||
16 |+def func4(arg1: bool | bool): | ||
17 17 | ... | ||
18 18 | | ||
19 19 | | ||
|
||
RUF038.py:20:17: RUF038 [*] `Literal[True, False]` can be replaced with a bare `bool` | ||
| | ||
20 | def func5(arg1: Literal[False, True]): | ||
| ^^^^^^^^^^^^^^^^^^^^ RUF038 | ||
21 | ... | ||
| | ||
= help: Replace with `bool` | ||
|
||
ℹ Unsafe fix | ||
17 17 | ... | ||
18 18 | | ||
19 19 | | ||
20 |-def func5(arg1: Literal[False, True]): | ||
20 |+def func5(arg1: bool): | ||
21 21 | ... | ||
22 22 | | ||
23 23 | | ||
|
||
RUF038.py:24:17: RUF038 `Literal[True, False, ...]` can be replaced with `Literal[...] | bool` | ||
| | ||
24 | def func6(arg1: Literal[True, False, "hello", "world"]): | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF038 | ||
25 | ... | ||
| | ||
= help: Replace with `bool` |
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.
Start off with a short sentence that clearly states that this is a rule concerned with readability and style rather than correctness: