[ruff] Implement unnecessary-nested-literal (RUF039)

crates/ruff_linter/resources/test/fixtures/ruff/RUF039.py

@@ -0,0 +1,31 @@
+from typing import Literal
+import typing as t
+import typing_extensions
+
+
+y: Literal[1, print("hello"), 3, Literal[4, 1]]
+Literal[1, Literal[1]]
+Literal[1, 2, Literal[1, 2]]
+Literal[1, Literal[1], Literal[1]]
+Literal[1, Literal[2], Literal[2]]
+t.Literal[1, t.Literal[2, t.Literal[1]]]
+Literal[
+    1, # comment 1
+    Literal[ # another comment
+        1 # yet annother comment
+    ]
+]  # once
+
+# Ensure issue is only raised once, even on nested literals
+MyType = Literal["foo", Literal[True, False, True], "bar"]
+
+# nested literals, all equivalent to `Literal[1]`
+Literal[Literal[1]]
+Literal[Literal[Literal[1], Literal[1]]]
+Literal[Literal[1], Literal[Literal[Literal[1]]]]
+
+# OK
+x: Literal[True, False, True, False]
+z: Literal[{1, 3, 5}, "foobar", {1,3,5}]
+typing_extensions.Literal[1, 1, 1]
+n: Literal["No", "duplicates", "here", 1, "1"]

crates/ruff_linter/resources/test/fixtures/ruff/RUF039.pyi

@@ -0,0 +1,31 @@
+from typing import Literal
+import typing as t
+import typing_extensions
+
+
+y: Literal[1, print("hello"), 3, Literal[4, 1]]
+Literal[1, Literal[1]]
+Literal[1, 2, Literal[1, 2]]
+Literal[1, Literal[1], Literal[1]]
+Literal[1, Literal[2], Literal[2]]
+t.Literal[1, t.Literal[2, t.Literal[1]]]
+Literal[
+    1, # comment 1
+    Literal[ # another comment
+        1 # yet annother comment
+    ]
+]  # once
+
+# Ensure issue is only raised once, even on nested literals
+MyType = Literal["foo", Literal[True, False, True], "bar"]
+
+# nested literals, all equivalent to `Literal[1]`
+Literal[Literal[1]]
+Literal[Literal[Literal[1], Literal[1]]]
+Literal[Literal[1], Literal[Literal[Literal[1]]]]
+
+# OK
+x: Literal[True, False, True, False]
+z: Literal[{1, 3, 5}, "foobar", {1,3,5}]
+typing_extensions.Literal[1, 1, 1]
+n: Literal["No", "duplicates", "here", 1, "1"]

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -100,9 +100,15 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             }
 
             // Ex) Literal[...]
-            if checker.enabled(Rule::DuplicateLiteralMember) {
+            if checker.any_enabled(&[Rule::DuplicateLiteralMember, Rule::UnnecessaryNestedLiteral])
+            {
                 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::UnnecessaryNestedLiteral) {
+                        ruff::rules::unnecessary_nested_literal(checker, expr);
+                    }
                 }
             }
 

crates/ruff_linter/src/codes.rs

@@ -968,6 +968,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Ruff, "033") => (RuleGroup::Preview, rules::ruff::rules::PostInitDefault),
         (Ruff, "034") => (RuleGroup::Preview, rules::ruff::rules::UselessIfElse),
         (Ruff, "035") => (RuleGroup::Preview, rules::ruff::rules::UnsafeMarkupUse),
+        (Ruff, "039") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryNestedLiteral),
         (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
         (Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA),
 

crates/ruff_linter/src/rules/ruff/mod.rs

@@ -62,6 +62,8 @@ mod tests {
     #[test_case(Rule::UselessIfElse, Path::new("RUF034.py"))]
     #[test_case(Rule::RedirectedNOQA, Path::new("RUF101.py"))]
     #[test_case(Rule::PostInitDefault, Path::new("RUF033.py"))]
+    #[test_case(Rule::UnnecessaryNestedLiteral, Path::new("RUF039.py"))]
+    #[test_case(Rule::UnnecessaryNestedLiteral, Path::new("RUF039.pyi"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff_linter/src/rules/ruff/rules/mod.rs

@@ -28,6 +28,7 @@ pub(crate) use static_key_dict_comprehension::*;
 pub(crate) use test_rules::*;
 pub(crate) use unnecessary_iterable_allocation_for_first_element::*;
 pub(crate) use unnecessary_key_check::*;
+pub(crate) use unnecessary_nested_literal::*;
 pub(crate) use unsafe_markup_use::*;
 pub(crate) use unused_async::*;
 pub(crate) use unused_noqa::*;
@@ -68,6 +69,7 @@ mod suppression_comment_visitor;
 pub(crate) mod test_rules;
 mod unnecessary_iterable_allocation_for_first_element;
 mod unnecessary_key_check;
+mod unnecessary_nested_literal;
 mod unsafe_markup_use;
 mod unused_async;
 mod unused_noqa;

crates/ruff_linter/src/rules/ruff/rules/unnecessary_nested_literal.rs

@@ -0,0 +1,132 @@
+use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::{AnyNodeRef, Expr, ExprContext, ExprSubscript, ExprTuple};
+use ruff_python_semantic::analyze::typing::traverse_literal;
+use ruff_text_size::{Ranged, TextRange};
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for unnecessary nested `Literal`.
+///
+/// ## Why is this bad?
+/// Prefer using a single `Literal`, which is equivalent and more concise.
+///
+/// Parameterization of literals by other literals is supported as an ergonomic
+/// feature as proposed in [PEP 586], to enable patterns such as:
+/// ```python
+/// ReadOnlyMode         = Literal["r", "r+"]
+/// WriteAndTruncateMode = Literal["w", "w+", "wt", "w+t"]
+/// WriteNoTruncateMode  = Literal["r+", "r+t"]
+/// AppendMode           = Literal["a", "a+", "at", "a+t"]
+///
+/// AllModes = Literal[ReadOnlyMode, WriteAndTruncateMode,
+///                   WriteNoTruncateMode, AppendMode]
+/// ```
+///
+/// As a consequence, type checkers also support nesting of literals
+/// which is less readable than a flat `Literal`:
+/// ```python
+/// AllModes = Literal[Literal["r", "r+"], Literal["w", "w+", "wt", "w+t"],
+///                   Literal["r+", "r+t"], Literal["a", "a+", "at", "a+t"]]
+/// ```
+///
+/// ## Example
+/// ```python
+/// AllModes = Literal[
+///     Literal["r", "r+"],
+///     Literal["w", "w+", "wt", "w+t"],
+///     Literal["r+", "r+t"],
+///     Literal["a", "a+", "at", "a+t"],
+/// ]
+/// ```
+///
+/// Use instead:
+/// ```python
+/// AllModes = Literal[
+///     "r", "r+", "w", "w+", "wt", "w+t", "r+", "r+t", "a", "a+", "at", "a+t"
+/// ]
+/// ```
+///
+/// or assign the literal to a variable as in the first example.
+///
+/// ## Fix safety
+/// The fix for this rule is marked as unsafe when the `Literal` slice is split
+/// across multiple lines and some of the lines have trailing comments.
+///
+/// ## 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)
+///
+/// [PEP 586](https://peps.python.org/pep-0586/)
+#[violation]
+pub struct UnnecessaryNestedLiteral;
+
+impl Violation for UnnecessaryNestedLiteral {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        "Unnecessary nested `Literal`".to_string()
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        Some("Replace with flattened `Literal`".to_string())
+    }
+}
+
+/// RUF039
+pub(crate) fn unnecessary_nested_literal<'a>(checker: &mut Checker, literal_expr: &'a Expr) {
+    let mut nodes: Vec<&Expr> = Vec::new();
+    let mut is_nested = false;
+
+    let mut check_for_duplicate_members = |expr: &'a Expr, parent: &'a Expr| {
+        // If the parent is not equal to the `literal_expr` then we know we are traversing recursively.
+        if !AnyNodeRef::ptr_eq(parent.into(), literal_expr.into()) {
+            is_nested = true;
+        };
+        nodes.push(expr);
+    };
+
+    // Traverse the type expressions in the `Literal`.
+    traverse_literal(
+        &mut check_for_duplicate_members,
+        checker.semantic(),
+        literal_expr,
+    );
+
+    if !is_nested {
+        return;
+    }
+
+    let mut diagnostic = Diagnostic::new(UnnecessaryNestedLiteral, literal_expr.range());
+
+    // Create a [`Fix`] that flattens all nodes.
+    if let Expr::Subscript(subscript) = literal_expr {
+        let subscript = Expr::Subscript(ExprSubscript {
+            slice: Box::new(if let [elt] = nodes.as_slice() {
+                (*elt).clone()
+            } else {
+                Expr::Tuple(ExprTuple {
+                    elts: nodes.into_iter().cloned().collect(),
+                    range: TextRange::default(),
+                    ctx: ExprContext::Load,
+                    parenthesized: false,
+                })
+            }),
+            value: subscript.value.clone(),
+            range: TextRange::default(),
+            ctx: ExprContext::Load,
+        });
+        let fix = Fix::applicable_edit(
+            Edit::range_replacement(checker.generator().expr(&subscript), literal_expr.range()),
+            if checker.comment_ranges().intersects(literal_expr.range()) {
+                Applicability::Unsafe
+            } else {
+                Applicability::Safe
+            },
+        );
+        diagnostic.set_fix(fix);
+    };
+
+    checker.diagnostics.push(diagnostic);
+}