From 4aec62d499212ea3ee1b15a40f72c5d8e2872975 Mon Sep 17 00:00:00 2001 From: Seo Sanghyeon Date: Tue, 20 Feb 2024 20:55:41 +0900 Subject: [PATCH 1/3] Detect literals with unary operators (UP018) --- .../test/fixtures/pyupgrade/UP018.py | 8 +- .../rules/pyupgrade/rules/native_literals.rs | 33 ++++--- ...er__rules__pyupgrade__tests__UP018.py.snap | 89 +++++++++++++++++-- 3 files changed, 114 insertions(+), 16 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP018.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP018.py index 0dfb97566664e..5b3a148a46eb1 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP018.py +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP018.py @@ -33,7 +33,7 @@ bool(1.0) int().denominator -# These become string or byte literals +# These become literals str() str("foo") str(""" @@ -53,3 +53,9 @@ # These become a literal but retain parentheses int(1).denominator + +# These too are literals in spirit +int(+1) +int(-1) +float(+1.0) +float(-1.0) diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs index 9e7392bb2125d..86ead3b062678 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs @@ -3,7 +3,7 @@ use std::str::FromStr; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::{self as ast, Expr, LiteralExpressionRef}; +use ruff_python_ast::{self as ast, Expr, LiteralExpressionRef, UnaryOp}; use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; @@ -198,7 +198,20 @@ pub(crate) fn native_literals( checker.diagnostics.push(diagnostic); } Some(arg) => { - let Some(literal_expr) = arg.as_literal_expr() else { + let (literal_expr, unary) = if let Some(literal_expr) = arg.as_literal_expr() { + (literal_expr, false) + } else if let Expr::UnaryOp(ast::ExprUnaryOp { + op: UnaryOp::UAdd | UnaryOp::USub, + operand, + .. + }) = arg + { + if let Some(literal_expr) = operand.as_literal_expr() { + (literal_expr, true) + } else { + return; + } + } else { return; }; @@ -215,20 +228,20 @@ pub(crate) fn native_literals( return; } + if unary { + if !matches!(literal_type, LiteralType::Int | LiteralType::Float) { + return; + } + } + let arg_code = checker.locator().slice(arg); // Attribute access on an integer requires the integer to be parenthesized to disambiguate from a float // Ex) `(7).denominator` is valid but `7.denominator` is not // Note that floats do not have this problem // Ex) `(1.0).real` is valid and `1.0.real` is too - let content = match (parent_expr, arg) { - ( - Some(Expr::Attribute(_)), - Expr::NumberLiteral(ast::ExprNumberLiteral { - value: ast::Number::Int(_), - .. - }), - ) => format!("({arg_code})"), + let content = match (parent_expr, literal_type) { + (Some(Expr::Attribute(_)), LiteralType::Int) => format!("({arg_code})"), _ => arg_code.to_string(), }; diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP018.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP018.py.snap index 46e6968df683d..a0a1f44188172 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP018.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP018.py.snap @@ -3,7 +3,7 @@ source: crates/ruff_linter/src/rules/pyupgrade/mod.rs --- UP018.py:37:1: UP018 [*] Unnecessary `str` call (rewrite as a literal) | -36 | # These become string or byte literals +36 | # These become literals 37 | str() | ^^^^^ UP018 38 | str("foo") @@ -14,7 +14,7 @@ UP018.py:37:1: UP018 [*] Unnecessary `str` call (rewrite as a literal) ℹ Safe fix 34 34 | int().denominator 35 35 | -36 36 | # These become string or byte literals +36 36 | # These become literals 37 |-str() 37 |+"" 38 38 | str("foo") @@ -23,7 +23,7 @@ UP018.py:37:1: UP018 [*] Unnecessary `str` call (rewrite as a literal) UP018.py:38:1: UP018 [*] Unnecessary `str` call (rewrite as a literal) | -36 | # These become string or byte literals +36 | # These become literals 37 | str() 38 | str("foo") | ^^^^^^^^^^ UP018 @@ -34,7 +34,7 @@ UP018.py:38:1: UP018 [*] Unnecessary `str` call (rewrite as a literal) ℹ Safe fix 35 35 | -36 36 | # These become string or byte literals +36 36 | # These become literals 37 37 | str() 38 |-str("foo") 38 |+"foo" @@ -55,7 +55,7 @@ UP018.py:39:1: UP018 [*] Unnecessary `str` call (rewrite as a literal) = help: Replace with string literal ℹ Safe fix -36 36 | # These become string or byte literals +36 36 | # These become literals 37 37 | str() 38 38 | str("foo") 39 |-str(""" @@ -304,6 +304,8 @@ UP018.py:55:1: UP018 [*] Unnecessary `int` call (rewrite as a literal) 54 | # These become a literal but retain parentheses 55 | int(1).denominator | ^^^^^^ UP018 +56 | +57 | # These too are literals in spirit | = help: Replace with integer literal @@ -313,5 +315,82 @@ UP018.py:55:1: UP018 [*] Unnecessary `int` call (rewrite as a literal) 54 54 | # These become a literal but retain parentheses 55 |-int(1).denominator 55 |+(1).denominator +56 56 | +57 57 | # These too are literals in spirit +58 58 | int(+1) + +UP018.py:58:1: UP018 [*] Unnecessary `int` call (rewrite as a literal) + | +57 | # These too are literals in spirit +58 | int(+1) + | ^^^^^^^ UP018 +59 | int(-1) +60 | float(+1.0) + | + = help: Replace with integer literal + +ℹ Safe fix +55 55 | int(1).denominator +56 56 | +57 57 | # These too are literals in spirit +58 |-int(+1) + 58 |++1 +59 59 | int(-1) +60 60 | float(+1.0) +61 61 | float(-1.0) + +UP018.py:59:1: UP018 [*] Unnecessary `int` call (rewrite as a literal) + | +57 | # These too are literals in spirit +58 | int(+1) +59 | int(-1) + | ^^^^^^^ UP018 +60 | float(+1.0) +61 | float(-1.0) + | + = help: Replace with integer literal + +ℹ Safe fix +56 56 | +57 57 | # These too are literals in spirit +58 58 | int(+1) +59 |-int(-1) + 59 |+-1 +60 60 | float(+1.0) +61 61 | float(-1.0) + +UP018.py:60:1: UP018 [*] Unnecessary `float` call (rewrite as a literal) + | +58 | int(+1) +59 | int(-1) +60 | float(+1.0) + | ^^^^^^^^^^^ UP018 +61 | float(-1.0) + | + = help: Replace with float literal + +ℹ Safe fix +57 57 | # These too are literals in spirit +58 58 | int(+1) +59 59 | int(-1) +60 |-float(+1.0) + 60 |++1.0 +61 61 | float(-1.0) + +UP018.py:61:1: UP018 [*] Unnecessary `float` call (rewrite as a literal) + | +59 | int(-1) +60 | float(+1.0) +61 | float(-1.0) + | ^^^^^^^^^^^ UP018 + | + = help: Replace with float literal + +ℹ Safe fix +58 58 | int(+1) +59 59 | int(-1) +60 60 | float(+1.0) +61 |-float(-1.0) + 61 |+-1.0 From dafbdc58178a2abf4f52494629b9f9310aa55ff9 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 20 Feb 2024 13:11:40 -0500 Subject: [PATCH 2/3] Move conditions around --- .../rules/pyupgrade/rules/native_literals.rs | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs index 86ead3b062678..6e24b817c18fa 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs @@ -198,16 +198,24 @@ pub(crate) fn native_literals( checker.diagnostics.push(diagnostic); } Some(arg) => { - let (literal_expr, unary) = if let Some(literal_expr) = arg.as_literal_expr() { - (literal_expr, false) + let literal_expr = if let Some(literal_expr) = arg.as_literal_expr() { + // Skip implicit concatenated strings. + if literal_expr.is_implicit_concatenated() { + return; + } else { + literal_expr + } } else if let Expr::UnaryOp(ast::ExprUnaryOp { op: UnaryOp::UAdd | UnaryOp::USub, operand, .. }) = arg { - if let Some(literal_expr) = operand.as_literal_expr() { - (literal_expr, true) + if let Some(literal_expr) = operand + .as_literal_expr() + .filter(|expr| matches!(expr, LiteralExpressionRef::NumberLiteral(_))) + { + literal_expr } else { return; } @@ -215,11 +223,6 @@ pub(crate) fn native_literals( return; }; - // Skip implicit string concatenations. - if literal_expr.is_implicit_concatenated() { - return; - } - let Ok(arg_literal_type) = LiteralType::try_from(literal_expr) else { return; }; @@ -228,12 +231,6 @@ pub(crate) fn native_literals( return; } - if unary { - if !matches!(literal_type, LiteralType::Int | LiteralType::Float) { - return; - } - } - let arg_code = checker.locator().slice(arg); // Attribute access on an integer requires the integer to be parenthesized to disambiguate from a float From 7d309e7ac8f4d3be1a97aa40935de5de1cf7f5fe Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 20 Feb 2024 13:13:07 -0500 Subject: [PATCH 3/3] Move if-else --- .../ruff_linter/src/rules/pyupgrade/rules/native_literals.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs index 6e24b817c18fa..d4123b8df4619 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/native_literals.rs @@ -202,9 +202,8 @@ pub(crate) fn native_literals( // Skip implicit concatenated strings. if literal_expr.is_implicit_concatenated() { return; - } else { - literal_expr } + literal_expr } else if let Expr::UnaryOp(ast::ExprUnaryOp { op: UnaryOp::UAdd | UnaryOp::USub, operand, @@ -217,6 +216,7 @@ pub(crate) fn native_literals( { literal_expr } else { + // Only allow unary operators for numbers. return; } } else {