Skip to content

Commit

Permalink
Add new lint type_param_mismatch
Browse files Browse the repository at this point in the history
Add new lint for checking if type parameters are consistent between type
definitions and impl blocks.
  • Loading branch information
arieluy committed May 15, 2022
1 parent c10bfae commit a0d5208
Show file tree
Hide file tree
Showing 7 changed files with 242 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3744,6 +3744,7 @@ Released 2018-09-13
[`trivially_copy_pass_by_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref
[`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err
[`type_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
[`type_param_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_param_mismatch
[`type_repetition_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds
[`undocumented_unsafe_blocks`]: https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks
[`undropped_manually_drops`]: https://rust-lang.github.io/rust-clippy/master/index.html#undropped_manually_drops
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,7 @@ store.register_lints(&[
transmute::WRONG_TRANSMUTE,
transmuting_null::TRANSMUTING_NULL,
try_err::TRY_ERR,
type_param_mismatch::TYPE_PARAM_MISMATCH,
types::BORROWED_BOX,
types::BOX_COLLECTION,
types::LINKEDLIST,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_pedantic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
LintId::of(stable_sort_primitive::STABLE_SORT_PRIMITIVE),
LintId::of(strings::STRING_ADD_ASSIGN),
LintId::of(transmute::TRANSMUTE_PTR_TO_PTR),
LintId::of(type_param_mismatch::TYPE_PARAM_MISMATCH),
LintId::of(types::LINKEDLIST),
LintId::of(types::OPTION_OPTION),
LintId::of(unicode::UNICODE_NOT_NFC),
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ mod trait_bounds;
mod transmute;
mod transmuting_null;
mod try_err;
mod type_param_mismatch;
mod types;
mod undocumented_unsafe_blocks;
mod unicode;
Expand Down Expand Up @@ -904,6 +905,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(strings::TrimSplitWhitespace));
store.register_late_pass(|| Box::new(rc_clone_in_vec_init::RcCloneInVecInit));
store.register_early_pass(|| Box::new(duplicate_mod::DuplicateMod::default()));
store.register_late_pass(|| Box::new(type_param_mismatch::TypeParamMismatch));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
113 changes: 113 additions & 0 deletions clippy_lints/src/type_param_mismatch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
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 TYPE_PARAM_MISMATCH,
pedantic,
"type parameter positioned inconsistently between type def and impl block"
}
declare_lint_pass!(TypeParamMismatch => [TYPE_PARAM_MISMATCH]);

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,
};

let type_name = segment.ident;

// 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();

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!("this type parameter is named {}, but it does not refer to {}::{}",
impl_param_name, type_name, impl_param_name);
span_lint_and_sugg(
cx,
TYPE_PARAM_MISMATCH,
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
}
60 changes: 60 additions & 0 deletions tests/ui/type_param_mismatch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#![warn(clippy::type_param_mismatch)]
#![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,
{
}
}
64 changes: 64 additions & 0 deletions tests/ui/type_param_mismatch.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
error: this type parameter is named B, but it does not refer to Foo::B
--> $DIR/type_param_mismatch.rs:11:20
|
LL | impl<B, A> Foo<B, A> {}
| ^ help: try: `A`
|
= note: `-D clippy::type-param-mismatch` implied by `-D warnings`

error: this type parameter is named A, but it does not refer to Foo::A
--> $DIR/type_param_mismatch.rs:11:23
|
LL | impl<B, A> Foo<B, A> {}
| ^ help: try: `B`

error: this type parameter is named A, but it does not refer to Foo::A
--> $DIR/type_param_mismatch.rs:14:23
|
LL | impl<C, A> Foo<C, A> {}
| ^ help: try: `B`

error: this type parameter is named B, but it does not refer to FooLifetime::B
--> $DIR/type_param_mismatch.rs:25:44
|
LL | impl<'m, 'l, B, A> FooLifetime<'m, 'l, B, A> {}
| ^ help: try: `A`

error: this type parameter is named A, but it does not refer to FooLifetime::A
--> $DIR/type_param_mismatch.rs:25:47
|
LL | impl<'m, 'l, B, A> FooLifetime<'m, 'l, B, A> {}
| ^ help: try: `B`

error: this type parameter is named C, but it does not refer to FooEnum::C
--> $DIR/type_param_mismatch.rs:41:27
|
LL | impl<C, A, B> FooEnum<C, A, B> {}
| ^ help: try: `A`

error: this type parameter is named A, but it does not refer to FooEnum::A
--> $DIR/type_param_mismatch.rs:41:30
|
LL | impl<C, A, B> FooEnum<C, A, B> {}
| ^ help: try: `B`

error: this type parameter is named B, but it does not refer to FooEnum::B
--> $DIR/type_param_mismatch.rs:41:33
|
LL | impl<C, A, B> FooEnum<C, A, B> {}
| ^ help: try: `C`

error: this type parameter is named B, but it does not refer to FooUnion::B
--> $DIR/type_param_mismatch.rs:52:31
|
LL | impl<B: Copy, A> FooUnion<B, A> where A: Copy {}
| ^ help: try: `A`

error: this type parameter is named A, but it does not refer to FooUnion::A
--> $DIR/type_param_mismatch.rs:52:34
|
LL | impl<B: Copy, A> FooUnion<B, A> where A: Copy {}
| ^ help: try: `B`

error: aborting due to 10 previous errors

0 comments on commit a0d5208

Please sign in to comment.