Associated types should not trigger a type_complexity message.

[moshevds]

This PR fixes #1013 by removing the check entirely. Whether this check is useful could be a point of discussion. But considering that type_complexity isn't applied to the generic types of a trait, I don't think it should be applied to the associated types of a trait either.

Still, A lint for the complexity of associated types could be useful. IMHO, it would be better to add that as a separate lint. (And maybe a different threshold.)

I have not contributed to rust-clippy before and did not dive deeply into this, so I might have missed some things. The fix is simple, but I would appreciate a careful review.

changelog: Don't trigger type_complexity for associated types.

[flip1995]

Thanks for directly opening a PR!

A few notes:

I think disabling it completely for TyAlias in impls isn't the right approach. E.g.

impl T for S {
    type X = Result<Option<Vec<u32>>, (String, Error)>;
}

// can be improved to

type OptVec = Option<Vec<u32>>;
type ErrTuple = (String, Error);
impl T for S {
    type X = Result<OptVec, ErrTuple>;
}

The problem at hand is rather, that Self::XYZ cannot be extracted. That means, we should only bail out if we find a assoc type from the same trait in the type description. This is not really harder, than what you did:

First you have to add a field to this visitor (assoc_type_found: bool or similar):

rust-clippy/clippy_lints/src/types.rs

Lines 1506 to 1511 in 63f818e

	struct TypeComplexityVisitor {
	/// total complexity score of the type
	score: u64,
	/// current nesting level
	nest: u64,
	}

After that you just have to find the assoc type in a impl type. This can be done by adding (Doc)

TyKind::Path(QPath::Resolved(Some(_), _)) => {
    self.assoc_type_found = true;
    return;
}

between those two arms:

rust-clippy/clippy_lints/src/types.rs

Lines 1519 to 1522 in 63f818e

	TyKind::Infer | TyKind::Ptr(..) | TyKind::Rptr(..) => (1, 0),
	// the "normal" components of a type: named types, arrays/tuples
	TyKind::Path(..) | TyKind::Slice(..) | TyKind::Tup(..) | TyKind::Array(..) => (10 * self.nest, 1),

After that just gate the linting not only on the score, but also on visitor.assoc_type_found, here:

rust-clippy/clippy_lints/src/types.rs

Lines 1488 to 1494 in 63f818e

	let score = {
	let mut visitor = TypeComplexityVisitor { score: 0, nest: 1 };
	visitor.visit_ty(ty);
	visitor.score
	};
	if score > self.threshold {

---

Also can you add the test to the already existing tests/ui/complex_types.rs file? Bonus points for renaming the test file to type_complexity.rs.

[phansch]

Hi @moshevds, are you still interested in finishing this PR up? Let us know if you need any further help =)

[moshevds]

Hi @phansch,

I did discuss it a bit more with @flip1995 on discord, and made some changes. But I did come across a couple of things I didn't immediately know how to solve. Other priorities came along, and I forgot to come back to this.

Looking back at what I did, I see that I added 3 commits related to the tests:
https://github.com/moshevds/rust-clippy/commits/type_complexity (see here)
If I remember correctly:

• The first commit is the filename change.
• The second commit is a test for the situation that shouldn't break while working on this.
• The third commit is a test for a situation that I think I assumed should also trigger the lint, but currently does not.

I also have a fix locally for the situation that third commit tests for, but I think I wasn't sure about the quality of that change.

All in all, the scope of this is a bit larger than I anticipated, in a project that I know almost nothing about. I'm still interested in improving this lint, but what is the best way to proceed?

• Perhaps I should rebase those first 2 commits and PR them independently?
• Next, maybe I should PR the third commit and add the fix that I have for it? I think it may need some quality improvements that we could discuss. (If it is even correct to extend the lint to that situation.)
• After that, proceed with the fix for Type complexity should not be considered when defining an associated type #1013?
• I also think that hints are an important addition to this lint, because in many situations the fix is not very obvious.

Perhaps you have another suggestion?

[phansch]

@moshevds Breaking down the task seems like a good plan, especially since you're just getting started. Feel free to open separate PRs for the commits 👍

[flip1995]

Thanks for contributing to Clippy! Sadly this PR was not updated in quite some time. If you waited on input from a reviewer, we're sorry that this fell under the radar. If you want to continue to work on this, just reopen the PR and/or ping a reviewer.

[jonboh]

@rustbot label -S-inactive-closed