Lint Reasons RFC

[myrrlyn]

This RFC makes two additions to the lint attributes in Rust code:

• add a reason field to the attributes that permits custom text when rendering
• add an expect lint attribute that acts as #[allow], but raises a lint when the lint it wants to allow is not raised.

Rendered
Tracking issue

[Manishearth]

cc @oli-obk @llogiq

[Ixrec]

Without prior knowledge of this RFC, if I saw #[expect(...)] in some random piece of code I'm not sure I'd realize it was lint-related at first. My first guess might be some sort of design-by-contract debug assertion on preconditions.

Come to think of it, why does the existing #[allow(...)] attribute not print a warning when the allowed lint doesn't happen anymore?

[scottmcm]

Come to think of it, why does the existing #[allow(...)] attribute not print a warning when the allowed lint doesn't happen anymore?

Probably in part because sometimes macros need to suppress things that only happen sometimes -- unreachable code in the error handling if the function returns -> Result<T, !>, for example.

[DavidMikeSimon]

Perhaps #[enforce(...)]?

[scottmcm]

Bikeshed: we have #[should_panic] in tests; maybe make this #[should_lint(...)]?

[jan-hudec]

I'd like to suggest an alternative for the second part:

Define an unused_allow lint that will generate a warning when there is an #[allow(…)], but the warning would not be generated anyway. Then the user can easily #![allow(unused_allow)] or #![warn(unused_allow)] as they prefer. Whether default state should be allow or warn is up to bikeshedding. As direct alternative it would start in allow, but I don't actually see much reason why it shouldn't default to warn.

[joshtriplett]

While I don't feel strongly about reason strings one way or another (and I definitely don't want the optional new comment form), I do very much like expect, and I'd like to see that. Almost any use of allow I've seen should become expect.

Also, you have a golden opportunity here: you should discuss the appropriate compiler behavior for #[expect(expectation_missing)] when applied to code that raises no warnings. The compiler clearly can neither warn nor not warn about the missing expectation. On the other hand, it can and should warn about a paradox.

[oli-obk]

Almost any use of allow I've seen should become expect.

That seems to me like an argument for the alternative with the unused_allow lint. If every occurrence of allow should become expect, then we should simply change the behaviour of allow.

[nikomatsakis]

I would prefer to keep allow with its current semantics, for two reasons:

• It feels like a breaking change to do otherwise, even if it strictly speaking conforms to our guidelines.
• Some projects, like LALRPOP, generate code that may trigger lints (notably exhaustiveness-style warnings) but also may not. In that case, it is useful to have allow -- if we only had expect, we would need some way to cover this case.

[joshtriplett]

We discussed this in the lang team meeting, and we were all quite excited about the expect attribute. There was strong consensus that we don't want to change the existing semantics of allow.

@rfcbot fcp merge

[rfcbot]

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged teams:

• @aturon
• @cramertj
• @eddyb
• @joshtriplett
• @nikomatsakis
• @nrc
• @pnkfelix
• @scottmcm
• @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[joshtriplett]

