Skip to content

Commit

Permalink
fix(toml): Provide a way to show unused manifest keys for dependencies
Browse files Browse the repository at this point in the history
  • Loading branch information
@Muscraft
Muscraft committed Jan 26, 2023
1 parent 2ccd950 commit 1caeab2
Show file tree
Hide file tree
Showing 3 changed files with 190 additions and 9 deletions.
94 changes: 87 additions & 7 deletions src/cargo/util/toml/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,15 @@ impl<'de, P: Deserialize<'de> + Clone> de::Deserialize<'de> for TomlDependency<P
}
}

impl TomlDependency {
fn unused_keys(&self) -> Vec<String> {
match self {
TomlDependency::Simple(_) => vec![],
TomlDependency::Detailed(detailed) => detailed.other.keys().cloned().collect(),
}
}
}

pub trait ResolveToPath {
fn resolve(&self, config: &Config) -> PathBuf;
}
Expand Down Expand Up @@ -312,6 +321,10 @@ pub struct DetailedTomlDependency<P: Clone = String> {
lib: Option<bool>,
/// A platform name, like `x86_64-apple-darwin`
target: Option<String>,
/// This is here to provide a way to see the "unused manifest keys" when deserializing
#[serde(skip_serializing)]
#[serde(flatten)]
other: BTreeMap<String, toml::Value>,
}

