user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Tighten up assignment operator representations.

Browse source 
In the AST, currently we use `BinOpKind` within `ExprKind::AssignOp` and
`AssocOp::AssignOp`, even though this allows some nonsensical
combinations. E.g. there is no `&&=` operator. Likewise for HIR and
THIR.

This commit introduces `AssignOpKind` which only includes the ten
assignable operators, and uses it in `ExprKind::AssignOp` and
`AssocOp::AssignOp`. (And does similar things for `hir::ExprKind` and
`thir::ExprKind`.) This avoids the possibility of nonsensical
combinations, as seen by the removal of the `bug!` case in
`lang_item_for_binop`.

The commit is mostly plumbing, including:
- Adds an `impl From<AssignOpKind> for BinOpKind` (AST) and `impl
  From<AssignOp> for BinOp` (MIR/THIR).
- `BinOpCategory` can now be created from both `BinOpKind` and
  `AssignOpKind`.
- Replaces the `IsAssign` type with `Op`, which has more information and
  a few methods.
- `suggest_swapping_lhs_and_rhs`: moves the condition to the call site,
  it's easier that way.
- `check_expr_inner`: had to factor out some code into a separate
  method.

I'm on the fence about whether avoiding the nonsensical combinations is
worth the extra code.
This commit is contained in:
Nicholas Nethercote 2024-12-20 10:15:05 +11:00
parent ac8ccf09b4
commit ddcb370bc6
26 changed files with 391 additions and 239 deletions
Download patch file Download diff file Expand all files Collapse all files

4
src/tools/clippy/clippy_lints/src/format_push_string.rs
View file

