Auto merge of rust-lang#7223 - ThibsG:FpUselessConversion7205, r=camsteffen

bors · bors · commit 1aad7541db90 · 2021-05-17T17:40:48.000Z
Fix FPs about generic args Fix 2 false positives in [`use_self`] and [`useless_conversion`] lints, by taking into account generic args and comparing them. Fixes: rust-lang#7205 Fixes: rust-lang#7206 changelog: Fix FPs about generic args in [`use_self`] and [`useless_conversion`] lints
diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs
@@ -1,20 +1,20 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_opt;
+use clippy_utils::ty::same_type_and_consts;
 use clippy_utils::{in_macro, meets_msrv, msrvs};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
-use rustc_hir as hir;
-use rustc_hir::def::DefKind;
 use rustc_hir::{
-    def,
+    self as hir,
+    def::{self, DefKind},
     def_id::LocalDefId,
     intravisit::{walk_ty, NestedVisitorMap, Visitor},
     Expr, ExprKind, FnRetTy, FnSig, GenericArg, HirId, Impl, ImplItemKind, Item, ItemKind, Node, Path, PathSegment,
     QPath, TyKind,
 };
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::hir::map::Map;
-use rustc_middle::ty::{AssocKind, Ty, TyS};
+use rustc_middle::ty::{AssocKind, Ty};
 use rustc_semver::RustcVersion;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::{BytePos, Span};
