Skip to content

integer_arithmetic: detect integer arithmetic on references. #5329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions clippy_dev/src/stderr_length_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use std::path::{Path, PathBuf};

use walkdir::WalkDir;

use clippy_dev::clippy_project_root;

// The maximum length allowed for stderr files.
//
// We limit this because small files are easier to deal with than bigger files.
Expand All @@ -14,22 +16,24 @@ pub fn check() {

if !exceeding_files.is_empty() {
eprintln!("Error: stderr files exceeding limit of {} lines:", LENGTH_LIMIT);
for path in exceeding_files {
println!("{}", path.display());
for (path, count) in exceeding_files {
println!("{}: {}", path.display(), count);
}
std::process::exit(1);
}
}

fn exceeding_stderr_files() -> Vec<PathBuf> {
fn exceeding_stderr_files() -> Vec<(PathBuf, usize)> {
// We use `WalkDir` instead of `fs::read_dir` here in order to recurse into subdirectories.
WalkDir::new("../tests/ui")
WalkDir::new(clippy_project_root().join("tests/ui"))
.into_iter()
.filter_map(Result::ok)
.filter(|f| !f.file_type().is_dir())
.filter_map(|e| {
let p = e.into_path();
if p.extension() == Some(OsStr::new("stderr")) && count_linenumbers(&p) > LENGTH_LIMIT {
Some(p)
let count = count_linenumbers(&p);
if p.extension() == Some(OsStr::new("stderr")) && count > LENGTH_LIMIT {
Some((p, count))
} else {
None
}
Expand Down
5 changes: 3 additions & 2 deletions clippy_lints/src/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Arithmetic {
| hir::BinOpKind::Gt => return,
_ => (),
}

let (l_ty, r_ty) = (cx.tables.expr_ty(l), cx.tables.expr_ty(r));
if l_ty.is_integral() && r_ty.is_integral() {
if l_ty.peel_refs().is_integral() && r_ty.peel_refs().is_integral() {
span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected");
self.expr_span = Some(expr.span);
} else if l_ty.is_floating_point() && r_ty.is_floating_point() {
} else if l_ty.peel_refs().is_floating_point() && r_ty.peel_refs().is_floating_point() {
span_lint(cx, FLOAT_ARITHMETIC, expr.span, "floating-point arithmetic detected");
self.expr_span = Some(expr.span);
}
Expand Down
127 changes: 0 additions & 127 deletions tests/ui/arithmetic.stderr

This file was deleted.

11 changes: 0 additions & 11 deletions tests/ui/checked_unwrap/complex_conditionals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,4 @@ fn test_complex_conditions() {
}
}

fn test_nested() {
fn nested() {
let x = Some(());
if x.is_some() {
x.unwrap(); // unnecessary
} else {
x.unwrap(); // will panic
}
}
}

fn main() {}
19 changes: 1 addition & 18 deletions tests/ui/checked_unwrap/complex_conditionals.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -188,22 +188,5 @@ LL | if x.is_ok() || !(y.is_ok() && z.is_err()) {
LL | z.unwrap_err(); // unnecessary
| ^^^^^^^^^^^^^^

error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/complex_conditionals.rs:58:13
|
LL | if x.is_some() {
| ----------- the check is happening here
LL | x.unwrap(); // unnecessary
| ^^^^^^^^^^

error: This call to `unwrap()` will always panic.
--> $DIR/complex_conditionals.rs:60:13
|
LL | if x.is_some() {
| ----------- because of this check
...
LL | x.unwrap(); // will panic
| ^^^^^^^^^^

error: aborting due to 22 previous errors
error: aborting due to 20 previous errors

15 changes: 15 additions & 0 deletions tests/ui/checked_unwrap/complex_conditionals_nested.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
#![allow(clippy::if_same_then_else)]

fn test_nested() {
fn nested() {
let x = Some(());
if x.is_some() {
x.unwrap(); // unnecessary
} else {
x.unwrap(); // will panic
}
}
}

fn main() {}
31 changes: 31 additions & 0 deletions tests/ui/checked_unwrap/complex_conditionals_nested.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/complex_conditionals_nested.rs:8:13
|
LL | if x.is_some() {
| ----------- the check is happening here
LL | x.unwrap(); // unnecessary
| ^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/complex_conditionals_nested.rs:1:35
|
LL | #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

error: This call to `unwrap()` will always panic.
--> $DIR/complex_conditionals_nested.rs:10:13
|
LL | if x.is_some() {
| ----------- because of this check
...
LL | x.unwrap(); // will panic
| ^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/complex_conditionals_nested.rs:1:9
|
LL | #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
| ^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

53 changes: 53 additions & 0 deletions tests/ui/float_arithmetic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#![warn(clippy::integer_arithmetic, clippy::float_arithmetic)]
#![allow(
unused,
clippy::shadow_reuse,
clippy::shadow_unrelated,
clippy::no_effect,
clippy::unnecessary_operation,
clippy::op_ref,
clippy::trivially_copy_pass_by_ref
)]

#[rustfmt::skip]
fn main() {
let mut f = 1.0f32;

f * 2.0;

1.0 + f;
f * 2.0;
f / 2.0;
f - 2.0 * 4.2;
-f;

f += 1.0;
f -= 1.0;
f *= 2.0;
f /= 2.0;
}

// also warn about floating point arith with references involved

pub fn float_arith_ref() {
3.1_f32 + &1.2_f32;
&3.4_f32 + 1.5_f32;
&3.5_f32 + &1.3_f32;
}

pub fn float_foo(f: &f32) -> f32 {
let a = 5.1;
a + f
}

pub fn float_bar(f1: &f32, f2: &f32) -> f32 {
f1 + f2
}

pub fn float_baz(f1: f32, f2: &f32) -> f32 {
f1 + f2
}

pub fn float_qux(f1: f32, f2: f32) -> f32 {
(&f1 + &f2)
}
Loading