about summary refs log tree commit diff
path: root/tests
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-10-08 21:24:54 +0000
committerbors <bors@rust-lang.org>2022-10-08 21:24:54 +0000
commit272bbfb857650e0d3d05dd83a5ce1a522c94b4bd (patch)
treeff6f3b941d23a479ff99eab4f44ae373cfa24d5c /tests
parent292e313259f422c8f4c31ecaedcc14058e8f4f8b (diff)
parent9cc8da222b3893bc13bc13c8827e93f8ea246854 (diff)
downloadrust-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.rs2
-rw-r--r--tests/ui/needless_borrow.fixed62
-rw-r--r--tests/ui/needless_borrow.rs60
-rw-r--r--tests/ui/needless_borrow.stderr96
-rw-r--r--tests/ui/unnecessary_to_owned.fixed2
-rw-r--r--tests/ui/unnecessary_to_owned.rs2
-rw-r--r--tests/versioncheck.rs2
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`")