compiler-team icon indicating copy to clipboard operation
compiler-team copied to clipboard

Report all lints, even if other errors already occurred.

Open oli-obk opened this issue 2 years ago • 7 comments

Proposal

Right now, if errors have occurred, we do not run our lints. This means that the user will see some errors, fix them, and once all of them are fixed, they will get new warnings and errors (the latter from deny lints).

If we always run lints, this reduces surprises of the "I fixed all the errors, now I'm getting new ones"-kind

This is already implemented: https://github.com/rust-lang/rust/pull/108813

Mentors or Reviewers

Reviewable by any compiler team member or compiler contributor.

Process

The main points of the Major Change Process are as follows:

  • [x] File an issue describing the proposal.
  • [ ] A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
    • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
    • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
  • [ ] Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

oli-obk avatar May 11 '23 11:05 oli-obk

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/compiler @rust-lang/compiler-contributors

rustbot avatar May 11 '23 11:05 rustbot

@rustbot second

tmiasko avatar May 11 '23 12:05 tmiasko

@rustbot concern errors-inherently-higher-prioirty

I'm worried about this change. Getting a wall of diagnostics can make it hard for users to tease out which things that they have to address to get their code to compile.

At the very least, are we going to offer a flag that lets someone opt back into the old behavior, for those of us who prefer it? (or is your thinking "if you want the old behavior, you can first compile with --cap-lints allow?)

pnkfelix avatar May 12 '23 15:05 pnkfelix

(or is your thinking "if you want the old behavior, you can first compile with --cap-lints allow?)

well... -Awarnings, but yes.

I think we should improve diagnostics in other ways than having this hard and very arbitrary limit. Not all our lints are run in the generic lint machinery, but reported throughout the compilation. Those are already reported before the lints I enable here, so this PR just treats all lints equally.

I would expect we'll figure out how to

  • make most lints incremental (per item) instead of running all of them over the entire crate
  • avoid emitting lints for items that have errors (instead of what this PR does, avoid emitting lints if there were any errors (even other denied lints))

oli-obk avatar May 16 '23 09:05 oli-obk

For what it's worth, I strongly prefer only seeing errors as long as there are errors (but yes, denied lints should count as errors). There's time to fix warnings once the code compiles (first priority) and the program runs/tests pass (second priority).

jdahlstrom avatar Oct 27 '23 22:10 jdahlstrom

I find it really depends on the lint. Unused imports? I (almost certainly, but not necessarily) don't want to see them. But lints that are (more frequently) tied to bugs? I often do. It's hard to predict. Often times I am editing one part of the code and deliberately ignoring errors in other parts.

I think anything that assumes people are going down the list of errors 1 by 1 is probably suspect, I tend to prefer a free-form approach, especially when I'm in an IDE.

nikomatsakis avatar Oct 28 '23 11:10 nikomatsakis

We already have one-off logic in diagnostics to silence redundant errors. We would have to extend those kind of checks to lints as well. Because that kind of silencing requires hand checking we can indeed take these kind of considerations when doing that.

estebank avatar Oct 30 '23 00:10 estebank