diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_literal_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_literal_union.rs index 6523cae84a..afcaec5c40 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_literal_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_literal_union.rs @@ -3,7 +3,7 @@ use std::fmt; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::{self as ast, Expr}; +use ruff_python_ast::{self as ast, Expr, LiteralExpressionRef}; use ruff_python_semantic::SemanticModel; use ruff_text_size::Ranged; @@ -59,7 +59,7 @@ impl Violation for RedundantLiteralUnion { /// PYI051 pub(crate) fn redundant_literal_union<'a>(checker: &mut Checker, union: &'a Expr) { - let mut literal_exprs = Vec::new(); + let mut typing_literal_exprs = Vec::new(); let mut builtin_types_in_union = FxHashSet::default(); // Adds a member to `literal_exprs` for each value in a `Literal`, and any builtin types @@ -68,9 +68,9 @@ pub(crate) fn redundant_literal_union<'a>(checker: &mut Checker, union: &'a Expr if let Expr::Subscript(ast::ExprSubscript { value, slice, .. }) = expr { if checker.semantic().match_typing_expr(value, "Literal") { if let Expr::Tuple(ast::ExprTuple { elts, .. }) = slice.as_ref() { - literal_exprs.extend(elts.iter()); + typing_literal_exprs.extend(elts.iter()); } else { - literal_exprs.push(slice); + typing_literal_exprs.push(slice); } } return; @@ -84,18 +84,20 @@ pub(crate) fn redundant_literal_union<'a>(checker: &mut Checker, union: &'a Expr traverse_union(&mut func, checker.semantic(), union, None); - for literal_expr in literal_exprs { - let Some(constant_type) = match_constant_type(literal_expr) else { + for typing_literal_expr in typing_literal_exprs { + let Some(literal_type) = match_literal_type(typing_literal_expr) else { continue; }; - if builtin_types_in_union.contains(&constant_type) { + if builtin_types_in_union.contains(&literal_type) { checker.diagnostics.push(Diagnostic::new( RedundantLiteralUnion { - literal: SourceCodeSnippet::from_str(checker.locator().slice(literal_expr)), - builtin_type: constant_type, + literal: SourceCodeSnippet::from_str( + checker.locator().slice(typing_literal_expr), + ), + builtin_type: literal_type, }, - literal_expr.range(), + typing_literal_expr.range(), )); } } @@ -143,19 +145,24 @@ fn match_builtin_type(expr: &Expr, semantic: &SemanticModel) -> Option Some(result) } -/// Return the [`ExprType`] of an [`Expr]` if it is a constant (e.g., an `int`, like `1`, or a +/// Return the [`ExprType`] of an [`Expr`] if it is a literal (e.g., an `int`, like `1`, or a /// `bool`, like `True`). -fn match_constant_type(expr: &Expr) -> Option { - let result = match expr { - Expr::BooleanLiteral(_) => ExprType::Bool, - Expr::StringLiteral(_) => ExprType::Str, - Expr::BytesLiteral(_) => ExprType::Bytes, - Expr::NumberLiteral(ast::ExprNumberLiteral { value, .. }) => match value { +fn match_literal_type(expr: &Expr) -> Option { + let Some(literal_expr) = expr.as_literal_expr() else { + return None; + }; + let result = match literal_expr { + LiteralExpressionRef::BooleanLiteral(_) => ExprType::Bool, + LiteralExpressionRef::StringLiteral(_) => ExprType::Str, + LiteralExpressionRef::BytesLiteral(_) => ExprType::Bytes, + LiteralExpressionRef::NumberLiteral(ast::ExprNumberLiteral { value, .. }) => match value { ast::Number::Int(_) => ExprType::Int, ast::Number::Float(_) => ExprType::Float, ast::Number::Complex { .. } => ExprType::Complex, }, - _ => return None, + LiteralExpressionRef::NoneLiteral(_) | LiteralExpressionRef::EllipsisLiteral(_) => { + return None; + } }; Some(result) } diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/if_else_block_instead_of_dict_lookup.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/if_else_block_instead_of_dict_lookup.rs index a3e70c0ca6..f69c46639d 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/rules/if_else_block_instead_of_dict_lookup.rs +++ b/crates/ruff_linter/src/rules/flake8_simplify/rules/if_else_block_instead_of_dict_lookup.rs @@ -2,7 +2,7 @@ use rustc_hash::FxHashSet; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::comparable::ComparableExpr; +use ruff_python_ast::comparable::ComparableLiteral; use ruff_python_ast::helpers::contains_effect; use ruff_python_ast::{self as ast, CmpOp, ElifElseClause, Expr, Stmt}; use ruff_python_semantic::analyze::typing::{is_sys_version_block, is_type_checking_block}; @@ -70,9 +70,9 @@ pub(crate) fn if_else_block_instead_of_dict_lookup(checker: &mut Checker, stmt_i let [expr] = comparators.as_slice() else { return; }; - if !expr.is_literal_expr() { + let Some(literal_expr) = expr.as_literal_expr() else { return; - } + }; let [Stmt::Return(ast::StmtReturn { value, range: _ })] = body.as_slice() else { return; }; @@ -95,8 +95,8 @@ pub(crate) fn if_else_block_instead_of_dict_lookup(checker: &mut Checker, stmt_i } // The `expr` was checked to be a literal above, so this is safe. - let mut literals: FxHashSet = FxHashSet::default(); - literals.insert(expr.into()); + let mut literals: FxHashSet = FxHashSet::default(); + literals.insert(literal_expr.into()); for clause in elif_else_clauses { let ElifElseClause { test, body, .. } = clause; @@ -133,9 +133,9 @@ pub(crate) fn if_else_block_instead_of_dict_lookup(checker: &mut Checker, stmt_i let [expr] = comparators.as_slice() else { return; }; - if !expr.is_literal_expr() { + let Some(literal_expr) = expr.as_literal_expr() else { return; - } + }; if value.as_ref().is_some_and(|value| { contains_effect(value, |id| checker.semantic().is_builtin(id)) @@ -144,7 +144,7 @@ pub(crate) fn if_else_block_instead_of_dict_lookup(checker: &mut Checker, stmt_i }; // The `expr` was checked to be a literal above, so this is safe. - literals.insert(expr.into()); + literals.insert(literal_expr.into()); } // Different `elif` _ => { diff --git a/crates/ruff_linter/src/rules/pylint/rules/magic_value_comparison.rs b/crates/ruff_linter/src/rules/pylint/rules/magic_value_comparison.rs index 7a53675996..c47f91ff62 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/magic_value_comparison.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/magic_value_comparison.rs @@ -1,5 +1,5 @@ use itertools::Itertools; -use ruff_python_ast::{self as ast, Expr, Int, UnaryOp}; +use ruff_python_ast::{self as ast, Expr, Int, LiteralExpressionRef, UnaryOp}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; @@ -57,39 +57,39 @@ impl Violation for MagicValueComparison { } } -/// If an [`Expr`] is a literal (or unary operation on a literal), return the literal [`Expr`]. -fn as_literal(expr: &Expr) -> Option<&Expr> { +/// If an [`Expr`] is a literal (or unary operation on a literal), return the [`LiteralExpressionRef`]. +fn as_literal(expr: &Expr) -> Option> { match expr { Expr::UnaryOp(ast::ExprUnaryOp { op: UnaryOp::UAdd | UnaryOp::USub | UnaryOp::Invert, operand, .. - }) if operand.is_literal_expr() => Some(operand.as_ref()), - expr if expr.is_literal_expr() => Some(expr), - _ => None, + }) => operand.as_literal_expr(), + _ => expr.as_literal_expr(), } } -fn is_magic_value(expr: &Expr, allowed_types: &[ConstantType]) -> bool { - if let Some(constant_type) = ConstantType::try_from_expr(expr) { +fn is_magic_value(literal_expr: LiteralExpressionRef, allowed_types: &[ConstantType]) -> bool { + if let Some(constant_type) = ConstantType::try_from_literal_expr(literal_expr) { if allowed_types.contains(&constant_type) { return false; } } - match expr { + match literal_expr { // Ignore `None`, `Bool`, and `Ellipsis` constants. - Expr::NoneLiteral(_) | Expr::BooleanLiteral(_) | Expr::EllipsisLiteral(_) => false, + LiteralExpressionRef::NoneLiteral(_) + | LiteralExpressionRef::BooleanLiteral(_) + | LiteralExpressionRef::EllipsisLiteral(_) => false, // Special-case some common string and integer types. - Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => { + LiteralExpressionRef::StringLiteral(ast::ExprStringLiteral { value, .. }) => { !matches!(value.as_str(), "" | "__main__") } - Expr::NumberLiteral(ast::ExprNumberLiteral { value, .. }) => match value { + LiteralExpressionRef::NumberLiteral(ast::ExprNumberLiteral { value, .. }) => match value { ast::Number::Int(value) => !matches!(*value, Int::ZERO | Int::ONE), _ => true, }, - Expr::BytesLiteral(_) => true, - _ => false, + LiteralExpressionRef::BytesLiteral(_) => true, } } diff --git a/crates/ruff_linter/src/rules/pylint/settings.rs b/crates/ruff_linter/src/rules/pylint/settings.rs index 10187e39e8..4925211952 100644 --- a/crates/ruff_linter/src/rules/pylint/settings.rs +++ b/crates/ruff_linter/src/rules/pylint/settings.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize}; use ruff_macros::CacheKey; -use ruff_python_ast::{Expr, ExprNumberLiteral, Number}; +use ruff_python_ast::{ExprNumberLiteral, LiteralExpressionRef, Number}; #[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, CacheKey)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] @@ -17,16 +17,18 @@ pub enum ConstantType { } impl ConstantType { - pub fn try_from_expr(expr: &Expr) -> Option { - match expr { - Expr::StringLiteral(_) => Some(Self::Str), - Expr::BytesLiteral(_) => Some(Self::Bytes), - Expr::NumberLiteral(ExprNumberLiteral { value, .. }) => match value { + pub fn try_from_literal_expr(literal_expr: LiteralExpressionRef<'_>) -> Option { + match literal_expr { + LiteralExpressionRef::StringLiteral(_) => Some(Self::Str), + LiteralExpressionRef::BytesLiteral(_) => Some(Self::Bytes), + LiteralExpressionRef::NumberLiteral(ExprNumberLiteral { value, .. }) => match value { Number::Int(_) => Some(Self::Int), Number::Float(_) => Some(Self::Float), Number::Complex { .. } => Some(Self::Complex), }, - _ => None, + LiteralExpressionRef::BooleanLiteral(_) + | LiteralExpressionRef::NoneLiteral(_) + | LiteralExpressionRef::EllipsisLiteral(_) => None, } } } diff --git a/crates/ruff_python_ast/src/comparable.rs b/crates/ruff_python_ast/src/comparable.rs index d0a219a336..8fdba131c2 100644 --- a/crates/ruff_python_ast/src/comparable.rs +++ b/crates/ruff_python_ast/src/comparable.rs @@ -644,6 +644,39 @@ pub struct ExprFString<'a> { values: Vec>, } +#[derive(Debug, PartialEq, Eq, Hash)] +pub enum ComparableLiteral<'a> { + None, + Ellipsis, + Bool(&'a bool), + Str { value: &'a str, unicode: &'a bool }, + Bytes { value: &'a [u8] }, + Number(ComparableNumber<'a>), +} + +impl<'a> From> for ComparableLiteral<'a> { + fn from(literal: ast::LiteralExpressionRef<'a>) -> Self { + match literal { + ast::LiteralExpressionRef::NoneLiteral(_) => Self::None, + ast::LiteralExpressionRef::EllipsisLiteral(_) => Self::Ellipsis, + ast::LiteralExpressionRef::BooleanLiteral(ast::ExprBooleanLiteral { + value, .. + }) => Self::Bool(value), + ast::LiteralExpressionRef::StringLiteral(ast::ExprStringLiteral { + value, + unicode, + .. + }) => Self::Str { value, unicode }, + ast::LiteralExpressionRef::BytesLiteral(ast::ExprBytesLiteral { value, .. }) => { + Self::Bytes { value } + } + ast::LiteralExpressionRef::NumberLiteral(ast::ExprNumberLiteral { value, .. }) => { + Self::Number(value.into()) + } + } + } +} + #[derive(Debug, PartialEq, Eq, Hash)] pub struct ExprStringLiteral<'a> { value: &'a str, diff --git a/crates/ruff_python_ast/src/expression.rs b/crates/ruff_python_ast/src/expression.rs index c1412b978b..83bac72724 100644 --- a/crates/ruff_python_ast/src/expression.rs +++ b/crates/ruff_python_ast/src/expression.rs @@ -336,3 +336,28 @@ impl Ranged for ExpressionRef<'_> { } } } + +/// Unowned pendant to all the literal variants of [`ast::Expr`] that stores a +/// reference instead of an owned value. +#[derive(Copy, Clone, Debug, PartialEq, is_macro::Is)] +pub enum LiteralExpressionRef<'a> { + StringLiteral(&'a ast::ExprStringLiteral), + BytesLiteral(&'a ast::ExprBytesLiteral), + NumberLiteral(&'a ast::ExprNumberLiteral), + BooleanLiteral(&'a ast::ExprBooleanLiteral), + NoneLiteral(&'a ast::ExprNoneLiteral), + EllipsisLiteral(&'a ast::ExprEllipsisLiteral), +} + +impl Ranged for LiteralExpressionRef<'_> { + fn range(&self) -> TextRange { + match self { + LiteralExpressionRef::StringLiteral(expression) => expression.range(), + LiteralExpressionRef::BytesLiteral(expression) => expression.range(), + LiteralExpressionRef::NumberLiteral(expression) => expression.range(), + LiteralExpressionRef::BooleanLiteral(expression) => expression.range(), + LiteralExpressionRef::NoneLiteral(expression) => expression.range(), + LiteralExpressionRef::EllipsisLiteral(expression) => expression.range(), + } + } +} diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index 5e3eea5a5c..12abaf1996 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -6,7 +6,7 @@ use std::fmt; use std::fmt::Debug; use std::ops::Deref; -use crate::int; +use crate::{int, LiteralExpressionRef}; use ruff_text_size::{Ranged, TextRange, TextSize}; /// See also [mod](https://docs.python.org/3/library/ast.html#ast.mod) @@ -640,6 +640,18 @@ impl Expr { ) } + pub fn as_literal_expr(&self) -> Option> { + match self { + Expr::StringLiteral(expr) => Some(LiteralExpressionRef::StringLiteral(expr)), + Expr::BytesLiteral(expr) => Some(LiteralExpressionRef::BytesLiteral(expr)), + Expr::NumberLiteral(expr) => Some(LiteralExpressionRef::NumberLiteral(expr)), + Expr::BooleanLiteral(expr) => Some(LiteralExpressionRef::BooleanLiteral(expr)), + Expr::NoneLiteral(expr) => Some(LiteralExpressionRef::NoneLiteral(expr)), + Expr::EllipsisLiteral(expr) => Some(LiteralExpressionRef::EllipsisLiteral(expr)), + _ => None, + } + } + pub fn is_implicit_concatenated_string(&self) -> bool { match self { Expr::StringLiteral(ExprStringLiteral {