diff --git a/src/ast/cmp.rs b/src/ast/cmp.rs deleted file mode 100644 index 18afcbf82e703..0000000000000 --- a/src/ast/cmp.rs +++ /dev/null @@ -1,394 +0,0 @@ -//! Compare two AST nodes for equality, ignoring locations. - -use rustpython_ast::{Arg, Arguments, Comprehension, Expr, ExprKind, Keyword}; - -/// Returns `true` if the two `Expr` are equal, ignoring locations. -pub fn expr(a: &Expr, b: &Expr) -> bool { - match (&a.node, &b.node) { - ( - ExprKind::BoolOp { - op: op_a, - values: values_a, - }, - ExprKind::BoolOp { - op: op_b, - values: values_b, - }, - ) => { - op_a == op_b - && values_a.len() == values_b.len() - && values_a.iter().zip(values_b).all(|(a, b)| expr(a, b)) - } - ( - ExprKind::NamedExpr { - target: target_a, - value: value_a, - }, - ExprKind::NamedExpr { - target: target_b, - value: value_b, - }, - ) => expr(target_a, target_b) && expr(value_a, value_b), - ( - ExprKind::BinOp { - left: left_a, - op: op_a, - right: right_a, - }, - ExprKind::BinOp { - left: left_b, - op: op_b, - right: right_b, - }, - ) => op_a == op_b && expr(left_a, left_b) && expr(right_a, right_b), - ( - ExprKind::UnaryOp { - op: op_a, - operand: operand_a, - }, - ExprKind::UnaryOp { - op: op_b, - operand: operand_b, - }, - ) => op_a == op_b && expr(operand_a, operand_b), - ( - ExprKind::Lambda { - args: args_a, - body: body_a, - }, - ExprKind::Lambda { - args: args_b, - body: body_b, - }, - ) => expr(body_a, body_b) && arguments(args_a, args_b), - ( - ExprKind::IfExp { - test: test_a, - body: body_a, - orelse: orelse_a, - }, - ExprKind::IfExp { - test: test_b, - body: body_b, - orelse: orelse_b, - }, - ) => expr(test_a, test_b) && expr(body_a, body_b) && expr(orelse_a, orelse_b), - ( - ExprKind::Dict { - keys: keys_a, - values: values_a, - }, - ExprKind::Dict { - keys: keys_b, - values: values_b, - }, - ) => { - keys_a.len() == keys_b.len() - && values_a.len() == values_b.len() - && keys_a.iter().zip(keys_b).all(|(a, b)| expr(a, b)) - && values_a.iter().zip(values_b).all(|(a, b)| expr(a, b)) - } - (ExprKind::Set { elts: elts_a }, ExprKind::Set { elts: elts_b }) => { - elts_a.len() == elts_b.len() && elts_a.iter().zip(elts_b).all(|(a, b)| expr(a, b)) - } - ( - ExprKind::ListComp { - elt: elt_a, - generators: generators_a, - }, - ExprKind::ListComp { - elt: elt_b, - generators: generators_b, - }, - ) => { - expr(elt_a, elt_b) - && generators_a.len() == generators_b.len() - && generators_a - .iter() - .zip(generators_b) - .all(|(a, b)| comprehension(a, b)) - } - ( - ExprKind::SetComp { - elt: elt_a, - generators: generators_a, - }, - ExprKind::SetComp { - elt: elt_b, - generators: generators_b, - }, - ) => { - expr(elt_a, elt_b) - && generators_a.len() == generators_b.len() - && generators_a - .iter() - .zip(generators_b) - .all(|(a, b)| comprehension(a, b)) - } - ( - ExprKind::DictComp { - key: key_a, - value: value_a, - generators: generators_a, - }, - ExprKind::DictComp { - key: key_b, - value: value_b, - generators: generators_b, - }, - ) => { - expr(key_a, key_b) - && expr(value_a, value_b) - && generators_a.len() == generators_b.len() - && generators_a - .iter() - .zip(generators_b) - .all(|(a, b)| comprehension(a, b)) - } - ( - ExprKind::GeneratorExp { - elt: elt_a, - generators: generators_a, - }, - ExprKind::GeneratorExp { - elt: elt_b, - generators: generators_b, - }, - ) => { - expr(elt_a, elt_b) - && generators_a.len() == generators_b.len() - && generators_a - .iter() - .zip(generators_b) - .all(|(a, b)| comprehension(a, b)) - } - (ExprKind::Await { value: value_a }, ExprKind::Await { value: value_b }) => { - expr(value_a, value_b) - } - (ExprKind::Yield { value: value_a }, ExprKind::Yield { value: value_b }) => { - option_expr(value_a.as_deref(), value_b.as_deref()) - } - (ExprKind::YieldFrom { value: value_a }, ExprKind::YieldFrom { value: value_b }) => { - expr(value_a, value_b) - } - ( - ExprKind::Compare { - left: left_a, - ops: ops_a, - comparators: comparators_a, - }, - ExprKind::Compare { - left: left_b, - ops: ops_b, - comparators: comparators_b, - }, - ) => { - expr(left_a, left_b) - && ops_a == ops_b - && comparators_a.len() == comparators_b.len() - && comparators_a - .iter() - .zip(comparators_b) - .all(|(a, b)| expr(a, b)) - } - ( - ExprKind::Call { - func: func_a, - args: args_a, - keywords: keywords_a, - }, - ExprKind::Call { - func: func_b, - args: args_b, - keywords: keywords_b, - }, - ) => { - expr(func_a, func_b) - && args_a.len() == args_b.len() - && args_a.iter().zip(args_b).all(|(a, b)| expr(a, b)) - && keywords_a.len() == keywords_b.len() - && keywords_a - .iter() - .zip(keywords_b) - .all(|(a, b)| keyword(a, b)) - } - ( - ExprKind::FormattedValue { - value: value_a, - conversion: conversion_a, - format_spec: format_spec_a, - }, - ExprKind::FormattedValue { - value: value_b, - conversion: conversion_b, - format_spec: format_spec_b, - }, - ) => { - expr(value_a, value_b) - && conversion_a == conversion_b - && option_expr(format_spec_a.as_deref(), format_spec_b.as_deref()) - } - (ExprKind::JoinedStr { values: values_a }, ExprKind::JoinedStr { values: values_b }) => { - values_a.len() == values_b.len() - && values_a.iter().zip(values_b).all(|(a, b)| expr(a, b)) - } - ( - ExprKind::Constant { - value: value_a, - kind: kind_a, - }, - ExprKind::Constant { - value: value_b, - kind: kind_b, - }, - ) => value_a == value_b && kind_a == kind_b, - ( - ExprKind::Attribute { - value: value_a, - attr: attr_a, - ctx: ctx_a, - }, - ExprKind::Attribute { - value: value_b, - attr: attr_b, - ctx: ctx_b, - }, - ) => attr_a == attr_b && ctx_a == ctx_b && expr(value_a, value_b), - ( - ExprKind::Subscript { - value: value_a, - slice: slice_a, - ctx: ctx_a, - }, - ExprKind::Subscript { - value: value_b, - slice: slice_b, - ctx: ctx_b, - }, - ) => ctx_a == ctx_b && expr(value_a, value_b) && expr(slice_a, slice_b), - ( - ExprKind::Starred { - value: value_a, - ctx: ctx_a, - }, - ExprKind::Starred { - value: value_b, - ctx: ctx_b, - }, - ) => ctx_a == ctx_b && expr(value_a, value_b), - ( - ExprKind::Name { - id: id_a, - ctx: ctx_a, - }, - ExprKind::Name { - id: id_b, - ctx: ctx_b, - }, - ) => id_a == id_b && ctx_a == ctx_b, - ( - ExprKind::List { - elts: elts_a, - ctx: ctx_a, - }, - ExprKind::List { - elts: elts_b, - ctx: ctx_b, - }, - ) => { - ctx_a == ctx_b - && elts_a.len() == elts_b.len() - && elts_a.iter().zip(elts_b).all(|(a, b)| expr(a, b)) - } - ( - ExprKind::Tuple { - elts: elts_a, - ctx: ctx_a, - }, - ExprKind::Tuple { - elts: elts_b, - ctx: ctx_b, - }, - ) => { - ctx_a == ctx_b - && elts_a.len() == elts_b.len() - && elts_a.iter().zip(elts_b).all(|(a, b)| expr(a, b)) - } - ( - ExprKind::Slice { - lower: lower_a, - upper: upper_a, - step: step_a, - }, - ExprKind::Slice { - lower: lower_b, - upper: upper_b, - step: step_b, - }, - ) => { - option_expr(lower_a.as_deref(), lower_b.as_deref()) - && option_expr(upper_a.as_deref(), upper_b.as_deref()) - && option_expr(step_a.as_deref(), step_b.as_deref()) - } - _ => false, - } -} - -fn option_expr(a: Option<&Expr>, b: Option<&Expr>) -> bool { - match (a, b) { - (Some(a), Some(b)) => expr(a, b), - (None, None) => true, - _ => false, - } -} - -fn arguments(a: &Arguments, b: &Arguments) -> bool { - a.posonlyargs.len() == b.posonlyargs.len() - && a.posonlyargs - .iter() - .zip(b.posonlyargs.iter()) - .all(|(a, b)| arg(a, b)) - && a.args.len() == b.args.len() - && a.args.iter().zip(b.args.iter()).all(|(a, b)| arg(a, b)) - && option_arg(a.vararg.as_deref(), b.vararg.as_deref()) - && a.kwonlyargs.len() == b.kwonlyargs.len() - && a.kwonlyargs - .iter() - .zip(b.kwonlyargs.iter()) - .all(|(a, b)| arg(a, b)) - && a.kw_defaults.len() == b.kw_defaults.len() - && a.kw_defaults - .iter() - .zip(b.kw_defaults.iter()) - .all(|(a, b)| expr(a, b)) - && option_arg(a.kwarg.as_deref(), b.kwarg.as_deref()) - && a.defaults.len() == b.defaults.len() - && a.defaults - .iter() - .zip(b.defaults.iter()) - .all(|(a, b)| expr(a, b)) -} - -fn arg(a: &Arg, b: &Arg) -> bool { - a.node.arg == b.node.arg - && option_expr(a.node.annotation.as_deref(), b.node.annotation.as_deref()) -} - -fn option_arg(a: Option<&Arg>, b: Option<&Arg>) -> bool { - match (a, b) { - (Some(a), Some(b)) => arg(a, b), - (None, None) => true, - _ => false, - } -} - -fn keyword(a: &Keyword, b: &Keyword) -> bool { - a.node.arg == b.node.arg && expr(&a.node.value, &b.node.value) -} - -fn comprehension(a: &Comprehension, b: &Comprehension) -> bool { - expr(&a.iter, &b.iter) - && expr(&a.target, &b.target) - && a.ifs.len() == b.ifs.len() - && a.ifs.iter().zip(b.ifs.iter()).all(|(a, b)| expr(a, b)) -} diff --git a/src/ast/comparable.rs b/src/ast/comparable.rs new file mode 100644 index 0000000000000..491fe73e43525 --- /dev/null +++ b/src/ast/comparable.rs @@ -0,0 +1,524 @@ +//! An equivalent object hierarchy to the `Expr` hierarchy, but with the ability +//! to compare expressions for equality (via `Eq` and `Hash`). + +use num_bigint::BigInt; +use rustpython_ast::{ + Arg, Arguments, Boolop, Cmpop, Comprehension, Constant, Expr, ExprContext, ExprKind, Keyword, + Operator, Unaryop, +}; + +#[derive(Debug, PartialEq, Eq, Hash)] +pub enum ComparableExprContext { + Load, + Store, + Del, +} + +impl From<&ExprContext> for ComparableExprContext { + fn from(ctx: &ExprContext) -> Self { + match ctx { + ExprContext::Load => Self::Load, + ExprContext::Store => Self::Store, + ExprContext::Del => Self::Del, + } + } +} + +#[derive(Debug, PartialEq, Eq, Hash)] +pub enum ComparableBoolop { + And, + Or, +} + +impl From<&Boolop> for ComparableBoolop { + fn from(op: &Boolop) -> Self { + match op { + Boolop::And => Self::And, + Boolop::Or => Self::Or, + } + } +} + +#[derive(Debug, PartialEq, Eq, Hash)] +pub enum ComparableOperator { + Add, + Sub, + Mult, + MatMult, + Div, + Mod, + Pow, + LShift, + RShift, + BitOr, + BitXor, + BitAnd, + FloorDiv, +} + +impl From<&Operator> for ComparableOperator { + fn from(op: &Operator) -> Self { + match op { + Operator::Add => Self::Add, + Operator::Sub => Self::Sub, + Operator::Mult => Self::Mult, + Operator::MatMult => Self::MatMult, + Operator::Div => Self::Div, + Operator::Mod => Self::Mod, + Operator::Pow => Self::Pow, + Operator::LShift => Self::LShift, + Operator::RShift => Self::RShift, + Operator::BitOr => Self::BitOr, + Operator::BitXor => Self::BitXor, + Operator::BitAnd => Self::BitAnd, + Operator::FloorDiv => Self::FloorDiv, + } + } +} + +#[derive(Debug, PartialEq, Eq, Hash)] +pub enum ComparableUnaryop { + Invert, + Not, + UAdd, + USub, +} + +impl From<&Unaryop> for ComparableUnaryop { + fn from(op: &Unaryop) -> Self { + match op { + Unaryop::Invert => Self::Invert, + Unaryop::Not => Self::Not, + Unaryop::UAdd => Self::UAdd, + Unaryop::USub => Self::USub, + } + } +} + +#[derive(Debug, PartialEq, Eq, Hash)] +pub enum ComparableCmpop { + Eq, + NotEq, + Lt, + LtE, + Gt, + GtE, + Is, + IsNot, + In, + NotIn, +} + +impl From<&Cmpop> for ComparableCmpop { + fn from(op: &Cmpop) -> Self { + match op { + Cmpop::Eq => Self::Eq, + Cmpop::NotEq => Self::NotEq, + Cmpop::Lt => Self::Lt, + Cmpop::LtE => Self::LtE, + Cmpop::Gt => Self::Gt, + Cmpop::GtE => Self::GtE, + Cmpop::Is => Self::Is, + Cmpop::IsNot => Self::IsNot, + Cmpop::In => Self::In, + Cmpop::NotIn => Self::NotIn, + } + } +} + +#[derive(Debug, PartialEq, Eq, Hash)] +pub enum ComparableConstant<'a> { + None, + Bool(&'a bool), + Str(&'a str), + Bytes(&'a [u8]), + Int(&'a BigInt), + Tuple(Vec>), + Float(u64), + Complex { real: u64, imag: u64 }, + Ellipsis, +} + +impl<'a> From<&'a Constant> for ComparableConstant<'a> { + fn from(constant: &'a Constant) -> Self { + match constant { + Constant::None => Self::None, + Constant::Bool(value) => Self::Bool(value), + Constant::Str(value) => Self::Str(value), + Constant::Bytes(value) => Self::Bytes(value), + Constant::Int(value) => Self::Int(value), + Constant::Tuple(value) => { + Self::Tuple(value.iter().map(std::convert::Into::into).collect()) + } + Constant::Float(value) => Self::Float(value.to_bits()), + Constant::Complex { real, imag } => Self::Complex { + real: real.to_bits(), + imag: imag.to_bits(), + }, + Constant::Ellipsis => Self::Ellipsis, + } + } +} + +#[derive(Debug, PartialEq, Eq, Hash)] +pub struct ComparableArguments<'a> { + pub posonlyargs: Vec>, + pub args: Vec>, + pub vararg: Option>, + pub kwonlyargs: Vec>, + pub kw_defaults: Vec>, + pub kwarg: Option>, + pub defaults: Vec>, +} + +impl<'a> From<&'a Arguments> for ComparableArguments<'a> { + fn from(arguments: &'a Arguments) -> Self { + Self { + posonlyargs: arguments + .posonlyargs + .iter() + .map(std::convert::Into::into) + .collect(), + args: arguments + .args + .iter() + .map(std::convert::Into::into) + .collect(), + vararg: arguments.vararg.as_ref().map(std::convert::Into::into), + kwonlyargs: arguments + .kwonlyargs + .iter() + .map(std::convert::Into::into) + .collect(), + kw_defaults: arguments + .kw_defaults + .iter() + .map(std::convert::Into::into) + .collect(), + kwarg: arguments.vararg.as_ref().map(std::convert::Into::into), + defaults: arguments + .defaults + .iter() + .map(std::convert::Into::into) + .collect(), + } + } +} + +impl<'a> From<&'a Box> for ComparableArg<'a> { + fn from(arg: &'a Box) -> Self { + (&**arg).into() + } +} + +#[derive(Debug, PartialEq, Eq, Hash)] +pub struct ComparableArg<'a> { + pub arg: &'a str, + pub annotation: Option>>, + pub type_comment: Option<&'a str>, +} + +impl<'a> From<&'a Arg> for ComparableArg<'a> { + fn from(arg: &'a Arg) -> Self { + Self { + arg: &arg.node.arg, + annotation: arg.node.annotation.as_ref().map(std::convert::Into::into), + type_comment: arg.node.type_comment.as_deref(), + } + } +} + +#[derive(Debug, PartialEq, Eq, Hash)] +pub struct ComparableKeyword<'a> { + pub arg: Option<&'a str>, + pub value: ComparableExpr<'a>, +} + +impl<'a> From<&'a Keyword> for ComparableKeyword<'a> { + fn from(keyword: &'a Keyword) -> Self { + Self { + arg: keyword.node.arg.as_deref(), + value: (&keyword.node.value).into(), + } + } +} + +#[derive(Debug, PartialEq, Eq, Hash)] +pub struct ComparableComprehension<'a> { + pub target: ComparableExpr<'a>, + pub iter: ComparableExpr<'a>, + pub ifs: Vec>, + pub is_async: &'a usize, +} + +impl<'a> From<&'a Comprehension> for ComparableComprehension<'a> { + fn from(comprehension: &'a Comprehension) -> Self { + Self { + target: (&comprehension.target).into(), + iter: (&comprehension.iter).into(), + ifs: comprehension + .ifs + .iter() + .map(std::convert::Into::into) + .collect(), + is_async: &comprehension.is_async, + } + } +} + +#[derive(Debug, PartialEq, Eq, Hash)] +pub enum ComparableExpr<'a> { + BoolOp { + op: ComparableBoolop, + values: Vec>, + }, + NamedExpr { + target: Box>, + value: Box>, + }, + BinOp { + left: Box>, + op: ComparableOperator, + right: Box>, + }, + UnaryOp { + op: ComparableUnaryop, + operand: Box>, + }, + Lambda { + args: ComparableArguments<'a>, + body: Box>, + }, + IfExp { + test: Box>, + body: Box>, + orelse: Box>, + }, + Dict { + keys: Vec>, + values: Vec>, + }, + Set { + elts: Vec>, + }, + ListComp { + elt: Box>, + generators: Vec>, + }, + SetComp { + elt: Box>, + generators: Vec>, + }, + DictComp { + key: Box>, + value: Box>, + generators: Vec>, + }, + GeneratorExp { + elt: Box>, + generators: Vec>, + }, + Await { + value: Box>, + }, + Yield { + value: Option>>, + }, + YieldFrom { + value: Box>, + }, + Compare { + left: Box>, + ops: Vec, + comparators: Vec>, + }, + Call { + func: Box>, + args: Vec>, + keywords: Vec>, + }, + FormattedValue { + value: Box>, + conversion: &'a usize, + format_spec: Option>>, + }, + JoinedStr { + values: Vec>, + }, + Constant { + value: ComparableConstant<'a>, + kind: Option<&'a str>, + }, + Attribute { + value: Box>, + attr: &'a str, + ctx: ComparableExprContext, + }, + Subscript { + value: Box>, + slice: Box>, + ctx: ComparableExprContext, + }, + Starred { + value: Box>, + ctx: ComparableExprContext, + }, + Name { + id: &'a str, + ctx: ComparableExprContext, + }, + List { + elts: Vec>, + ctx: ComparableExprContext, + }, + Tuple { + elts: Vec>, + ctx: ComparableExprContext, + }, + Slice { + lower: Option>>, + upper: Option>>, + step: Option>>, + }, +} + +impl<'a> From<&'a Box> for Box> { + fn from(expr: &'a Box) -> Self { + Box::new((&**expr).into()) + } +} + +impl<'a> From<&'a Expr> for ComparableExpr<'a> { + fn from(expr: &'a Expr) -> Self { + match &expr.node { + ExprKind::BoolOp { op, values } => Self::BoolOp { + op: op.into(), + values: values.iter().map(std::convert::Into::into).collect(), + }, + ExprKind::NamedExpr { target, value } => Self::NamedExpr { + target: target.into(), + value: value.into(), + }, + ExprKind::BinOp { left, op, right } => Self::BinOp { + left: left.into(), + op: op.into(), + right: right.into(), + }, + ExprKind::UnaryOp { op, operand } => Self::UnaryOp { + op: op.into(), + operand: operand.into(), + }, + ExprKind::Lambda { args, body } => Self::Lambda { + args: (&**args).into(), + body: body.into(), + }, + ExprKind::IfExp { test, body, orelse } => Self::IfExp { + test: test.into(), + body: body.into(), + orelse: orelse.into(), + }, + ExprKind::Dict { keys, values } => Self::Dict { + keys: keys.iter().map(std::convert::Into::into).collect(), + values: values.iter().map(std::convert::Into::into).collect(), + }, + ExprKind::Set { elts } => Self::Set { + elts: elts.iter().map(std::convert::Into::into).collect(), + }, + ExprKind::ListComp { elt, generators } => Self::ListComp { + elt: elt.into(), + generators: generators.iter().map(std::convert::Into::into).collect(), + }, + ExprKind::SetComp { elt, generators } => Self::SetComp { + elt: elt.into(), + generators: generators.iter().map(std::convert::Into::into).collect(), + }, + ExprKind::DictComp { + key, + value, + generators, + } => Self::DictComp { + key: key.into(), + value: value.into(), + generators: generators.iter().map(std::convert::Into::into).collect(), + }, + ExprKind::GeneratorExp { elt, generators } => Self::GeneratorExp { + elt: elt.into(), + generators: generators.iter().map(std::convert::Into::into).collect(), + }, + ExprKind::Await { value } => Self::Await { + value: value.into(), + }, + ExprKind::Yield { value } => Self::Yield { + value: value.as_ref().map(std::convert::Into::into), + }, + ExprKind::YieldFrom { value } => Self::YieldFrom { + value: value.into(), + }, + ExprKind::Compare { + left, + ops, + comparators, + } => Self::Compare { + left: left.into(), + ops: ops.iter().map(std::convert::Into::into).collect(), + comparators: comparators.iter().map(std::convert::Into::into).collect(), + }, + ExprKind::Call { + func, + args, + keywords, + } => Self::Call { + func: func.into(), + args: args.iter().map(std::convert::Into::into).collect(), + keywords: keywords.iter().map(std::convert::Into::into).collect(), + }, + ExprKind::FormattedValue { + value, + conversion, + format_spec, + } => Self::FormattedValue { + value: value.into(), + conversion, + format_spec: format_spec.as_ref().map(std::convert::Into::into), + }, + ExprKind::JoinedStr { values } => Self::JoinedStr { + values: values.iter().map(std::convert::Into::into).collect(), + }, + ExprKind::Constant { value, kind } => Self::Constant { + value: value.into(), + kind: kind.as_ref().map(String::as_str), + }, + ExprKind::Attribute { value, attr, ctx } => Self::Attribute { + value: value.into(), + attr, + ctx: ctx.into(), + }, + ExprKind::Subscript { value, slice, ctx } => Self::Subscript { + value: value.into(), + slice: slice.into(), + ctx: ctx.into(), + }, + ExprKind::Starred { value, ctx } => Self::Starred { + value: value.into(), + ctx: ctx.into(), + }, + ExprKind::Name { id, ctx } => Self::Name { + id, + ctx: ctx.into(), + }, + ExprKind::List { elts, ctx } => Self::List { + elts: elts.iter().map(std::convert::Into::into).collect(), + ctx: ctx.into(), + }, + ExprKind::Tuple { elts, ctx } => Self::Tuple { + elts: elts.iter().map(std::convert::Into::into).collect(), + ctx: ctx.into(), + }, + ExprKind::Slice { lower, upper, step } => Self::Slice { + lower: lower.as_ref().map(std::convert::Into::into), + upper: upper.as_ref().map(std::convert::Into::into), + step: step.as_ref().map(std::convert::Into::into), + }, + } + } +} diff --git a/src/ast/mod.rs b/src/ast/mod.rs index a4e54e0bc244a..9121e23a5860f 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -1,6 +1,6 @@ pub mod branch_detection; pub mod cast; -pub mod cmp; +pub mod comparable; pub mod function_type; pub mod helpers; pub mod operations; diff --git a/src/pyflakes/plugins/repeated_keys.rs b/src/pyflakes/plugins/repeated_keys.rs index 143f0c7c871d7..2b485f42d9df1 100644 --- a/src/pyflakes/plugins/repeated_keys.rs +++ b/src/pyflakes/plugins/repeated_keys.rs @@ -1,29 +1,25 @@ use std::hash::{BuildHasherDefault, Hash}; -use rustc_hash::FxHashMap; +use rustc_hash::{FxHashMap, FxHashSet}; use rustpython_ast::{Expr, ExprKind}; -use crate::ast::cmp; +use crate::ast::comparable::{ComparableConstant, ComparableExpr}; use crate::ast::helpers::unparse_expr; use crate::ast::types::Range; use crate::autofix::Fix; use crate::checkers::ast::Checker; use crate::registry::{Check, CheckCode}; -use crate::source_code_style::SourceCodeStyleDetector; use crate::violations; #[derive(Debug, Eq, PartialEq, Hash)] enum DictionaryKey<'a> { - Constant(String), + Constant(ComparableConstant<'a>), Variable(&'a str), } -fn into_dictionary_key<'a>( - expr: &'a Expr, - stylist: &SourceCodeStyleDetector, -) -> Option> { +fn into_dictionary_key(expr: &Expr) -> Option { match &expr.node { - ExprKind::Constant { .. } => Some(DictionaryKey::Constant(unparse_expr(expr, stylist))), + ExprKind::Constant { value, .. } => Some(DictionaryKey::Constant(value.into())), ExprKind::Name { id, .. } => Some(DictionaryKey::Variable(id)), _ => None, } @@ -32,23 +28,26 @@ fn into_dictionary_key<'a>( /// F601, F602 pub fn repeated_keys(checker: &mut Checker, keys: &[Expr], values: &[Expr]) { // Generate a map from key to (index, value). - let mut seen: FxHashMap> = + let mut seen: FxHashMap> = FxHashMap::with_capacity_and_hasher(keys.len(), BuildHasherDefault::default()); // Detect duplicate keys. for (i, key) in keys.iter().enumerate() { - if let Some(key) = into_dictionary_key(key, checker.style) { + if let Some(key) = into_dictionary_key(key) { if let Some(seen_values) = seen.get_mut(&key) { match key { - DictionaryKey::Constant(key) => { + DictionaryKey::Constant(..) => { if checker.settings.enabled.contains(&CheckCode::F601) { - let repeated_value = - seen_values.iter().any(|value| cmp::expr(value, &values[i])); + let comparable_value: ComparableExpr = (&values[i]).into(); + let is_duplicate_value = seen_values.contains(&comparable_value); let mut check = Check::new( - violations::MultiValueRepeatedKeyLiteral(key, repeated_value), + violations::MultiValueRepeatedKeyLiteral( + unparse_expr(&keys[i], checker.style), + is_duplicate_value, + ), Range::from_located(&keys[i]), ); - if repeated_value { + if is_duplicate_value { if checker.patch(&CheckCode::F601) { check.amend(Fix::deletion( values[i - 1].end_location.unwrap(), @@ -56,23 +55,23 @@ pub fn repeated_keys(checker: &mut Checker, keys: &[Expr], values: &[Expr]) { )); } } else { - seen_values.push(&values[i]); + seen_values.insert(comparable_value); } checker.checks.push(check); } } DictionaryKey::Variable(key) => { if checker.settings.enabled.contains(&CheckCode::F602) { - let repeated_value = - seen_values.iter().any(|value| cmp::expr(value, &values[i])); + let comparable_value: ComparableExpr = (&values[i]).into(); + let is_duplicate_value = seen_values.contains(&comparable_value); let mut check = Check::new( violations::MultiValueRepeatedKeyVariable( key.to_string(), - repeated_value, + is_duplicate_value, ), Range::from_located(&keys[i]), ); - if repeated_value { + if is_duplicate_value { if checker.patch(&CheckCode::F602) { check.amend(Fix::deletion( values[i - 1].end_location.unwrap(), @@ -80,14 +79,14 @@ pub fn repeated_keys(checker: &mut Checker, keys: &[Expr], values: &[Expr]) { )); } } else { - seen_values.push(&values[i]); + seen_values.insert(comparable_value); } checker.checks.push(check); } } } } else { - seen.insert(key, vec![&values[i]]); + seen.insert(key, FxHashSet::from_iter([(&values[i]).into()])); } } }