rust-clippy/clippy_lints/src/eq_op.rs

259 lines
12 KiB
Rust
Raw Normal View History

use clippy_utils::diagnostics::{multispan_sugg, span_lint, span_lint_and_then};
use clippy_utils::source::snippet;
2021-03-13 23:01:03 +00:00
use clippy_utils::ty::{implements_trait, is_copy};
use clippy_utils::{ast_utils::is_useless_with_eq_exprs, eq_expr_value, higher, is_expn_of, is_in_test_function};
use if_chain::if_chain;
use rustc_errors::Applicability;
add suspicious_operation_groupings lint run `cargo dev new_lint --category correctness --name suspicious_chained_operators --pass early` add (currently failing) tests for suspicious_chained_operators add some tests to answer a question that came up during implementation write usage code for functions we'll need to find or create Complete left-right tracking TODO get it compiling with several `todo!` invocations. refactor to a set of incomplete functions that don't expect to be able to edit a `Span` create placeholder for `suggestion_with_swapped_ident` function and correct some comments add `inside_larger_boolean_expression` test fill out `get_ident` and `suggestion_with_swapped_ident` Implementi the `IdentIter` start on implementing the `IdentIter` handle the `ExprKind::Path` case in `IdentIter` on second thought, make the iterator type dynamic so we don't need an explicit type for each one we will need handle `ExprKind::MacCall` in `IdentIter` Try handling `box x` expressions restructure `IdentIter` set `self.done` when returning `None` Handle `ExprKind::Array` reduce duplication with a macro that we expect to use several more times handle ExprKind::Call add `new_p` convenience method handle `MethodCall` handle `Tup` and `Binary` handle `Unary` simplify by not returning an additional `Expr` from the `IdentIter` add cross product test against false positives rename suspicious_chained_operators to suspicious_operation_groupings within files For the record, the exact commands run were: find . -type f -name "*.md" -exec sed -i 's/suspicious_chained_operators/suspicious_operation_groupings/g' {} + find . -type f -name "*.rs" -exec sed -i 's/suspicious_chained_operators/suspicious_operation_groupings/g' {} + find . -type f -name "*.rs" -exec sed -i 's/SUSPICIOUS_CHAINED_OPERATORS/SUSPICIOUS_OPERATION_GROUPINGS/g' {} + find . -type f -name "*.rs" -exec sed -i 's/SuspiciousChainedOperators/SuspiciousOperationGroupings/g' {} + Also: rename file to match module name rename test file to match lint name start implementing `IdentDifference` creation add `IdentIter` utility use `ident_iter::IdentIter` fix bug in `suggestion_with_swapped_ident` add `inside_if_statements` test implement `Add` `todo`s register `SuspiciousOperationGroupings` lint pass fill in `chained_binops`, and fill in a stopgap version of `ident_difference_expr`, but then notice that the lint does not seem to ever be run in the tests run `cargo dev update_lints` and not that the `suspicious_operation_groupings` lint still does not seem to be run fix base index incrementing bug fix paired_identifiers bug, and remove ident from `Single` change help prefix and note our first successful lint messages! add odd_number_of_pairs test get the `non_boolean_operators` test passing, with two copies of the error message extract `is_useless_with_eq_exprs` so we can know when `eq_op` will already handle something add `not_caught_by_eq_op` tests since `s1.b * s1.b` was (reasonably) not caught by `eq_op` cover the case where the change should be made on either side of the expression with `not_caught_by_eq_op` tests produce the expected suggestion on the `not_caught_by_eq_op_middle_change_left` test confirm that the previous tests still pass and update references fix early continue bug and get `not_caught_by_eq_op_middle_change_right` passing note that `not_caught_by_eq_op_start` already passes fix bugs based on misunderstanding of what `Iterator::skip` does, and note that `not_caught_by_eq_op_end` now passes add several parens tests and make some of them pass handle parens inside `chained_binops_helper` and note that this makes several tests pass get `inside_larger_boolean_expression_with_unsorted_ops` test passing by extracting out `check_same_op_binops` function also run `cargo dev fmt` note that `inside_function_call` already passes add another `if_statement` test remove the matching op requirement, making `inside_larger_boolean_expression_with_unsorted_ops` pass prevent non-change suggestions from being emitted get the `Nested` tests passing, and remove apparently false note about eq_op add a test to justify comment in `ident_difference_expr_with_base_location` but find that the failure mode seems different than expected complete `todo` making `do_not_give_bad_suggestions_for_this_unusual_expr` pass and add some more tests that already pass add test to `eq_op` note that `inside_fn_with_similar_expression` already passes fix `inside_an_if_statement` and note that it already passes attempt to implement if statement extraction and notice that we don't seem to handle unary ops correctly add `maximum_unary_minus_right_tree` test and make it pass add two tests and note one of them passes filter out unary operations in several places, and find that the issue seems to be that we don't currently recognize the error in `multiple_comparison_types_and_unary_minus` even so. remove filtering that was causing bad suggestions remove tests that were deemed too much for now run `cargo dev fmt` correct eq_op post-merge fill out the description and delete debugging code run `cargo dev update_lints` update eq_op references add parens to work around rustfmt issue #3666 and run rustfmt https://github.com/rust-lang/rustfmt/issues/3666#issuecomment-714612257 update references after formatting fix dogfood issues fix multi-cursor edit fix missed dogfood error fix more dogfood pedantic issues, including function length even more nesting insert hidden definition of Vec3 so docs compile add spaces to second struct def reword test description comment Co-authored-by: llogiq <bogusandre@gmail.com> add local `use BinOpKind::*;` Apply suggestions from code review Co-authored-by: llogiq <bogusandre@gmail.com> switch `SUSPICIOUS_OPERATION_GROUPINGS` to a style lint run `cargo dev update_lints` put both usages of `op_types` in the same closure to satisfy `borrowck` fix compile error
2020-11-07 23:00:42 +00:00
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
2020-01-11 11:37:08 +00:00
use rustc_session::{declare_lint_pass, declare_tool_lint};
2018-03-28 13:24:26 +00:00
declare_clippy_lint! {
/// ### What it does
/// Checks for equal operands to comparison, logical and
/// bitwise, difference and division binary operators (`==`, `>`, etc., `&&`,
/// `||`, `&`, `|`, `^`, `-` and `/`).
///
/// ### Why is this bad?
/// This is usually just a typo or a copy and paste error.
///
/// ### Known problems
/// False negatives: We had some false positives regarding
/// calls (notably [racer](https://github.com/phildawes/racer) had one instance
/// of `x.pop() && x.pop()`), so we removed matching any function or method
/// calls. We may introduce a list of known pure functions in the future.
///
/// ### Example
/// ```rust
/// # let x = 1;
/// if x + 1 == x + 1 {}
/// ```
/// or
/// ```rust
/// # let a = 3;
/// # let b = 4;
/// assert_eq!(a, a);
/// ```
Added `clippy::version` attribute to all normal lints So, some context for this, well, more a story. I'm not used to scripting, I've never really scripted anything, even if it's a valuable skill. I just never really needed it. Now, `@flip1995` correctly suggested using a script for this in `rust-clippy#7813`... And I decided to write a script using nushell because why not? This was a mistake... I spend way more time on this than I would like to admit. It has definitely been more than 4 hours. It shouldn't take that long, but me being new to scripting and nushell just wasn't a good mixture... Anyway, here is the script that creates another script which adds the versions. Fun... Just execute this on the `gh-pages` branch and the resulting `replacer.sh` in `clippy_lints` and it should all work. ```nu mv v0.0.212 rust-1.00.0; mv beta rust-1.57.0; mv master rust-1.58.0; let paths = (open ./rust-1.58.0/lints.json | select id id_span | flatten | select id path); let versions = ( ls | where name =~ "rust-" | select name | format {name}/lints.json | each { open $it | select id | insert version $it | str substring "5,11" version} | group-by id | rotate counter-clockwise id version | update version {get version | first 1} | flatten | select id version); $paths | each { |row| let version = ($versions | where id == ($row.id) | format {version}) let idu = ($row.id | str upcase) $"sed -i '0,/($idu),/{s/pub ($idu),/#[clippy::version = "($version)"]\n pub ($idu),/}' ($row.path)" } | str collect ";" | str find-replace --all '1.00.0' 'pre 1.29.0' | save "replacer.sh"; ``` And this still has some problems, but at this point I just want to be done -.-
2021-10-21 19:06:26 +00:00
#[clippy::version = "pre 1.29.0"]
pub EQ_OP,
2018-03-28 13:24:26 +00:00
correctness,
2019-01-31 01:15:29 +00:00
"equal operands on both sides of a comparison or bitwise combination (e.g., `x == x`)"
}
2018-03-28 13:24:26 +00:00
declare_clippy_lint! {
/// ### What it does
/// Checks for arguments to `==` which have their address
/// taken to satisfy a bound
/// and suggests to dereference the other argument instead
///
/// ### Why is this bad?
/// It is more idiomatic to dereference the other argument.
///
/// ### Known problems
/// None
///
/// ### Example
2019-03-05 22:23:50 +00:00
/// ```ignore
/// // Bad
/// &x == y
///
/// // Good
/// x == *y
/// ```
Added `clippy::version` attribute to all normal lints So, some context for this, well, more a story. I'm not used to scripting, I've never really scripted anything, even if it's a valuable skill. I just never really needed it. Now, `@flip1995` correctly suggested using a script for this in `rust-clippy#7813`... And I decided to write a script using nushell because why not? This was a mistake... I spend way more time on this than I would like to admit. It has definitely been more than 4 hours. It shouldn't take that long, but me being new to scripting and nushell just wasn't a good mixture... Anyway, here is the script that creates another script which adds the versions. Fun... Just execute this on the `gh-pages` branch and the resulting `replacer.sh` in `clippy_lints` and it should all work. ```nu mv v0.0.212 rust-1.00.0; mv beta rust-1.57.0; mv master rust-1.58.0; let paths = (open ./rust-1.58.0/lints.json | select id id_span | flatten | select id path); let versions = ( ls | where name =~ "rust-" | select name | format {name}/lints.json | each { open $it | select id | insert version $it | str substring "5,11" version} | group-by id | rotate counter-clockwise id version | update version {get version | first 1} | flatten | select id version); $paths | each { |row| let version = ($versions | where id == ($row.id) | format {version}) let idu = ($row.id | str upcase) $"sed -i '0,/($idu),/{s/pub ($idu),/#[clippy::version = "($version)"]\n pub ($idu),/}' ($row.path)" } | str collect ";" | str find-replace --all '1.00.0' 'pre 1.29.0' | save "replacer.sh"; ``` And this still has some problems, but at this point I just want to be done -.-
2021-10-21 19:06:26 +00:00
#[clippy::version = "pre 1.29.0"]
pub OP_REF,
2018-03-28 13:24:26 +00:00
style,
"taking a reference to satisfy the type constraints on `==`"
}
2019-04-08 20:43:55 +00:00
declare_lint_pass!(EqOp => [EQ_OP, OP_REF]);
const ASSERT_MACRO_NAMES: [&str; 4] = ["assert_eq", "assert_ne", "debug_assert_eq", "debug_assert_ne"];
impl<'tcx> LateLintPass<'tcx> for EqOp {
2019-01-13 15:19:02 +00:00
#[allow(clippy::similar_names, clippy::too_many_lines)]
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
if let ExprKind::Block(block, _) = e.kind {
for stmt in block.stmts {
for amn in &ASSERT_MACRO_NAMES {
if_chain! {
if is_expn_of(stmt.span, amn).is_some();
if let StmtKind::Semi(matchexpr) = stmt.kind;
if let Some(macro_args) = higher::extract_assert_macro_args(matchexpr);
if macro_args.len() == 2;
let (lhs, rhs) = (macro_args[0], macro_args[1]);
if eq_expr_value(cx, lhs, rhs);
2021-10-12 17:18:22 +00:00
if !is_in_test_function(cx.tcx, e.hir_id);
then {
span_lint(
cx,
EQ_OP,
lhs.span.to(rhs.span),
&format!("identical args used in this `{}!` macro call", amn),
);
}
}
}
}
}
if let ExprKind::Binary(op, left, right) = e.kind {
2019-08-19 16:30:32 +00:00
if e.span.from_expansion() {
return;
}
let macro_with_not_op = |expr_kind: &ExprKind<'_>| {
if let ExprKind::Unary(_, expr) = *expr_kind {
expr.span.from_expansion()
} else {
false
}
};
if macro_with_not_op(&left.kind) || macro_with_not_op(&right.kind) {
return;
}
2021-10-12 17:18:22 +00:00
if is_useless_with_eq_exprs(op.node.into())
&& eq_expr_value(cx, left, right)
&& !is_in_test_function(cx.tcx, e.hir_id)
{
2017-08-09 07:30:56 +00:00
span_lint(
cx,
EQ_OP,
e.span,
&format!("equal expressions as operands to `{}`", op.node.as_str()),
);
2017-04-28 16:13:09 +00:00
return;
2017-04-28 15:03:47 +00:00
}
let (trait_id, requires_ref) = match op.node {
2018-07-12 07:50:09 +00:00
BinOpKind::Add => (cx.tcx.lang_items().add_trait(), false),
BinOpKind::Sub => (cx.tcx.lang_items().sub_trait(), false),
BinOpKind::Mul => (cx.tcx.lang_items().mul_trait(), false),
BinOpKind::Div => (cx.tcx.lang_items().div_trait(), false),
BinOpKind::Rem => (cx.tcx.lang_items().rem_trait(), false),
2017-04-28 15:03:47 +00:00
// don't lint short circuiting ops
2018-07-12 07:50:09 +00:00
BinOpKind::And | BinOpKind::Or => return,
BinOpKind::BitXor => (cx.tcx.lang_items().bitxor_trait(), false),
BinOpKind::BitAnd => (cx.tcx.lang_items().bitand_trait(), false),
BinOpKind::BitOr => (cx.tcx.lang_items().bitor_trait(), false),
BinOpKind::Shl => (cx.tcx.lang_items().shl_trait(), false),
BinOpKind::Shr => (cx.tcx.lang_items().shr_trait(), false),
BinOpKind::Ne | BinOpKind::Eq => (cx.tcx.lang_items().eq_trait(), true),
2018-11-27 20:14:15 +00:00
BinOpKind::Lt | BinOpKind::Le | BinOpKind::Ge | BinOpKind::Gt => {
(cx.tcx.lang_items().partial_ord_trait(), true)
2018-11-27 20:14:15 +00:00
},
2017-04-28 15:03:47 +00:00
};
if let Some(trait_id) = trait_id {
2018-08-01 20:48:41 +00:00
#[allow(clippy::match_same_arms)]
2019-09-27 15:16:06 +00:00
match (&left.kind, &right.kind) {
2017-04-28 15:03:47 +00:00
// do not suggest to dereference literals
2018-07-12 07:30:57 +00:00
(&ExprKind::Lit(..), _) | (_, &ExprKind::Lit(..)) => {},
2017-04-28 15:03:47 +00:00
// &foo == &bar
(&ExprKind::AddrOf(BorrowKind::Ref, _, l), &ExprKind::AddrOf(BorrowKind::Ref, _, r)) => {
2020-07-17 08:47:04 +00:00
let lty = cx.typeck_results().expr_ty(l);
let rty = cx.typeck_results().expr_ty(r);
let lcpy = is_copy(cx, lty);
let rcpy = is_copy(cx, rty);
2017-04-28 15:03:47 +00:00
// either operator autorefs or both args are copyable
if (requires_ref || (lcpy && rcpy)) && implements_trait(cx, lty, trait_id, &[rty.into()]) {
2017-08-09 07:30:56 +00:00
span_lint_and_then(
cx,
OP_REF,
e.span,
"needlessly taken reference of both operands",
|diag| {
2017-08-09 07:30:56 +00:00
let lsnip = snippet(cx, l.span, "...").to_string();
let rsnip = snippet(cx, r.span, "...").to_string();
multispan_sugg(
diag,
"use the values directly",
2017-08-09 07:30:56 +00:00
vec![(left.span, lsnip), (right.span, rsnip)],
);
},
);
2018-11-27 20:14:15 +00:00
} else if lcpy
&& !rcpy
2020-07-17 08:47:04 +00:00
&& implements_trait(cx, lty, trait_id, &[cx.typeck_results().expr_ty(right).into()])
2018-11-27 20:14:15 +00:00
{
2020-04-17 06:09:09 +00:00
span_lint_and_then(
cx,
OP_REF,
e.span,
"needlessly taken reference of left operand",
|diag| {
let lsnip = snippet(cx, l.span, "...").to_string();
diag.span_suggestion(
left.span,
"use the left value directly",
lsnip,
Applicability::MaybeIncorrect, // FIXME #2597
);
},
);
2018-11-27 20:14:15 +00:00
} else if !lcpy
&& rcpy
2020-07-17 08:47:04 +00:00
&& implements_trait(cx, cx.typeck_results().expr_ty(left), trait_id, &[rty.into()])
2018-11-27 20:14:15 +00:00
{
2017-08-09 07:30:56 +00:00
span_lint_and_then(
cx,
OP_REF,
e.span,
"needlessly taken reference of right operand",
|diag| {
2017-08-09 07:30:56 +00:00
let rsnip = snippet(cx, r.span, "...").to_string();
diag.span_suggestion(
right.span,
"use the right value directly",
rsnip,
Applicability::MaybeIncorrect, // FIXME #2597
2018-09-18 15:07:54 +00:00
);
2017-08-09 07:30:56 +00:00
},
);
2017-04-28 15:03:47 +00:00
}
},
// &foo == bar
(&ExprKind::AddrOf(BorrowKind::Ref, _, l), _) => {
2020-07-17 08:47:04 +00:00
let lty = cx.typeck_results().expr_ty(l);
let lcpy = is_copy(cx, lty);
2018-11-27 20:14:15 +00:00
if (requires_ref || lcpy)
2020-07-17 08:47:04 +00:00
&& implements_trait(cx, lty, trait_id, &[cx.typeck_results().expr_ty(right).into()])
2018-11-27 20:14:15 +00:00
{
2020-04-17 06:09:09 +00:00
span_lint_and_then(
cx,
OP_REF,
e.span,
"needlessly taken reference of left operand",
|diag| {
let lsnip = snippet(cx, l.span, "...").to_string();
diag.span_suggestion(
left.span,
"use the left value directly",
lsnip,
Applicability::MaybeIncorrect, // FIXME #2597
);
},
);
2017-04-28 15:03:47 +00:00
}
2021-05-25 06:05:52 +00:00
},
2017-04-28 15:03:47 +00:00
// foo == &bar
(_, &ExprKind::AddrOf(BorrowKind::Ref, _, r)) => {
2020-07-17 08:47:04 +00:00
let rty = cx.typeck_results().expr_ty(r);
let rcpy = is_copy(cx, rty);
2018-11-27 20:14:15 +00:00
if (requires_ref || rcpy)
2020-07-17 08:47:04 +00:00
&& implements_trait(cx, cx.typeck_results().expr_ty(left), trait_id, &[rty.into()])
2018-11-27 20:14:15 +00:00
{
span_lint_and_then(cx, OP_REF, e.span, "taken reference of right operand", |diag| {
2017-04-28 15:03:47 +00:00
let rsnip = snippet(cx, r.span, "...").to_string();
diag.span_suggestion(
2018-09-18 15:07:54 +00:00
right.span,
"use the right value directly",
rsnip,
Applicability::MaybeIncorrect, // FIXME #2597
2018-09-18 15:07:54 +00:00
);
});
}
2017-04-28 15:03:47 +00:00
},
_ => {},
}
}
}
}
}