@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::higher;
use clippy_utils::ty::is_type_lang_item;
use rustc_hir::{BinOpKind, Expr, ExprKind, LangItem, MatchSource};
use rustc_hir::{AssignOpKind, Expr, ExprKind, LangItem, MatchSource};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::sym;
@ -77,7 +77,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatPushString {
return;
}
},
ExprKind::AssignOp(op, left, arg) if op.node == BinOpKind::Add && is_string(cx, left) => arg,
ExprKind::AssignOp(op, left, arg) if op.node == AssignOpKind::AddAssign && is_string(cx, left) => arg,
_ => return,
};
if is_format(cx, arg) {

4
src/tools/clippy/clippy_lints/src/implicit_saturating_add.rs
View file

@ -5,7 +5,7 @@ use clippy_utils::source::snippet_with_context;
use rustc_ast::ast::{LitIntType, LitKind};
use rustc_data_structures::packed::Pu128;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, Stmt, StmtKind};
use rustc_hir::{AssignOpKind, BinOpKind, Block, Expr, ExprKind, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{IntTy, Ty, UintTy};
use rustc_session::declare_lint_pass;
@ -68,7 +68,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingAdd {
&& ex.span.ctxt() == ctxt
&& expr1.span.ctxt() == ctxt
&& clippy_utils::SpanlessEq::new(cx).eq_expr(l, target)
&& BinOpKind::Add == op1.node
&& AssignOpKind::AddAssign == op1.node
&& let ExprKind::Lit(lit) = value.kind
&& let LitKind::Int(Pu128(1), LitIntType::Unsuffixed) = lit.node
&& block.expr.is_none()

4
src/tools/clippy/clippy_lints/src/implicit_saturating_sub.rs
View file

@ -8,7 +8,7 @@ use clippy_utils::{
use rustc_ast::ast::LitKind;
use rustc_data_structures::packed::Pu128;
use rustc_errors::Applicability;
use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind, QPath};
use rustc_hir::{AssignOpKind, BinOp, BinOpKind, Expr, ExprKind, QPath};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::impl_lint_pass;
use rustc_span::Span;
@ -366,7 +366,7 @@ fn subtracts_one<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<&'a Exp
match peel_blocks_with_stmt(expr).kind {
ExprKind::AssignOp(ref op1, target, value) => {
// Check if literal being subtracted is one
(BinOpKind::Sub == op1.node && is_integer_literal(value, 1)).then_some(target)
(AssignOpKind::SubAssign == op1.node && is_integer_literal(value, 1)).then_some(target)
},
ExprKind::Assign(target, value, _) => {
if let ExprKind::Binary(ref op1, left1, right1) = value.kind

6
src/tools/clippy/clippy_lints/src/loops/utils.rs
View file

@ -3,7 +3,9 @@ use clippy_utils::{get_parent_expr, is_integer_const, path_to_local, path_to_loc
use rustc_ast::ast::{LitIntType, LitKind};
use rustc_errors::Applicability;
use rustc_hir::intravisit::{Visitor, walk_expr, walk_local};
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, HirId, HirIdMap, LetStmt, Mutability, PatKind};
use rustc_hir::{
AssignOpKind, BorrowKind, Expr, ExprKind, HirId, HirIdMap, LetStmt, Mutability, PatKind
};
use rustc_lint::LateContext;
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::{self, Ty};
@ -58,7 +60,7 @@ impl<'tcx> Visitor<'tcx> for IncrementVisitor<'_, 'tcx> {
match parent.kind {
ExprKind::AssignOp(op, lhs, rhs) => {
if lhs.hir_id == expr.hir_id {
*state = if op.node == BinOpKind::Add
*state = if op.node == AssignOpKind::AddAssign
&& is_integer_const(self.cx, rhs, 1)
&& *state == IncrementVisitorVarState::Initial
&& self.depth == 0

5
src/tools/clippy/clippy_lints/src/mixed_read_write_in_expression.rs
View file

@ -261,10 +261,11 @@ fn check_expr<'tcx>(vis: &mut ReadVisitor<'_, 'tcx>, expr: &'tcx Expr<'_>) -> St
| ExprKind::Assign(..)
| ExprKind::Index(..)
| ExprKind::Repeat(_, _)
| ExprKind::Struct(_, _, _) => {
| ExprKind::Struct(_, _, _)
| ExprKind::AssignOp(_, _, _) => {
walk_expr(vis, expr);
},
ExprKind::Binary(op, _, _) | ExprKind::AssignOp(op, _, _) => {
ExprKind::Binary(op, _, _) => {
if op.node == BinOpKind::And || op.node == BinOpKind::Or {
// x && y and x || y always evaluate x first, so these are
// strictly sequenced.

5
src/tools/clippy/clippy_lints/src/operators/arithmetic_side_effects.rs
View file

@ -335,9 +335,12 @@ impl<'tcx> LateLintPass<'tcx> for ArithmeticSideEffects {
return;
}
match &expr.kind {
hir::ExprKind::AssignOp(op, lhs, rhs) | hir::ExprKind::Binary(op, lhs, rhs) => {
hir::ExprKind::Binary(op, lhs, rhs) => {
self.manage_bin_ops(cx, expr, op.node, lhs, rhs);
},
hir::ExprKind::AssignOp(op, lhs, rhs) => {
self.manage_bin_ops(cx, expr, op.node.into(), lhs, rhs);
},
hir::ExprKind::MethodCall(ps, receiver, args, _) => {
self.manage_method_call(args, cx, expr, ps, receiver);
},

7
src/tools/clippy/clippy_lints/src/operators/mod.rs
View file

@ -913,9 +913,10 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
);
},
ExprKind::AssignOp(op, lhs, rhs) => {
self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);
misrefactored_assign_op::check(cx, e, op.node, lhs, rhs);
modulo_arithmetic::check(cx, e, op.node, lhs, rhs, false);
let bin_op = op.node.into();
self.arithmetic_context.check_binary(cx, e, bin_op, lhs, rhs);
misrefactored_assign_op::check(cx, e, bin_op, lhs, rhs);
modulo_arithmetic::check(cx, e, bin_op, lhs, rhs, false);
},
ExprKind::Assign(lhs, rhs, _) => {
assign_op_pattern::check(cx, e, lhs, rhs);

28
src/tools/clippy/clippy_lints/src/suspicious_trait_impl.rs
View file

@ -5,6 +5,7 @@ use core::ops::ControlFlow;
use rustc_hir as hir;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::Span;
declare_clippy_lint! {
/// ### What it does
@ -56,8 +57,27 @@ declare_lint_pass!(SuspiciousImpl => [SUSPICIOUS_ARITHMETIC_IMPL, SUSPICIOUS_OP_
impl<'tcx> LateLintPass<'tcx> for SuspiciousImpl {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
if let hir::ExprKind::Binary(binop, _, _) | hir::ExprKind::AssignOp(binop, ..) = expr.kind
&& let Some((binop_trait_lang, op_assign_trait_lang)) = binop_traits(binop.node)
match expr.kind {
hir::ExprKind::Binary(op, _, _) => {
self.check_expr_inner(cx, expr, op.node, op.span);
}
hir::ExprKind::AssignOp(op, _, _) => {
self.check_expr_inner(cx, expr, op.node.into(), op.span);
}
_ => {}
}
}
}
impl<'tcx> SuspiciousImpl {
fn check_expr_inner(
&mut self,
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
binop: hir::BinOpKind,
span: Span,
) {
if let Some((binop_trait_lang, op_assign_trait_lang)) = binop_traits(binop)
&& let Some(binop_trait_id) = cx.tcx.lang_items().get(binop_trait_lang)
&& let Some(op_assign_trait_id) = cx.tcx.lang_items().get(op_assign_trait_lang)
@ -82,10 +102,10 @@ impl<'tcx> LateLintPass<'tcx> for SuspiciousImpl {
span_lint(
cx,
lint,
binop.span,
span,
format!(
"suspicious use of `{}` in `{}` impl",
binop.node.as_str(),
binop.as_str(),
cx.tcx.item_name(trait_id)
),
);

4
src/tools/clippy/clippy_lints/src/swap.rs
View file

@ -10,7 +10,7 @@ use rustc_data_structures::fx::FxIndexSet;
use rustc_hir::intravisit::{Visitor, walk_expr};
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, LetStmt, PatKind, QPath, Stmt, StmtKind};
use rustc_hir::{AssignOpKind, Block, Expr, ExprKind, LetStmt, PatKind, QPath, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::ty;
use rustc_session::declare_lint_pass;
@ -307,7 +307,7 @@ fn extract_sides_of_xor_assign<'a, 'hir>(
if let StmtKind::Semi(expr) = stmt.kind
&& let ExprKind::AssignOp(
Spanned {
node: BinOpKind::BitXor,
node: AssignOpKind::BitXorAssign,
..
},
lhs,

2
src/tools/clippy/clippy_utils/src/sugg.rs
View file

@ -357,7 +357,7 @@ fn binop_to_string(op: AssocOp, lhs: &str, rhs: &str) -> String {
match op {
AssocOp::Binary(op) => format!("{lhs} {} {rhs}", op.as_str()),
AssocOp::Assign => format!("{lhs} = {rhs}"),
AssocOp::AssignOp(op) => format!("{lhs} {}= {rhs}", op.as_str()),
AssocOp::AssignOp(op) => format!("{lhs} {} {rhs}", op.as_str()),
AssocOp::Cast => format!("{lhs} as {rhs}"),
AssocOp::Range(limits) => format!("{lhs}{}{rhs}", limits.as_str()),
}