Rust-clippy: lint against impl trait in the argument position

Created on 22 May 2018  路  11Comments  路  Source: rust-lang/rust-clippy

while rust has just introduced impl trait to stable, impl trait in the argument position should be an official discouraged practice because there are now three ways to write a generic function.

by linting against this we can encourage the older syntax more often
it also allows those who disagree with it to discourage it in their code base

Most helpful comment

bother to read the why

uhm... I literally read and categorized every lint description two months ago

The else if without else lint has an actual safety standard notice:

Some coding guidelines require this (e.g. MISRA-C:2004 Rule 14.10).

The shadow* lints say

  • Still, some may opt to avoid it in their code base
  • Still, some argue that name shadowing like this hurts readability, because a value may be bound to different things depending on position in the code.
  • Name shadowing can hurt readability, especially in large code bases, because it is easy to lose track of the active binding at any place in the code

If you prefer to have citations of coding guidelines or safety standards here, please open an issue or better yet a PR.

Our lints and their descriptions depend on community members maintaining them, this is not our dayjob.

All 11 comments

I don't think it currently is an officially discouraged practice especially since the RFC landed pretty recently.

It currently isnt a discouraged practice just that one of the three ways to write generic function. With that, i think impl trait in args should be discouraged in code bases that disagree with using it. Having a lint for those who disagree with it would beneficial to those people. Those who dont just leave clippy lint
on allow which would hardly affect them, thus I see no reason to close this so suddenly just because,
it isnt officially discouraged yet.

just because it isnt officially discouraged yet.

https://github.com/rust-lang/rfcs/pull/2444#issuecomment-390719558 seems to suggest that that will never happen.

While we do have some unusual restriction lints, this is a special situation as there's loads of emotional stuff going on. I do not want to drag clippy into that discussion by offering a lint that goes against the majority of the community and against the official process for making changes.

Agreed. Clippy does have restriction lints but those are not for "I dislike this", those are for specialized situations where you want additional restrictions.

And yeah, as an emotional subject we're probably not going to touch it anyway.

Clippy isn't an escape hatch around community process.

I agree that dragging clippy into emotional battle would be absolutely horrible idea.
if you accept nearly every lint, but that lint always start out as allow in the beginning, then via some formal process for making these denied by default, clippy wont be drag into battle. yes the formal process might be a bit of a debate but that doesn't stop clippy for accepting any lint.

the point, I want to make crystal clear is that if someone feels strongly enough about the topic to write a lint against the behaivor, I think having the ability for clippy to accept even though it might go against the grain but set to allow by default seems perfectly reasonable. if clippy cant do that for a techinical reason then possibly a new project needs to be created for unusual lints "community members" would like to support.

in way, by being inclusive, clippy informal way of showing what kinda of code the community actually likes compared to what other "community members" or leaders thinks the community likes.

Yes, clippy _can_ accept such a lint, that doesn't mean it should.

A separate project for this sounds great.

for your comment @oli-obk, clippy has definitely did accept a lint that falls under "I dislike this".
and those lints are shadow_reuse, shadow_same, shadow_ as well as else if without else.

The shadow lints and the "else if without else" actually catch bugs in projects that have high safety requirements.

The only lint that comes to mind that violates this rule is the assign_ops lint, and I'll happily accept a PR that removes this lint!

I have to disagree with @oli-obk, if you actually bother to read the why for those clipply lints, they all mention, "community members" believe having those features makes readability worse.
readability is inherently subjective.

on, a finer note, being against the official process, isn't necessarily bad thing, It can be a good thing to show, how possibly broken an official process is. (I don't believe it is quite yet). my hope is that lint inclusion wasn't as bureaucratic as RFC process. however it is looking like it is.
thank you for reading all of my comments, I appreciate any thought you have put into it.

bother to read the why

uhm... I literally read and categorized every lint description two months ago

The else if without else lint has an actual safety standard notice:

Some coding guidelines require this (e.g. MISRA-C:2004 Rule 14.10).

The shadow* lints say

  • Still, some may opt to avoid it in their code base
  • Still, some argue that name shadowing like this hurts readability, because a value may be bound to different things depending on position in the code.
  • Name shadowing can hurt readability, especially in large code bases, because it is easy to lose track of the active binding at any place in the code

If you prefer to have citations of coding guidelines or safety standards here, please open an issue or better yet a PR.

Our lints and their descriptions depend on community members maintaining them, this is not our dayjob.

The "why" in the codebase need not be the full set of reasons why. We're not great about documentation.

Shadowing actually _is_ problematic, but not too problematic. Those in safety-critical environments may like it.

Was this page helpful?
0 / 5 - 0 ratings