Skip to content

Commit

Permalink
Update tests, address review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Nov 27, 2024
1 parent e194d94 commit b30d865
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 66 deletions.
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
Rule::DuplicateLiteralMember,
Rule::RedundantBoolLiteral,
Rule::RedundantNoneLiteral,
Rule::UnnecessaryNestedLiteral
Rule::UnnecessaryNestedLiteral,
]) {
if !checker.semantic.in_nested_literal() {
if checker.enabled(Rule::DuplicateLiteralMember) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::{AnyNodeRef, Expr, ExprContext, ExprSubscript, ExprTuple};
use ruff_python_semantic::analyze::typing::traverse_literal;
use ruff_text_size::{Ranged, TextRange};
Expand Down Expand Up @@ -58,8 +58,8 @@ use crate::checkers::ast::Checker;
/// - [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;
#[derive(ViolationMetadata)]
pub(crate) struct UnnecessaryNestedLiteral;

impl Violation for UnnecessaryNestedLiteral {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
Expand All @@ -76,20 +76,16 @@ impl Violation for UnnecessaryNestedLiteral {

/// 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,
&mut |_: &'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;
};
},
checker.semantic(),
literal_expr,
);
Expand All @@ -98,6 +94,17 @@ pub(crate) fn unnecessary_nested_literal<'a>(checker: &mut Checker, literal_expr
return;
}

// Collect the literal nodes for the fix
let mut nodes: Vec<&Expr> = Vec::new();

traverse_literal(
&mut |expr, _| {
nodes.push(expr);
},
checker.semantic(),
literal_expr,
);

let mut diagnostic = Diagnostic::new(UnnecessaryNestedLiteral, literal_expr.range());

// Create a [`Fix`] that flattens all nodes.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
snapshot_kind: text
---
RUF039.py:6:4: RUF039 [*] Unnecessary nested `Literal`
RUF041.py:6:4: RUF041 [*] Unnecessary nested `Literal`
|
6 | y: Literal[1, print("hello"), 3, Literal[4, 1]]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF041
7 | Literal[1, Literal[1]]
8 | Literal[1, 2, Literal[1, 2]]
|
Expand All @@ -20,11 +21,11 @@ RUF039.py:6:4: RUF039 [*] Unnecessary nested `Literal`
8 8 | Literal[1, 2, Literal[1, 2]]
9 9 | Literal[1, Literal[1], Literal[1]]

RUF039.py:7:1: RUF039 [*] Unnecessary nested `Literal`
RUF041.py:7:1: RUF041 [*] Unnecessary nested `Literal`
|
6 | y: Literal[1, print("hello"), 3, Literal[4, 1]]
7 | Literal[1, Literal[1]]
| ^^^^^^^^^^^^^^^^^^^^^^ RUF039
| ^^^^^^^^^^^^^^^^^^^^^^ RUF041
8 | Literal[1, 2, Literal[1, 2]]
9 | Literal[1, Literal[1], Literal[1]]
|
Expand All @@ -40,12 +41,12 @@ RUF039.py:7:1: RUF039 [*] Unnecessary nested `Literal`
9 9 | Literal[1, Literal[1], Literal[1]]
10 10 | Literal[1, Literal[2], Literal[2]]

RUF039.py:8:1: RUF039 [*] Unnecessary nested `Literal`
RUF041.py:8:1: RUF041 [*] Unnecessary nested `Literal`
|
6 | y: Literal[1, print("hello"), 3, Literal[4, 1]]
7 | Literal[1, Literal[1]]
8 | Literal[1, 2, Literal[1, 2]]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF041
9 | Literal[1, Literal[1], Literal[1]]
10 | Literal[1, Literal[2], Literal[2]]
|
Expand All @@ -61,12 +62,12 @@ RUF039.py:8:1: RUF039 [*] Unnecessary nested `Literal`
10 10 | Literal[1, Literal[2], Literal[2]]
11 11 | t.Literal[1, t.Literal[2, t.Literal[1]]]

