Skip to content

Commit

Permalink
feat: Stabilize lints
Browse files Browse the repository at this point in the history
Fixes #12115
  • Loading branch information
epage committed Sep 8, 2023
1 parent 4518131 commit e539380
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 294 deletions.
9 changes: 4 additions & 5 deletions src/cargo/ops/cargo_new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -817,11 +817,10 @@ fn mk(config: &Config, opts: &MkOptions<'_>) -> CargoResult<()> {
}

// Try to inherit the workspace lints key if it exists.
if config.cli_unstable().lints
&& workspace_document
.get("workspace")
.and_then(|workspace| workspace.get("lints"))
.is_some()
if workspace_document
.get("workspace")
.and_then(|workspace| workspace.get("lints"))
.is_some()
{
let mut table = toml_edit::Table::new();
table["workspace"] = toml_edit::value(true);
Expand Down
71 changes: 13 additions & 58 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ pub struct TomlManifest {
patch: Option<BTreeMap<String, BTreeMap<String, TomlDependency>>>,
workspace: Option<TomlWorkspace>,
badges: Option<MaybeWorkspaceBtreeMap>,
lints: Option<toml::Value>,
lints: Option<MaybeWorkspaceLints>,
}

#[derive(Deserialize, Serialize, Clone, Debug, Default)]
Expand Down Expand Up @@ -1456,7 +1456,7 @@ pub struct TomlWorkspace {
// Properties that can be inherited by members.
package: Option<InheritableFields>,
dependencies: Option<BTreeMap<String, TomlDependency>>,
lints: Option<toml::Value>,
lints: Option<TomlLints>,

// Note that this field must come last due to the way toml serialization
// works which requires tables to be emitted after all values.
Expand Down Expand Up @@ -1882,7 +1882,7 @@ impl TomlManifest {
let mut inheritable = toml_config.package.clone().unwrap_or_default();
inheritable.update_ws_path(package_root.to_path_buf());
inheritable.update_deps(toml_config.dependencies.clone());
let lints = parse_unstable_lints(toml_config.lints.clone(), config, &mut warnings)?;
let lints = toml_config.lints.clone();
let lints = verify_lints(lints)?;
inheritable.update_lints(lints);
if let Some(ws_deps) = &inheritable.dependencies {
Expand Down Expand Up @@ -2143,10 +2143,11 @@ impl TomlManifest {
&inherit_cell,
)?;

let lints =
parse_unstable_lints::<MaybeWorkspaceLints>(me.lints.clone(), config, cx.warnings)?
.map(|mw| mw.resolve(|| inherit()?.lints()))
.transpose()?;
let lints = me
.lints
.clone()
.map(|mw| mw.resolve(|| inherit()?.lints()))
.transpose()?;
let lints = verify_lints(lints)?;
let default = TomlLints::default();
let rustflags = lints_to_rustflags(lints.as_ref().unwrap_or(&default));
Expand Down Expand Up @@ -2447,7 +2448,10 @@ impl TomlManifest {
.badges
.as_ref()
.map(|_| MaybeWorkspace::Defined(metadata.badges.clone())),
lints: lints.map(|lints| toml::Value::try_from(lints).unwrap()),
lints: lints.map(|lints| MaybeWorkspaceLints {
workspace: false,
lints,
}),
};
let mut manifest = Manifest::new(
summary,
Expand Down Expand Up @@ -2579,7 +2583,7 @@ impl TomlManifest {
let mut inheritable = toml_config.package.clone().unwrap_or_default();
inheritable.update_ws_path(root.to_path_buf());
inheritable.update_deps(toml_config.dependencies.clone());
let lints = parse_unstable_lints(toml_config.lints.clone(), config, &mut warnings)?;
let lints = toml_config.lints.clone();
let lints = verify_lints(lints)?;
inheritable.update_lints(lints);
let ws_root_config = WorkspaceRootConfig::new(
Expand Down Expand Up @@ -2726,55 +2730,6 @@ impl TomlManifest {
}
}

fn parse_unstable_lints<T: Deserialize<'static>>(
lints: Option<toml::Value>,
config: &Config,
warnings: &mut Vec<String>,
) -> CargoResult<Option<T>> {
let Some(lints) = lints else {
return Ok(None);
};

if !config.cli_unstable().lints {
warn_for_lint_feature(config, warnings);
return Ok(None);
}

lints.try_into().map(Some).map_err(|err| err.into())
}

fn warn_for_lint_feature(config: &Config, warnings: &mut Vec<String>) {
use std::fmt::Write as _;

let key_name = "lints";
let feature_name = "lints";

let mut message = String::new();

let _ = write!(
message,
"unused manifest key `{key_name}` (may be supported in a future version)"
);
if config.nightly_features_allowed {
let _ = write!(
message,
"
consider passing `-Z{feature_name}` to enable this feature."
);
} else {
let _ = write!(
message,
"
this Cargo does not support nightly features, but if you
switch to nightly channel you can pass
`-Z{feature_name}` to enable this feature.",
);
}
warnings.push(message);
}

fn verify_lints(lints: Option<TomlLints>) -> CargoResult<Option<TomlLints>> {
let Some(lints) = lints else {
return Ok(None);
Expand Down
41 changes: 41 additions & 0 deletions src/doc/src/reference/manifest.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ Every manifest file consists of the following sections:
* [`[target]`](specifying-dependencies.md#platform-specific-dependencies) --- Platform-specific dependencies.
* [`[badges]`](#the-badges-section) --- Badges to display on a registry.
* [`[features]`](features.md) --- Conditional compilation features.
* [`[lints]`](#the-lints-section) --- Configure linters for this package.
* [`[patch]`](overriding-dependencies.md#the-patch-section) --- Override dependencies.
* [`[replace]`](overriding-dependencies.md#the-replace-section) --- Override dependencies (deprecated).
* [`[profile]`](profiles.md) --- Compiler settings and optimizations.
Expand Down Expand Up @@ -530,6 +531,46 @@ both `src/bin/a.rs` and `src/bin/b.rs`:
default-run = "a"
```

#### The `lints` section

Override the default level of lints from different tools by assigning them to a new level in a
table, for example:
```toml
[lints.rust]
unsafe_code = "forbid"
```

This is short-hand for:
```toml
[lints.rust]
unsafe_code = { level = "forbid", priority = 0 }
```

`level` corresponds to the lint levels in `rustc`:
- `forbid`
- `deny`
- `warn`
- `allow`

`priority` is a signed integer that controls which lints or lint groups override other lint groups:
- lower (particularly negative) numbers have lower priority, being overridden
by higher numbers, and show up first on the command-line to tools like
`rustc`

To know which table under `[lints]` a particular lint belongs under, it is the part before `::` in the lint
name. If there isn't a `::`, then the tool is `rust`. For example a warning
about `unsafe_code` would be `lints.rust.unsafe_code` but a lint about
`clippy::enum_glob_use` would be `lints.clippy.enum_glob_use`.

For example:
```toml
[lints.rust]
unsafe_code = "forbid"

[lints.clippy]
enum_glob_use = "deny"
```

## The `[badges]` section

The `[badges]` section is for specifying status badges that can be displayed
Expand Down
98 changes: 5 additions & 93 deletions src/doc/src/reference/unstable.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ For the latest nightly, see the [nightly version] of this page.
* [codegen-backend](#codegen-backend) --- Select the codegen backend used by rustc.
* [per-package-target](#per-package-target) --- Sets the `--target` to use for each individual package.
* [artifact dependencies](#artifact-dependencies) --- Allow build artifacts to be included into other build artifacts and build them for different targets.
* [`[lints]`](#lints) --- Configure lint levels for various linter tools.
* Information and metadata
* [Build-plan](#build-plan) --- Emits JSON information on which commands will be run.
* [unit-graph](#unit-graph) --- Emits JSON for Cargo's internal graph structure.
Expand Down Expand Up @@ -1571,100 +1570,8 @@ Differences between `cargo run --manifest-path <path>` and `cargo <path>`
- `cargo <path>` runs with the config for `<path>` and not the current dir, more like `cargo install --path <path>`
- `cargo <path>` is at a verbosity level below the normal default. Pass `-v` to get normal output.

## `[lints]`

* Tracking Issue: [#12115](https://github.com/rust-lang/cargo/issues/12115)

A new `lints` table would be added to configure lints:
```toml
[lints.rust]
unsafe = "forbid"
```
and `cargo` would pass these along as flags to `rustc`, `clippy`, or other lint tools when `-Zlints` is used.

This would work with
[RFC 2906 `workspace-deduplicate`](https://rust-lang.github.io/rfcs/2906-cargo-workspace-deduplicate.html):
```toml
[lints]
workspace = true
[workspace.lints.rust]
unsafe = "forbid"
```

### Documentation Updates

#### The `lints` section

*as a new ["Manifest Format" entry](./manifest.html#the-manifest-format)*

Override the default level of lints from different tools by assigning them to a new level in a
table, for example:
```toml
[lints.rust]
unsafe = "forbid"
```

This is short-hand for:
```toml
[lints.rust]
unsafe = { level = "forbid", priority = 0 }
```

`level` corresponds to the lint levels in `rustc`:
- `forbid`
- `deny`
- `warn`
- `allow`

`priority` is a signed integer that controls which lints or lint groups override other lint groups:
- lower (particularly negative) numbers have lower priority, being overridden
by higher numbers, and show up first on the command-line to tools like
`rustc`

To know which table under `[lints]` a particular lint belongs under, it is the part before `::` in the lint
name. If there isn't a `::`, then the tool is `rust`. For example a warning
about `unsafe` would be `lints.rust.unsafe` but a lint about
`clippy::enum_glob_use` would be `lints.clippy.enum_glob_use`.
For example:
```toml
[lints.rust]
unsafe = "forbid"
[lints.clippy]
enum_glob_use = "deny"
```
#### The `lints` table
*as a new [`[workspace]` entry](./workspaces.html#the-workspace-section)*
The `workspace.lints` table is where you define lint configuration to be inherited by members of a workspace.
Specifying a workspace lint configuration is similar to package lints.
Example:
```toml
# [PROJECT_DIR]/Cargo.toml
[workspace]
members = ["crates/*"]
[workspace.lints.rust]
unsafe = "forbid"
```
```toml
# [PROJECT_DIR]/crates/bar/Cargo.toml
[package]
name = "bar"
version = "0.1.0"
[lints]
workspace = true
```
# Stabilized and removed features

## Compile progress
Expand Down Expand Up @@ -1886,3 +1793,8 @@ for more information about the working directory for compiling and running tests
The `--keep-going` option has been stabilized in the 1.74 release. See the
[`--keep-going` flag](../commands/cargo-build.html#option-cargo-build---keep-going)
in `cargo build` as an example for more details.

## `[lints]`

[`[lints]`](manifest.html#the-lints-section) (enabled via `-Zlints`) has been stabilized in the 1.74 release.

28 changes: 28 additions & 0 deletions src/doc/src/reference/workspaces.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ In the `Cargo.toml`, the `[workspace]` table supports the following sections:
* [`default-members`](#the-default-members-field) --- Packages to operate on when a specific package wasn't selected.
* [`package`](#the-package-table) --- Keys for inheriting in packages.
* [`dependencies`](#the-dependencies-table) --- Keys for inheriting in package dependencies.
* [`lints`](#the-lints-table) --- Keys for inheriting in package lints.
* [`metadata`](#the-metadata-table) --- Extra settings for external tools.
* [`[patch]`](overriding-dependencies.md#the-patch-section) --- Override dependencies.
* [`[replace]`](overriding-dependencies.md#the-replace-section) --- Override dependencies (deprecated).
Expand Down Expand Up @@ -222,6 +223,33 @@ cc.workspace = true
rand.workspace = true
```

## The `lints` table

The `workspace.lints` table is where you define lint configuration to be inherited by members of a workspace.

Specifying a workspace lint configuration is similar to package lints.

Example:

```toml
# [PROJECT_DIR]/Cargo.toml
[workspace]
members = ["crates/*"]

[workspace.lints.rust]
unsafe_code = "forbid"
```

```toml
# [PROJECT_DIR]/crates/bar/Cargo.toml
[package]
name = "bar"
version = "0.1.0"

[lints]
workspace = true
```

## The `metadata` table

The `workspace.metadata` table is ignored by Cargo and will not be warned
Expand Down
4 changes: 1 addition & 3 deletions tests/testsuite/cargo_new/inherit_workspace_lints/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::curr_dir;
use cargo_test_support::CargoCommand;
use cargo_test_support::ChannelChanger;
use cargo_test_support::Project;

#[cargo_test]
Expand All @@ -12,9 +11,8 @@ fn case() {

snapbox::cmd::Command::cargo_ui()
.arg("new")
.args(["crates/foo", "-Zlints"])
.args(["crates/foo"])
.current_dir(cwd)
.masquerade_as_nightly_cargo(&["lints"])
.assert()
.success()
.stdout_matches_path(curr_dir!().join("stdout.log"))
Expand Down
Loading

0 comments on commit e539380

Please sign in to comment.