Skip to content

Inconsistent type-parameter order #7147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lukaslueg opened this issue Apr 30, 2021 · 3 comments · Fixed by #8831
Closed

Inconsistent type-parameter order #7147

lukaslueg opened this issue Apr 30, 2021 · 3 comments · Fixed by #8831
Assignees
Labels
A-lint Area: New lints good first issue These issues are a good way to get started with Clippy

Comments

@lukaslueg
Copy link
Contributor

lukaslueg commented Apr 30, 2021

What it does

Warn if type parameters are named identically but ordered inconsistently in type definitions and impl-blocks. This is a cousin to the new inconsistent_struct_constructor lint. With literal constructors, the order of variables does not matter (Foo { a, b } is the same as Foo { b, a }). Type parameters, on the other hand, are never referred to by name, but only by position, which is exactly reversed to the behavior of struct constructors. For example:

struct Foo<A, B> {
    a: A,
    b: B,
}

// Potential warning: `B` does not refer to `B` in the struct-definition, because of position.
impl<B, A> Foo<B, A> {
    fn type_names(&self) {
        let name_a = std::any::type_name::<A>();
        let name_b = std::any::type_name::<B>();
        println!("A: {}, B: {}", name_a, name_b);
    }

    fn values(&self)
    where
        A: std::fmt::Debug,
        B: std::fmt::Debug,
    {
        println!("a: {:?}, b: {:?}", self.a, self.b);
    }
}

fn main() {
    let foo = Foo { a: 0, b: "foo" };

    // Prints 'A: &str, B: i32'
    foo.type_names();

    // Prints 'a: 0, b: "foo"'
    foo.values();

    // Wait, what? A `i32` valued as "foo" ?
}

Categories (optional)

  • Kind: Possibly clippy::correctness, because getting this wrong may cause you to refer to the wrong type parameter.

What is the advantage of the recommended code over the original code

  • Naming the type parameters consistently avoids confusion between type definition and type implementation.

Drawbacks

None.

Example

// Note::
struct Foo<A, B> {
// ...

impl<B, A> Foo<B, A> {
//   ^--|- This type parameter refers to `Foo::A`, but is named `B`.
//      ^- This type parameter refers to `Foo::B`, but is named `A`

Could be written as:

impl<A, B> Foo<A, B> {
@lukaslueg lukaslueg added the A-lint Area: New lints label Apr 30, 2021
@camsteffen
Copy link
Contributor

Also consider cases like impl<T, A> Foo<T, A> where just the A is out of place. Maybe name the lint inconsistent_type_param_position.

We could also make a stricter lint which fires any time the variable name does not match. Then Foo<C, D> would lint because it expects <A, B>. type_param_mismatch

@camsteffen camsteffen added the good first issue These issues are a good way to get started with Clippy label May 3, 2021
@0alternis
Copy link

@rustbot claim

@0alternis 0alternis removed their assignment Oct 24, 2021
@arieluy
Copy link
Contributor

arieluy commented May 4, 2022

@rustbot claim

bors added a commit that referenced this issue Jun 3, 2022
Add new lint `mismatching_type_param_order`

changelog:
Add new lint `mismatching_type_param_order` for checking if type parameters
are consistent between type definitions and impl blocks.

fixes #7147
@bors bors closed this as completed in 7c0d649 Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good first issue These issues are a good way to get started with Clippy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants