diff options
| author | bors <bors@rust-lang.org> | 2022-10-08 21:24:54 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-10-08 21:24:54 +0000 |
| commit | 272bbfb857650e0d3d05dd83a5ce1a522c94b4bd (patch) | |
| tree | ff6f3b941d23a479ff99eab4f44ae373cfa24d5c /tests | |
| parent | 292e313259f422c8f4c31ecaedcc14058e8f4f8b (diff) | |
| parent | 9cc8da222b3893bc13bc13c8827e93f8ea246854 (diff) | |
| download | rust-272bbfb857650e0d3d05dd83a5ce1a522c94b4bd.tar.gz rust-272bbfb857650e0d3d05dd83a5ce1a522c94b4bd.zip | |
Auto merge of #9386 - smoelius:further-enhance-needless-borrow, r=Jarcho
Further enhance `needless_borrow`, mildly refactor `redundant_clone` This PR does the following: * Moves some code from `redundant_clone` into a new `clippy_utils` module called `mir`, and wraps that code in a function called `dropped_without_further_use`. * Relaxes the "is copyable" condition condition from #9136 by also suggesting to remove borrows from values dropped without further use. The changes involve the just mentioned function. * Separates `redundant_clone` into modules. Strictly speaking, the last bullet is independent of the others. `redundant_clone` is somewhat hairy, IMO. Separating it into modules makes it slightly less so, by helping to delineate what depends upon what. I've tried to break everything up into digestible commits. r? `@Jarcho` (`@Jarcho` I hope you don't mind.) changelog: continuation of #9136
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/compile-test.rs | 2 | ||||
| -rw-r--r-- | tests/ui/needless_borrow.fixed | 62 | ||||
| -rw-r--r-- | tests/ui/needless_borrow.rs | 60 | ||||
| -rw-r--r-- | tests/ui/needless_borrow.stderr | 96 | ||||
| -rw-r--r-- | tests/ui/unnecessary_to_owned.fixed | 2 | ||||
| -rw-r--r-- | tests/ui/unnecessary_to_owned.rs | 2 | ||||
| -rw-r--r-- | tests/versioncheck.rs | 2 |
7 files changed, 189 insertions, 37 deletions
diff --git a/tests/compile-test.rs b/tests/compile-test.rs index fa769222d1a..c10ee969c01 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -283,7 +283,7 @@ fn run_ui_cargo() { env::set_current_dir(&src_path)?; let cargo_toml_path = case.path().join("Cargo.toml"); - let cargo_content = fs::read(&cargo_toml_path)?; + let cargo_content = fs::read(cargo_toml_path)?; let cargo_parsed: toml::Value = toml::from_str( std::str::from_utf8(&cargo_content).expect("`Cargo.toml` is not a valid utf-8 file!"), ) diff --git a/tests/ui/needless_borrow.fixed b/tests/ui/needless_borrow.fixed index aa2687159ef..340e89d2db1 100644 --- a/tests/ui/needless_borrow.fixed +++ b/tests/ui/needless_borrow.fixed @@ -3,7 +3,11 @@ #[warn(clippy::all, clippy::needless_borrow)] #[allow(unused_variables)] -#[allow(clippy::uninlined_format_args, clippy::unnecessary_mut_passed)] +#[allow( + clippy::uninlined_format_args, + clippy::unnecessary_mut_passed, + clippy::unnecessary_to_owned +)] fn main() { let a = 5; let ref_a = &a; @@ -134,6 +138,7 @@ fn main() { multiple_constraints([[""]]); multiple_constraints_normalizes_to_same(X, X); let _ = Some("").unwrap_or(""); + let _ = std::fs::write("x", "".to_string()); only_sized(&""); // Don't lint. `Sized` is only bound let _ = std::any::Any::type_id(&""); // Don't lint. `Any` is only bound @@ -276,8 +281,9 @@ mod copyable_iterator { fn dont_warn(mut x: Iter) { takes_iter(&mut x); } + #[allow(unused_mut)] fn warn(mut x: &mut Iter) { - takes_iter(&mut x) + takes_iter(x) } } @@ -327,3 +333,55 @@ fn issue9383() { ManuallyDrop::drop(&mut ocean.coral); } } + +#[allow(dead_code)] +fn closure_test() { + let env = "env".to_owned(); + let arg = "arg".to_owned(); + let f = |arg| { + let loc = "loc".to_owned(); + let _ = std::fs::write("x", &env); // Don't lint. In environment + let _ = std::fs::write("x", arg); + let _ = std::fs::write("x", loc); + }; + let _ = std::fs::write("x", &env); // Don't lint. Borrowed by `f` + f(arg); +} + +#[allow(dead_code)] +mod significant_drop { + #[derive(Debug)] + struct X; + + #[derive(Debug)] + struct Y; + + impl Drop for Y { + fn drop(&mut self) {} + } + + fn foo(x: X, y: Y) { + debug(x); + debug(&y); // Don't lint. Has significant drop + } + + fn debug(_: impl std::fmt::Debug) {} +} + +#[allow(dead_code)] +mod used_exactly_once { + fn foo(x: String) { + use_x(x); + } + fn use_x(_: impl AsRef<str>) {} +} + +#[allow(dead_code)] +mod used_more_than_once { + fn foo(x: String) { + use_x(&x); + use_x_again(&x); + } + fn use_x(_: impl AsRef<str>) {} + fn use_x_again(_: impl AsRef<str>) {} +} diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index d41251e8f6a..c93711ac8e2 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -3,7 +3,11 @@ #[warn(clippy::all, clippy::needless_borrow)] #[allow(unused_variables)] -#[allow(clippy::uninlined_format_args, clippy::unnecessary_mut_passed)] +#[allow( + clippy::uninlined_format_args, + clippy::unnecessary_mut_passed, + clippy::unnecessary_to_owned +)] fn main() { let a = 5; let ref_a = &a; @@ -134,6 +138,7 @@ fn main() { multiple_constraints(&[[""]]); multiple_constraints_normalizes_to_same(&X, X); let _ = Some("").unwrap_or(&""); + let _ = std::fs::write("x", &"".to_string()); only_sized(&""); // Don't lint. `Sized` is only bound let _ = std::any::Any::type_id(&""); // Don't lint. `Any` is only bound @@ -276,6 +281,7 @@ mod copyable_iterator { fn dont_warn(mut x: Iter) { takes_iter(&mut x); } + #[allow(unused_mut)] fn warn(mut x: &mut Iter) { takes_iter(&mut x) } @@ -327,3 +333,55 @@ fn issue9383() { ManuallyDrop::drop(&mut ocean.coral); } } + +#[allow(dead_code)] +fn closure_test() { + let env = "env".to_owned(); + let arg = "arg".to_owned(); + let f = |arg| { + let loc = "loc".to_owned(); + let _ = std::fs::write("x", &env); // Don't lint. In environment + let _ = std::fs::write("x", &arg); + let _ = std::fs::write("x", &loc); + }; + let _ = std::fs::write("x", &env); // Don't lint. Borrowed by `f` + f(arg); +} + +#[allow(dead_code)] +mod significant_drop { + #[derive(Debug)] + struct X; + + #[derive(Debug)] + struct Y; + + impl Drop for Y { + fn drop(&mut self) {} + } + + fn foo(x: X, y: Y) { + debug(&x); + debug(&y); // Don't lint. Has significant drop + } + + fn debug(_: impl std::fmt::Debug) {} +} + +#[allow(dead_code)] +mod used_exactly_once { + fn foo(x: String) { + use_x(&x); + } + fn use_x(_: impl AsRef<str>) {} +} + +#[allow(dead_code)] +mod used_more_than_once { + fn foo(x: String) { + use_x(&x); + use_x_again(&x); + } + fn use_x(_: impl AsRef<str>) {} + fn use_x_again(_: impl AsRef<str>) {} +} diff --git a/tests/ui/needless_borrow.stderr b/tests/ui/needless_borrow.stderr index 5af68706d4b..8b593268bec 100644 --- a/tests/ui/needless_borrow.stderr +++ b/tests/ui/needless_borrow.stderr @@ -1,5 +1,5 @@ error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:11:15 + --> $DIR/needless_borrow.rs:15:15 | LL | let _ = x(&&a); // warn | ^^^ help: change this to: `&a` @@ -7,172 +7,208 @@ LL | let _ = x(&&a); // warn = note: `-D clippy::needless-borrow` implied by `-D warnings` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:15:13 + --> $DIR/needless_borrow.rs:19:13 | LL | mut_ref(&mut &mut b); // warn | ^^^^^^^^^^^ help: change this to: `&mut b` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:27:13 + --> $DIR/needless_borrow.rs:31:13 | LL | &&a | ^^^ help: change this to: `&a` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:29:15 + --> $DIR/needless_borrow.rs:33:15 | LL | 46 => &&a, | ^^^ help: change this to: `&a` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:35:27 + --> $DIR/needless_borrow.rs:39:27 | LL | break &ref_a; | ^^^^^^ help: change this to: `ref_a` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:42:15 + --> $DIR/needless_borrow.rs:46:15 | LL | let _ = x(&&&a); | ^^^^ help: change this to: `&a` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:43:15 + --> $DIR/needless_borrow.rs:47:15 | LL | let _ = x(&mut &&a); | ^^^^^^^^ help: change this to: `&a` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:44:15 + --> $DIR/needless_borrow.rs:48:15 | LL | let _ = x(&&&mut b); | ^^^^^^^^ help: change this to: `&mut b` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:45:15 + --> $DIR/needless_borrow.rs:49:15 | LL | let _ = x(&&ref_a); | ^^^^^^^ help: change this to: `ref_a` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:48:11 + --> $DIR/needless_borrow.rs:52:11 | LL | x(&b); | ^^ help: change this to: `b` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:55:13 + --> $DIR/needless_borrow.rs:59:13 | LL | mut_ref(&mut x); | ^^^^^^ help: change this to: `x` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:56:13 + --> $DIR/needless_borrow.rs:60:13 | LL | mut_ref(&mut &mut x); | ^^^^^^^^^^^ help: change this to: `x` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:57:23 + --> $DIR/needless_borrow.rs:61:23 | LL | let y: &mut i32 = &mut x; | ^^^^^^ help: change this to: `x` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:58:23 + --> $DIR/needless_borrow.rs:62:23 | LL | let y: &mut i32 = &mut &mut x; | ^^^^^^^^^^^ help: change this to: `x` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:67:14 + --> $DIR/needless_borrow.rs:71:14 | LL | 0 => &mut x, | ^^^^^^ help: change this to: `x` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:73:14 + --> $DIR/needless_borrow.rs:77:14 | LL | 0 => &mut x, | ^^^^^^ help: change this to: `x` error: this expression borrows a value the compiler would automatically borrow - --> $DIR/needless_borrow.rs:85:13 + --> $DIR/needless_borrow.rs:89:13 | LL | let _ = (&x).0; | ^^^^ help: change this to: `x` error: this expression borrows a value the compiler would automatically borrow - --> $DIR/needless_borrow.rs:87:22 + --> $DIR/needless_borrow.rs:91:22 | LL | let _ = unsafe { (&*x).0 }; | ^^^^^ help: change this to: `(*x)` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:97:5 + --> $DIR/needless_borrow.rs:101:5 | LL | (&&()).foo(); | ^^^^^^ help: change this to: `(&())` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:106:5 + --> $DIR/needless_borrow.rs:110:5 | LL | (&&5).foo(); | ^^^^^ help: change this to: `(&5)` error: the borrowed expression implements the required traits - --> $DIR/needless_borrow.rs:131:51 + --> $DIR/needless_borrow.rs:135:51 | LL | let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap(); | ^^^^^^^^^^^^^ help: change this to: `["-a", "-l"]` error: the borrowed expression implements the required traits - --> $DIR/needless_borrow.rs:132:44 + --> $DIR/needless_borrow.rs:136:44 | LL | let _ = std::path::Path::new(".").join(&&"."); | ^^^^^ help: change this to: `"."` error: the borrowed expression implements the required traits - --> $DIR/needless_borrow.rs:133:23 + --> $DIR/needless_borrow.rs:137:23 | LL | deref_target_is_x(&X); | ^^ help: change this to: `X` error: the borrowed expression implements the required traits - --> $DIR/needless_borrow.rs:134:26 + --> $DIR/needless_borrow.rs:138:26 | LL | multiple_constraints(&[[""]]); | ^^^^^^^ help: change this to: `[[""]]` error: the borrowed expression implements the required traits - --> $DIR/needless_borrow.rs:135:45 + --> $DIR/needless_borrow.rs:139:45 | LL | multiple_constraints_normalizes_to_same(&X, X); | ^^ help: change this to: `X` error: this expression creates a reference which is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:136:32 + --> $DIR/needless_borrow.rs:140:32 | LL | let _ = Some("").unwrap_or(&""); | ^^^ help: change this to: `""` +error: the borrowed expression implements the required traits + --> $DIR/needless_borrow.rs:141:33 + | +LL | let _ = std::fs::write("x", &"".to_string()); + | ^^^^^^^^^^^^^^^ help: change this to: `"".to_string()` + error: this expression borrows a value the compiler would automatically borrow - --> $DIR/needless_borrow.rs:187:13 + --> $DIR/needless_borrow.rs:192:13 | LL | (&self.f)() | ^^^^^^^^^ help: change this to: `(self.f)` error: this expression borrows a value the compiler would automatically borrow - --> $DIR/needless_borrow.rs:196:13 + --> $DIR/needless_borrow.rs:201:13 | LL | (&mut self.f)() | ^^^^^^^^^^^^^ help: change this to: `(self.f)` error: the borrowed expression implements the required traits - --> $DIR/needless_borrow.rs:298:55 + --> $DIR/needless_borrow.rs:286:20 + | +LL | takes_iter(&mut x) + | ^^^^^^ help: change this to: `x` + +error: the borrowed expression implements the required traits + --> $DIR/needless_borrow.rs:304:55 | LL | let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap(); | ^^^^^^^^^^^^^ help: change this to: `["-a", "-l"]` -error: aborting due to 29 previous errors +error: the borrowed expression implements the required traits + --> $DIR/needless_borrow.rs:344:37 + | +LL | let _ = std::fs::write("x", &arg); + | ^^^^ help: change this to: `arg` + +error: the borrowed expression implements the required traits + --> $DIR/needless_borrow.rs:345:37 + | +LL | let _ = std::fs::write("x", &loc); + | ^^^^ help: change this to: `loc` + +error: the borrowed expression implements the required traits + --> $DIR/needless_borrow.rs:364:15 + | +LL | debug(&x); + | ^^ help: change this to: `x` + +error: the borrowed expression implements the required traits + --> $DIR/needless_borrow.rs:374:15 + | +LL | use_x(&x); + | ^^ help: change this to: `x` + +error: aborting due to 35 previous errors diff --git a/tests/ui/unnecessary_to_owned.fixed b/tests/ui/unnecessary_to_owned.fixed index f97583aa22f..fe09aad06bc 100644 --- a/tests/ui/unnecessary_to_owned.fixed +++ b/tests/ui/unnecessary_to_owned.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![allow(clippy::ptr_arg)] +#![allow(clippy::needless_borrow, clippy::ptr_arg)] #![warn(clippy::unnecessary_to_owned)] #![feature(custom_inner_attributes)] diff --git a/tests/ui/unnecessary_to_owned.rs b/tests/ui/unnecessary_to_owned.rs index aa5394a5657..3de6d0903c0 100644 --- a/tests/ui/unnecessary_to_owned.rs +++ b/tests/ui/unnecessary_to_owned.rs @@ -1,6 +1,6 @@ // run-rustfix -#![allow(clippy::ptr_arg)] +#![allow(clippy::needless_borrow, clippy::ptr_arg)] #![warn(clippy::unnecessary_to_owned)] #![feature(custom_inner_attributes)] diff --git a/tests/versioncheck.rs b/tests/versioncheck.rs index 9e07769a8e4..a6d8d0307ce 100644 --- a/tests/versioncheck.rs +++ b/tests/versioncheck.rs @@ -48,7 +48,7 @@ fn check_that_clippy_has_the_same_major_version_as_rustc() { // `RUSTC_REAL` if Clippy is build in the Rust repo with `./x.py`. let rustc = std::env::var("RUSTC_REAL").unwrap_or_else(|_| "rustc".to_string()); let rustc_version = String::from_utf8( - std::process::Command::new(&rustc) + std::process::Command::new(rustc) .arg("--version") .output() .expect("failed to run `rustc --version`") |
