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 31, 2019 · 255a713 · 255a713
2 parents aa4e57c + 08ca236
commit 255a713
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