Add impl_trait_in_params lint
#10197
Conversation
|
r? @Jarcho (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
the
S-waiting-on-review
blyxyas
force-pushed
the
impl_trait_param
branch
from
b99a1d5 to
942d2ac
Compare
|
This should be a restriction lint, |
| } | ||
| } | ||
|
|
||
| fn next_valid_letter(generics: &Generics<'_>) -> char { |
There was a problem hiding this comment.
If you really do want to suggest a name, it should be a 'good' name that matches the trait if possible. e.g.
Iterator/IntoIter/DoubleEndedIterator/ExactSizeIterator/TrustedLen/FusedIterator->I/J/KFn(...) -> bool->PFn(...) -> _->FHasher->HRead/BufRead/RangeBounds->Rio::Write/fmt::Write->W- Eveything else ->
T/U/V
If the name isn't available it would be better to leave a placeholder for the name rather than use any random name.
Just using a placeholder is also a reasonable option. Or even not implementing the list and just using T/U/V also works. If all three are already taken it's already beyond a reasonable number of meaningless names.
There was a problem hiding this comment.
ping @blyxyas. Are you still working on this?
There was a problem hiding this comment.
Yes I am still working on it. I thought the lint didn't need it because "Just using a placeholder is also a reasonable option" so I didn't change anything. Sorry for not seeing the ping before. I'll implement this right now.
blyxyas
force-pushed
the
impl_trait_param
branch
from
21479be to
7941073
Compare
|
What is wrong here? |
|
You're a few days behind master. Rebasing onto that and running |
|
I did that, using |
|
You're currently 27 commits behind. You need to pull the latest changes first. |
blyxyas
force-pushed
the
impl_trait_param
branch
from
7941073 to
452b474
Compare
|
☔ The latest upstream changes (presumably #10313) made this pull request unmergeable. Please resolve the merge conflicts. |
As this is a lint about "style", and a purely cosmetical choice (using `<A: Trait>` over `impl Trait`), a lot of other files needed to be allowed this lint.
blyxyas
force-pushed
the
impl_trait_param
branch
from
cdbf6a5 to
6ef34bf
Compare
|
Sorry for coming so late with 89fde4a, I didn't see the ping and had a very big confussion thinking that the PR was finished and needed no further changes to it. This new commit adds placeholders instead of name suggestions. |
|
remark failed because of a curl error when installing mdbook: Is this my fault? |
|
I think that's just a spurious error. Hopefully this works. @bors r+ Thank you. |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
As this is a lint about style, and using
impl Traitis purely cosmetical (even with downsides), a lot of unrelated files needed to allow this lint.Resolves #10030
changelog: New lint: [
impl_trait_in_params]10197