13 KiB

Raw Permalink Blame History

Feature Name: lint_reasons
Start Date: 2018-04-02
RFC PR: rust-lang/rfcs#2383
Rust Issue: rust-lang/rust#54503

Summary

Rust has a number of code lints, both built into the compiler and provided through external tools, which provide guidelines for code style. The linter behavior can be customized by attaching attributes to regions of code to allow, warn, or forbid, certain lint checks.

The decision for allowing, warning on, or forbidding, specific lints is occasionally placed in a comment above the attribute or, more often, left unstated. This RFC proposes adding syntax to the lint attributes to encode the documented reason for a lint configuration choice.

Motivation

The style guide for a project, team, or company can cover far more than just syntax layout. Rules for the semantic shape of a codebase are documented in natural language and often in automated checking programs, such as the Rust compiler and Clippy. Because the decisions about what rules to follow or ignore shape the codebase and its growth, the decisions are worth storing in the project directly with the settings they affect.

It is common wisdom that only the text the environment can read stays true; text it ignores will drift out of sync with the code over time, if it was even in sync to begin. Lint settings should have an explanation for their use to explain why they were chosen and where they are or are not applicable. As they are text that is read by some lint program, they have an opportunity to include an explanation similar to the way Rust documentation is a first-class attribute on code.

The RFC template asks three questions for motivation:

Why are we doing this?

We are adding this behavior to give projects a mechanism for storing human design choices about code in a manner that the tools can track and use to empower human work. For example, the compiler could use the contents of the lint explanation when it emits lint messages, or the documenter could collect them into a set of code style information.

What use cases does it support?

This supports the use cases of projects, teams, or organizations using specific sets of code style guidelines beyond the Rust defaults. This also enables the creation and maintenance of documented practices and preferences that can be standardized in a useful way. Furthermore, this provides a standardized means of explaining decisions when a style guide must be violated by attaching an overriding lint attribute to a specific item.

What is the expected outcome?

The expected outcome of this RFC is that projects will have more information about the decisions and expectations of the project, and can have support from the tools to maintain and inform these decisions. Global and specific choices can have their information checked and maintained by the tools, and the Rust ecosystem can have a somewhat more uniform means of establishing code guidelines and practices.

I expect Clippy will be a significant benefactor of this RFC, as Clippy lints are far more specific and plentiful than the compiler lints, and from personal experience much more likely to want explanation for their use or disuse.

Guide-level explanation

When a linting tool such as the compiler or Clippy encounter a code span that they determine breaks one of their rules, they emit a message indicating the problem and, often, how to fix it. These messages explain how to make the linter program happy, but carry very little information on why the code may be a problem from a human perspective.

These lints can be configured away from the default settings by the use of an attribute modifying the code span that triggers a lint, or by setting the linter behavior for a module or crate, with attributes like #[allow(rule)] and #![deny(rule)].

It is generally good practice to include an explanation for why certain rules are set so that programmers working on a project can know what the project expects from their work. These explanations can be embedded directly in the lint attribute with the reason = "Your reasoning here" attribute.

For example, if you are implementing Ord on an enum where the discriminants are not the correct ordering, you might have code like this:

enum Severity { Red, Blue, Green, Yellow }
impl Ord for Severity {
    fn cmp(&self, other: &Self) -> Ordering {
        use Severity::*;
        use Ordering::*;
        match (*self, *other) {
            (Red, Red) |
            (Blue, Blue) |
            (Green, Green) |
            (Yellow, Yellow) => Equal,

            (Blue, _) => Greater,
            (Red, _) => Less,

            (Green, Blue) => Less,
            (Green, _) => Greater,

            (Yellow, Red) => Greater,
            (Yellow, _) => Less,
        }
    }
}

The ordering of the left hand side of the match branches is significant, and allows a compact number of match arms. However, if you're using Clippy, this will cause the match_same_arms lint to trip! You can silence the lint in this spot, and provide an explanation that indicates you are doing so deliberately, by placing this attribute above the match line:

#[allow(match_same_arms, reason = "The arms are order-dependent")]

Now, when the lints run, no warning will be raised on this specific instance, and there is an explanation of why you disabled the lint, directly in the lint command.

Similarly, if you want to increase the strictness of a lint, you can explain why you think the lint is worth warning or forbidding directly in it:

#![deny(float_arithmetic, reason = "This code runs in a context without floats")]

With a warning or denial marker, when a linting tool encounters such a lint trap it will emit its builtin diagnostic, but also include the reason in its output.

For instance, using the above Clippy lint and some floating-point arithmetic code will result in the following lint output:

error: floating-point arithmetic detected
reason: This code runs in a context without floats
 --> src/lib.rs:4:2
  |
4 |     a + b
  |     ^^^^^
  |
note: lint level defined here
 --> src/lib.rs:1:44
  |
1 | #![cfg_attr(deny(float_arithmetic, reason = "..."))]
  |                  ^^^^^^^^^^^^^^^^
  = help: for further information visit ...