(Also, I wasn't entirely joking about the semantics of #[expect(expectation_missing)]; that should probably have an explicit definition just for the sake of not leaving the weird corner case.)

[scottmcm]

A few questions here:

• Does the reason on an allow get used anywhere?
• What happens with groups like #[expect(warnings)]?
• If I have crate-level #![expect(unused_mut)], is that usefully different from just allow?

[joshtriplett]

If I have crate-level #![expect(unused_mut)], is that usefully different from just allow?

Somewhat: the compiler will tell you if you don't have any unused_mut warnings, so you can remove it. It's more useful to put it directly where you need it, but you can put it at the crate level too and still get value from it.

[myrrlyn]

@scottmcm

Does the reason on an allow get used anywhere?

Not in the rustc linting system at present, which does nothing with allow flags. I have included it so that all the lint flags share the same grammar, and so that it can be left open for future use. At present it is able to be a comment viewed in source code, or there may be a hypothetical tool that collects all lint flags for display and renders what behavior is allowed/warned/denied/expected and why. But in short, allow(reason) is at present unused in the compiler.

What happens with groups?

Anything that is a member of the group matches; this is as true of warn and deny as it is of expect.

@joshtriplett

semantics of #[expect(expectation_missing)]

Let me see if I understand this correctly:

mod foo {
  #![expect(expectation_missing)]

  #[expect(unused_mut)]
  fn bar() {
    let mut v = v[3, 2, 4, 1, 5];
    v.sort();
  }
}

1. v mutates with .sort(), and so the mut binding is used. The linter does not raise an unused_mut event.

2. fn bar expects an unused_mut event and does not receive one. It therefore raises an expectation_missing event.

3. mod foo receives an expectation_missing event and traps it, and no lint event exits the module.

Now eliminate v.sort().

1. v is mutable and never mutated. The linter raises an unused_mut event.

2. fn bar receives an unused_mut event and traps it.

3. mod foo expects an expectation_missing event and does not receive one. It therefore raises an expectation_missing event, and a lint event exits the module. If this event is not silenced with allow or expect, then it will reach the crate boundary and the linter will display that mod foo expected to receive an expectation_missing event, but no such event occurred; please look at mod foo.

As far as I can understand this, these are consistent and non-paradoxical behaviors. The expect attribute is essentially a logical NOT gate: when a (matching) lint arrives, no lint departs; when a lint does not arrive, a lint departs.

All:

Should allow take on the lint-raising behavior of the proposed expect?

I agree that allow should remain unchanged for the reasons given.

Should expect be named something else?

I have become more aware that we use attributes for altering control flow (#[async]) and therefore an attribute name that looks like a control-flow term should be avoided for something that does not affect the actual generated code. I like should_lint as it matches an existing expectation of raised events, and I also like enforce as a single word to match the other lint attribute names. I am in favor of moving away from expect as the new linter, and have no strong preference for either name put forward so far. I suppose I would go with should_lint, as using enforce feels stronger than is warranted to me, and should_lint clearly (to me) conveys the idea that if the behavior that should happen does not, then an event will be raised.

---

I definitely don't want the optional new comment form

Agreed. I included it solely as a brainstorm, and to provided a prior-art reference for if a formalization of magic comments ever does arise. It is unrelated to the main thrust of this RFC and I will happily strip it from the text if/when this is approved to merge.

[joshtriplett]

@myrrlyn So, you're suggesting that an "expectation_missing" lint doesn't get looked at at the level it's produced, only at the next level out? So, in particular, if you'd put #[expect(expectation_missing)] on bar then your first case wouldn't work because the expectation_missing event would start working outward from mod foo and not from fn bar?

That's...consistent, but it seems wrong somehow. I think I'd prefer to have expectation_missing start at the same level as the expect that produced it, and then have a special case for expect(expectation_missing) or similar.

[myrrlyn]

... I think I don't know how lints travel the scope tree. The way I have always thought about it, they start at the scope where they were raised and then the linter searches "down", that is, through the progressively enclosing scopes, for a modifier and halts the search at the first one it finds. I also might be misunderstanding where lint control attributes are placed.

Suppose we have the following code:

#[expect(expectation_missing)]
mod foo {
  #[expect(expectation_missing)]
  fn bar() {
    #[expect(unused_mut)]
    let mut v = 0;
  }
}

This is what I think is the chronological order of events when this code gets linted.

• The linter begins searching crate scope. The crate has no lint handlers attached.
  • The linter begins searching mod foo scope. The module scope has a lint handler attached, and so the linter sets a trap for expectation_missing.
    • The linter begins searching fn bar scope, which also has a handler, and the linter sets a trap here as well.
      • The linter begins searching statement scope. The statement has a handler, so the linter sets that up.
        • The linter observes that v does not mutate, and adds unused_mut to its set of lints.
        • The statement ends, and the linter unwinds one step, to be inside fn bar scope.
      • When the linter crosses the statement scope boundary into fn bar, it runs the statement lint controllers.
      • The expect handler consumes the unused_mut lint, and then having received what it expected, disarms.
      • The linter continues searching fn bar scope (empty).
      • The linter unwinds one step, to be inside mod foo scope.
    • When the linter crosses the fn bar scope boundary into mod foo, it runs the lint controllers on fn bar. expect(expectation_missing) executes, and has no lint to consume, so it adds expectation_missing to the set of active lint events.
    • The linter continues searching mod foo scope (empty).
    • The linter unwinds one step, to be inside crate scope.
  • The linter has a standing lint item: expectation_missing. When the linter crosses the mod foo boundary into crate scope, it runs the lint controllers on mod foo. expect(expectation_missing) consumes the pending lint, and disarms.
  • The linter continues searching crate scope (empty).
  • The linter unwinds one step, to be OUTSIDE crate scope.
• The linter did not carry any lints outside crate scope, and the departure from crate scope did not run any expect traps, so the linter exits quietly.

Assuming I know how lints work, which is very questionable, each scope generates lints internally, that are then processed whenever the linter passes a semicolon or a closing brace.

Each expect handler can only run once. It either receives a lint from the set of lints generated in the scopes over which it has domain and destroys the lint, or it does not receive one and inserts an expectation_missing into the set of lints generated in the scope in which it resides. After either of these actions, the linter moves forward and will never revisit that handler. Therefore, any expectation_missing lint can only be processed when the linter departs the scope in which that lint was created.

This seems like it makes sense in my head but that's predicated on my current model of how the lint system works.

---

Update: I ran an experiment rather than make shit up. Consecutive lints on an item are processed as a stack.

#[deny(keyboard_smash)] // made up lint name
#[deny(unknown_lints)]
mod foo {}

When deny(unknown_lints) runs first, there are no lint events raised by the linter saying "I don't know what you said".

When deny(keyboard_smash) runs second, the linter says "I don't know what this is" and adds unknown_lint to its set of standing lints.

It runs the default unknown_lints handler at exit (warn) and compiles.

If I switch those two lines around, so keyboard_smash runs and lints before unknown_lints, which detects the lint, the compilation halts.

The expect handler and expectation_missing lint should behave the same way. They only process lints raised below them in scope and after them in peers on the same item, and the lints they themselves raise are only processed by handlers above them in scope or before them in peers on the same item.

#[expect(expectation_missing)]
#[expect(unused_mut)]
let v = 0;

No unused_mut is raised, so the lower expect fires. An expectation_missing is raised, so the upper expect disarms. No lints leave.

#[expect(unused_mut)]
#[expect(expectation_missing)]
let v = 0;

No expectation_missing is raised, so the lower lint fires. No unused_mut is raised, so the upper expect fires. Two lints leave.

[myrrlyn]

If lints aren't a stack machine then I don't really have any ideas :/

I feel like expect can really only work for running linters and catching lints down-scope of each expect declaration though, especially since expect is directly dependent on the state of the linter itself rather than being a pure analysis of the source code.

[nikomatsakis]

@rfcbot reviewed -- d'oh, thought I did this already

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

[Centril]

Huzzah! This RFC has been merged!

Tracking issue: rust-lang/rust#54503