Rollup merge of rust-lang#65112 - jack-t:type-parens-lint, r=varkor

Add lint and tests for unnecessary parens around types This is my first contribution to the Rust project, so I apologize if I'm not doing things the right way. The PR fixes rust-lang#64169. It adds a lint and tests for unnecessary parentheses around types. I've run `tidy` and `rustfmt` — I'm not totally sure it worked right, though — and I've tried to follow the instructions linked in the readme. I tried to think through all the variants of `ast::TyKind` to find exceptions to this lint, and I could only find the one mentioned in the original issue, which concerns types with `dyn`. I'm not a Rust expert, thought, so I may well be missing something. There's also a problem with getting this to build. The new lint catches several things in the, e.g., `core`. Because `x.py` seems to build with an equivalent of `-Werror`, what would have been warnings cause the build to break. I got it to build and the tests to pass with `--warnings warn` on my `x.py build` and `x.py test` commands.
tmandry · Oct 30, 2019 · da5d7a7 · da5d7a7
2 parents ab9b789 + 08ca236
commit da5d7a7
Show file tree

Hide file tree

Showing 14 changed files with 65 additions and 27 deletions.
diff --git a/src/librustc/infer/error_reporting/nice_region_error/find_anon_type.rs b/src/librustc/infer/error_reporting/nice_region_error/find_anon_type.rs
@@ -62,7 +62,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
         &self,
         arg: &'tcx hir::Ty,
         br: &ty::BoundRegion,
-    ) -> Option<(&'tcx hir::Ty)> {
+    ) -> Option<&'tcx hir::Ty> {
         let mut nested_visitor = FindNestedTypeVisitor {
             tcx: self.tcx(),
             bound_region: *br,

diff --git a/src/librustc_data_structures/owning_ref/mod.rs b/src/librustc_data_structures/owning_ref/mod.rs
@@ -1046,14 +1046,14 @@ unsafe impl<O, T: ?Sized> CloneStableAddress for OwningRef<O, T>
     where O: CloneStableAddress {}
 
 unsafe impl<O, T: ?Sized> Send for OwningRef<O, T>
-    where O: Send, for<'a> (&'a T): Send {}
+    where O: Send, for<'a> &'a T: Send {}
 unsafe impl<O, T: ?Sized> Sync for OwningRef<O, T>
-    where O: Sync, for<'a> (&'a T): Sync {}
+    where O: Sync, for<'a> &'a T: Sync {}
 
 unsafe impl<O, T: ?Sized> Send for OwningRefMut<O, T>
-    where O: Send, for<'a> (&'a mut T): Send {}
+    where O: Send, for<'a> &'a mut T: Send {}
 unsafe impl<O, T: ?Sized> Sync for OwningRefMut<O, T>