// Explicit implementation so we avoid pulling in P: Default
Expand All @@ -335,6 +348,7 @@ impl<P: Clone> Default for DetailedTomlDependency<P> {
artifact: Default::default(),
lib: Default::default(),
target: Default::default(),
other: Default::default(),
}
}
}
Expand Down Expand Up @@ -1053,6 +1067,15 @@ impl<T, W: WorkspaceInherit> MaybeWorkspace<T, W> {

type MaybeWorkspaceDependency = MaybeWorkspace<TomlDependency, TomlWorkspaceDependency>;

impl MaybeWorkspaceDependency {
fn unused_keys(&self) -> Vec<String> {
match self {
MaybeWorkspaceDependency::Defined(d) => d.unused_keys(),
MaybeWorkspaceDependency::Workspace(w) => w.other.keys().cloned().collect(),
}
}
}

#[derive(Deserialize, Serialize, Clone, Debug)]
#[serde(rename_all = "kebab-case")]
pub struct TomlWorkspaceDependency {
Expand All @@ -1062,6 +1085,10 @@ pub struct TomlWorkspaceDependency {
#[serde(rename = "default_features")]
default_features2: Option<bool>,
optional: Option<bool>,
/// This is here to provide a way to see the "unused manifest keys" when deserializing
#[serde(skip_serializing)]
#[serde(flatten)]
other: BTreeMap<String, toml::Value>,
}

impl WorkspaceInherit for TomlWorkspaceDependency {
Expand Down Expand Up @@ -1724,6 +1751,16 @@ 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());
if let Some(ws_deps) = &inheritable.dependencies {
for (name, dep) in ws_deps {
unused_dep_keys(
name,
"workspace.dependencies",
dep.unused_keys(),
&mut warnings,
);
}
}
let ws_root_config = WorkspaceRootConfig::new(
package_root,
&toml_config.members,
Expand Down Expand Up @@ -1922,7 +1959,21 @@ impl TomlManifest {
.clone()
.resolve_with_self(n, |dep| dep.resolve(n, inheritable, cx))?;
let dep = resolved.to_dependency(n, cx, kind)?;
validate_package_name(dep.name_in_toml().as_str(), "dependency name", "")?;
let name_in_toml = dep.name_in_toml().as_str();
validate_package_name(name_in_toml, "dependency name", "")?;
let kind_name = &kind
.map(|k| match k {
DepKind::Normal => "dependencies",
DepKind::Development => "dev-dependencies",
DepKind::Build => "build-dependencies",
})
.unwrap_or("dependencies");
let table_in_toml = if let Some(platform) = &cx.platform {
format!("target.{}.{kind_name}", platform.to_string())
} else {
kind_name.to_string()
};
unused_dep_keys(name_in_toml, &table_in_toml, v.unused_keys(), cx.warnings);
cx.deps.push(dep);
deps.insert(n.to_string(), MaybeWorkspace::Defined(resolved.clone()));
}
Expand Down Expand Up @@ -2450,6 +2501,12 @@ impl TomlManifest {
spec
)
})?;
unused_dep_keys(
dep.name_in_toml().as_str(),
"replace",
replacement.unused_keys(),
&mut cx.warnings,
);
dep.set_version_req(VersionReq::exact(version))
.lock_version(version);
replace.push((spec, dep));
Expand All @@ -2459,21 +2516,32 @@ impl TomlManifest {

fn patch(&self, cx: &mut Context<'_, '_>) -> CargoResult<HashMap<Url, Vec<Dependency>>> {
let mut patch = HashMap::new();
for (url, deps) in self.patch.iter().flatten() {
let url = match &url[..] {
for (toml_url, deps) in self.patch.iter().flatten() {
let url = match &toml_url[..] {
CRATES_IO_REGISTRY => CRATES_IO_INDEX.parse().unwrap(),
_ => cx
.config
.get_registry_index(url)
.or_else(|_| url.into_url())
.get_registry_index(toml_url)
.or_else(|_| toml_url.into_url())
.with_context(|| {
format!("[patch] entry `{}` should be a URL or registry name", url)
format!(
"[patch] entry `{}` should be a URL or registry name",
toml_url
)
})?,
};
patch.insert(
url,
deps.iter()
.map(|(name, dep)| dep.to_dependency(name, cx, None))
.map(|(name, dep)| {
unused_dep_keys(
name,
&format!("patch.{toml_url}",),
dep.unused_keys(),
&mut cx.warnings,
);
dep.to_dependency(name, cx, None)
})
.collect::<CargoResult<Vec<_>>>()?,
);
}
Expand Down Expand Up @@ -2513,6 +2581,18 @@ impl TomlManifest {
}
}

fn unused_dep_keys(
dep_name: &str,
kind: &str,
unused_keys: Vec<String>,
warnings: &mut Vec<String>,
) {
for unused in unused_keys {
let key = format!("unused manifest key: {kind}.{dep_name}.{unused}");
warnings.push(key);
}
}

fn inheritable_from_path(
config: &Config,
workspace_path: PathBuf,
Expand Down
60 changes: 60 additions & 0 deletions tests/testsuite/check.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1459,3 +1459,63 @@ fn check_fixable_warning_for_clippy() {
.with_stderr_contains("[..] (run `cargo clippy --fix --lib -p foo` to apply 1 suggestion)")
.run();
}

#[cargo_test]
fn check_unused_manifest_keys() {
Package::new("dep", "0.1.0").publish();
Package::new("foo", "0.1.0").publish();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "bar"
version = "0.2.0"
authors = []
[dependencies]
dep = { version = "0.1.0", wxz = "wxz" }
foo = { version = "0.1.0", abc = "abc" }
[dev-dependencies]
foo = { version = "0.1.0", wxz = "wxz" }
[build-dependencies]
foo = { version = "0.1.0", wxz = "wxz" }
[target.'cfg(windows)'.dependencies]
foo = { version = "0.1.0", wxz = "wxz" }
[target.x86_64-pc-windows-gnu.dev-dependencies]
foo = { version = "0.1.0", wxz = "wxz" }
[target.bar.build-dependencies]
foo = { version = "0.1.0", wxz = "wxz" }
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("check")
.with_stderr(
"\
[WARNING] unused manifest key: dependencies.dep.wxz
[WARNING] unused manifest key: dependencies.foo.abc
[WARNING] unused manifest key: dev-dependencies.foo.wxz
[WARNING] unused manifest key: build-dependencies.foo.wxz
[WARNING] unused manifest key: target.bar.build-dependencies.foo.wxz
[WARNING] unused manifest key: target.cfg(windows).dependencies.foo.wxz
[WARNING] unused manifest key: target.x86_64-pc-windows-gnu.dev-dependencies.foo.wxz
[UPDATING] `[..]` index
[DOWNLOADING] crates ...
[DOWNLOADED] foo v0.1.0 ([..])
[DOWNLOADED] dep v0.1.0 ([..])
[CHECKING] [..]
[CHECKING] [..]
[CHECKING] bar v0.2.0 ([CWD])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
",
)
.run();
}
45 changes: 43 additions & 2 deletions tests/testsuite/inheritable_workspace_fields.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1212,8 +1212,8 @@ fn error_workspace_dependency_looked_for_workspace_itself() {
.with_status(101)
.with_stderr(
"\
[WARNING] [CWD]/Cargo.toml: dependency (dep) specified without providing a local path, Git repository, or version to use. This will be considered an error in future versions
[WARNING] [CWD]/Cargo.toml: unused manifest key: workspace.dependencies.dep.workspace
[WARNING] [CWD]/Cargo.toml: dependency (dep) specified without providing a local path, Git repository, or version to use. This will be considered an error in future versions
[UPDATING] `dummy-registry` index
[ERROR] no matching package named `dep` found
location searched: registry `crates-io`
Expand Down Expand Up @@ -1532,8 +1532,8 @@ fn cannot_inherit_in_patch() {
.with_status(101)
.with_stderr(
"\
[WARNING] [CWD]/Cargo.toml: dependency (bar) specified without providing a local path, Git repository, or version to use. This will be considered an error in future versions
[WARNING] [CWD]/Cargo.toml: unused manifest key: patch.crates-io.bar.workspace
[WARNING] [CWD]/Cargo.toml: dependency (bar) specified without providing a local path, Git repository, or version to use. This will be considered an error in future versions
[UPDATING] `dummy-registry` index
[ERROR] failed to resolve patches for `https://github.com/rust-lang/crates.io-index`
Expand All @@ -1543,3 +1543,44 @@ Caused by:
)
.run();
}

#[cargo_test]
fn warn_inherit_unused_manifest() {
Package::new("dep", "0.1.0").publish();

let p = project()
.file(
"Cargo.toml",
r#"
[workspace]
members = []
[workspace.dependencies]
dep = { version = "0.1", wxz = "wxz" }
[package]
name = "bar"
version = "0.2.0"
authors = []
[dependencies]
dep = { workspace = true, wxz = "wxz" }
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("check")
.with_stderr(
"\
[WARNING] [CWD]/Cargo.toml: unused manifest key: workspace.dependencies.dep.wxz
[WARNING] [CWD]/Cargo.toml: unused manifest key: dependencies.dep.wxz
[UPDATING] `[..]` index
[DOWNLOADING] crates ...
[DOWNLOADED] dep v0.1.0 ([..])
[CHECKING] [..]
[CHECKING] bar v0.2.0 ([CWD])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
",
)
.run();
}

0 comments on commit 1caeab2

Please sign in to comment.