Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Stabilize lints #12648

Merged
merged 1 commit into from
Sep 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that both unsafe_code and unsafe-code works. It may not be worth a mention in doc but the error message always shows dash version, which is a bit unfortunate.

  = note: requested on the command line with `-D unsafe-code`

```

```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