Skip to content

Conversation

theodorebje
Copy link
Contributor

Motivation

This PR is part of the broader PR #3394 (Activate more lints)

Repeating type names over and over again quickly becomes a problem if the name of the type ever changes, or a refactored is required. It also makes it more difficult to copy code from one method to another.

Solution

In impls, Self can be used instead of the struct/enum being implemented, to reduce unnecessary repetition. This PR uses Self as often as possible. This fixes the use_self clippy lint.

In `impl`s, `Self` can be used instead of the struct/enum being
implemented, to reduce unnecessary repetition. This commit uses `Self`
as often as possible. This fixes the `use_self` clippy lint.
@theodorebje theodorebje mentioned this pull request Jul 3, 2025
5 tasks
@theodorebje
Copy link
Contributor Author

Copying over the previous discussion from #3394

Originally posted by mladedav in #3394 (comment)

Personally, I'm not a fan of this on 3rd party structs like Option<T> here or String somewhere else. Self::from_utf8 looks just weird to me.

So I wouldn't add this lint, but I get that this is stylistic, so if other people have different opinions, I don't mind.
Originally posted by theodorebje in #3394 (comment)

jplatte What do you think about this? Should we only use Self for Axum's types?

@jplatte
Copy link
Member

jplatte commented Jul 3, 2025

Yeah, let's be a bit more conservative and not change any of the places that David had some concerns about. You can still enable the lint, just #[allow] it in those specific places (there weren't many, right?).

NOTE: This commit adds multiple `#[allow]`s where Clippy would otherwise
warn about `Self` being allowed.
Copy link
Collaborator

@mladedav mladedav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think everything here is fine, if I didn't miss something, it's really just constructors and enum variants (and a few signatures).

@theodorebje
Copy link
Contributor Author

With all checks passing, this is ready to merge 👌

@mladedav
Copy link
Collaborator

mladedav commented Jul 4, 2025

@jplatte if you want to change some of the allows to Self, we can, otherwise let's merge this.

@jplatte jplatte enabled auto-merge (squash) July 4, 2025 20:45
@jplatte jplatte merged commit 0f255c3 into tokio-rs:main Jul 4, 2025
18 checks passed
@theodorebje theodorebje deleted the clippy-use-self branch July 4, 2025 21:34
@theodorebje
Copy link
Contributor Author

Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants