diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI062.py b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI062.py new file mode 100644 index 0000000000000..9e3b46c1f80c0 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI062.py @@ -0,0 +1,21 @@ +from typing import Literal +import typing as t +import typing_extensions + +x: Literal[True, False, True, False] # PYI062 twice here + +y: Literal[1, print("hello"), 3, Literal[4, 1]] # PYI062 on the last 1 + +z: Literal[{1, 3, 5}, "foobar", {1,3,5}] # PYI062 on the set literal + +Literal[1, Literal[1]] # once +Literal[1, 2, Literal[1, 2]] # twice +Literal[1, Literal[1], Literal[1]] # twice +Literal[1, Literal[2], Literal[2]] # once +t.Literal[1, t.Literal[2, t.Literal[1]]] # once +typing_extensions.Literal[1, 1, 1] # twice + +# Ensure issue is only raised once, even on nested literals +MyType = Literal["foo", Literal[True, False, True], "bar"] # PYI062 + +n: Literal["No", "duplicates", "here", 1, "1"] diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI062.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI062.pyi new file mode 100644 index 0000000000000..38d04bcd9df68 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI062.pyi @@ -0,0 +1,21 @@ +from typing import Literal +import typing as t +import typing_extensions + +x: Literal[True, False, True, False] # PY062 twice here + +y: Literal[1, print("hello"), 3, Literal[4, 1]] # PY062 on the last 1 + +z: Literal[{1, 3, 5}, "foobar", {1,3,5}] # PY062 on the set literal + +Literal[1, Literal[1]] # once +Literal[1, 2, Literal[1, 2]] # twice +Literal[1, Literal[1], Literal[1]] # twice +Literal[1, Literal[2], Literal[2]] # once +t.Literal[1, t.Literal[2, t.Literal[1]]] # once +typing_extensions.Literal[1, 1, 1] # twice + +# Ensure issue is only raised once, even on nested literals +MyType = Literal["foo", Literal[True, False, True], "bar"] # PYI062 + +n: Literal["No", "duplicates", "here", 1, "1"] diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index c1c912188afc9..b526dd37777da 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -97,6 +97,13 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { } } + // Ex) Literal[...] + if checker.enabled(Rule::DuplicateLiteralMember) { + if !checker.semantic.in_nested_literal() { + flake8_pyi::rules::duplicate_literal_member(checker, expr); + } + } + if checker.enabled(Rule::NeverUnion) { ruff::rules::never_union(checker, expr); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index d6f675f19db1f..05906db3c906b 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -809,6 +809,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Pyi, "056") => (RuleGroup::Stable, rules::flake8_pyi::rules::UnsupportedMethodCallOnAll), (Flake8Pyi, "058") => (RuleGroup::Stable, rules::flake8_pyi::rules::GeneratorReturnFromIterMethod), (Flake8Pyi, "059") => (RuleGroup::Preview, rules::flake8_pyi::rules::GenericNotLastBaseClass), + (Flake8Pyi, "062") => (RuleGroup::Preview, rules::flake8_pyi::rules::DuplicateLiteralMember), // flake8-pytest-style (Flake8PytestStyle, "001") => (RuleGroup::Stable, rules::flake8_pytest_style::rules::PytestFixtureIncorrectParenthesesStyle), diff --git a/crates/ruff_linter/src/rules/flake8_pyi/mod.rs b/crates/ruff_linter/src/rules/flake8_pyi/mod.rs index 75a5387db3c08..e69606adfbd5c 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/mod.rs @@ -33,6 +33,8 @@ mod tests { #[test_case(Rule::CustomTypeVarReturnType, Path::new("PYI019.pyi"))] #[test_case(Rule::DocstringInStub, Path::new("PYI021.py"))] #[test_case(Rule::DocstringInStub, Path::new("PYI021.pyi"))] + #[test_case(Rule::DuplicateLiteralMember, Path::new("PYI062.py"))] + #[test_case(Rule::DuplicateLiteralMember, Path::new("PYI062.pyi"))] #[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.py"))] #[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.pyi"))] #[test_case(Rule::EllipsisInNonEmptyClassBody, Path::new("PYI013.py"))] diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_literal_member.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_literal_member.rs new file mode 100644 index 0000000000000..4c3a8001dc6d6 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_literal_member.rs @@ -0,0 +1,67 @@ +use std::collections::HashSet; + +use rustc_hash::FxHashSet; + +use ruff_diagnostics::{Diagnostic, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::comparable::ComparableExpr; +use ruff_python_ast::Expr; +use ruff_python_semantic::analyze::typing::traverse_literal; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for duplicate members in a `typing.Literal[]` slice. +/// +/// ## Why is this bad? +/// Duplicate literal members are redundant and should be removed. +/// +/// ## Example +/// ```python +/// foo: Literal["a", "b", "a"] +/// ``` +/// +/// Use instead: +/// ```python +/// foo: Literal["a", "b"] +/// ``` +/// +/// ## References +/// - [Python documentation: `typing.Literal`](https://docs.python.org/3/library/typing.html#typing.Literal) +#[violation] +pub struct DuplicateLiteralMember { + duplicate_name: String, +} + +impl Violation for DuplicateLiteralMember { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + format!("Duplicate literal member `{}`", self.duplicate_name) + } +} + +/// PYI062 +pub(crate) fn duplicate_literal_member<'a>(checker: &mut Checker, expr: &'a Expr) { + let mut seen_nodes: HashSet, _> = FxHashSet::default(); + let mut diagnostics: Vec = Vec::new(); + + // Adds a member to `literal_exprs` if it is a `Literal` annotation + let mut check_for_duplicate_members = |expr: &'a Expr, _: &'a Expr| { + // If we've already seen this literal member, raise a violation. + if !seen_nodes.insert(expr.into()) { + diagnostics.push(Diagnostic::new( + DuplicateLiteralMember { + duplicate_name: checker.generator().expr(expr), + }, + expr.range(), + )); + } + }; + + // Traverse the literal, collect all diagnostic members + traverse_literal(&mut check_for_duplicate_members, checker.semantic(), expr); + checker.diagnostics.append(&mut diagnostics); +} diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/mod.rs index dbf28eefd7327..18b50b8e78408 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/mod.rs @@ -6,6 +6,7 @@ pub(crate) use complex_assignment_in_stub::*; pub(crate) use complex_if_statement_in_stub::*; pub(crate) use custom_type_var_return_type::*; pub(crate) use docstring_in_stubs::*; +pub(crate) use duplicate_literal_member::*; pub(crate) use duplicate_union_member::*; pub(crate) use ellipsis_in_non_empty_class_body::*; pub(crate) use exit_annotations::*; @@ -45,6 +46,7 @@ mod complex_assignment_in_stub; mod complex_if_statement_in_stub; mod custom_type_var_return_type; mod docstring_in_stubs; +mod duplicate_literal_member; mod duplicate_union_member; mod ellipsis_in_non_empty_class_body; mod exit_annotations; diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI062_PYI062.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI062_PYI062.py.snap new file mode 100644 index 0000000000000..2ed8215957160 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI062_PYI062.py.snap @@ -0,0 +1,138 @@ +--- +source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs +--- +PYI062.py:5:25: PYI062 Duplicate literal member `True` + | +3 | import typing_extensions +4 | +5 | x: Literal[True, False, True, False] # PYI062 twice here + | ^^^^ PYI062 +6 | +7 | y: Literal[1, print("hello"), 3, Literal[4, 1]] # PYI062 on the last 1 + | + +PYI062.py:5:31: PYI062 Duplicate literal member `False` + | +3 | import typing_extensions +4 | +5 | x: Literal[True, False, True, False] # PYI062 twice here + | ^^^^^ PYI062 +6 | +7 | y: Literal[1, print("hello"), 3, Literal[4, 1]] # PYI062 on the last 1 + | + +PYI062.py:7:45: PYI062 Duplicate literal member `1` + | +5 | x: Literal[True, False, True, False] # PYI062 twice here +6 | +7 | y: Literal[1, print("hello"), 3, Literal[4, 1]] # PYI062 on the last 1 + | ^ PYI062 +8 | +9 | z: Literal[{1, 3, 5}, "foobar", {1,3,5}] # PYI062 on the set literal + | + +PYI062.py:9:33: PYI062 Duplicate literal member `{1, 3, 5}` + | + 7 | y: Literal[1, print("hello"), 3, Literal[4, 1]] # PYI062 on the last 1 + 8 | + 9 | z: Literal[{1, 3, 5}, "foobar", {1,3,5}] # PYI062 on the set literal + | ^^^^^^^ PYI062 +10 | +11 | Literal[1, Literal[1]] # once + | + +PYI062.py:11:20: PYI062 Duplicate literal member `1` + | + 9 | z: Literal[{1, 3, 5}, "foobar", {1,3,5}] # PYI062 on the set literal +10 | +11 | Literal[1, Literal[1]] # once + | ^ PYI062 +12 | Literal[1, 2, Literal[1, 2]] # twice +13 | Literal[1, Literal[1], Literal[1]] # twice + | + +PYI062.py:12:23: PYI062 Duplicate literal member `1` + | +11 | Literal[1, Literal[1]] # once +12 | Literal[1, 2, Literal[1, 2]] # twice + | ^ PYI062 +13 | Literal[1, Literal[1], Literal[1]] # twice +14 | Literal[1, Literal[2], Literal[2]] # once + | + +PYI062.py:12:26: PYI062 Duplicate literal member `2` + | +11 | Literal[1, Literal[1]] # once +12 | Literal[1, 2, Literal[1, 2]] # twice + | ^ PYI062 +13 | Literal[1, Literal[1], Literal[1]] # twice +14 | Literal[1, Literal[2], Literal[2]] # once + | + +PYI062.py:13:20: PYI062 Duplicate literal member `1` + | +11 | Literal[1, Literal[1]] # once +12 | Literal[1, 2, Literal[1, 2]] # twice +13 | Literal[1, Literal[1], Literal[1]] # twice + | ^ PYI062 +14 | Literal[1, Literal[2], Literal[2]] # once +15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once + | + +PYI062.py:13:32: PYI062 Duplicate literal member `1` + | +11 | Literal[1, Literal[1]] # once +12 | Literal[1, 2, Literal[1, 2]] # twice +13 | Literal[1, Literal[1], Literal[1]] # twice + | ^ PYI062 +14 | Literal[1, Literal[2], Literal[2]] # once +15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once + | + +PYI062.py:14:32: PYI062 Duplicate literal member `2` + | +12 | Literal[1, 2, Literal[1, 2]] # twice +13 | Literal[1, Literal[1], Literal[1]] # twice +14 | Literal[1, Literal[2], Literal[2]] # once + | ^ PYI062 +15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once +16 | typing_extensions.Literal[1, 1, 1] # twice + | + +PYI062.py:15:37: PYI062 Duplicate literal member `1` + | +13 | Literal[1, Literal[1], Literal[1]] # twice +14 | Literal[1, Literal[2], Literal[2]] # once +15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once + | ^ PYI062 +16 | typing_extensions.Literal[1, 1, 1] # twice + | + +PYI062.py:16:30: PYI062 Duplicate literal member `1` + | +14 | Literal[1, Literal[2], Literal[2]] # once +15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once +16 | typing_extensions.Literal[1, 1, 1] # twice + | ^ PYI062 +17 | +18 | # Ensure issue is only raised once, even on nested literals + | + +PYI062.py:16:33: PYI062 Duplicate literal member `1` + | +14 | Literal[1, Literal[2], Literal[2]] # once +15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once +16 | typing_extensions.Literal[1, 1, 1] # twice + | ^ PYI062 +17 | +18 | # Ensure issue is only raised once, even on nested literals + | + +PYI062.py:19:46: PYI062 Duplicate literal member `True` + | +18 | # Ensure issue is only raised once, even on nested literals +19 | MyType = Literal["foo", Literal[True, False, True], "bar"] # PYI062 + | ^^^^ PYI062 +20 | +21 | n: Literal["No", "duplicates", "here", 1, "1"] + | diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI062_PYI062.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI062_PYI062.pyi.snap new file mode 100644 index 0000000000000..069024753f74b --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI062_PYI062.pyi.snap @@ -0,0 +1,138 @@ +--- +source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs +--- +PYI062.pyi:5:25: PYI062 Duplicate literal member `True` + | +3 | import typing_extensions +4 | +5 | x: Literal[True, False, True, False] # PY062 twice here + | ^^^^ PYI062 +6 | +7 | y: Literal[1, print("hello"), 3, Literal[4, 1]] # PY062 on the last 1 + | + +PYI062.pyi:5:31: PYI062 Duplicate literal member `False` + | +3 | import typing_extensions +4 | +5 | x: Literal[True, False, True, False] # PY062 twice here + | ^^^^^ PYI062 +6 | +7 | y: Literal[1, print("hello"), 3, Literal[4, 1]] # PY062 on the last 1 + | + +PYI062.pyi:7:45: PYI062 Duplicate literal member `1` + | +5 | x: Literal[True, False, True, False] # PY062 twice here +6 | +7 | y: Literal[1, print("hello"), 3, Literal[4, 1]] # PY062 on the last 1 + | ^ PYI062 +8 | +9 | z: Literal[{1, 3, 5}, "foobar", {1,3,5}] # PY062 on the set literal + | + +PYI062.pyi:9:33: PYI062 Duplicate literal member `{1, 3, 5}` + | + 7 | y: Literal[1, print("hello"), 3, Literal[4, 1]] # PY062 on the last 1 + 8 | + 9 | z: Literal[{1, 3, 5}, "foobar", {1,3,5}] # PY062 on the set literal + | ^^^^^^^ PYI062 +10 | +11 | Literal[1, Literal[1]] # once + | + +PYI062.pyi:11:20: PYI062 Duplicate literal member `1` + | + 9 | z: Literal[{1, 3, 5}, "foobar", {1,3,5}] # PY062 on the set literal +10 | +11 | Literal[1, Literal[1]] # once + | ^ PYI062 +12 | Literal[1, 2, Literal[1, 2]] # twice +13 | Literal[1, Literal[1], Literal[1]] # twice + | + +PYI062.pyi:12:23: PYI062 Duplicate literal member `1` + | +11 | Literal[1, Literal[1]] # once +12 | Literal[1, 2, Literal[1, 2]] # twice + | ^ PYI062 +13 | Literal[1, Literal[1], Literal[1]] # twice +14 | Literal[1, Literal[2], Literal[2]] # once + | + +PYI062.pyi:12:26: PYI062 Duplicate literal member `2` + | +11 | Literal[1, Literal[1]] # once +12 | Literal[1, 2, Literal[1, 2]] # twice + | ^ PYI062 +13 | Literal[1, Literal[1], Literal[1]] # twice +14 | Literal[1, Literal[2], Literal[2]] # once + | + +PYI062.pyi:13:20: PYI062 Duplicate literal member `1` + | +11 | Literal[1, Literal[1]] # once +12 | Literal[1, 2, Literal[1, 2]] # twice +13 | Literal[1, Literal[1], Literal[1]] # twice + | ^ PYI062 +14 | Literal[1, Literal[2], Literal[2]] # once +15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once + | + +PYI062.pyi:13:32: PYI062 Duplicate literal member `1` + | +11 | Literal[1, Literal[1]] # once +12 | Literal[1, 2, Literal[1, 2]] # twice +13 | Literal[1, Literal[1], Literal[1]] # twice + | ^ PYI062 +14 | Literal[1, Literal[2], Literal[2]] # once +15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once + | + +PYI062.pyi:14:32: PYI062 Duplicate literal member `2` + | +12 | Literal[1, 2, Literal[1, 2]] # twice +13 | Literal[1, Literal[1], Literal[1]] # twice +14 | Literal[1, Literal[2], Literal[2]] # once + | ^ PYI062 +15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once +16 | typing_extensions.Literal[1, 1, 1] # twice + | + +PYI062.pyi:15:37: PYI062 Duplicate literal member `1` + | +13 | Literal[1, Literal[1], Literal[1]] # twice +14 | Literal[1, Literal[2], Literal[2]] # once +15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once + | ^ PYI062 +16 | typing_extensions.Literal[1, 1, 1] # twice + | + +PYI062.pyi:16:30: PYI062 Duplicate literal member `1` + | +14 | Literal[1, Literal[2], Literal[2]] # once +15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once +16 | typing_extensions.Literal[1, 1, 1] # twice + | ^ PYI062 +17 | +18 | # Ensure issue is only raised once, even on nested literals + | + +PYI062.pyi:16:33: PYI062 Duplicate literal member `1` + | +14 | Literal[1, Literal[2], Literal[2]] # once +15 | t.Literal[1, t.Literal[2, t.Literal[1]]] # once +16 | typing_extensions.Literal[1, 1, 1] # twice + | ^ PYI062 +17 | +18 | # Ensure issue is only raised once, even on nested literals + | + +PYI062.pyi:19:46: PYI062 Duplicate literal member `True` + | +18 | # Ensure issue is only raised once, even on nested literals +19 | MyType = Literal["foo", Literal[True, False, True], "bar"] # PYI062 + | ^^^^ PYI062 +20 | +21 | n: Literal["No", "duplicates", "here", 1, "1"] + | diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index 88d0441e68667..08875307d0c98 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -418,6 +418,49 @@ where inner(func, semantic, expr, None); } +/// Traverse a "literal" type annotation, applying `func` to each literal member. +/// +/// The function is called with each expression in the literal (excluding declarations of nested +/// literals) and the parent expression. +pub fn traverse_literal<'a, F>(func: &mut F, semantic: &SemanticModel, expr: &'a Expr) +where + F: FnMut(&'a Expr, &'a Expr), +{ + fn inner<'a, F>( + func: &mut F, + semantic: &SemanticModel, + expr: &'a Expr, + parent: Option<&'a Expr>, + ) where + F: FnMut(&'a Expr, &'a Expr), + { + // Ex) Literal[x, y] + if let Expr::Subscript(ast::ExprSubscript { value, slice, .. }) = expr { + if semantic.match_typing_expr(value, "Literal") { + match &**slice { + Expr::Tuple(ast::ExprTuple { elts, .. }) => { + // Traverse each element of the tuple within the literal recursively to handle cases + // such as `Literal[..., Literal[...]] + for elt in elts { + inner(func, semantic, elt, Some(expr)); + } + } + other => { + inner(func, semantic, other, Some(expr)); + } + } + } + } else { + // Otherwise, call the function on expression, if it's not the top-level expression. + if let Some(parent) = parent { + func(expr, parent); + } + } + } + + inner(func, semantic, expr, None); +} + /// Abstraction for a type checker, conservatively checks for the intended type(s). pub trait TypeChecker { /// Check annotation expression to match the intended type(s). diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 8e0c0c1f47d29..ea6ca0e0e2e9a 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -1350,6 +1350,15 @@ impl<'a> SemanticModel<'a> { false } + /// Return `true` if the model is in a nested literal expression (e.g., the inner `Literal` in + /// `Literal[Literal[int, str], float]`). + pub fn in_nested_literal(&self) -> bool { + // Ex) `Literal[Literal[int, str], float]` + self.current_expression_grandparent() + .and_then(Expr::as_subscript_expr) + .is_some_and(|parent| self.match_typing_expr(&parent.value, "Literal")) + } + /// Returns `true` if `left` and `right` are in the same branches of an `if`, `match`, or /// `try` statement. /// diff --git a/ruff.schema.json b/ruff.schema.json index 4b374adba80b8..33cc16342c78f 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3574,6 +3574,8 @@ "PYI056", "PYI058", "PYI059", + "PYI06", + "PYI062", "Q", "Q0", "Q00",