Auto merge of #14567 - Urgau:check-cfg-cleanup-lints, r=epage

Cleanup duplicated check-cfg lint logic

### What does this PR try to resolve?

This PR clean-ups some duplication left by #13958, because of Cargo MSRV.

Fixes #13975

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

The tests in `tests/testsuite/check_cfg.rs` show no change in behaviours (except for the better error messages). I suggest maybe reviewing commit by commit.

Author: bors

src/cargo/core/compiler/build_runner/mod.rs

@@ -246,7 +246,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
                 let mut args = compiler::extern_args(&self, unit, &mut unstable_opts)?;
                 args.extend(compiler::lto_args(&self, unit));
                 args.extend(compiler::features_args(unit));
-                args.extend(compiler::check_cfg_args(unit)?);
+                args.extend(compiler::check_cfg_args(unit));
 
                 let script_meta = self.find_build_script_metadata(unit);
                 if let Some(meta) = script_meta {

src/cargo/core/compiler/fingerprint/mod.rs

@@ -1421,34 +1421,12 @@ fn calculate_normal(
     }
     .to_vec();
 
-    // Include all the args from `[lints.rust.unexpected_cfgs.check-cfg]`
-    //
-    // HACK(#13975): duplicating the lookup logic here until `--check-cfg` is supported
-    // on Cargo's MSRV and we can centralize the logic in `lints_to_rustflags`
-    let mut lint_check_cfg = Vec::new();
-    if let Ok(Some(lints)) = unit.pkg.manifest().normalized_toml().normalized_lints() {
-        if let Some(rust_lints) = lints.get("rust") {
-            if let Some(unexpected_cfgs) = rust_lints.get("unexpected_cfgs") {
-                if let Some(config) = unexpected_cfgs.config() {
-                    if let Some(check_cfg) = config.get("check-cfg") {
-                        if let Ok(check_cfgs) =
-                            toml::Value::try_into::<Vec<String>>(check_cfg.clone())
-                        {
-                            lint_check_cfg = check_cfgs;
-                        }
-                    }
-                }
-            }
-        }
-    }
-
     let profile_hash = util::hash_u64((
         &unit.profile,
         unit.mode,
         build_runner.bcx.extra_args_for(unit),
         build_runner.lto[unit],
         unit.pkg.manifest().lint_rustflags(),
-        lint_check_cfg,
     ));
     // Include metadata since it is exposed as environment variables.
     let m = unit.pkg.manifest().metadata();

src/cargo/core/compiler/mod.rs

@@ -63,7 +63,7 @@ use std::io::{BufRead, Write};
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
-use anyhow::{bail, Context as _, Error};
+use anyhow::{Context as _, Error};
 use lazycell::LazyCell;
 use tracing::{debug, trace};
 
@@ -743,7 +743,7 @@ fn prepare_rustdoc(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> CargoResu
     let doc_dir = build_runner.files().out_dir(unit);
     rustdoc.arg("-o").arg(&doc_dir);
     rustdoc.args(&features_args(unit));
-    rustdoc.args(&check_cfg_args(unit)?);
+    rustdoc.args(&check_cfg_args(unit));
 
     add_error_format_and_color(build_runner, &mut rustdoc);
     add_allow_features(build_runner, &mut rustdoc);
@@ -1140,7 +1140,7 @@ fn build_base_args(
     }
 
     cmd.args(&features_args(unit));
-    cmd.args(&check_cfg_args(unit)?);
+    cmd.args(&check_cfg_args(unit));
 
     let meta = build_runner.files().metadata(unit);
     cmd.arg("-C").arg(&format!("metadata={}", meta));
@@ -1354,7 +1354,7 @@ fn package_remap(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> OsString {
 }
 
 /// Generates the `--check-cfg` arguments for the `unit`.
-fn check_cfg_args(unit: &Unit) -> CargoResult<Vec<OsString>> {
+fn check_cfg_args(unit: &Unit) -> Vec<OsString> {
     // The routine below generates the --check-cfg arguments. Our goals here are to
     // enable the checking of conditionals and pass the list of declared features.
     //
@@ -1391,39 +1391,12 @@ fn check_cfg_args(unit: &Unit) -> CargoResult<Vec<OsString>> {
     // Cargo and docs.rs than rustc and docs.rs. In particular, all users of docs.rs use
     // Cargo, but not all users of rustc (like Rust-for-Linux) use docs.rs.
 
-    let mut args = vec![
+    vec![
         OsString::from("--check-cfg"),
         OsString::from("cfg(docsrs)"),
         OsString::from("--check-cfg"),
         arg_feature,
-    ];
-
-    // Also include the custom arguments specified in `[lints.rust.unexpected_cfgs.check_cfg]`
-    if let Ok(Some(lints)) = unit.pkg.manifest().normalized_toml().normalized_lints() {
-        if let Some(rust_lints) = lints.get("rust") {
-            if let Some(unexpected_cfgs) = rust_lints.get("unexpected_cfgs") {
-                if let Some(config) = unexpected_cfgs.config() {
-                    if let Some(check_cfg) = config.get("check-cfg") {
-                        if let Ok(check_cfgs) =
-                            toml::Value::try_into::<Vec<String>>(check_cfg.clone())
-                        {
-                            for check_cfg in check_cfgs {
-                                args.push(OsString::from("--check-cfg"));
-                                args.push(OsString::from(check_cfg));
-                            }
-                        // error about `check-cfg` not being a list-of-string
-                        } else {
-                            bail!(
-                                "`lints.rust.unexpected_cfgs.check-cfg` must be a list of string"
-                            );
-                        }
-                    }
-                }
-            }
-        }
-    }
-
-    Ok(args)
+    ]
 }
 
 /// Adds LTO related codegen flags.

src/cargo/util/toml/mod.rs

@@ -1446,7 +1446,7 @@ pub fn to_real_manifest(
             .normalized_lints()
             .expect("previously normalized")
             .unwrap_or(&default),
-    );
+    )?;
 
     let metadata = ManifestMetadata {
         description: normalized_package
@@ -2545,7 +2545,7 @@ switch to nightly channel you can pass
     warnings.push(message);
 }
 
-fn lints_to_rustflags(lints: &manifest::TomlLints) -> Vec<String> {
+fn lints_to_rustflags(lints: &manifest::TomlLints) -> CargoResult<Vec<String>> {
     let mut rustflags = lints
         .iter()
         // We don't want to pass any of the `cargo` lints to `rustc`
@@ -2575,7 +2575,30 @@ fn lints_to_rustflags(lints: &manifest::TomlLints) -> Vec<String> {
         })
         .collect::<Vec<_>>();
     rustflags.sort();
-    rustflags.into_iter().map(|(_, _, option)| option).collect()
+
+    let mut rustflags: Vec<_> = rustflags.into_iter().map(|(_, _, option)| option).collect();
+
+    // Also include the custom arguments specified in `[lints.rust.unexpected_cfgs.check_cfg]`
+    if let Some(rust_lints) = lints.get("rust") {
+        if let Some(unexpected_cfgs) = rust_lints.get("unexpected_cfgs") {
+            if let Some(config) = unexpected_cfgs.config() {
+                if let Some(check_cfg) = config.get("check-cfg") {
+                    if let Ok(check_cfgs) = toml::Value::try_into::<Vec<String>>(check_cfg.clone())
+                    {
+                        for check_cfg in check_cfgs {
+                            rustflags.push("--check-cfg".to_string());
+                            rustflags.push(check_cfg);
+                        }
+                    // error about `check-cfg` not being a list-of-string
+                    } else {
+                        bail!("`lints.rust.unexpected_cfgs.check-cfg` must be a list of string");
+                    }
+                }
+            }
+        }
+    }
+
+    Ok(rustflags)
 }
 
 fn emit_diagnostic(

tests/testsuite/check_cfg.rs

@@ -707,8 +707,11 @@ fn config_invalid_not_list() {
     p.cargo("check")
         .with_status(101)
         .with_stderr_data(str![[r#"
-[ERROR] `lints.rust.unexpected_cfgs.check-cfg` must be a list of string
-...
+[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
+
+Caused by:
+  `lints.rust.unexpected_cfgs.check-cfg` must be a list of string
+
 "#]])
         .run();
 }
@@ -734,8 +737,11 @@ fn config_invalid_not_list_string() {
     p.cargo("check")
         .with_status(101)
         .with_stderr_data(str![[r#"
-[ERROR] `lints.rust.unexpected_cfgs.check-cfg` must be a list of string
-...
+[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
+
+Caused by:
+  `lints.rust.unexpected_cfgs.check-cfg` must be a list of string
+
 "#]])
         .run();
 }