@@ -459,7 +459,7 @@ fn in_impl(cx: &LateContext<'tcx>, hir_ty: &hir::Ty<'_>) -> bool {
 
 fn should_lint_ty(hir_ty: &hir::Ty<'_>, ty: Ty<'_>, self_ty: Ty<'_>) -> bool {
     if_chain! {
-        if TyS::same_type(ty, self_ty);
+        if same_type_and_consts(ty, self_ty);
         if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind;
         then {
             !matches!(path.res, def::Res::SelfTy(..))
diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs
@@ -1,13 +1,13 @@
 use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
 use clippy_utils::source::{snippet, snippet_with_macro_callsite};
 use clippy_utils::sugg::Sugg;
-use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::ty::{is_type_diagnostic_item, same_type_and_consts};
 use clippy_utils::{get_parent_expr, match_def_path, match_trait_method, paths};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, ExprKind, HirId, MatchSource};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty::{self, TyS};
+use rustc_middle::ty;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::sym;
 
@@ -67,7 +67,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
                 if match_trait_method(cx, e, &paths::INTO) && &*name.ident.as_str() == "into" {
                     let a = cx.typeck_results().expr_ty(e);
                     let b = cx.typeck_results().expr_ty(&args[0]);
-                    if TyS::same_type(a, b) {
+                    if same_type_and_consts(a, b) {
                         let sugg = snippet_with_macro_callsite(cx, args[0].span, "<expr>").to_string();
                         span_lint_and_sugg(
                             cx,
@@ -90,7 +90,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
                     }
                     let a = cx.typeck_results().expr_ty(e);
                     let b = cx.typeck_results().expr_ty(&args[0]);
-                    if TyS::same_type(a, b) {
+                    if same_type_and_consts(a, b) {
                         let sugg = snippet(cx, args[0].span, "<expr>").into_owned();
                         span_lint_and_sugg(
                             cx,
@@ -110,7 +110,8 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
                     if is_type_diagnostic_item(cx, a, sym::result_type);
                     if let ty::Adt(_, substs) = a.kind();
                     if let Some(a_type) = substs.types().next();
-                    if TyS::same_type(a_type, b);
+                    if same_type_and_consts(a_type, b);
+
                     then {
                         span_lint_and_help(
                             cx,
@@ -137,7 +138,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
                             if is_type_diagnostic_item(cx, a, sym::result_type);
                             if let ty::Adt(_, substs) = a.kind();
                             if let Some(a_type) = substs.types().next();
-                            if TyS::same_type(a_type, b);
+                            if same_type_and_consts(a_type, b);
 
                             then {
                                 let hint = format!("consider removing `{}()`", snippet(cx, path.span, "TryFrom::try_from"));
@@ -154,7 +155,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
 
                         if_chain! {
                             if match_def_path(cx, def_id, &paths::FROM_FROM);
-                            if TyS::same_type(a, b);
+                            if same_type_and_consts(a, b);
 
                             then {
                                 let sugg = Sugg::hir_with_macro_callsite(cx, &args[0], "<expr>").maybe_par();
diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs
@@ -322,3 +322,27 @@ pub fn walk_ptrs_ty_depth(ty: Ty<'_>) -> (Ty<'_>, usize) {
     }
     inner(ty, 0)
 }
+
+/// Returns `true` if types `a` and `b` are same types having same `Const` generic args,
+/// otherwise returns `false`
+pub fn same_type_and_consts(a: Ty<'tcx>, b: Ty<'tcx>) -> bool {
+    match (&a.kind(), &b.kind()) {
+        (&ty::Adt(did_a, substs_a), &ty::Adt(did_b, substs_b)) => {
+            if did_a != did_b {
+                return false;
+            }
+
+            substs_a
+                .iter()
+                .zip(substs_b.iter())
+                .all(|(arg_a, arg_b)| match (arg_a.unpack(), arg_b.unpack()) {
+                    (GenericArgKind::Const(inner_a), GenericArgKind::Const(inner_b)) => inner_a == inner_b,
+                    (GenericArgKind::Type(type_a), GenericArgKind::Type(type_b)) => {
+                        same_type_and_consts(type_a, type_b)
+                    },
+                    _ => true,
+                })
+        },
+        _ => a == b,
+    }
+}
diff --git a/tests/ui/use_self.fixed b/tests/ui/use_self.fixed
@@ -462,3 +462,33 @@ mod issue6818 {
         a: i32,
     }
 }
+
+mod issue7206 {
+    struct MyStruct<const C: char>;
+    impl From<MyStruct<'a'>> for MyStruct<'b'> {
+        fn from(_s: MyStruct<'a'>) -> Self {
+            Self
+        }
+    }
+
+    // keep linting non-`Const` generic args
+    struct S<'a> {
+        inner: &'a str,
+    }
+
+    struct S2<T> {
+        inner: T,
+    }
+
+    impl<T> S2<T> {
+        fn new() -> Self {
+            unimplemented!();
+        }
+    }
+
+    impl<'a> S2<S<'a>> {
+        fn new_again() -> Self {
+            Self::new()
+        }
+    }
+}
diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs
@@ -462,3 +462,33 @@ mod issue6818 {
         a: i32,
     }
 }
+
+mod issue7206 {
+    struct MyStruct<const C: char>;
+    impl From<MyStruct<'a'>> for MyStruct<'b'> {
+        fn from(_s: MyStruct<'a'>) -> Self {
+            Self
+        }
+    }
+
+    // keep linting non-`Const` generic args
+    struct S<'a> {
+        inner: &'a str,
+    }
+
+    struct S2<T> {
+        inner: T,
+    }
+
+    impl<T> S2<T> {
+        fn new() -> Self {
+            unimplemented!();
+        }
+    }
+
+    impl<'a> S2<S<'a>> {
+        fn new_again() -> Self {
+            S2::new()
+        }
+    }
+}
diff --git a/tests/ui/use_self.stderr b/tests/ui/use_self.stderr
@@ -162,5 +162,11 @@ error: unnecessary structure name repetition
 LL |             A::new::<submod::B>(submod::B {})
    |             ^ help: use the applicable keyword: `Self`
 
-error: aborting due to 27 previous errors
+error: unnecessary structure name repetition
+  --> $DIR/use_self.rs:491:13
+   |
+LL |             S2::new()
+   |             ^^ help: use the applicable keyword: `Self`
+
+error: aborting due to 28 previous errors
 
diff --git a/tests/ui/useless_conversion.fixed b/tests/ui/useless_conversion.fixed
@@ -70,4 +70,23 @@ fn main() {
     let a: i32 = 1;
     let b: i32 = 1;
     let _ = (a + b) * 3;
+
+    // see #7205
+    let s: Foo<'a'> = Foo;
+    let _: Foo<'b'> = s.into();
+    let s2: Foo<'a'> = Foo;
+    let _: Foo<'a'> = s2;
+    let s3: Foo<'a'> = Foo;
+    let _ = s3;
+    let s4: Foo<'a'> = Foo;
+    let _ = vec![s4, s4, s4].into_iter();
+}
+
+#[derive(Copy, Clone)]
+struct Foo<const C: char>;
+
+impl From<Foo<'a'>> for Foo<'b'> {
+    fn from(_s: Foo<'a'>) -> Self {
+        Foo
+    }
 }
diff --git a/tests/ui/useless_conversion.rs b/tests/ui/useless_conversion.rs
@@ -70,4 +70,23 @@ fn main() {
     let a: i32 = 1;
     let b: i32 = 1;
     let _ = i32::from(a + b) * 3;
+
+    // see #7205
+    let s: Foo<'a'> = Foo;
+    let _: Foo<'b'> = s.into();
+    let s2: Foo<'a'> = Foo;
+    let _: Foo<'a'> = s2.into();
+    let s3: Foo<'a'> = Foo;
+    let _ = Foo::<'a'>::from(s3);
+    let s4: Foo<'a'> = Foo;
+    let _ = vec![s4, s4, s4].into_iter().into_iter();
+}
+
+#[derive(Copy, Clone)]
+struct Foo<const C: char>;
+
+impl From<Foo<'a'>> for Foo<'b'> {
+    fn from(_s: Foo<'a'>) -> Self {
+        Foo
+    }
 }
diff --git a/tests/ui/useless_conversion.stderr b/tests/ui/useless_conversion.stderr
@@ -70,5 +70,23 @@ error: useless conversion to the same type: `i32`
 LL |     let _ = i32::from(a + b) * 3;
    |             ^^^^^^^^^^^^^^^^ help: consider removing `i32::from()`: `(a + b)`
 
-error: aborting due to 11 previous errors
+error: useless conversion to the same type: `Foo<'a'>`
+  --> $DIR/useless_conversion.rs:78:23
+   |
+LL |     let _: Foo<'a'> = s2.into();
+   |                       ^^^^^^^^^ help: consider removing `.into()`: `s2`
+
+error: useless conversion to the same type: `Foo<'a'>`
+  --> $DIR/useless_conversion.rs:80:13
+   |
+LL |     let _ = Foo::<'a'>::from(s3);
+   |             ^^^^^^^^^^^^^^^^^^^^ help: consider removing `Foo::<'a'>::from()`: `s3`
+
+error: useless conversion to the same type: `std::vec::IntoIter<Foo<'a'>>`
+  --> $DIR/useless_conversion.rs:82:13
+   |
+LL |     let _ = vec![s4, s4, s4].into_iter().into_iter();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![s4, s4, s4].into_iter()`
+
+error: aborting due to 14 previous errors
 
