Website: Show which lints are auto-correctable with cargo fix

[phansch]

Now that cargo fix support is on the way for Clippy, it would be nice to update the lint docs to show which lints support auto-correct.

Since a lint can be emitted in different ways with different applicabilities, we have to decide which lints to include:

• Lints where all emitted diagnostics are MachineApplicable
• Lints that emit at least one MachineApplicable diagnostic

Apart from that we also have to somehow obtain this data. This is probably not going to be easy since our existing python tools for the website have pretty much no way to find out the applicability of lints.

[flip1995]

That would be great.

Maybe we could write an internal Clippy lint, that does this analysis? For performance reasons I would hide the registration of this lint behind a feature flag though.

[phansch]

Maybe we could write an internal Clippy lint, that does this analysis?

Seems like the best option. And I really want to have this feature.. So I'm going to try and write an internal 'lint'/data collector for this 😃

[xFrednet]

@phansch are you still working on this issue? It sounds interesting, and I would like to work on it, if that's okay for you? 🙃

And if so, have you already crated a plan how to implement it? I have a rough idea how it can be done, but I would be happy about pointers if you have already put some thoughts into it.

[flip1995]

This is probably not going to be easy since our existing python tools for the website have pretty much no way to find out the applicability of lints.

This suggest that it is time to RIIR our documentation generation scripts.

So I see 2 ways how to do this:

1. Implement an internal lint, that collects the data and writes it to a JSON or similar file, that can then be processed by the python scripts.
   a. If it is implemented in clippy_lints: We would have to figure out a system on how to suppress/enable the generation of this JSON file, since we don't want to always run this, only if we generate the documentation (deploy CI workflow)
   b. If it is implemented in clippy_dev: We would need to use rustc_private and add a minimal rustc wrapper to clippy_dev
2. RIIR our python documentation scripts and then include the fix data collector there, to avoid having to go through an output file (I think this could only be done with implementing variant 1.b. though)

1.a. would be the quick and dirty solution, 1.b. the not so quick, but still dirty, and 2. the slow and clean way.

How would you go about implementing this? @xFrednet

[phansch]

I'm not working on this currently, feel free to pick it up again =)

When I started working on this, I went with the 1.a approach from @flip1995. I have an unfinished WIP commit here: phansch@444d2c1#diff-132b8beb349994a7a16a8740b14a5cb5b1402f746d3c2a75295c2079cad6b70aR695

Essentially, we would have an internal lint that looks for span_lint* calls with arguments of type Applicability (this was working already). It would then figure out the name of the emitted lint and all the associated information we currently collect with the python script. The result of executing that lint code would be a JSON file.

[xFrednet]

Thank you for the detailed responses. I actually though of simply implementing a lint like the issue first suggested, that would be option 1a. The lint would then collect the Applicability information like @phansch described. My approach would have been similar to his WIP implementation.

Thinking more about it makes me like option 2(Rewriting the JSON export as a part of clippy_dev) a lot more. This is as you said @flip1995 the clean way and seams to be the correct place to implement it. However, I'm less sure how to do this in comparison to option 1a. I'm still happy to try working on it, but I'll probably need some more mentoring if that is okay? 😅

I already have one question if you don't mind:

• The need for rustc bindings is clear. But what is the wrapper for? Is it just to make sure that we can react to changes in rustc quickly or is there another reason?

---

TIL what RIIR, and it's an awesome abbreviation ^^

[phansch]

avoid having to go through an output file

Hm, why would that be a bad thing? I think that file could be useful for other things as well - For example we could use that file instead of all the Regex-based lint collection in clippy_dev

I think in any case, work could be started on a working lint data collector prototype, the rest is just a matter of deciding how it is integrated/used exactly =)

[flip1995]

Hm, why would that be a bad thing?

I meant a file that just contains the information about the auto fixable, which is then thrown away, after generating the documentation.

I think having a output file, which we can use for more than that, I have nothing against that. So we could have an internal lint that collects all the data for the lints and saves it in a JSON file. After that we can use that file as input for clippy_dev tools.

But what is the wrapper for?

So Clippy lints work by registering them as Callbacks in the RunCompiler struct. This is basically what I mean by wrapper. So if we should decide to implement this in clippy_dev instead of another internal lint, we would just need to implement it with RunCompiler and run it on clippy_lints.

---

I think (without trying it out myself) that in the long run it would be easier to implement it in clippy_dev. It feels weird to hack something into clippy_lints that emits a file.

EDIT: or maybe not. I think the easiest way would be to implement it as an internal lint, and hide it behind a feature. Then add a cargo dev command, that runs clippy on itself with just that internal lint enabled (and the feature ofc)

[xFrednet]

This sounds like a plan. Thank you for the input! 🙃

I'll finish the lint that I'm working on and then start work on this. I'll also try to implement #1303 and #6492 while I'm at it

[flip1995]

Thanks! This would be really great to have, but I can see that it will involve much experimentation. So if you need support for something, don't hesitate to ping me here or on zulip!

[xFrednet]

@rustbot claim

[xFrednet]

Hey @flip1995 and everyone else, I've been working on this in the form of a general metadata extractor which can currently extract the following information:

• lint_id
• lint_group
• lint declaration file and line
• documentation
• hard-coded applicability

A big thanks to @phansch for his previous work. It helped me a lot to get started!

---

The next step is to track dynamic applicability that is stored within values. I've implemented a tracking system that basically builds a value life pipeline. The following rust code:

let mut applicability = Applicability::MachineApplicable;
let snip = snippet_with_applicability(cx, block.span, "..", &mut applicability);
span_lint_and_sugg(
    cx,
    LINT_NAME,
    block.span,
    "...",
    "...",
    snip,
    applicability,
);

produces a tracking vector that looks something like this:

[ConstValue(MachineApplicable), Modifier(clippy_lints::utils::snippet_with_applicability), LintEmit("LINT_NAME")]

I would now like to have some kind of map for the Modifiers which allows us to define which applicabilities can lead to which new applicabilities. For example: The applicability modifications of snippet_with_applicability are based on the original value. The map for that function would look something like this:

Input	Possible output
MachineApplicable	[MaybeIncorrect, HasPlaceholders, MachineApplicable]
MaybeIncorrect	[MaybeIncorrect]
HasPlaceholders	[MaybeIncorrect, HasPlaceholders]
Unspecified	[Unspecified]

Do you have a recommendation what's the best way to store this? (Or maybe some completely different idea?)

The two solutions I came up with are:

1. The maps are just hard-coded in the metadata collector file. (This seems a bit inflexible to me but is of course simple.)
2. The map is in the method documentation or an attribute of the function. (This would be more flexible but would make the metadata collector not self-contained)

---

The current implementation state can be found here

[flip1995]

I think the only thing you have to deal with is if MachineApplicable may be modified anywhere. I would just mark it with a *. So if the applicability is literal Applicability::Anything I would just write Anything in the documentation. If the applicability starts with MachineApplicable and may be modified , I would put MachineApplicable* in the documentation.. If any other applicability may be modified, I would just put that applicability in the documentation, because it doesn't really matter to the user what the reason is that it isn't auto applicable..

[xFrednet]

That makes sense and sounds like an elegant enough solution :)