From 58cd01c2fcda07f97efcd908b50e5256cf084593 Mon Sep 17 00:00:00 2001
From: Ariel Uy <ariel.b.uy@gmail.com>
Date: Fri, 13 May 2022 23:48:52 -0700
Subject: [PATCH] Add new lint mismatching_type_param_order
Add new lint for checking if type parameters are consistent between type
definitions and impl blocks.
---
CHANGELOG.md | 1 +
clippy_lints/src/lib.register_lints.rs | 1 +
clippy_lints/src/lib.register_pedantic.rs | 1 +
clippy_lints/src/lib.rs | 2 +
.../src/mismatching_type_param_order.rs | 116 ++++++++++++++++++
tests/ui/mismatching_type_param_order.rs | 60 +++++++++
tests/ui/mismatching_type_param_order.stderr | 83 +++++++++++++
7 files changed, 264 insertions(+)
create mode 100644 clippy_lints/src/mismatching_type_param_order.rs
create mode 100644 tests/ui/mismatching_type_param_order.rs
create mode 100644 tests/ui/mismatching_type_param_order.stderr
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1c7ebfdf035..0a44ffdd3d4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3560,6 +3560,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
index ea8040a319b..29bfc660d29 100644
--- a/clippy_lints/src/lib.register_lints.rs
+++ b/clippy_lints/src/lib.register_lints.rs
@@ -380,6 +380,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
index 5684972b8be..2e47a287d5c 100644
--- a/clippy_lints/src/lib.register_pedantic.rs
+++ b/clippy_lints/src/lib.register_pedantic.rs
@@ -67,6 +67,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
index 89b65ce5469..6c3d7594f5c 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -298,6 +298,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;
@@ -917,6 +918,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(|| Box::new(unused_rounding::UnusedRounding));
store.register_early_pass(move || Box::new(almost_complete_letter_range::AlmostCompleteLetterRange::new(msrv)));
store.register_late_pass(|| Box::new(swap_ptr_to_ref::SwapPtrToRef));
+ 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
new file mode 100644
index 00000000000..d466d54a6ba
--- /dev/null
+++ b/clippy_lints/src/mismatching_type_param_order.rs
@@ -0,0 +1,116 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use rustc_data_structures::fx::FxHashMap;
+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();
+ // hashmap of name -> index for mismatch_param_name
+ let type_param_names_hashmap: FxHashMap<&String, usize> =
+ type_param_names.iter().enumerate().map(|(i, param)| (param, i)).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_hashmap) {
+ let msg = format!("`{}` has a similarly named generic type parameter `{}` in its declaration, but in a different order",
+ type_name, impl_param_name);
+ let help = format!("try `{}`, or a name that does not conflict with `{}`'s generic params",
+ type_param_names[i], type_name);
+ span_lint_and_help(
+ cx,
+ MISMATCHING_TYPE_PARAM_ORDER,
+ impl_param_span,
+ &msg,
+ None,
+ &help
+ );
+ }
+ }
+ }
+ }
+ }
+}
+
+// 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: &String, type_param_names: &FxHashMap<&String, usize>) -> bool {
+ if let Some(j) = type_param_names.get(impl_param_name) {
+ if i != *j {
+ return true;
+ }
+ }
+ false
+}
diff --git a/tests/ui/mismatching_type_param_order.rs b/tests/ui/mismatching_type_param_order.rs
new file mode 100644
index 00000000000..8f286c9304c
--- /dev/null
+++ 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
new file mode 100644
index 00000000000..cb720256c50
--- /dev/null
+++ b/tests/ui/mismatching_type_param_order.stderr
@@ -0,0 +1,83 @@
+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> {}
+ | ^
+ |
+ = note: `-D clippy::mismatching-type-param-order` implied by `-D warnings`
+ = help: try `A`, or a name that does not conflict with `Foo`'s generic params
+
+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`, or a name that does not conflict with `Foo`'s generic params
+
+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`, or a name that does not conflict with `Foo`'s generic params
+
+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`, or a name that does not conflict with `FooLifetime`'s generic params
+
+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`, or a name that does not conflict with `FooLifetime`'s generic params
+
+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`, or a name that does not conflict with `FooEnum`'s generic params
+
+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`, or a name that does not conflict with `FooEnum`'s generic params
+
+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`, or a name that does not conflict with `FooEnum`'s generic params
+
+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`, or a name that does not conflict with `FooUnion`'s generic params
+
+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`, or a name that does not conflict with `FooUnion`'s generic params
+
+error: aborting due to 10 previous errors
+