rust-clippy/tests/ui/suspicious_operation_groupings.rs
Ryan1729 af1cc5c911 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-27 17:50:16 -07:00

207 lines
5.2 KiB
Rust

#![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() {}