refactor(toml): Consolidate how we track unused keys

This makes it act more like everything else, making this easier to
evolve over time.
This commit is contained in:
Ed Page 2024-03-14 20:56:06 -05:00
parent 714359289d
commit ff454fd452
5 changed files with 29 additions and 19 deletions

2
Cargo.lock generated
View File

@ -469,7 +469,7 @@ dependencies = [
[[package]] [[package]]
name = "cargo-util-schemas" name = "cargo-util-schemas"
version = "0.2.1" version = "0.3.0"
dependencies = [ dependencies = [
"semver", "semver",
"serde", "serde",

View File

@ -33,7 +33,7 @@ cargo-platform = { path = "crates/cargo-platform", version = "0.1.5" }
cargo-test-macro = { path = "crates/cargo-test-macro" } cargo-test-macro = { path = "crates/cargo-test-macro" }
cargo-test-support = { path = "crates/cargo-test-support" } cargo-test-support = { path = "crates/cargo-test-support" }
cargo-util = { version = "0.2.9", path = "crates/cargo-util" } cargo-util = { version = "0.2.9", path = "crates/cargo-util" }
cargo-util-schemas = { version = "0.2.1", path = "crates/cargo-util-schemas" } cargo-util-schemas = { version = "0.3.0", path = "crates/cargo-util-schemas" }
cargo_metadata = "0.18.1" cargo_metadata = "0.18.1"
clap = "4.5.1" clap = "4.5.1"
color-print = "0.3.5" color-print = "0.3.5"

View File

@ -1,6 +1,6 @@
[package] [package]
name = "cargo-util-schemas" name = "cargo-util-schemas"
version = "0.2.1" version = "0.3.0"
rust-version = "1.76.0" # MSRV:1 rust-version = "1.76.0" # MSRV:1
edition.workspace = true edition.workspace = true
license.workspace = true license.workspace = true

View File

@ -6,6 +6,7 @@
//! - Keys that exist for bookkeeping but don't correspond to the schema have a `_` prefix //! - Keys that exist for bookkeeping but don't correspond to the schema have a `_` prefix
use std::collections::BTreeMap; use std::collections::BTreeMap;
use std::collections::BTreeSet;
use std::fmt::{self, Display, Write}; use std::fmt::{self, Display, Write};
use std::path::PathBuf; use std::path::PathBuf;
use std::str; use std::str;
@ -28,6 +29,7 @@ pub use rust_version::RustVersionError;
#[derive(Debug, Deserialize, Serialize)] #[derive(Debug, Deserialize, Serialize)]
#[serde(rename_all = "kebab-case")] #[serde(rename_all = "kebab-case")]
pub struct TomlManifest { pub struct TomlManifest {
// when adding new fields, be sure to check whether `requires_package` should disallow them
pub cargo_features: Option<Vec<String>>, pub cargo_features: Option<Vec<String>>,
pub package: Option<Box<TomlPackage>>, pub package: Option<Box<TomlPackage>>,
pub project: Option<Box<TomlPackage>>, pub project: Option<Box<TomlPackage>>,
@ -51,7 +53,11 @@ pub struct TomlManifest {
pub workspace: Option<TomlWorkspace>, pub workspace: Option<TomlWorkspace>,
pub badges: Option<InheritableBtreeMap>, pub badges: Option<InheritableBtreeMap>,
pub lints: Option<InheritableLints>, pub lints: Option<InheritableLints>,
// when adding new fields, be sure to check whether `requires_package` should disallow them
/// Report unused keys (see also nested `_unused_keys`)
/// Note: this is populated by the caller, rather than automatically
#[serde(skip)]
pub _unused_keys: BTreeSet<String>,
} }
impl TomlManifest { impl TomlManifest {

View File

@ -18,7 +18,7 @@ use url::Url;
use crate::core::compiler::{CompileKind, CompileTarget}; use crate::core::compiler::{CompileKind, CompileTarget};
use crate::core::dependency::{Artifact, ArtifactTarget, DepKind}; use crate::core::dependency::{Artifact, ArtifactTarget, DepKind};
use crate::core::manifest::{ManifestMetadata, TargetSourcePath, Warnings}; use crate::core::manifest::{ManifestMetadata, TargetSourcePath};
use crate::core::resolver::ResolveBehavior; use crate::core::resolver::ResolveBehavior;
use crate::core::{find_workspace_root, resolve_relative_path, CliUnstable, FeatureValue}; use crate::core::{find_workspace_root, resolve_relative_path, CliUnstable, FeatureValue};
use crate::core::{Dependency, Manifest, PackageId, Summary, Target}; use crate::core::{Dependency, Manifest, PackageId, Summary, Target};
@ -52,11 +52,10 @@ pub fn read_manifest(
read_toml_string(path, gctx).map_err(|err| ManifestError::new(err, path.into()))?; read_toml_string(path, gctx).map_err(|err| ManifestError::new(err, path.into()))?;
let document = let document =
parse_document(&contents).map_err(|e| emit_diagnostic(e.into(), &contents, path, gctx))?; parse_document(&contents).map_err(|e| emit_diagnostic(e.into(), &contents, path, gctx))?;
let mut unused = BTreeSet::new(); let toml = deserialize_toml(&document)
let toml = deserialize_toml(&document, &mut unused)
.map_err(|e| emit_diagnostic(e.into(), &contents, path, gctx))?; .map_err(|e| emit_diagnostic(e.into(), &contents, path, gctx))?;
convert_toml(toml, unused, path, source_id, gctx).map_err(|err| { convert_toml(toml, path, source_id, gctx).map_err(|err| {
ManifestError::new( ManifestError::new(
err.context(format!("failed to parse manifest at `{}`", path.display())), err.context(format!("failed to parse manifest at `{}`", path.display())),
path.into(), path.into(),
@ -85,14 +84,16 @@ fn parse_document(contents: &str) -> Result<toml_edit::ImDocument<String>, toml_
#[tracing::instrument(skip_all)] #[tracing::instrument(skip_all)]
fn deserialize_toml( fn deserialize_toml(
document: &toml_edit::ImDocument<String>, document: &toml_edit::ImDocument<String>,
unused: &mut BTreeSet<String>,
) -> Result<manifest::TomlManifest, toml_edit::de::Error> { ) -> Result<manifest::TomlManifest, toml_edit::de::Error> {
let mut unused = BTreeSet::new();
let deserializer = toml_edit::de::Deserializer::from(document.clone()); let deserializer = toml_edit::de::Deserializer::from(document.clone());
serde_ignored::deserialize(deserializer, |path| { let mut document: manifest::TomlManifest = serde_ignored::deserialize(deserializer, |path| {
let mut key = String::new(); let mut key = String::new();
stringify(&mut key, &path); stringify(&mut key, &path);
unused.insert(key); unused.insert(key);
}) })?;
document._unused_keys = unused;
Ok(document)
} }
/// See also `bin/cargo/commands/run.rs`s `is_manifest_command` /// See also `bin/cargo/commands/run.rs`s `is_manifest_command`
@ -170,7 +171,6 @@ fn emit_diagnostic(
#[tracing::instrument(skip_all)] #[tracing::instrument(skip_all)]
fn convert_toml( fn convert_toml(
manifest: manifest::TomlManifest, manifest: manifest::TomlManifest,
unused: BTreeSet<String>,
manifest_file: &Path, manifest_file: &Path,
source_id: SourceId, source_id: SourceId,
gctx: &GlobalContext, gctx: &GlobalContext,
@ -193,13 +193,11 @@ fn convert_toml(
} }
return if manifest.package().is_some() { return if manifest.package().is_some() {
let embedded = is_embedded(manifest_file); let embedded = is_embedded(manifest_file);
let (mut manifest, paths) = let (manifest, paths) =
to_real_manifest(manifest, embedded, source_id, package_root, gctx)?; to_real_manifest(manifest, embedded, source_id, package_root, gctx)?;
warn_on_unused(&unused, manifest.warnings_mut());
Ok((EitherManifest::Real(manifest), paths)) Ok((EitherManifest::Real(manifest), paths))
} else { } else {
let (mut m, paths) = to_virtual_manifest(manifest, source_id, package_root, gctx)?; let (m, paths) = to_virtual_manifest(manifest, source_id, package_root, gctx)?;
warn_on_unused(&unused, m.warnings_mut());
Ok((EitherManifest::Virtual(m), paths)) Ok((EitherManifest::Virtual(m), paths))
}; };
} }
@ -238,11 +236,11 @@ fn warn_on_deprecated(new_path: &str, name: &str, kind: &str, warnings: &mut Vec
)) ))
} }
fn warn_on_unused(unused: &BTreeSet<String>, warnings: &mut Warnings) { fn warn_on_unused(unused: &BTreeSet<String>, warnings: &mut Vec<String>) {
for key in unused { for key in unused {
warnings.add_warning(format!("unused manifest key: {}", key)); warnings.push(format!("unused manifest key: {}", key));
if key == "profiles.debug" { if key == "profiles.debug" {
warnings.add_warning("use `[profile.dev]` to configure debug builds".to_string()); warnings.push("use `[profile.dev]` to configure debug builds".to_string());
} }
} }
} }
@ -375,6 +373,7 @@ pub fn prepare_for_publish(
badges: me.badges.clone(), badges: me.badges.clone(),
cargo_features: me.cargo_features.clone(), cargo_features: me.cargo_features.clone(),
lints: me.lints.clone(), lints: me.lints.clone(),
_unused_keys: Default::default(),
}; };
strip_features(&mut manifest); strip_features(&mut manifest);
return Ok(manifest); return Ok(manifest);
@ -523,6 +522,8 @@ pub fn to_real_manifest(
let mut warnings = vec![]; let mut warnings = vec![];
let mut errors = vec![]; let mut errors = vec![];
warn_on_unused(&me._unused_keys, &mut warnings);
// Parse features first so they will be available when parsing other parts of the TOML. // Parse features first so they will be available when parsing other parts of the TOML.
let empty = Vec::new(); let empty = Vec::new();
let cargo_features = me.cargo_features.as_ref().unwrap_or(&empty); let cargo_features = me.cargo_features.as_ref().unwrap_or(&empty);
@ -1225,6 +1226,7 @@ pub fn to_real_manifest(
workspace: false, workspace: false,
lints, lints,
}), }),
_unused_keys: Default::default(),
}; };
let mut manifest = Manifest::new( let mut manifest = Manifest::new(
Rc::new(resolved_toml), Rc::new(resolved_toml),
@ -1292,6 +1294,8 @@ fn to_virtual_manifest(
let cargo_features = me.cargo_features.as_ref().unwrap_or(&empty); let cargo_features = me.cargo_features.as_ref().unwrap_or(&empty);
let features = Features::new(cargo_features, gctx, &mut warnings, source_id.is_path())?; let features = Features::new(cargo_features, gctx, &mut warnings, source_id.is_path())?;
warn_on_unused(&me._unused_keys, &mut warnings);
let (replace, patch) = { let (replace, patch) = {
let mut manifest_ctx = ManifestContext { let mut manifest_ctx = ManifestContext {
deps: &mut deps, deps: &mut deps,