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
This commit is contained in:
Ryan1729
parent
84ba08f391
commit
af1cc5c911
@ -2073,6 +2073,7 @@ Released 2018-09-13
|
||||
[`suspicious_else_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting
|
||||
[`suspicious_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_map
|
||||
[`suspicious_op_assign_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_op_assign_impl
|
||||
[`suspicious_operation_groupings`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_operation_groupings
|
||||
[`suspicious_unary_op_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_unary_op_formatting
|
||||
[`tabs_in_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#tabs_in_doc_comments
|
||||
[`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment
|
||||
|
||||
@ -1,10 +1,10 @@
|
||||
use crate::utils::{
|
||||
eq_expr_value, higher, implements_trait, in_macro, is_copy, is_expn_of, multispan_sugg, snippet, span_lint,
|
||||
span_lint_and_then,
|
||||
ast_utils::is_useless_with_eq_exprs, eq_expr_value, higher, implements_trait, in_macro, is_copy, is_expn_of,
|
||||
multispan_sugg, snippet, span_lint, span_lint_and_then,
|
||||
};
|
||||
use if_chain::if_chain;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{BinOp, BinOpKind, BorrowKind, Expr, ExprKind, StmtKind};
|
||||
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, StmtKind};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
|
||||
@ -102,7 +102,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
|
||||
if macro_with_not_op(&left.kind) || macro_with_not_op(&right.kind) {
|
||||
return;
|
||||
}
|
||||
if is_valid_operator(op) && eq_expr_value(cx, left, right) {
|
||||
if is_useless_with_eq_exprs(higher::binop(op.node)) && eq_expr_value(cx, left, right) {
|
||||
span_lint(
|
||||
cx,
|
||||
EQ_OP,
|
||||
@ -245,22 +245,3 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn is_valid_operator(op: BinOp) -> bool {
|
||||
matches!(
|
||||
op.node,
|
||||
BinOpKind::Sub
|
||||
| BinOpKind::Div
|
||||
| BinOpKind::Eq
|
||||
| BinOpKind::Lt
|
||||
| BinOpKind::Le
|
||||
| BinOpKind::Gt
|
||||
| BinOpKind::Ge
|
||||
| BinOpKind::Ne
|
||||
| BinOpKind::And
|
||||
| BinOpKind::Or
|
||||
| BinOpKind::BitXor
|
||||
| BinOpKind::BitAnd
|
||||
| BinOpKind::BitOr
|
||||
)
|
||||
}
|
||||
|
||||
@ -308,6 +308,7 @@ macro_rules! declare_clippy_lint {
|
||||
mod slow_vector_initialization;
|
||||
mod stable_sort_primitive;
|
||||
mod strings;
|
||||
mod suspicious_operation_groupings;
|
||||
mod suspicious_trait_impl;
|
||||
mod swap;
|
||||
mod tabs_in_doc_comments;
|
||||
@ -834,6 +835,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
&strings::STRING_LIT_AS_BYTES,
|
||||
&strings::STRING_TO_STRING,
|
||||
&strings::STR_TO_STRING,
|
||||
&suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS,
|
||||
&suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL,
|
||||
&suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL,
|
||||
&swap::ALMOST_SWAPPED,
|
||||
@ -1066,6 +1068,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
store.register_late_pass(|| box types::UnitArg);
|
||||
store.register_late_pass(|| box double_comparison::DoubleComparisons);
|
||||
store.register_late_pass(|| box question_mark::QuestionMark);
|
||||
store.register_early_pass(|| box suspicious_operation_groupings::SuspiciousOperationGroupings);
|
||||
store.register_late_pass(|| box suspicious_trait_impl::SuspiciousImpl);
|
||||
store.register_late_pass(|| box map_unit_fn::MapUnit);
|
||||
store.register_late_pass(|| box inherent_impl::MultipleInherentImpl::default());
|
||||
@ -1547,6 +1550,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
LintId::of(&slow_vector_initialization::SLOW_VECTOR_INITIALIZATION),
|
||||
LintId::of(&stable_sort_primitive::STABLE_SORT_PRIMITIVE),
|
||||
LintId::of(&strings::STRING_FROM_UTF8_AS_BYTES),
|
||||
LintId::of(&suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS),
|
||||
LintId::of(&suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
|
||||
LintId::of(&suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
|
||||
LintId::of(&swap::ALMOST_SWAPPED),
|
||||
@ -1698,6 +1702,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
LintId::of(&returns::LET_AND_RETURN),
|
||||
LintId::of(&returns::NEEDLESS_RETURN),
|
||||
LintId::of(&single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
|
||||
LintId::of(&suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS),
|
||||
LintId::of(&tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),
|
||||
LintId::of(&to_digit_is_some::TO_DIGIT_IS_SOME),
|
||||
LintId::of(&try_err::TRY_ERR),
|
||||
|
||||
693
clippy_lints/src/suspicious_operation_groupings.rs
Normal file
693
clippy_lints/src/suspicious_operation_groupings.rs
Normal file
@ -0,0 +1,693 @@
|
||||
use crate::utils::ast_utils::{eq_id, is_useless_with_eq_exprs, IdentIter};
|
||||
use crate::utils::{snippet_with_applicability, span_lint_and_sugg};
|
||||
use core::ops::{Add, AddAssign};
|
||||
use if_chain::if_chain;
|
||||
use rustc_ast::ast::{BinOpKind, Expr, ExprKind, StmtKind};
|
||||
use rustc_data_structures::fx::FxHashSet;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_lint::{EarlyContext, EarlyLintPass};
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
use rustc_span::source_map::Spanned;
|
||||
use rustc_span::symbol::Ident;
|
||||
use rustc_span::Span;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:**
|
||||
/// Checks for unlikely usages of binary operators that are almost
|
||||
/// certainly typos and/or copy/paste errors, given the other usages
|
||||
/// of binary operators nearby.
|
||||
/// **Why is this bad?**
|
||||
/// They are probably bugs and if they aren't then they look like bugs
|
||||
/// and you should add a comment explaining why you are doing such an
|
||||
/// odd set of operations.
|
||||
/// **Known problems:**
|
||||
/// There may be some false positives if you are trying to do something
|
||||
/// unusual that happens to look like a typo.
|
||||
///
|
||||
/// **Example:**
|
||||
///
|
||||
/// ```rust
|
||||
/// struct Vec3 {
|
||||
/// x: f64,
|
||||
/// y: f64,
|
||||
/// z: f64,
|
||||
/// }
|
||||
///
|
||||
/// impl Eq for Vec3 {}
|
||||
///
|
||||
/// impl PartialEq for Vec3 {
|
||||
/// fn eq(&self, other: &Self) -> bool {
|
||||
/// // This should trigger the lint because `self.x` is compared to `other.y`
|
||||
/// self.x == other.y && self.y == other.y && self.z == other.z
|
||||
/// }
|
||||
/// }
|
||||
/// ```
|
||||
/// Use instead:
|
||||
/// ```rust
|
||||
/// # struct Vec3 {
|
||||
/// # x: f64,
|
||||
/// # y: f64,
|
||||
/// # z: f64,
|
||||
/// # }
|
||||
/// // same as above except:
|
||||
/// impl PartialEq for Vec3 {
|
||||
/// fn eq(&self, other: &Self) -> bool {
|
||||
/// // Note we now compare other.x to self.x
|
||||
/// self.x == other.x && self.y == other.y && self.z == other.z
|
||||
/// }
|
||||
/// }
|
||||
/// ```
|
||||
pub SUSPICIOUS_OPERATION_GROUPINGS,
|
||||
style,
|
||||
"groupings of binary operations that look suspiciously like typos"
|
||||
}
|
||||
|
||||
declare_lint_pass!(SuspiciousOperationGroupings => [SUSPICIOUS_OPERATION_GROUPINGS]);
|
||||
|
||||
impl EarlyLintPass for SuspiciousOperationGroupings {
|
||||
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
|
||||
if expr.span.from_expansion() {
|
||||
return;
|
||||
}
|
||||
|
||||
if let Some(binops) = extract_related_binops(&expr.kind) {
|
||||
check_binops(cx, &binops.iter().collect::<Vec<_>>());
|
||||
|
||||
let mut op_types = Vec::with_capacity(binops.len());
|
||||
// We could use a hashmap, etc. to avoid being O(n*m) here, but
|
||||
// we want the lints to be emitted in a consistent order. Besides,
|
||||
// m, (the number of distinct `BinOpKind`s in `binops`)
|
||||
// will often be small, and does have an upper limit.
|
||||
binops.iter().map(|b| b.op).for_each(|op| {
|
||||
if !op_types.contains(&op) {
|
||||
op_types.push(op);
|
||||
}
|
||||
});
|
||||
|
||||
for op_type in op_types {
|
||||
let ops: Vec<_> = binops.iter().filter(|b| b.op == op_type).collect();
|
||||
|
||||
check_binops(cx, &ops);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_binops(cx: &EarlyContext<'_>, binops: &[&BinaryOp<'_>]) {
|
||||
let binop_count = binops.len();
|
||||
if binop_count < 2 {
|
||||
// Single binary operation expressions would likely be false
|
||||
// positives.
|
||||
return;
|
||||
}
|
||||
|
||||
let mut one_ident_difference_count = 0;
|
||||
let mut no_difference_info = None;
|
||||
let mut double_difference_info = None;
|
||||
let mut expected_ident_loc = None;
|
||||
|
||||
let mut paired_identifiers = FxHashSet::default();
|
||||
|
||||
for (i, BinaryOp { left, right, op, .. }) in binops.iter().enumerate() {
|
||||
match ident_difference_expr(left, right) {
|
||||
IdentDifference::NoDifference => {
|
||||
if is_useless_with_eq_exprs(*op) {
|
||||
// The `eq_op` lint should catch this in this case.
|
||||
return;
|
||||
}
|
||||
|
||||
no_difference_info = Some(i);
|
||||
},
|
||||
IdentDifference::Single(ident_loc) => {
|
||||
one_ident_difference_count += 1;
|
||||
if let Some(previous_expected) = expected_ident_loc {
|
||||
if previous_expected != ident_loc {
|
||||
// This expression doesn't match the form we're
|
||||
// looking for.
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
expected_ident_loc = Some(ident_loc);
|
||||
}
|
||||
|
||||
// If there was only a single difference, all other idents
|
||||
// must have been the same, and thus were paired.
|
||||
for id in skip_index(IdentIter::from(*left), ident_loc.index) {
|
||||
paired_identifiers.insert(id);
|
||||
}
|
||||
},
|
||||
IdentDifference::Double(ident_loc1, ident_loc2) => {
|
||||
double_difference_info = Some((i, ident_loc1, ident_loc2));
|
||||
},
|
||||
IdentDifference::Multiple | IdentDifference::NonIdent => {
|
||||
// It's too hard to know whether this is a bug or not.
|
||||
return;
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
let mut applicability = Applicability::MachineApplicable;
|
||||
|
||||
if let Some(expected_loc) = expected_ident_loc {
|
||||
match (no_difference_info, double_difference_info) {
|
||||
(Some(i), None) => attempt_to_emit_no_difference_lint(cx, binops, i, expected_loc),
|
||||
(None, Some((double_difference_index, ident_loc1, ident_loc2))) => {
|
||||
if_chain! {
|
||||
if one_ident_difference_count == binop_count - 1;
|
||||
if let Some(binop) = binops.get(double_difference_index);
|
||||
then {
|
||||
let changed_loc = if ident_loc1 == expected_loc {
|
||||
ident_loc2
|
||||
} else if ident_loc2 == expected_loc {
|
||||
ident_loc1
|
||||
} else {
|
||||
// This expression doesn't match the form we're
|
||||
// looking for.
|
||||
return;
|
||||
};
|
||||
|
||||
if let Some(sugg) = ident_swap_sugg(
|
||||
cx,
|
||||
&paired_identifiers,
|
||||
binop,
|
||||
changed_loc,
|
||||
&mut applicability,
|
||||
) {
|
||||
emit_suggestion(
|
||||
cx,
|
||||
binop.span,
|
||||
sugg,
|
||||
applicability,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn attempt_to_emit_no_difference_lint(
|
||||
cx: &EarlyContext<'_>,
|
||||
binops: &[&BinaryOp<'_>],
|
||||
i: usize,
|
||||
expected_loc: IdentLocation,
|
||||
) {
|
||||
if let Some(binop) = binops.get(i).cloned() {
|
||||
// We need to try and figure out which identifier we should
|
||||
// suggest using instead. Since there could be multiple
|
||||
// replacement candidates in a given expression, and we're
|
||||
// just taking the first one, we may get some bad lint
|
||||
// messages.
|
||||
let mut applicability = Applicability::MaybeIncorrect;
|
||||
|
||||
// We assume that the correct ident is one used elsewhere in
|
||||
// the other binops, in a place that there was a single
|
||||
// difference between idents before.
|
||||
let old_left_ident = get_ident(binop.left, expected_loc);
|
||||
let old_right_ident = get_ident(binop.right, expected_loc);
|
||||
|
||||
for b in skip_index(binops.iter(), i) {
|
||||
if_chain! {
|
||||
if let (Some(old_ident), Some(new_ident)) =
|
||||
(old_left_ident, get_ident(b.left, expected_loc));
|
||||
if old_ident != new_ident;
|
||||
if let Some(sugg) = suggestion_with_swapped_ident(
|
||||
cx,
|
||||
binop.left,
|
||||
expected_loc,
|
||||
new_ident,
|
||||
&mut applicability,
|
||||
);
|
||||
then {
|
||||
emit_suggestion(
|
||||
cx,
|
||||
binop.span,
|
||||
replace_left_sugg(cx, &binop, &sugg, &mut applicability),
|
||||
applicability,
|
||||
);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if_chain! {
|
||||
if let (Some(old_ident), Some(new_ident)) =
|
||||
(old_right_ident, get_ident(b.right, expected_loc));
|
||||
if old_ident != new_ident;
|
||||
if let Some(sugg) = suggestion_with_swapped_ident(
|
||||
cx,
|
||||
binop.right,
|
||||
expected_loc,
|
||||
new_ident,
|
||||
&mut applicability,
|
||||
);
|
||||
then {
|
||||
emit_suggestion(
|
||||
cx,
|
||||
binop.span,
|
||||
replace_right_sugg(cx, &binop, &sugg, &mut applicability),
|
||||
applicability,
|
||||
);
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn emit_suggestion(cx: &EarlyContext<'_>, span: Span, sugg: String, applicability: Applicability) {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
SUSPICIOUS_OPERATION_GROUPINGS,
|
||||
span,
|
||||
"This sequence of operators looks suspiciously like a bug.",
|
||||
"I think you meant",
|
||||
sugg,
|
||||
applicability,
|
||||
)
|
||||
}
|
||||
|
||||
fn ident_swap_sugg(
|
||||
cx: &EarlyContext<'_>,
|
||||
paired_identifiers: &FxHashSet<Ident>,
|
||||
binop: &BinaryOp<'_>,
|
||||
location: IdentLocation,
|
||||
applicability: &mut Applicability,
|
||||
) -> Option<String> {
|
||||
let left_ident = get_ident(&binop.left, location)?;
|
||||
let right_ident = get_ident(&binop.right, location)?;
|
||||
|
||||
let sugg = match (
|
||||
paired_identifiers.contains(&left_ident),
|
||||
paired_identifiers.contains(&right_ident),
|
||||
) {
|
||||
(true, true) | (false, false) => {
|
||||
// We don't have a good guess of what ident should be
|
||||
// used instead, in these cases.
|
||||
*applicability = Applicability::MaybeIncorrect;
|
||||
|
||||
// We arbitraily choose one side to suggest changing,
|
||||
// since we don't have a better guess. If the user
|
||||
// ends up duplicating a clause, the `logic_bug` lint
|
||||
// should catch it.
|
||||
|
||||
let right_suggestion =
|
||||
suggestion_with_swapped_ident(cx, &binop.right, location, left_ident, applicability)?;
|
||||
|
||||
replace_right_sugg(cx, binop, &right_suggestion, applicability)
|
||||
},
|
||||
(false, true) => {
|
||||
// We haven't seen a pair involving the left one, so
|
||||
// it's probably what is wanted.
|
||||
|
||||
let right_suggestion =
|
||||
suggestion_with_swapped_ident(cx, &binop.right, location, left_ident, applicability)?;
|
||||
|
||||
replace_right_sugg(cx, binop, &right_suggestion, applicability)
|
||||
},
|
||||
(true, false) => {
|
||||
// We haven't seen a pair involving the right one, so
|
||||
// it's probably what is wanted.
|
||||
let left_suggestion = suggestion_with_swapped_ident(cx, &binop.left, location, right_ident, applicability)?;
|
||||
|
||||
replace_left_sugg(cx, binop, &left_suggestion, applicability)
|
||||
},
|
||||
};
|
||||
|
||||
Some(sugg)
|
||||
}
|
||||
|
||||
fn replace_left_sugg(
|
||||
cx: &EarlyContext<'_>,
|
||||
binop: &BinaryOp<'_>,
|
||||
left_suggestion: &str,
|
||||
applicability: &mut Applicability,
|
||||
) -> String {
|
||||
format!(
|
||||
"{} {} {}",
|
||||
left_suggestion,
|
||||
binop.op.to_string(),
|
||||
snippet_with_applicability(cx, binop.right.span, "..", applicability),
|
||||
)
|
||||
}
|
||||
|
||||
fn replace_right_sugg(
|
||||
cx: &EarlyContext<'_>,
|
||||
binop: &BinaryOp<'_>,
|
||||
right_suggestion: &str,
|
||||
applicability: &mut Applicability,
|
||||
) -> String {
|
||||
format!(
|
||||
"{} {} {}",
|
||||
snippet_with_applicability(cx, binop.left.span, "..", applicability),
|
||||
binop.op.to_string(),
|
||||
right_suggestion,
|
||||
)
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
struct BinaryOp<'exprs> {
|
||||
op: BinOpKind,
|
||||
span: Span,
|
||||
left: &'exprs Expr,
|
||||
right: &'exprs Expr,
|
||||
}
|
||||
|
||||
impl BinaryOp<'exprs> {
|
||||
fn new(op: BinOpKind, span: Span, (left, right): (&'exprs Expr, &'exprs Expr)) -> Self {
|
||||
Self { op, span, left, right }
|
||||
}
|
||||
}
|
||||
|
||||
fn strip_non_ident_wrappers(expr: &Expr) -> &Expr {
|
||||
let mut output = expr;
|
||||
loop {
|
||||
output = match &output.kind {
|
||||
ExprKind::Paren(ref inner) | ExprKind::Unary(_, ref inner) => inner,
|
||||
_ => {
|
||||
return output;
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
fn extract_related_binops(kind: &ExprKind) -> Option<Vec<BinaryOp<'_>>> {
|
||||
append_opt_vecs(chained_binops(kind), if_statment_binops(kind))
|
||||
}
|
||||
|
||||
fn if_statment_binops(kind: &ExprKind) -> Option<Vec<BinaryOp<'_>>> {
|
||||
match kind {
|
||||
ExprKind::If(ref condition, _, _) => chained_binops(&condition.kind),
|
||||
ExprKind::Paren(ref e) => if_statment_binops(&e.kind),
|
||||
ExprKind::Block(ref block, _) => {
|
||||
let mut output = None;
|
||||
for stmt in &block.stmts {
|
||||
match stmt.kind {
|
||||
StmtKind::Expr(ref e) | StmtKind::Semi(ref e) => {
|
||||
output = append_opt_vecs(output, if_statment_binops(&e.kind));
|
||||
},
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
output
|
||||
},
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn append_opt_vecs<A>(target_opt: Option<Vec<A>>, source_opt: Option<Vec<A>>) -> Option<Vec<A>> {
|
||||
match (target_opt, source_opt) {
|
||||
(Some(mut target), Some(mut source)) => {
|
||||
target.reserve(source.len());
|
||||
for op in source.drain(..) {
|
||||
target.push(op);
|
||||
}
|
||||
Some(target)
|
||||
},
|
||||
(Some(v), None) | (None, Some(v)) => Some(v),
|
||||
(None, None) => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn chained_binops(kind: &ExprKind) -> Option<Vec<BinaryOp<'_>>> {
|
||||
match kind {
|
||||
ExprKind::Binary(_, left_outer, right_outer) => chained_binops_helper(left_outer, right_outer),
|
||||
ExprKind::Paren(ref e) | ExprKind::Unary(_, ref e) => chained_binops(&e.kind),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn chained_binops_helper(left_outer: &'expr Expr, right_outer: &'expr Expr) -> Option<Vec<BinaryOp<'expr>>> {
|
||||
match (&left_outer.kind, &right_outer.kind) {
|
||||
(
|
||||
ExprKind::Paren(ref left_e) | ExprKind::Unary(_, ref left_e),
|
||||
ExprKind::Paren(ref right_e) | ExprKind::Unary(_, ref right_e),
|
||||
) => chained_binops_helper(left_e, right_e),
|
||||
(ExprKind::Paren(ref left_e) | ExprKind::Unary(_, ref left_e), _) => chained_binops_helper(left_e, right_outer),
|
||||
(_, ExprKind::Paren(ref right_e) | ExprKind::Unary(_, ref right_e)) => {
|
||||
chained_binops_helper(left_outer, right_e)
|
||||
},
|
||||
(
|
||||
ExprKind::Binary(Spanned { node: left_op, .. }, ref left_left, ref left_right),
|
||||
ExprKind::Binary(Spanned { node: right_op, .. }, ref right_left, ref right_right),
|
||||
) => match (
|
||||
chained_binops_helper(left_left, left_right),
|
||||
chained_binops_helper(right_left, right_right),
|
||||
) {
|
||||
(Some(mut left_ops), Some(mut right_ops)) => {
|
||||
left_ops.reserve(right_ops.len());
|
||||
for op in right_ops.drain(..) {
|
||||
left_ops.push(op);
|
||||
}
|
||||
Some(left_ops)
|
||||
},
|
||||
(Some(mut left_ops), _) => {
|
||||
left_ops.push(BinaryOp::new(*right_op, right_outer.span, (right_left, right_right)));
|
||||
Some(left_ops)
|
||||
},
|
||||
(_, Some(mut right_ops)) => {
|
||||
right_ops.insert(0, BinaryOp::new(*left_op, left_outer.span, (left_left, left_right)));
|
||||
Some(right_ops)
|
||||
},
|
||||
(None, None) => Some(vec![
|
||||
BinaryOp::new(*left_op, left_outer.span, (left_left, left_right)),
|
||||
BinaryOp::new(*right_op, right_outer.span, (right_left, right_right)),
|
||||
]),
|
||||
},
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, PartialEq, Eq, Default, Debug)]
|
||||
struct IdentLocation {
|
||||
index: usize,
|
||||
}
|
||||
|
||||
impl Add for IdentLocation {
|
||||
type Output = IdentLocation;
|
||||
|
||||
fn add(self, other: Self) -> Self::Output {
|
||||
Self {
|
||||
index: self.index + other.index,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl AddAssign for IdentLocation {
|
||||
fn add_assign(&mut self, other: Self) {
|
||||
*self = *self + other
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
enum IdentDifference {
|
||||
NoDifference,
|
||||
Single(IdentLocation),
|
||||
Double(IdentLocation, IdentLocation),
|
||||
Multiple,
|
||||
NonIdent,
|
||||
}
|
||||
|
||||
impl Add for IdentDifference {
|
||||
type Output = IdentDifference;
|
||||
|
||||
fn add(self, other: Self) -> Self::Output {
|
||||
match (self, other) {
|
||||
(Self::NoDifference, output) | (output, Self::NoDifference) => output,
|
||||
(Self::Multiple, _)
|
||||
| (_, Self::Multiple)
|
||||
| (Self::Double(_, _), Self::Single(_))
|
||||
| (Self::Single(_) | Self::Double(_, _), Self::Double(_, _)) => Self::Multiple,
|
||||
(Self::NonIdent, _) | (_, Self::NonIdent) => Self::NonIdent,
|
||||
(Self::Single(il1), Self::Single(il2)) => Self::Double(il1, il2),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl AddAssign for IdentDifference {
|
||||
fn add_assign(&mut self, other: Self) {
|
||||
*self = *self + other
|
||||
}
|
||||
}
|
||||
|
||||
impl IdentDifference {
|
||||
/// Returns true if learning about more differences will not change the value
|
||||
/// of this `IdentDifference`, and false otherwise.
|
||||
fn is_complete(&self) -> bool {
|
||||
match self {
|
||||
Self::NoDifference | Self::Single(_) | Self::Double(_, _) => false,
|
||||
Self::Multiple | Self::NonIdent => true,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn ident_difference_expr(left: &Expr, right: &Expr) -> IdentDifference {
|
||||
ident_difference_expr_with_base_location(left, right, IdentLocation::default()).0
|
||||
}
|
||||
|
||||
fn ident_difference_expr_with_base_location(
|
||||
left: &Expr,
|
||||
right: &Expr,
|
||||
mut base: IdentLocation,
|
||||
) -> (IdentDifference, IdentLocation) {
|
||||
// Ideally, this function should not use IdentIter because it should return
|
||||
// early if the expressions have any non-ident differences. We want that early
|
||||
// return because if without that restriction the lint would lead to false
|
||||
// positives.
|
||||
//
|
||||
// But, we cannot (easily?) use a `rustc_ast::visit::Visitor`, since we need
|
||||
// the two expressions to be walked in lockstep. And without a `Visitor`, we'd
|
||||
// have to do all the AST traversal ourselves, which is a lot of work, since to
|
||||
// do it properly we'd need to be able to handle more or less every possible
|
||||
// AST node since `Item`s can be written inside `Expr`s.
|
||||
//
|
||||
// In practice, it seems likely that expressions, above a certain size, that
|
||||
// happen to use the exact same idents in the exact same order, and which are
|
||||
// not structured the same, would be rare. Therefore it seems likely that if
|
||||
// we do only the first layer of matching ourselves and eventually fallback on
|
||||
// IdentIter, then the output of this function will be almost always be correct
|
||||
// in practice.
|
||||
//
|
||||
// If it turns out that problematic cases are more prelavent than we assume,
|
||||
// then we should be able to change this function to do the correct traversal,
|
||||
// without needing to change the rest of the code.
|
||||
|
||||
#![allow(clippy::enum_glob_use)]
|
||||
use ExprKind::*;
|
||||
|
||||
match (
|
||||
&strip_non_ident_wrappers(left).kind,
|
||||
&strip_non_ident_wrappers(right).kind,
|
||||
) {
|
||||
(Yield(_), Yield(_))
|
||||
| (Try(_), Try(_))
|
||||
| (Paren(_), Paren(_))
|
||||
| (Repeat(_, _), Repeat(_, _))
|
||||
| (Struct(_, _, _), Struct(_, _, _))
|
||||
| (MacCall(_), MacCall(_))
|
||||
| (LlvmInlineAsm(_), LlvmInlineAsm(_))
|
||||
| (InlineAsm(_), InlineAsm(_))
|
||||
| (Ret(_), Ret(_))
|
||||
| (Continue(_), Continue(_))
|
||||
| (Break(_, _), Break(_, _))
|
||||
| (AddrOf(_, _, _), AddrOf(_, _, _))
|
||||
| (Path(_, _), Path(_, _))
|
||||
| (Range(_, _, _), Range(_, _, _))
|
||||
| (Index(_, _), Index(_, _))
|
||||
| (Field(_, _), Field(_, _))
|
||||
| (AssignOp(_, _, _), AssignOp(_, _, _))
|
||||
| (Assign(_, _, _), Assign(_, _, _))
|
||||
| (TryBlock(_), TryBlock(_))
|
||||
| (Await(_), Await(_))
|
||||
| (Async(_, _, _), Async(_, _, _))
|
||||
| (Block(_, _), Block(_, _))
|
||||
| (Closure(_, _, _, _, _, _), Closure(_, _, _, _, _, _))
|
||||
| (Match(_, _), Match(_, _))
|
||||
| (Loop(_, _), Loop(_, _))
|
||||
| (ForLoop(_, _, _, _), ForLoop(_, _, _, _))
|
||||
| (While(_, _, _), While(_, _, _))
|
||||
| (If(_, _, _), If(_, _, _))
|
||||
| (Let(_, _), Let(_, _))
|
||||
| (Type(_, _), Type(_, _))
|
||||
| (Cast(_, _), Cast(_, _))
|
||||
| (Lit(_), Lit(_))
|
||||
| (Unary(_, _), Unary(_, _))
|
||||
| (Binary(_, _, _), Binary(_, _, _))
|
||||
| (Tup(_), Tup(_))
|
||||
| (MethodCall(_, _, _), MethodCall(_, _, _))
|
||||
| (Call(_, _), Call(_, _))
|
||||
| (ConstBlock(_), ConstBlock(_))
|
||||
| (Array(_), Array(_))
|
||||
| (Box(_), Box(_)) => {
|
||||
// keep going
|
||||
},
|
||||
_ => {
|
||||
return (IdentDifference::NonIdent, base);
|
||||
},
|
||||
}
|
||||
|
||||
let mut difference = IdentDifference::NoDifference;
|
||||
|
||||
for (left_attr, right_attr) in left.attrs.iter().zip(right.attrs.iter()) {
|
||||
let (new_difference, new_base) =
|
||||
ident_difference_via_ident_iter_with_base_location(left_attr, right_attr, base);
|
||||
base = new_base;
|
||||
difference += new_difference;
|
||||
if difference.is_complete() {
|
||||
return (difference, base);
|
||||
}
|
||||
}
|
||||
|
||||
let (new_difference, new_base) = ident_difference_via_ident_iter_with_base_location(left, right, base);
|
||||
base = new_base;
|
||||
difference += new_difference;
|
||||
|
||||
(difference, base)
|
||||
}
|
||||
|
||||
fn ident_difference_via_ident_iter_with_base_location<Iterable: Into<IdentIter>>(
|
||||
left: Iterable,
|
||||
right: Iterable,
|
||||
mut base: IdentLocation,
|
||||
) -> (IdentDifference, IdentLocation) {
|
||||
// See the note in `ident_difference_expr_with_base_location` about `IdentIter`
|
||||
let mut difference = IdentDifference::NoDifference;
|
||||
|
||||
let mut left_iterator = left.into();
|
||||
let mut right_iterator = right.into();
|
||||
|
||||
loop {
|
||||
match (left_iterator.next(), right_iterator.next()) {
|
||||
(Some(left_ident), Some(right_ident)) => {
|
||||
if !eq_id(left_ident, right_ident) {
|
||||
difference += IdentDifference::Single(base);
|
||||
if difference.is_complete() {
|
||||
return (difference, base);
|
||||
}
|
||||
}
|
||||
},
|
||||
(Some(_), None) | (None, Some(_)) => {
|
||||
return (IdentDifference::NonIdent, base);
|
||||
},
|
||||
(None, None) => {
|
||||
return (difference, base);
|
||||
},
|
||||
}
|
||||
base += IdentLocation { index: 1 };
|
||||
}
|
||||
}
|
||||
|
||||
fn get_ident(expr: &Expr, location: IdentLocation) -> Option<Ident> {
|
||||
IdentIter::from(expr).nth(location.index)
|
||||
}
|
||||
|
||||
fn suggestion_with_swapped_ident(
|
||||
cx: &EarlyContext<'_>,
|
||||
expr: &Expr,
|
||||
location: IdentLocation,
|
||||
new_ident: Ident,
|
||||
applicability: &mut Applicability,
|
||||
) -> Option<String> {
|
||||
get_ident(expr, location).and_then(|current_ident| {
|
||||
if eq_id(current_ident, new_ident) {
|
||||
// We never want to suggest a non-change
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(format!(
|
||||
"{}{}{}",
|
||||
snippet_with_applicability(cx, expr.span.with_hi(current_ident.span.lo()), "..", applicability),
|
||||
new_ident.to_string(),
|
||||
snippet_with_applicability(cx, expr.span.with_lo(current_ident.span.hi()), "..", applicability),
|
||||
))
|
||||
})
|
||||
}
|
||||
|
||||
fn skip_index<A, Iter>(iter: Iter, index: usize) -> impl Iterator<Item = A>
|
||||
where
|
||||
Iter: Iterator<Item = A>,
|
||||
{
|
||||
iter.enumerate()
|
||||
.filter_map(move |(i, a)| if i == index { None } else { Some(a) })
|
||||
}
|
||||
@ -10,6 +10,17 @@
|
||||
use rustc_span::symbol::Ident;
|
||||
use std::mem;
|
||||
|
||||
pub mod ident_iter;
|
||||
pub use ident_iter::IdentIter;
|
||||
|
||||
pub fn is_useless_with_eq_exprs(kind: BinOpKind) -> bool {
|
||||
use BinOpKind::*;
|
||||
matches!(
|
||||
kind,
|
||||
Sub | Div | Eq | Lt | Le | Gt | Ge | Ne | And | Or | BitXor | BitAnd | BitOr
|
||||
)
|
||||
}
|
||||
|
||||
/// Checks if each element in the first slice is contained within the latter as per `eq_fn`.
|
||||
pub fn unordered_over<X>(left: &[X], right: &[X], mut eq_fn: impl FnMut(&X, &X) -> bool) -> bool {
|
||||
left.len() == right.len() && left.iter().all(|l| right.iter().any(|r| eq_fn(l, r)))
|
||||
|
||||
45
clippy_lints/src/utils/ast_utils/ident_iter.rs
Normal file
45
clippy_lints/src/utils/ast_utils/ident_iter.rs
Normal file
@ -0,0 +1,45 @@
|
||||
use core::iter::FusedIterator;
|
||||
use rustc_ast::visit::{walk_attribute, walk_expr, Visitor};
|
||||
use rustc_ast::{Attribute, Expr};
|
||||
use rustc_span::symbol::Ident;
|
||||
|
||||
pub struct IdentIter(std::vec::IntoIter<Ident>);
|
||||
|
||||
impl Iterator for IdentIter {
|
||||
type Item = Ident;
|
||||
|
||||
fn next(&mut self) -> Option<Self::Item> {
|
||||
self.0.next()
|
||||
}
|
||||
}
|
||||
|
||||
impl FusedIterator for IdentIter {}
|
||||
|
||||
impl From<&Expr> for IdentIter {
|
||||
fn from(expr: &Expr) -> Self {
|
||||
let mut visitor = IdentCollector::default();
|
||||
|
||||
walk_expr(&mut visitor, expr);
|
||||
|
||||
IdentIter(visitor.0.into_iter())
|
||||
}
|
||||
}
|
||||
|
||||
impl From<&Attribute> for IdentIter {
|
||||
fn from(attr: &Attribute) -> Self {
|
||||
let mut visitor = IdentCollector::default();
|
||||
|
||||
walk_attribute(&mut visitor, attr);
|
||||
|
||||
IdentIter(visitor.0.into_iter())
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct IdentCollector(Vec<Ident>);
|
||||
|
||||
impl Visitor<'_> for IdentCollector {
|
||||
fn visit_ident(&mut self, ident: Ident) {
|
||||
self.0.push(ident);
|
||||
}
|
||||
}
|
||||
@ -86,3 +86,12 @@ fn check_ignore_macro() {
|
||||
// checks if the lint ignores macros with `!` operator
|
||||
!bool_macro!(1) && !bool_macro!("");
|
||||
}
|
||||
|
||||
struct Nested {
|
||||
inner: ((i32,), (i32,), (i32,)),
|
||||
}
|
||||
|
||||
fn check_nested(n1: &Nested, n2: &Nested) -> bool {
|
||||
// `n2.inner.0.0` mistyped as `n1.inner.0.0`
|
||||
(n1.inner.0).0 == (n1.inner.0).0 && (n1.inner.1).0 == (n2.inner.1).0 && (n1.inner.2).0 == (n2.inner.2).0
|
||||
}
|
||||
|
||||
@ -162,5 +162,13 @@ error: equal expressions as operands to `/`
|
||||
LL | const D: u32 = A / A;
|
||||
| ^^^^^
|
||||
|
||||
error: aborting due to 27 previous errors
|
||||
error: equal expressions as operands to `==`
|
||||
--> $DIR/eq_op.rs:96:5
|
||||
|
|
||||
LL | (n1.inner.0).0 == (n1.inner.0).0 && (n1.inner.1).0 == (n2.inner.1).0 && (n1.inner.2).0 == (n2.inner.2).0
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: `#[deny(clippy::eq_op)]` on by default
|
||||
|
||||
error: aborting due to 28 previous errors
|
||||
|
||||
|
||||
207
tests/ui/suspicious_operation_groupings.rs
Normal file
207
tests/ui/suspicious_operation_groupings.rs
Normal file
@ -0,0 +1,207 @@
|
||||
#![warn(clippy::suspicious_operation_groupings)]
|
||||
|
||||
struct Vec3 {
|
||||
x: f64,
|
||||
y: f64,
|
||||
z: f64,
|
||||
}
|
||||
|
||||
impl Eq for Vec3 {}
|
||||
|
||||
impl PartialEq for Vec3 {
|
||||
fn eq(&self, other: &Self) -> bool {
|
||||
// This should trigger the lint because `self.x` is compared to `other.y`
|
||||
self.x == other.y && self.y == other.y && self.z == other.z
|
||||
}
|
||||
}
|
||||
|
||||
struct S {
|
||||
a: i32,
|
||||
b: i32,
|
||||
c: i32,
|
||||
d: i32,
|
||||
}
|
||||
|
||||
fn buggy_ab_cmp(s1: &S, s2: &S) -> bool {
|
||||
// There's no `s1.b`
|
||||
s1.a < s2.a && s1.a < s2.b
|
||||
}
|
||||
|
||||
struct SAOnly {
|
||||
a: i32,
|
||||
}
|
||||
|
||||
impl S {
|
||||
fn a(&self) -> i32 {
|
||||
0
|
||||
}
|
||||
}
|
||||
|
||||
fn do_not_give_bad_suggestions_for_this_unusual_expr(s1: &S, s2: &SAOnly) -> bool {
|
||||
// This is superficially similar to `buggy_ab_cmp`, but we should not suggest
|
||||
// `s2.b` since that is invalid.
|
||||
s1.a < s2.a && s1.a() < s1.b
|
||||
}
|
||||
|
||||
fn do_not_give_bad_suggestions_for_this_macro_expr(s1: &S, s2: &SAOnly) -> bool {
|
||||
macro_rules! s1 {
|
||||
() => {
|
||||
S {
|
||||
a: 1,
|
||||
b: 1,
|
||||
c: 1,
|
||||
d: 1,
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
// This is superficially similar to `buggy_ab_cmp`, but we should not suggest
|
||||
// `s2.b` since that is invalid.
|
||||
s1.a < s2.a && s1!().a < s1.b
|
||||
}
|
||||
|
||||
fn do_not_give_bad_suggestions_for_this_incorrect_expr(s1: &S, s2: &SAOnly) -> bool {
|
||||
// There's two `s1.b`, but we should not suggest `s2.b` since that is invalid
|
||||
s1.a < s2.a && s1.b < s1.b
|
||||
}
|
||||
|
||||
fn permissable(s1: &S, s2: &S) -> bool {
|
||||
// Something like this seems like it might actually be what is desired.
|
||||
s1.a == s2.b
|
||||
}
|
||||
|
||||
fn non_boolean_operators(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s2.c`
|
||||
s1.a * s2.a + s1.b * s2.b + s1.c * s2.b + s1.d * s2.d
|
||||
}
|
||||
|
||||
fn odd_number_of_pairs(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s2.b`
|
||||
s1.a * s2.a + s1.b * s2.c + s1.c * s2.c
|
||||
}
|
||||
|
||||
fn not_caught_by_eq_op_middle_change_left(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s1.b`
|
||||
s1.a * s2.a + s2.b * s2.b + s1.c * s2.c
|
||||
}
|
||||
|
||||
fn not_caught_by_eq_op_middle_change_right(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s2.b`
|
||||
s1.a * s2.a + s1.b * s1.b + s1.c * s2.c
|
||||
}
|
||||
|
||||
fn not_caught_by_eq_op_start(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s2.a`
|
||||
s1.a * s1.a + s1.b * s2.b + s1.c * s2.c
|
||||
}
|
||||
|
||||
fn not_caught_by_eq_op_end(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s2.c`
|
||||
s1.a * s2.a + s1.b * s2.b + s1.c * s1.c
|
||||
}
|
||||
|
||||
fn the_cross_product_should_not_lint(s1: &S, s2: &S) -> (i32, i32, i32) {
|
||||
(
|
||||
s1.b * s2.c - s1.c * s2.b,
|
||||
s1.c * s2.a - s1.a * s2.c,
|
||||
s1.a * s2.b - s1.b * s2.a,
|
||||
)
|
||||
}
|
||||
|
||||
fn outer_parens_simple(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s2.b`
|
||||
(s1.a * s2.a + s1.b * s1.b)
|
||||
}
|
||||
|
||||
fn outer_parens(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s2.c`
|
||||
(s1.a * s2.a + s1.b * s2.b + s1.c * s2.b + s1.d * s2.d)
|
||||
}
|
||||
|
||||
fn inner_parens(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s2.c`
|
||||
(s1.a * s2.a) + (s1.b * s2.b) + (s1.c * s2.b) + (s1.d * s2.d)
|
||||
}
|
||||
|
||||
fn outer_and_some_inner_parens(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s2.c`
|
||||
((s1.a * s2.a) + (s1.b * s2.b) + (s1.c * s2.b) + (s1.d * s2.d))
|
||||
}
|
||||
|
||||
fn all_parens_balanced_tree(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s2.c`
|
||||
(((s1.a * s2.a) + (s1.b * s2.b)) + ((s1.c * s2.b) + (s1.d * s2.d)))
|
||||
}
|
||||
|
||||
fn all_parens_left_tree(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s2.c`
|
||||
(((s1.a * s2.a) + (s1.b * s2.b) + (s1.c * s2.b)) + (s1.d * s2.d))
|
||||
}
|
||||
|
||||
fn all_parens_right_tree(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s2.c`
|
||||
((s1.a * s2.a) + ((s1.b * s2.b) + (s1.c * s2.b) + (s1.d * s2.d)))
|
||||
}
|
||||
|
||||
fn inside_other_binop_expression(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s1.b`
|
||||
(s1.a * s2.a + s2.b * s2.b) / 2
|
||||
}
|
||||
|
||||
fn inside_function_call(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s1.b`
|
||||
i32::swap_bytes(s1.a * s2.a + s2.b * s2.b)
|
||||
}
|
||||
|
||||
fn inside_larger_boolean_expression(s1: &S, s2: &S) -> bool {
|
||||
// There's no `s1.c`
|
||||
s1.a > 0 && s1.b > 0 && s1.d == s2.c && s1.d == s2.d
|
||||
}
|
||||
|
||||
fn inside_larger_boolean_expression_with_unsorted_ops(s1: &S, s2: &S) -> bool {
|
||||
// There's no `s1.c`
|
||||
s1.a > 0 && s1.d == s2.c && s1.b > 0 && s1.d == s2.d
|
||||
}
|
||||
|
||||
struct Nested {
|
||||
inner: ((i32,), (i32,), (i32,)),
|
||||
}
|
||||
|
||||
fn changed_middle_ident(n1: &Nested, n2: &Nested) -> bool {
|
||||
// There's no `n2.inner.2.0`
|
||||
(n1.inner.0).0 == (n2.inner.0).0 && (n1.inner.1).0 == (n2.inner.1).0 && (n1.inner.2).0 == (n2.inner.1).0
|
||||
}
|
||||
|
||||
// `eq_op` should catch this one.
|
||||
fn changed_initial_ident(n1: &Nested, n2: &Nested) -> bool {
|
||||
// There's no `n2.inner.0.0`
|
||||
(n1.inner.0).0 == (n1.inner.0).0 && (n1.inner.1).0 == (n2.inner.1).0 && (n1.inner.2).0 == (n2.inner.2).0
|
||||
}
|
||||
|
||||
fn inside_fn_with_similar_expression(s1: &S, s2: &S, strict: bool) -> bool {
|
||||
if strict {
|
||||
s1.a < s2.a && s1.b < s2.b
|
||||
} else {
|
||||
// There's no `s1.b` in this subexpression
|
||||
s1.a <= s2.a && s1.a <= s2.b
|
||||
}
|
||||
}
|
||||
|
||||
fn inside_an_if_statement(s1: &S, s2: &S) {
|
||||
// There's no `s1.b`
|
||||
if s1.a < s2.a && s1.a < s2.b {
|
||||
s1.c = s2.c;
|
||||
}
|
||||
}
|
||||
|
||||
fn maximum_unary_minus_right_tree(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s2.c`
|
||||
-(-(-s1.a * -s2.a) + (-(-s1.b * -s2.b) + -(-s1.c * -s2.b) + -(-s1.d * -s2.d)))
|
||||
}
|
||||
|
||||
fn unary_minus_and_an_if_expression(s1: &S, s2: &S) -> i32 {
|
||||
// There's no `s1.b`
|
||||
-(if -s1.a < -s2.a && -s1.a < -s2.b { s1.c } else { s2.a })
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
166
tests/ui/suspicious_operation_groupings.stderr
Normal file
166
tests/ui/suspicious_operation_groupings.stderr
Normal file
@ -0,0 +1,166 @@
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:14:9
|
||||
|
|
||||
LL | self.x == other.y && self.y == other.y && self.z == other.z
|
||||
| ^^^^^^^^^^^^^^^^^ help: I think you meant: `self.x == other.x`
|
||||
|
|
||||
= note: `-D clippy::suspicious-operation-groupings` implied by `-D warnings`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:14:9
|
||||
|
|
||||
LL | self.x == other.y && self.y == other.y && self.z == other.z
|
||||
| ^^^^^^^^^^^^^^^^^ help: I think you meant: `self.x == other.x`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:27:20
|
||||
|
|
||||
LL | s1.a < s2.a && s1.a < s2.b
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.b < s2.b`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:75:33
|
||||
|
|
||||
LL | s1.a * s2.a + s1.b * s2.b + s1.c * s2.b + s1.d * s2.d
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.c * s2.c`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:80:19
|
||||
|
|
||||
LL | s1.a * s2.a + s1.b * s2.c + s1.c * s2.c
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.b * s2.b`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:80:19
|
||||
|
|
||||
LL | s1.a * s2.a + s1.b * s2.c + s1.c * s2.c
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.b * s2.b`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:85:19
|
||||
|
|
||||
LL | s1.a * s2.a + s2.b * s2.b + s1.c * s2.c
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.b * s2.b`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:90:19
|
||||
|
|
||||
LL | s1.a * s2.a + s1.b * s1.b + s1.c * s2.c
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.b * s2.b`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:95:5
|
||||
|
|
||||
LL | s1.a * s1.a + s1.b * s2.b + s1.c * s2.c
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.a * s2.a`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:100:33
|
||||
|
|
||||
LL | s1.a * s2.a + s1.b * s2.b + s1.c * s1.c
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.c * s2.c`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:113:20
|
||||
|
|
||||
LL | (s1.a * s2.a + s1.b * s1.b)
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.b * s2.b`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:118:34
|
||||
|
|
||||
LL | (s1.a * s2.a + s1.b * s2.b + s1.c * s2.b + s1.d * s2.d)
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.c * s2.c`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:123:38
|
||||
|
|
||||
LL | (s1.a * s2.a) + (s1.b * s2.b) + (s1.c * s2.b) + (s1.d * s2.d)
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.c * s2.c`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:128:39
|
||||
|
|
||||
LL | ((s1.a * s2.a) + (s1.b * s2.b) + (s1.c * s2.b) + (s1.d * s2.d))
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.c * s2.c`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:133:42
|
||||
|
|
||||
LL | (((s1.a * s2.a) + (s1.b * s2.b)) + ((s1.c * s2.b) + (s1.d * s2.d)))
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.c * s2.c`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:133:42
|
||||
|
|
||||
LL | (((s1.a * s2.a) + (s1.b * s2.b)) + ((s1.c * s2.b) + (s1.d * s2.d)))
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.c * s2.c`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:138:40
|
||||
|
|
||||
LL | (((s1.a * s2.a) + (s1.b * s2.b) + (s1.c * s2.b)) + (s1.d * s2.d))
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.c * s2.c`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:143:40
|
||||
|
|
||||
LL | ((s1.a * s2.a) + ((s1.b * s2.b) + (s1.c * s2.b) + (s1.d * s2.d)))
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.c * s2.c`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:148:20
|
||||
|
|
||||
LL | (s1.a * s2.a + s2.b * s2.b) / 2
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.b * s2.b`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:153:35
|
||||
|
|
||||
LL | i32::swap_bytes(s1.a * s2.a + s2.b * s2.b)
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.b * s2.b`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:158:29
|
||||
|
|
||||
LL | s1.a > 0 && s1.b > 0 && s1.d == s2.c && s1.d == s2.d
|
||||
| ^^^^^^^^^^^^ help: I think you meant: `s1.c == s2.c`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:163:17
|
||||
|
|
||||
LL | s1.a > 0 && s1.d == s2.c && s1.b > 0 && s1.d == s2.d
|
||||
| ^^^^^^^^^^^^ help: I think you meant: `s1.c == s2.c`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:172:77
|
||||
|
|
||||
LL | (n1.inner.0).0 == (n2.inner.0).0 && (n1.inner.1).0 == (n2.inner.1).0 && (n1.inner.2).0 == (n2.inner.1).0
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: I think you meant: `(n1.inner.2).0 == (n2.inner.2).0`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:186:25
|
||||
|
|
||||
LL | s1.a <= s2.a && s1.a <= s2.b
|
||||
| ^^^^^^^^^^^^ help: I think you meant: `s1.b <= s2.b`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:192:23
|
||||
|
|
||||
LL | if s1.a < s2.a && s1.a < s2.b {
|
||||
| ^^^^^^^^^^^ help: I think you meant: `s1.b < s2.b`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:199:48
|
||||
|
|
||||
LL | -(-(-s1.a * -s2.a) + (-(-s1.b * -s2.b) + -(-s1.c * -s2.b) + -(-s1.d * -s2.d)))
|
||||
| ^^^^^^^^^^^^^ help: I think you meant: `-s1.c * -s2.c`
|
||||
|
||||
error: This sequence of operators looks suspiciously like a bug.
|
||||
--> $DIR/suspicious_operation_groupings.rs:204:27
|
||||
|
|
||||
LL | -(if -s1.a < -s2.a && -s1.a < -s2.b { s1.c } else { s2.a })
|
||||
| ^^^^^^^^^^^^^ help: I think you meant: `-s1.b < -s2.b`
|
||||
|
||||
error: aborting due to 27 previous errors
|
||||
|
||||
Loading…
Reference in New Issue
Block a user