Add new lint type_param_mismatch

arieluy · arieluy · commit d6062886d6b7 · 2022-05-27T23:08:00.000-07:00
Add new lint for checking if type parameters are consistent between type
definitions and impl blocks.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3557,6 +3557,7 @@ Released 2018-09-13
 [`min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_max
 [`misaligned_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#misaligned_transmute
 [`mismatched_target_os`]: https://rust-lang.github.io/rust-clippy/master/index.html#mismatched_target_os
+[`mismatching_type_param_order`]: https://rust-lang.github.io/rust-clippy/master/index.html#mismatching_type_param_order
 [`misrefactored_assign_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#misrefactored_assign_op
 [`missing_const_for_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn
 [`missing_docs_in_private_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
@@ -379,6 +379,7 @@ store.register_lints(&[
     misc_early::UNNEEDED_WILDCARD_PATTERN,
     misc_early::UNSEPARATED_LITERAL_SUFFIX,
     misc_early::ZERO_PREFIXED_LITERAL,
+    mismatching_type_param_order::MISMATCHING_TYPE_PARAM_ORDER,
     missing_const_for_fn::MISSING_CONST_FOR_FN,
     missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS,
     missing_enforced_import_rename::MISSING_ENFORCED_IMPORT_RENAMES,
diff --git a/clippy_lints/src/lib.register_pedantic.rs b/clippy_lints/src/lib.register_pedantic.rs
@@ -66,6 +66,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
     LintId::of(methods::UNNECESSARY_JOIN),
     LintId::of(misc::FLOAT_CMP),
     LintId::of(misc::USED_UNDERSCORE_BINDING),
+    LintId::of(mismatching_type_param_order::MISMATCHING_TYPE_PARAM_ORDER),
     LintId::of(mut_mut::MUT_MUT),
     LintId::of(needless_bitwise_bool::NEEDLESS_BITWISE_BOOL),
     LintId::of(needless_continue::NEEDLESS_CONTINUE),
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -296,6 +296,7 @@ mod methods;
 mod minmax;
 mod misc;
 mod misc_early;
+mod mismatching_type_param_order;
 mod missing_const_for_fn;
 mod missing_doc;
 mod missing_enforced_import_rename;
@@ -909,6 +910,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_early_pass(|| Box::new(duplicate_mod::DuplicateMod::default()));
     store.register_late_pass(|| Box::new(get_first::GetFirst));
     store.register_early_pass(|| Box::new(unused_rounding::UnusedRounding));
+    store.register_late_pass(|| Box::new(mismatching_type_param_order::TypeParamMismatch));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 
diff --git a/clippy_lints/src/mismatching_type_param_order.rs b/clippy_lints/src/mismatching_type_param_order.rs
@@ -0,0 +1,112 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use rustc_errors::Applicability;
+use rustc_hir::def::{DefKind, Res};
+use rustc_hir::{GenericArg, Item, ItemKind, QPath, Ty, TyKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::GenericParamDefKind;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for type parameters which are positioned inconsistently between
+    /// a type definition and impl block. Specifically, a paramater in an impl
+    /// block which has the same name as a parameter in the type def, but is in
+    /// a different place.
+    ///
+    /// ### Why is this bad?
+    /// Type parameters are determined by their position rather than name.
+    /// Naming type parameters inconsistently may cause you to refer to the
+    /// wrong type parameter.
+    ///
+    /// ### Example
+    /// ```rust
+    /// struct Foo<A, B> {
+    ///     x: A,
+    ///     y: B,
+    /// }
+    /// // inside the impl, B refers to Foo::A
+    /// impl<B, A> Foo<B, A> {}
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// struct Foo<A, B> {
+    ///     x: A,
+    ///     y: B,
+    /// }
+    /// impl<A, B> Foo<A, B> {}
+    /// ```
+    #[clippy::version = "1.62.0"]
+    pub MISMATCHING_TYPE_PARAM_ORDER,
+    pedantic,
+    "type parameter positioned inconsistently between type def and impl block"
+}
+declare_lint_pass!(TypeParamMismatch => [MISMATCHING_TYPE_PARAM_ORDER]);
+
+impl<'tcx> LateLintPass<'tcx> for TypeParamMismatch {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
+        if_chain! {
+            if !item.span.from_expansion();
+            if let ItemKind::Impl(imp) = &item.kind;
+            if let TyKind::Path(QPath::Resolved(_, path)) = &imp.self_ty.kind;
+            if let Some(segment) = path.segments.iter().next();
+            if let Some(generic_args) = segment.args;
+            if !generic_args.args.is_empty();
+            then {
+                // get the name and span of the generic parameters in the Impl
+                let impl_params = generic_args.args.iter()
+                .filter_map(|p|
+                    match p {
+                        GenericArg::Type(Ty {kind: TyKind::Path(QPath::Resolved(_, path)), ..}) =>
+                            Some((path.segments[0].ident.to_string(), path.span)),
+                        _ => None,
+                    }
+                );
+
+                // find the type that the Impl is for
+                // only lint on struct/enum/union for now
+                let defid = match path.res {
+                    Res::Def(DefKind::Struct | DefKind::Enum | DefKind::Union, defid) => defid,
+                    _ => return,
+                };
+
+                // get the names of the generic parameters in the type
+                let type_params = &cx.tcx.generics_of(defid).params;
+                let type_param_names: Vec<_> = type_params.iter()
+                .filter_map(|p|
+                    match p.kind {
+                        GenericParamDefKind::Type {..} => Some(p.name.to_string()),
+                        _ => None,
+                    }
+                ).collect();
+
+                let type_name = segment.ident;
+                for (i, (impl_param_name, impl_param_span)) in impl_params.enumerate() {
+                    if mismatch_param_name(i, &impl_param_name, &type_param_names) {
+                        let msg = format!("`{}` has a similarly named generic type parameter `{}` in its declaration, but in a different order",
+                                  type_name, impl_param_name);
+                            span_lint_and_sugg(
+                                cx,
+                                MISMATCHING_TYPE_PARAM_ORDER,
+                                impl_param_span,
+                                &msg,
+                                "try",
+                                type_param_names[i].to_string(),
+                                Applicability::MaybeIncorrect
+                            );
+                    }
+                }
+            }
+        }
+    }
+}
+
+// Checks if impl_param_name is the same as one of type_param_names,
+// and is in a different position
+fn mismatch_param_name(i: usize, impl_param_name: &str, type_param_names: &[String]) -> bool {
+    for (j, type_param_name) in type_param_names.iter().enumerate() {
+        if impl_param_name == type_param_name && i != j {
+            return true;
+        }
+    }
+    false
+}
diff --git a/tests/ui/mismatching_type_param_order.rs b/tests/ui/mismatching_type_param_order.rs
@@ -0,0 +1,60 @@
+#![warn(clippy::mismatching_type_param_order)]
+#![allow(clippy::blacklisted_name)]
+
+fn main() {
+    struct Foo<A, B> {
+        x: A,
+        y: B,
+    }
+
+    // lint on both params
+    impl<B, A> Foo<B, A> {}
+
+    // lint on the 2nd param
+    impl<C, A> Foo<C, A> {}
+
+    // should not lint
+    impl<A, B> Foo<A, B> {}
+
+    struct FooLifetime<'l, 'm, A, B> {
+        x: &'l A,
+        y: &'m B,
+    }
+
+    // should not lint on lifetimes
+    impl<'m, 'l, B, A> FooLifetime<'m, 'l, B, A> {}
+
+    struct Bar {
+        x: i32,
+    }
+
+    // should not lint
+    impl Bar {}
+
+    // also works for enums
+    enum FooEnum<A, B, C> {
+        X(A),
+        Y(B),
+        Z(C),
+    }
+
+    impl<C, A, B> FooEnum<C, A, B> {}
+
+    // also works for unions
+    union FooUnion<A: Copy, B>
+    where
+        B: Copy,
+    {
+        x: A,
+        y: B,
+    }
+
+    impl<B: Copy, A> FooUnion<B, A> where A: Copy {}
+
+    impl<A, B> FooUnion<A, B>
+    where
+        A: Copy,
+        B: Copy,
+    {
+    }
+}
diff --git a/tests/ui/mismatching_type_param_order.stderr b/tests/ui/mismatching_type_param_order.stderr
@@ -0,0 +1,64 @@
+error: `Foo` has a similarly named generic type parameter `B` in its declaration, but in a different order
+  --> $DIR/mismatching_type_param_order.rs:11:20
+   |
+LL |     impl<B, A> Foo<B, A> {}
+   |                    ^ help: try: `A`
+   |
+   = note: `-D clippy::mismatching-type-param-order` implied by `-D warnings`
+
+error: `Foo` has a similarly named generic type parameter `A` in its declaration, but in a different order
+  --> $DIR/mismatching_type_param_order.rs:11:23
+   |
+LL |     impl<B, A> Foo<B, A> {}
+   |                       ^ help: try: `B`
+
+error: `Foo` has a similarly named generic type parameter `A` in its declaration, but in a different order
+  --> $DIR/mismatching_type_param_order.rs:14:23
+   |
+LL |     impl<C, A> Foo<C, A> {}
+   |                       ^ help: try: `B`
+
+error: `FooLifetime` has a similarly named generic type parameter `B` in its declaration, but in a different order
+  --> $DIR/mismatching_type_param_order.rs:25:44
+   |
+LL |     impl<'m, 'l, B, A> FooLifetime<'m, 'l, B, A> {}
+   |                                            ^ help: try: `A`
+
+error: `FooLifetime` has a similarly named generic type parameter `A` in its declaration, but in a different order
+  --> $DIR/mismatching_type_param_order.rs:25:47
+   |
+LL |     impl<'m, 'l, B, A> FooLifetime<'m, 'l, B, A> {}
+   |                                               ^ help: try: `B`
+
+error: `FooEnum` has a similarly named generic type parameter `C` in its declaration, but in a different order
+  --> $DIR/mismatching_type_param_order.rs:41:27
+   |
+LL |     impl<C, A, B> FooEnum<C, A, B> {}
+   |                           ^ help: try: `A`
+
+error: `FooEnum` has a similarly named generic type parameter `A` in its declaration, but in a different order
+  --> $DIR/mismatching_type_param_order.rs:41:30
+   |
+LL |     impl<C, A, B> FooEnum<C, A, B> {}
+   |                              ^ help: try: `B`
+
+error: `FooEnum` has a similarly named generic type parameter `B` in its declaration, but in a different order
+  --> $DIR/mismatching_type_param_order.rs:41:33
+   |
+LL |     impl<C, A, B> FooEnum<C, A, B> {}
+   |                                 ^ help: try: `C`
+
+error: `FooUnion` has a similarly named generic type parameter `B` in its declaration, but in a different order
+  --> $DIR/mismatching_type_param_order.rs:52:31
+   |
+LL |     impl<B: Copy, A> FooUnion<B, A> where A: Copy {}
+   |                               ^ help: try: `A`
+
+error: `FooUnion` has a similarly named generic type parameter `A` in its declaration, but in a different order
+  --> $DIR/mismatching_type_param_order.rs:52:34
+   |
+LL |     impl<B: Copy, A> FooUnion<B, A> where A: Copy {}
+   |                                  ^ help: try: `B`
+
+error: aborting due to 10 previous errors
+