`expect` Lint Attribute

This RFC adds an expect lint attribute that functions identically to allow, but will cause a lint to be emitted when the code it decorates does not raise a lint warning. This lint was inspired by Yehuda Katz:

@ManishEarth has anyone ever asked for something like #[expect(lint)] which would basically be like #[allow(lint)] but give you a lint warning if the problem ever went away?

I basically want to mark things as ok while doing initial work, but I need to know when safe to remove

— Yehuda Katz (@wycats)

March 30, 2018

When the lint passes run, the expect attribute suppresses a lint generated by the span to which it attached. It does not swallow any other lint raised, and when it does not receive a lint to suppress, it raises a lint warning itself. expect can take a reason field, which is printed when the lint is raised, just as with the allow/warn/deny markers.

This is used when prototyping and using code that will generate lints for now, but will eventually be refactored and stop generating lints and thus no longer need the permission.

#[expect(unused_mut, reason = "Everything is mut until I get this figured out")]
fn foo() -> usize {
    let mut a = Vec::new();
    a.len()
}

will remain quiet while you're not mutating a, but when you do write code that mutates it, or decide you don't need it mutable and strip the mut, the expect lint will fire and inform you that there is no unused mutation in the span.

#[expect(unused_mut, reason = "...")]
fn foo() {
    let a = Vec::new();
    a.len()
}

will emit

warning: expected lint `unused_mut` did not appear
reason: Everything is mut until I get this figured out
 --> src/lib.rs:1:1
  |
1 | #[expect(unused_mut, reason = "...")]
  |   -------^^^^^^^^^^-----------------
  |   |
  |   help: remove this `#[expect(...)]`
  |
  = note: #[warn(expectation_missing)] on by default

Reference-level explanation

This RFC adds a reason = STRING element to the three lint attributes. The diagnostic emitter in the compiler and other lint tools such as Clippy will need to be made aware of this element so that they can emit it in diagnostic text.

This RFC also adds the expect(lint_name, reason = STRING) lint attribute. The expect attribute uses the same lint-suppression mechanism that allow does, but will raise a new lint, expectation_missing (name open to change), when the lint it expects does not arrive.

The expectation_missing lint is itself subject to allow/expect/warn/deny attributes in a higher scope, so it is possible to suppress expectation failures, lint when no expectation failures occur, or fail the build when one occurs. The default setting is #![warn(expectation_missing)].

That’s pretty much it, for technical details.

OPTIONAL — Yet Another Comment Syntax

A sugar for lint text MAY be the line comment //# or the block comment /*# #*/ with U+0023 # NUMBER SIGN as the signifying character. These comments MUST be placed immediately above a lint attribute. They are collected into a single string and collapsed as the text content of the attribute they decorate using the same processing logic that documentation comments (/// and //! and their block variants) currently use. Example:

//# Floating Point Arithmetic Unsupported
//#
//# This crate is written to be run on an AVR processor which does not have
//# floating-point capability in hardware. As such, all floating-point work is
//# done in software routines that can take a significant amount of time and
//# space to perform. Rather than pay this cost, floating-point work is
//# statically disabled. All arithmetic is in fixed-point integer math, using
//# the `FixedPoint` wrapper over integer primitives.
#![deny(float_arithmetic)]

The # character is chosen as the signifying character to provide room for possible future expansion – these comments MAY in the future be repurposed as sugar for writing the text of an attribute that declares a string parameter that can accept such comments.

This comment syntax already pushes the edge of the scope of this RFC, and extension of all attributes is certainly beyond it.

Implementing this comment syntax would require extending the existing transform pass that replaces documentation comments with documentation attributes. Specifically, the transform pass would ensure that all lint comments are directly attached to a lint attribute, and then use the strip-and-trim method that the documentation comments experience to remove the comment markers and collapse the comment text, across multiple consecutive comment spans, into a single string that is then inserted as reason = STRING into the attribute.

Given that this is a lot of work and a huge addition to the comment grammar, the author does not expect it to be included in the main RFC at all, and is writing it solely to be a published prior art in case of future desire for such a feature.

Drawbacks

Why should we not do this?

Possibly low value add for the effort.

Rationale and alternatives

Why is this design the best in the space of possible designs?

Attributes taking descriptive text is a common pattern in Rust.
What other designs have been considered and what is the rationale for not choosing them?

None.
What is the impact of not doing this?

None.

Prior art

The stable and unstable attributes both take descriptive text parameters that appear in diagnostic and documentation output.

Unresolved questions

What parts of the design do you expect to resolve through the RFC process before this gets merged?

The name of the reason parameter.
What parts of the design do you expect to resolve through the implementation of this feature before stabilization?

The use sites of the reason parameter.
What related issues do you consider out of scope for this RFC that could be addressed in the future independently of the solution that comes out of this RFC?

The means of filling the reason parameter, or what tools like rustdoc would do with them.

13 KiB Raw Permalink Blame History Unescape Escape