RUF039.py:9:1: RUF039 [*] Unnecessary nested `Literal`
RUF041.py:9:1: RUF041 [*] Unnecessary nested `Literal`
|
7 | Literal[1, Literal[1]]
8 | Literal[1, 2, Literal[1, 2]]
9 | Literal[1, Literal[1], Literal[1]]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF041
10 | Literal[1, Literal[2], Literal[2]]
11 | t.Literal[1, t.Literal[2, t.Literal[1]]]
|
Expand All @@ -82,12 +83,12 @@ RUF039.py:9:1: RUF039 [*] Unnecessary nested `Literal`
11 11 | t.Literal[1, t.Literal[2, t.Literal[1]]]
12 12 | Literal[

RUF039.py:10:1: RUF039 [*] Unnecessary nested `Literal`
RUF041.py:10:1: RUF041 [*] Unnecessary nested `Literal`
|
8 | Literal[1, 2, Literal[1, 2]]
9 | Literal[1, Literal[1], Literal[1]]
10 | Literal[1, Literal[2], Literal[2]]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF041
11 | t.Literal[1, t.Literal[2, t.Literal[1]]]
12 | Literal[
|
Expand All @@ -103,12 +104,12 @@ RUF039.py:10:1: RUF039 [*] Unnecessary nested `Literal`
12 12 | Literal[
13 13 | 1, # comment 1

RUF039.py:11:1: RUF039 [*] Unnecessary nested `Literal`
RUF041.py:11:1: RUF041 [*] Unnecessary nested `Literal`
|
9 | Literal[1, Literal[1], Literal[1]]
10 | Literal[1, Literal[2], Literal[2]]
11 | t.Literal[1, t.Literal[2, t.Literal[1]]]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF041
12 | Literal[
13 | 1, # comment 1
|
Expand All @@ -124,7 +125,7 @@ RUF039.py:11:1: RUF039 [*] Unnecessary nested `Literal`
13 13 | 1, # comment 1
14 14 | Literal[ # another comment

RUF039.py:12:1: RUF039 [*] Unnecessary nested `Literal`
RUF041.py:12:1: RUF041 [*] Unnecessary nested `Literal`
|
10 | Literal[1, Literal[2], Literal[2]]
11 | t.Literal[1, t.Literal[2, t.Literal[1]]]
Expand All @@ -134,7 +135,7 @@ RUF039.py:12:1: RUF039 [*] Unnecessary nested `Literal`
15 | | 1 # yet annother comment
16 | | ]
17 | | ] # once
| |_^ RUF039
| |_^ RUF041
18 |
19 | # Ensure issue is only raised once, even on nested literals
|
Expand All @@ -155,11 +156,11 @@ RUF039.py:12:1: RUF039 [*] Unnecessary nested `Literal`
19 14 | # Ensure issue is only raised once, even on nested literals
20 15 | MyType = Literal["foo", Literal[True, False, True], "bar"]

RUF039.py:20:10: RUF039 [*] Unnecessary nested `Literal`
RUF041.py:20:10: RUF041 [*] Unnecessary nested `Literal`
|
19 | # Ensure issue is only raised once, even on nested literals
20 | MyType = Literal["foo", Literal[True, False, True], "bar"]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF041
21 |
22 | # nested literals, all equivalent to `Literal[1]`
|
Expand All @@ -175,11 +176,11 @@ RUF039.py:20:10: RUF039 [*] Unnecessary nested `Literal`
22 22 | # nested literals, all equivalent to `Literal[1]`
23 23 | Literal[Literal[1]]

RUF039.py:23:1: RUF039 [*] Unnecessary nested `Literal`
RUF041.py:23:1: RUF041 [*] Unnecessary nested `Literal`
|
22 | # nested literals, all equivalent to `Literal[1]`
23 | Literal[Literal[1]]
| ^^^^^^^^^^^^^^^^^^^ RUF039
| ^^^^^^^^^^^^^^^^^^^ RUF041
24 | Literal[Literal[Literal[1], Literal[1]]]
25 | Literal[Literal[1], Literal[Literal[Literal[1]]]]
|
Expand All @@ -195,12 +196,12 @@ RUF039.py:23:1: RUF039 [*] Unnecessary nested `Literal`
25 25 | Literal[Literal[1], Literal[Literal[Literal[1]]]]
26 26 |

RUF039.py:24:1: RUF039 [*] Unnecessary nested `Literal`
RUF041.py:24:1: RUF041 [*] Unnecessary nested `Literal`
|
22 | # nested literals, all equivalent to `Literal[1]`
23 | Literal[Literal[1]]
24 | Literal[Literal[Literal[1], Literal[1]]]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF041
25 | Literal[Literal[1], Literal[Literal[Literal[1]]]]
|
= help: Replace with flattened `Literal`
Expand All @@ -215,12 +216,12 @@ RUF039.py:24:1: RUF039 [*] Unnecessary nested `Literal`
26 26 |
27 27 | # OK

RUF039.py:24:9: RUF039 [*] Unnecessary nested `Literal`
RUF041.py:24:9: RUF041 [*] Unnecessary nested `Literal`
|
22 | # nested literals, all equivalent to `Literal[1]`
23 | Literal[Literal[1]]
24 | Literal[Literal[Literal[1], Literal[1]]]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF041
25 | Literal[Literal[1], Literal[Literal[Literal[1]]]]
|
= help: Replace with flattened `Literal`
Expand All @@ -235,12 +236,12 @@ RUF039.py:24:9: RUF039 [*] Unnecessary nested `Literal`
26 26 |
27 27 | # OK

RUF039.py:25:1: RUF039 [*] Unnecessary nested `Literal`
RUF041.py:25:1: RUF041 [*] Unnecessary nested `Literal`
|
23 | Literal[Literal[1]]
24 | Literal[Literal[Literal[1], Literal[1]]]
25 | Literal[Literal[1], Literal[Literal[Literal[1]]]]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF039
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF041
26 |
27 | # OK
|
Expand All @@ -256,12 +257,12 @@ RUF039.py:25:1: RUF039 [*] Unnecessary nested `Literal`
27 27 | # OK
28 28 | x: Literal[True, False, True, False]

RUF039.py:25:29: RUF039 [*] Unnecessary nested `Literal`
RUF041.py:25:29: RUF041 [*] Unnecessary nested `Literal`
|
23 | Literal[Literal[1]]
24 | Literal[Literal[Literal[1], Literal[1]]]
25 | Literal[Literal[1], Literal[Literal[Literal[1]]]]
| ^^^^^^^^^^^^^^^^^^^ RUF039
| ^^^^^^^^^^^^^^^^^^^ RUF041
26 |
27 | # OK
|
Expand Down
Loading

0 comments on commit b30d865

Please sign in to comment.