New internal lint: unusual_names

[samueltardieu]

This lint aims at detecting unusual names used in Clippy source code, such as appl or application for a rustc_errors::Applicability variable, instead of app and applicability which are commonly used throughout Clippy.

This helps maintaining the consistency of the Clippy source code.

It is currently implemented for:

• Applicability: app or applicability
• EarlyContext/LateContext: cx
• TyCtxt: tcx

changelog: none

[rustbot]

r? @Jarcho

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Jarcho]

Seems reasonable. Will wait a bit if anyone on the team has anything to add.

cc @rust-lang/clippy

[Alexendoo]

Sounds fine to me, feels like something we could make into a disallowed- style lint

[allowed-names]
"rustc_errors::Applicability" = ["app", "applicability"]
"rustc_middle::ty::TyCtxt" = "tcx"
# etc

[samueltardieu]

I thought about this and as long as it is an internal lint only we can edit the source as easily as we would edit the configuration file. But if we make it a general lint, it would be a good idea to make it configurable of course.

[blyxyas]

We don't want to register symbols into the general compiler if they're only used for an internal lint. Could we either cfg-gate them or move this logic to an internal module?

[samueltardieu]

We already do that a lot: sym::expn_data, sym::outer_expn, sym::lint_vec, sym::check_attributes, etc. Why do it differently for this particular lint?

Maybe you should file a separate issue if you think it is worth the complexity to have a separate symbol list. I'm not sure this is needed, as those symbols are created at compile time as constants, and they are few so that they probably don't create many collisions when dynamically interning new strings when parsing code.

[blyxyas]

In that case, I'll do an experiment and open a Pr performing the optimization if it turns out to be significant. Although in a mini-benchmark with -Aclippy::all this PR showed to be about +0.2% on tokio 1.38.1, and that's what really brought me to post this comment.

[Jarcho]

Do not cfg these out. Having to rebuild all of clippy to switch on the internal feature is not a good trade off.

You're almost certainly measuring noise. Symbol interning shouldn't even be 0.1% of the runtime and that's the only thing this affects.

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.