Skip to content

Commit

Permalink
Auto merge of #12168 - epage:lints2, r=weihanglo
Browse files Browse the repository at this point in the history
fix(lints): Switch to -Zlints so stable projects can experiment

### What does this PR try to resolve?

In #12115, we explored how we can let stable projects
experiment with `[lints]` to provide feedback.  What we settled on is
switching from the `cargo-features` manifest key to the `-Z` flag as
`cargo-features` always requires nightly while `-Z` only requires it
when being passed in.  This means a project can have a `[lints]` table
and have CI / contributors run `cargo +nightly check -Zlints` when they
care about warnings.

### How should we test and review this PR?

Demonstrate how you test this change and guide reviewers through your PR.
With a smooth review process, a pull request usually gets reviewed quicker.

If you don't know how to write and run your tests, please read the guide:
https://doc.crates.io/contrib/tests

### Additional information

I considered reworking the code to show the user the errors they would encounter once the feature is stable but held off.  I wasn't quite sure what language to use and most likely a user would have something doing error reporting, like CI, so it should be fine.
  • Loading branch information
bors committed May 23, 2023
2 parents 50d56ca + 08fdd86 commit feb9bcf
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 86 deletions.
5 changes: 2 additions & 3 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,9 +483,6 @@ features! {

// Allow specifying rustflags directly in a profile
(stable, workspace_inheritance, "1.64", "reference/unstable.html#workspace-inheritance"),

// Allow specifying rustflags directly in a profile
(unstable, lints, "", "reference/unstable.html#lints"),
}

pub struct Feature {
Expand Down Expand Up @@ -734,6 +731,7 @@ unstable_cli_options!(
skip_rustdoc_fingerprint: bool = (HIDDEN),
rustdoc_scrape_examples: bool = ("Allows Rustdoc to scrape code examples from reverse-dependencies"),
msrv_policy: bool = ("Enable rust-version aware policy within cargo"),
lints: bool = ("Pass `[lints]` to the linting tools"),
);

const STABILIZED_COMPILE_PROGRESS: &str = "The progress bar is now always \
Expand Down Expand Up @@ -1097,6 +1095,7 @@ impl CliUnstable {
"codegen-backend" => self.codegen_backend = parse_empty(k, v)?,
"profile-rustflags" => self.profile_rustflags = parse_empty(k, v)?,
"msrv-policy" => self.msrv_policy = parse_empty(k, v)?,
"lints" => self.lints = parse_empty(k, v)?,
_ => bail!("unknown `-Z` flag specified: {}", k),
}

Expand Down
44 changes: 16 additions & 28 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2045,12 +2045,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(),
&features,
config,
&mut warnings,
)?;
let lints = parse_unstable_lints(toml_config.lints.clone(), config, &mut warnings)?;
let lints = verify_lints(lints)?;
inheritable.update_lints(lints);
if let Some(ws_deps) = &inheritable.dependencies {
Expand Down Expand Up @@ -2316,14 +2311,10 @@ impl TomlManifest {
&inherit_cell,
)?;

let lints = parse_unstable_lints::<MaybeWorkspaceLints>(
me.lints.clone(),
&features,
config,
cx.warnings,
)?
.map(|mw| mw.resolve("lints", || inherit()?.lints()))
.transpose()?;
let lints =
parse_unstable_lints::<MaybeWorkspaceLints>(me.lints.clone(), config, cx.warnings)?
.map(|mw| mw.resolve("lints", || 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 @@ -2757,12 +2748,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(),
&features,
config,
&mut warnings,
)?;
let lints = parse_unstable_lints(toml_config.lints.clone(), config, &mut warnings)?;
let lints = verify_lints(lints)?;
inheritable.update_lints(lints);
let ws_root_config = WorkspaceRootConfig::new(
Expand Down Expand Up @@ -2911,44 +2897,46 @@ impl TomlManifest {

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

if !features.is_enabled(Feature::lints()) {
warn_for_feature("lints", config, warnings);
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_feature(name: &str, config: &Config, warnings: &mut Vec<String>) {
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,
"feature `{name}` is not supported on this version of Cargo and will be ignored"
"unused manifest key `{key_name}` (may be supported in a future version)"
);
if config.nightly_features_allowed {
let _ = write!(
message,
"
consider adding `cargo-features = [\"{name}\"]` to the manifest"
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 add
`cargo-features = [\"{name}\"]` to enable this feature",
switch to nightly channel you can pass
`-Z{feature_name}` to enable this feature.",
);
}
warnings.push(message);
Expand Down
6 changes: 1 addition & 5 deletions src/doc/src/reference/unstable.md
Original file line number Diff line number Diff line change
Expand Up @@ -1398,18 +1398,14 @@ Valid operations are the following:

A new `lints` table would be added to configure lints:
```toml
cargo-features = ["lints"]

[lints.rust]
unsafe = "forbid"
```
and `cargo` would pass these along as flags to `rustc`, `clippy`, or other lint tools.
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
cargo-features = ["lints"]

[lints]
workspace = true

Expand Down
Loading

0 comments on commit feb9bcf

Please sign in to comment.