-    where O: Sync, for<'a> (&'a mut T): Sync {}
+    where O: Sync, for<'a> &'a mut T: Sync {}
 
 impl Debug for dyn Erased {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {

diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs
@@ -598,6 +598,25 @@ impl EarlyLintPass for UnusedParens {
     fn check_arm(&mut self, cx: &EarlyContext<'_>, arm: &ast::Arm) {
         self.check_unused_parens_pat(cx, &arm.pat, false, false);
     }
+
+    fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) {
+        if let &ast::TyKind::Paren(ref r) = &ty.kind {
+            match &r.kind {
+                &ast::TyKind::TraitObject(..) => {}
+                &ast::TyKind::ImplTrait(_, ref bounds) if bounds.len() > 1 => {}
+                _ => {
+                    let pattern_text = if let Ok(snippet) = cx.sess().source_map()
+                        .span_to_snippet(ty.span) {
+                            snippet
+                        } else {
+                            pprust::ty_to_string(ty)
+                        };
+
+                    Self::remove_outer_parens(cx, ty.span, &pattern_text, "type", (false, false));
+                }
+            }
+        }
+    }
 }
 
 declare_lint! {

diff --git a/src/librustc_mir/borrow_check/prefixes.rs b/src/librustc_mir/borrow_check/prefixes.rs
@@ -29,7 +29,7 @@ pub(super) struct Prefixes<'cx, 'tcx> {
     body: &'cx Body<'tcx>,
     tcx: TyCtxt<'tcx>,
     kind: PrefixSet,
-    next: Option<(PlaceRef<'cx, 'tcx>)>,
+    next: Option<PlaceRef<'cx, 'tcx>>,
 }
 
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]

diff --git a/src/libstd/collections/hash/map.rs b/src/libstd/collections/hash/map.rs
@@ -1818,7 +1818,7 @@ impl<'a, K, V> Iterator for Keys<'a, K, V> {
     type Item = &'a K;
 
     #[inline]
-    fn next(&mut self) -> Option<(&'a K)> {
+    fn next(&mut self) -> Option<&'a K> {
         self.inner.next().map(|(k, _)| k)
     }
     #[inline]
@@ -1841,7 +1841,7 @@ impl<'a, K, V> Iterator for Values<'a, K, V> {
     type Item = &'a V;
 
     #[inline]
-    fn next(&mut self) -> Option<(&'a V)> {
+    fn next(&mut self) -> Option<&'a V> {
         self.inner.next().map(|(_, v)| v)
     }
     #[inline]
@@ -1864,7 +1864,7 @@ impl<'a, K, V> Iterator for ValuesMut<'a, K, V> {
     type Item = &'a mut V;
 
     #[inline]
-    fn next(&mut self) -> Option<(&'a mut V)> {
+    fn next(&mut self) -> Option<&'a mut V> {
         self.inner.next().map(|(_, v)| v)
     }
     #[inline]

diff --git a/src/test/ui/alignment-gep-tup-like-1.rs b/src/test/ui/alignment-gep-tup-like-1.rs
@@ -28,7 +28,7 @@ fn f<A:Clone + 'static>(a: A, b: u16) -> Box<dyn Invokable<A>+'static> {
     box Invoker {
         a: a,
         b: b,
-    } as (Box<dyn Invokable<A>+'static>)
+    } as Box<dyn Invokable<A>+'static>
 }
 
 pub fn main() {

diff --git a/src/test/ui/as-precedence.rs b/src/test/ui/as-precedence.rs
@@ -1,5 +1,6 @@
 // run-pass
 
+#[allow(unused_parens)]
 fn main() {
     assert_eq!(3 as usize * 3, 9);
     assert_eq!(3 as (usize) * 3, 9);

diff --git a/src/test/ui/close-over-big-then-small-data.rs b/src/test/ui/close-over-big-then-small-data.rs
@@ -30,7 +30,7 @@ fn f<A:Clone + 'static>(a: A, b: u16) -> Box<dyn Invokable<A>+'static> {
     box Invoker {
         a: a,
         b: b,
-    } as (Box<dyn Invokable<A>+'static>)
+    } as Box<dyn Invokable<A>+'static>
 }
 
 pub fn main() {

diff --git a/src/test/ui/functions-closures/closure-to-fn-coercion.rs b/src/test/ui/functions-closures/closure-to-fn-coercion.rs
@@ -10,7 +10,7 @@ const BAR: [fn(&mut u32); 5] = [
     |v: &mut u32| *v += 3,
     |v: &mut u32| *v += 4,
 ];
-fn func_specific() -> (fn() -> u32) {
+fn func_specific() -> fn() -> u32 {
     || return 42
 }
 

diff --git a/src/test/ui/lint/lint-unnecessary-parens.rs b/src/test/ui/lint/lint-unnecessary-parens.rs
@@ -13,6 +13,18 @@ fn bar(y: bool) -> X {
     return (X { y }); //~ ERROR unnecessary parentheses around `return` value
 }
 
+fn unused_parens_around_return_type() -> (u32) { //~ ERROR unnecessary parentheses around type
+    panic!()
+}
+
+trait Trait {
+    fn test(&self);
+}
+
+fn passes_unused_parens_lint() -> &'static (dyn Trait) {
+    panic!()
+}
+
 fn main() {
     foo();
     bar((true)); //~ ERROR unnecessary parentheses around function argument

diff --git a/src/test/ui/lint/lint-unnecessary-parens.stderr b/src/test/ui/lint/lint-unnecessary-parens.stderr
@@ -16,73 +16,79 @@ error: unnecessary parentheses around `return` value
 LL |     return (X { y });
    |            ^^^^^^^^^ help: remove these parentheses
 
+error: unnecessary parentheses around type
+  --> $DIR/lint-unnecessary-parens.rs:16:42
+   |
+LL | fn unused_parens_around_return_type() -> (u32) {
+   |                                          ^^^^^ help: remove these parentheses
+
 error: unnecessary parentheses around function argument
-  --> $DIR/lint-unnecessary-parens.rs:18:9
+  --> $DIR/lint-unnecessary-parens.rs:30:9
    |
 LL |     bar((true));
    |         ^^^^^^ help: remove these parentheses
 
 error: unnecessary parentheses around `if` condition
-  --> $DIR/lint-unnecessary-parens.rs:20:8
+  --> $DIR/lint-unnecessary-parens.rs:32:8
    |
 LL |     if (true) {}
    |        ^^^^^^ help: remove these parentheses
 
 error: unnecessary parentheses around `while` condition
-  --> $DIR/lint-unnecessary-parens.rs:21:11
+  --> $DIR/lint-unnecessary-parens.rs:33:11
    |
 LL |     while (true) {}
    |           ^^^^^^ help: remove these parentheses
 
 warning: denote infinite loops with `loop { ... }`
-  --> $DIR/lint-unnecessary-parens.rs:21:5
+  --> $DIR/lint-unnecessary-parens.rs:33:5
    |
 LL |     while (true) {}
    |     ^^^^^^^^^^^^ help: use `loop`
    |
    = note: `#[warn(while_true)]` on by default
 
 error: unnecessary parentheses around `match` head expression
-  --> $DIR/lint-unnecessary-parens.rs:23:11
+  --> $DIR/lint-unnecessary-parens.rs:35:11
    |
 LL |     match (true) {
    |           ^^^^^^ help: remove these parentheses
 
 error: unnecessary parentheses around `let` head expression
-  --> $DIR/lint-unnecessary-parens.rs:26:16
+  --> $DIR/lint-unnecessary-parens.rs:38:16
    |
 LL |     if let 1 = (1) {}
    |                ^^^ help: remove these parentheses
 
 error: unnecessary parentheses around `let` head expression
-  --> $DIR/lint-unnecessary-parens.rs:27:19
+  --> $DIR/lint-unnecessary-parens.rs:39:19
    |
 LL |     while let 1 = (2) {}
    |                   ^^^ help: remove these parentheses
 
 error: unnecessary parentheses around method argument
-  --> $DIR/lint-unnecessary-parens.rs:41:24
+  --> $DIR/lint-unnecessary-parens.rs:53:24
    |
 LL |     X { y: false }.foo((true));
    |                        ^^^^^^ help: remove these parentheses
 
 error: unnecessary parentheses around assigned value
-  --> $DIR/lint-unnecessary-parens.rs:43:18
+  --> $DIR/lint-unnecessary-parens.rs:55:18
    |
 LL |     let mut _a = (0);
    |                  ^^^ help: remove these parentheses
 
 error: unnecessary parentheses around assigned value
-  --> $DIR/lint-unnecessary-parens.rs:44:10
+  --> $DIR/lint-unnecessary-parens.rs:56:10
    |
 LL |     _a = (0);
    |          ^^^ help: remove these parentheses
 
 error: unnecessary parentheses around assigned value
-  --> $DIR/lint-unnecessary-parens.rs:45:11
+  --> $DIR/lint-unnecessary-parens.rs:57:11
    |
 LL |     _a += (1);
    |           ^^^ help: remove these parentheses
 
-error: aborting due to 12 previous errors
+error: aborting due to 13 previous errors
 
diff --git a/src/test/ui/single-use-lifetime/zero-uses-in-fn.fixed b/src/test/ui/single-use-lifetime/zero-uses-in-fn.fixed
@@ -15,7 +15,7 @@ fn october<'b, T>(s: &'b T) -> &'b T {
     s
 }
 
-fn november<'a>(s: &'a str) -> (&'a str) {
+fn november<'a>(s: &'a str) -> &'a str {
     //~^ ERROR lifetime parameter `'b` never used
     //~| HELP elide the unused lifetime
     s

diff --git a/src/test/ui/single-use-lifetime/zero-uses-in-fn.rs b/src/test/ui/single-use-lifetime/zero-uses-in-fn.rs
@@ -15,7 +15,7 @@ fn october<'a, 'b, T>(s: &'b T) -> &'b T {
     s
 }
 
-fn november<'a, 'b>(s: &'a str) -> (&'a str) {
+fn november<'a, 'b>(s: &'a str) -> &'a str {
     //~^ ERROR lifetime parameter `'b` never used
     //~| HELP elide the unused lifetime
     s

diff --git a/src/test/ui/single-use-lifetime/zero-uses-in-fn.stderr b/src/test/ui/single-use-lifetime/zero-uses-in-fn.stderr
@@ -21,7 +21,7 @@ LL | fn october<'a, 'b, T>(s: &'b T) -> &'b T {
 error: lifetime parameter `'b` never used
   --> $DIR/zero-uses-in-fn.rs:18:17
    |
-LL | fn november<'a, 'b>(s: &'a str) -> (&'a str) {
+LL | fn november<'a, 'b>(s: &'a str) -> &'a str {
    |               --^^
    |               |
    |               help: elide the unused lifetime