diff --git a/Cargo.lock b/Cargo.lock index ee251fe03..aeb714162 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -248,6 +248,7 @@ dependencies = [ "askalono", "bitvec", "camino", + "cfg-expr", "clap", "codespan", "codespan-reporting", @@ -261,6 +262,7 @@ dependencies = [ "insta", "krates", "log", + "memchr", "nu-ansi-term", "parking_lot", "rayon", @@ -294,29 +296,6 @@ dependencies = [ "url", ] -[[package]] -name = "cargo-platform" -version = "0.1.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "24b1f0365a6c6bb4020cd05806fd0d33c44d38046b8bd7f0e40814b9763cabfc" -dependencies = [ - "serde", -] - -[[package]] -name = "cargo_metadata" -version = "0.18.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d886547e41f740c616ae73108f6eb70afe6d940c7bc697cb30f13daec073037" -dependencies = [ - "camino", - "cargo-platform", - "semver", - "serde", - "serde_json", - "thiserror", -] - [[package]] name = "cc" version = "1.0.98" @@ -1783,15 +1762,16 @@ dependencies = [ [[package]] name = "krates" -version = "0.16.10" +version = "0.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7fcb3baf2360eb25ad31f0ada3add63927ada6db457791979b82ac199f835cb9" +checksum = "9d0956eb8f64d0353d56c69544657bbf3bc143c3c0f0883cc4fa9ec1b252a404" dependencies = [ - "cargo-platform", - "cargo_metadata", + "camino", "cfg-expr", "petgraph", "semver", + "serde", + "serde_json", ] [[package]] @@ -2652,9 +2632,9 @@ dependencies = [ [[package]] name = "tame-index" -version = "0.12.0" +version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "418b7a56dbbf693afbf024c12f90863ac25894fd209d374d4af39cf9e43da36c" +checksum = "e9632d46ad4e0c6a07d4354fc43c6eae4654a09beef8e9e7c7c899f983185c45" dependencies = [ "bytes", "camino", @@ -2832,9 +2812,9 @@ dependencies = [ [[package]] name = "toml-span" -version = "0.2.0" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "369db38ce6d1fc320a54ea3f032d07c07a232ca19c40e287246aff06d57c2abe" +checksum = "ce0e1be49e3b9bf33d1a8077c081a3b7afcfc94e4bc1002c80376784381bc106" dependencies = [ "codespan-reporting", "serde", diff --git a/Cargo.toml b/Cargo.toml index a2db36b99..0b5545c0e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ askalono = { version = "0.4", default-features = false } bitvec = { version = "1.0", features = ["alloc"] } # Much nicer paths camino = "1.1" +cfg-expr = "0.15" # Allows us to do eg cargo metadata operations without relying on an external cargo #cargo = { version = "0.71", optional = true } # Argument parsing, kept aligned with cargo @@ -75,9 +76,11 @@ goblin = { version = "0.8", default-features = false, features = [ # We need to figure out HOME/CARGO_HOME in some cases home = "0.5" # Provides graphs on top of cargo_metadata -krates = { version = "0.16", features = ["targets"] } +krates = { version = "0.17", features = ["targets"] } # Log macros log = "0.4" +# Faster char searching +memchr = "2.7" # Nicer sync primitives parking_lot = "0.12" # Moar brrrr @@ -112,7 +115,7 @@ time = { version = "0.3", default-features = false, features = [ "macros", ] } # Deserialization of configuration files and crate manifests -toml-span = { version = "0.2", features = ["reporting"] } +toml-span = { version = "0.3", features = ["reporting"] } # Small fast hash crate twox-hash = { version = "1.5", default-features = false } # Url parsing/manipulation @@ -138,7 +141,7 @@ fs_extra = "1.3" insta = { version = "1.21", features = ["json"] } tame-index = { version = "0.12", features = ["local-builder"] } time = { version = "0.3", features = ["serde"] } -toml-span = { version = "0.2", features = ["serde"] } +toml-span = { version = "0.3", features = ["serde"] } # We use this for creating fake crate directories for crawling license files on disk tempfile = "3.1.0" # divan = "0.1" diff --git a/clippy.toml b/clippy.toml index da6189cb6..278890147 100644 --- a/clippy.toml +++ b/clippy.toml @@ -17,3 +17,10 @@ disallowed-types = [ { path = "ring::digest::SHA1_FOR_LEGACY_USE_ONLY", reason = "SHA-1 is cryptographically broken, and we are building new code so should not use it" }, ] +disallowed-macros = [ + "std::print", + "std::println", + "std::eprint", + "std::eprintln", + "std::dbg", +] diff --git a/examples/06_advisories/Cargo.lock b/examples/06_advisories/Cargo.lock index fcbc74cac..59b39d43b 100644 --- a/examples/06_advisories/Cargo.lock +++ b/examples/06_advisories/Cargo.lock @@ -24,10 +24,10 @@ dependencies = [ "ammonia 0.7.0", "artifact_serde", "axum-core", - "const-cstr", "dirs", "failure", "lettre", + "static_type_map", "trust-dns-resolver", ] @@ -191,12 +191,6 @@ dependencies = [ "bitflags", ] -[[package]] -name = "const-cstr" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed3d0b5ff30645a68f35ece8cea4556ca14ef8a1651455f789a099a0513532a6" - [[package]] name = "core-foundation" version = "0.9.1" @@ -1347,6 +1341,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "static_type_map" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f41d6352756e5e8bfb4dc0ad306fa62f41876dc2229d83669415fb4f56e34732" + [[package]] name = "string_cache" version = "0.6.2" diff --git a/examples/06_advisories/Cargo.toml b/examples/06_advisories/Cargo.toml index 3ab03da89..3e791b1f4 100644 --- a/examples/06_advisories/Cargo.toml +++ b/examples/06_advisories/Cargo.toml @@ -24,9 +24,9 @@ dirs = "4.0" # Failure has an unsound advisory (and is unmaintained) failure = "=0.1.8" -# const-cstr is unmaintained -# https://github.com/rustsec/advisory-db/blob/463e8405f85bb74eef17149f7e704b07723ce46e/crates/const-cstr/RUSTSEC-2023-0020.md -const-cstr = "0.3" +# atty is unmaintained +# https://github.com/rustsec/advisory-db/blob/8eb99abe8c369b48bbd4ca04133e1f05be22a778/crates/static_type_map/RUSTSEC-2022-0023.md +static_type_map = "0.3" # The advisory applies to 0.10.0-alpha.1 >= && < 0.10.0-alpha.4 # https://github.com/RustSec/advisory-db/blob/c71cfec8c3fe313c9445a9ab0ae9b7faedda850a/crates/lettre/RUSTSEC-2020-0069.md diff --git a/src/advisories.rs b/src/advisories.rs index 16a66d7ec..eb62c2f51 100644 --- a/src/advisories.rs +++ b/src/advisories.rs @@ -74,10 +74,9 @@ pub fn check( let mut ignore_yanked_hits: BitVec = BitVec::repeat(false, ctx.cfg.ignore_yanked.len()); // Emit diagnostics for any advisories found that matched crates in the graph - for (krate, krate_index, advisory) in &report.advisories { + for (krate, advisory) in &report.advisories { let diag = ctx.diag_for_advisory( krate, - *krate_index, &advisory.metadata, Some(&advisory.versions), |index| { @@ -89,14 +88,9 @@ pub fn check( } for (krate, status) in yanked { - let Some(ind) = ctx.krates.nid_for_kid(&krate.id) else { - log::warn!("failed to locate node id for '{krate}'"); - continue; - }; - if let Some(e) = status { if ctx.cfg.yanked.value != LintLevel::Allow { - sink.push(ctx.diag_for_index_failure(krate, ind, e)); + sink.push(ctx.diag_for_index_failure(krate, e)); } } else { // Check to see if the user has added an ignore for the yanked @@ -113,7 +107,7 @@ pub fn check( sink.push(ctx.diag_for_yanked_ignore(krate, i)); ignore_yanked_hits.as_mut_bitslice().set(i, true); } else { - sink.push(ctx.diag_for_yanked(krate, ind)); + sink.push(ctx.diag_for_yanked(krate)); } } } diff --git a/src/advisories/cfg.rs b/src/advisories/cfg.rs index 084d7fb7e..1439ccfbf 100644 --- a/src/advisories/cfg.rs +++ b/src/advisories/cfg.rs @@ -198,7 +198,7 @@ impl<'de> Deserialize<'de> for Config { v.set(ValueInner::String(s)); } ValueInner::Table(tab) => { - if tab.contains_key(&"id".into()) { + if tab.contains_key("id") { v.set(ValueInner::Table(tab)); match IgnoreId::deserialize(&mut v) { Ok(iid) => u.push(Spanned::with_span(iid, v.span)), @@ -994,7 +994,7 @@ expansions = [ let toml_span::value::ValueInner::Table(mut tab) = tv.take() else { unreachable!() }; - let mut expansions = tab.remove(&"expansions".into()).unwrap(); + let mut expansions = tab.remove("expansions").unwrap(); let toml_span::value::ValueInner::Array(exp) = expansions.take() else { unreachable!() }; @@ -1002,7 +1002,7 @@ expansions = [ use toml_span::Deserialize as _; let mut files = crate::diag::Files::new(); - let cfg_id = files.add("expansions.toml", toml.into()); + let cfg_id = files.add("expansions.toml", toml); let mut output = String::new(); diff --git a/src/advisories/diags.rs b/src/advisories/diags.rs index ac1aaccff..72f5624b2 100644 --- a/src/advisories/diags.rs +++ b/src/advisories/diags.rs @@ -70,7 +70,6 @@ impl<'a> crate::CheckCtx<'a, super::cfg::ValidConfig> { pub(crate) fn diag_for_advisory( &self, krate: &crate::Krate, - krate_index: krates::NodeId, advisory: &Metadata, versions: Option<&Versions>, mut on_ignore: F, @@ -179,9 +178,11 @@ impl<'a> crate::CheckCtx<'a, super::cfg::ValidConfig> { let diag = pack.push( Diagnostic::new(severity) .with_message(advisory.title.clone()) - .with_labels(vec![self - .krate_spans - .label_for_index(krate_index.index(), message)]) + .with_labels(vec![Label::primary( + self.krate_spans.lock_id, + self.krate_spans.lock_span(&krate.id).total, + ) + .with_message(message)]) .with_code(code) .with_notes(notes), ); @@ -193,11 +194,7 @@ impl<'a> crate::CheckCtx<'a, super::cfg::ValidConfig> { pack } - pub(crate) fn diag_for_yanked( - &self, - krate: &crate::Krate, - krate_index: krates::NodeId, - ) -> Pack { + pub(crate) fn diag_for_yanked(&self, krate: &crate::Krate) -> Pack { let mut pack = Pack::with_kid(Check::Advisories, krate.id.clone()); pack.push( Diagnostic::new(self.cfg.yanked.value.into()) @@ -206,9 +203,11 @@ impl<'a> crate::CheckCtx<'a, super::cfg::ValidConfig> { krate.name )) .with_code(Code::Yanked) - .with_labels(vec![self - .krate_spans - .label_for_index(krate_index.index(), "yanked version")]), + .with_labels(vec![Label::primary( + self.krate_spans.lock_id, + self.krate_spans.lock_span(&krate.id).total, + ) + .with_message("yanked version")]), ); pack @@ -229,13 +228,13 @@ impl<'a> crate::CheckCtx<'a, super::cfg::ValidConfig> { pub(crate) fn diag_for_index_failure( &self, krate: &crate::Krate, - krate_index: krates::NodeId, error: D, ) -> Pack { - let mut labels = vec![self.krate_spans.label_for_index( - krate_index.index(), - "crate whose registry we failed to query", - )]; + let mut labels = vec![Label::secondary( + self.krate_spans.lock_id, + self.krate_spans.lock_span(&krate.id).total, + ) + .with_message("crate whose registry we failed to query")]; // Don't show the config location if it's the default, since it just points // to the beginning and confuses users diff --git a/src/advisories/helpers/db.rs b/src/advisories/helpers/db.rs index 5a3b433d7..b6cf5e764 100644 --- a/src/advisories/helpers/db.rs +++ b/src/advisories/helpers/db.rs @@ -530,7 +530,7 @@ fn fetch_via_cli(url: &str, db_path: &Path) -> anyhow::Result<()> { } pub struct Report<'db, 'k> { - pub advisories: Vec<(&'k Krate, krates::NodeId, &'db rustsec::Advisory)>, + pub advisories: Vec<(&'k Krate, &'db rustsec::Advisory)>, /// For backwards compatibility with cargo-audit, we optionally serialize the /// reports to JSON and output them in addition to the normal cargo-deny /// diagnostics @@ -599,7 +599,7 @@ impl<'db, 'k> Report<'db, 'k> { return None; } - Some((km.krate, km.node_id, advisory)) + Some((km.krate, advisory)) }) }) .collect(); @@ -608,7 +608,7 @@ impl<'db, 'k> Report<'db, 'k> { let mut warnings = std::collections::BTreeMap::<_, Vec>::new(); let mut vulns = Vec::new(); - for (krate, _nid, advisory) in &db_advisories { + for (krate, advisory) in &db_advisories { let package = rustsec::package::Package { // :( name: krate.name.parse().unwrap(), @@ -687,7 +687,7 @@ impl<'db, 'k> Report<'db, 'k> { advisories.append(&mut db_advisories); } - advisories.sort_by(|a, b| a.1.cmp(&b.1)); + advisories.sort_by(|a, b| a.0.cmp(b.0)); Self { advisories, diff --git a/src/advisories/helpers/index.rs b/src/advisories/helpers/index.rs index 8490f18a1..e73128064 100644 --- a/src/advisories/helpers/index.rs +++ b/src/advisories/helpers/index.rs @@ -12,13 +12,13 @@ pub enum Entry { } pub struct Indices<'k> { - pub indices: Vec<(&'k Source, Result)>, + pub indices: Vec<(&'k Source, Result, Error>)>, pub cache: BTreeMap<(&'k str, &'k Source), Entry>, } impl<'k> Indices<'k> { pub fn load(krates: &'k Krates, cargo_home: crate::PathBuf) -> Self { - let mut indices = Vec::<(&Source, Result)>::new(); + let mut indices = Vec::<(&Source, Result, Error>)>::new(); for source in krates .krates() @@ -39,7 +39,12 @@ impl<'k> Indices<'k> { }; let index = index_url.and_then(|iu| { + // // If the registry has been replaced with a local registry just ignore it + // if matches!(&iu, IndexUrl::Local(_)) { + // return Ok(None); + // }; ComboIndexCache::new(IndexLocation::new(iu).with_root(Some(cargo_home.clone()))) + .map(Some) }); indices.push((source, index)); @@ -82,7 +87,7 @@ impl<'k> Indices<'k> { .find_map(|(url, index)| (src == *url).then_some(index)) .ok_or_else(|| "unable to locate index".to_owned())? { - Ok(index) => { + Ok(Some(index)) => { match index.cached_krate( name.try_into() .map_err(|e: tame_index::Error| e.to_string())?, @@ -98,6 +103,9 @@ impl<'k> Indices<'k> { Err(err) => Entry::Error(format!("{err:#}")), } } + Ok(None) => { + Entry::Error("unable to locate index entry for crate".to_owned()) + } Err(err) => Entry::Error(format!("{err:#}")), }; diff --git a/src/advisories/snapshots/cargo_deny__advisories__cfg__test__deserializes_advisories_cfg-2.snap b/src/advisories/snapshots/cargo_deny__advisories__cfg__test__deserializes_advisories_cfg-2.snap index 34ba38944..24682268f 100644 --- a/src/advisories/snapshots/cargo_deny__advisories__cfg__test__deserializes_advisories_cfg-2.snap +++ b/src/advisories/snapshots/cargo_deny__advisories__cfg__test__deserializes_advisories_cfg-2.snap @@ -3,7 +3,7 @@ source: src/advisories/cfg.rs expression: validated --- { - "file_id": 1, + "file_id": 0, "db_path": "/home/you/.cargo/advisory-dbs", "db_urls": [ "https://github.com/RustSec/advisory-db" diff --git a/src/bans.rs b/src/bans.rs index 14748865a..6abb385a9 100644 --- a/src/bans.rs +++ b/src/bans.rs @@ -192,7 +192,6 @@ use crate::diag::{Check, Diag, Pack, Severity}; pub fn check( ctx: crate::CheckCtx<'_, ValidConfig>, output_graph: Option>, - cargo_spans: diag::CargoSpans, sink: impl Into, ) { let ValidConfig { @@ -206,6 +205,8 @@ pub fn check( skipped, multiple_versions, multiple_versions_include_dev, + workspace_duplicates, + unused_workspace_dependencies, highlight, tree_skipped, wildcards, @@ -409,18 +410,12 @@ pub fn check( let mut kids = smallvec::SmallVec::<[Dupe; 2]>::new(); for dup in multi_detector.dupes.iter().cloned() { - let span = &ctx.krate_spans[dup].total; - - if span.start < all_start { - all_start = span.start; - } - - if span.end > all_end { - all_end = span.end; - } - let krate = &ctx.krates[dup]; + let span = &ctx.krate_spans.lock_span(&krate.id).total; + all_start = all_start.min(span.start); + all_end = all_end.max(span.end); + if let Err(i) = kids.binary_search_by(|other| match other.version.cmp(&krate.version) { std::cmp::Ordering::Equal => other.id.cmp(&krate.id), ord => ord, @@ -440,7 +435,7 @@ pub fn check( krate_name: multi_detector.name, num_dupes: kids.len(), krates_coord: KrateCoord { - file: krate_spans.file_id, + file: krate_spans.lock_id, span: (all_start..all_end).into(), }, severity, @@ -502,7 +497,7 @@ pub fn check( } } - let (mut tx, rx) = if let Some(bc) = build { + let (mut tx, build_config) = if let Some(bc) = build { let (tx, rx) = crossbeam::channel::unbounded(); (Sink::Build(tx), Some((bc, rx))) @@ -510,8 +505,49 @@ pub fn check( (Sink::NoBuild(sink.clone()), None) }; - let (_, build_packs) = rayon::join( - || { + struct BuildCheckCtx { + bypasses: parking_lot::Mutex, + diag_packs: parking_lot::Mutex>, + cargo_home: Option, + build_config: ValidBuildConfig, + } + + let build_check_ctx = build_config.map(|(build_config, rx)| { + // Make all paths reported in build diagnostics be relative to cargo_home + let cargo_home = home::cargo_home() + .map_err(|err| { + log::error!("unable to locate $CARGO_HOME: {err}"); + err + }) + .ok() + .and_then(|pb| { + crate::PathBuf::from_path_buf(pb) + .map_err(|pb| { + log::error!("$CARGO_HOME path '{}' is not utf-8", pb.display()); + }) + .ok() + }); + + // Keep track of the individual crate configs so we can emit warnings + // if they're configured but not actually used + let bypasses = + parking_lot::Mutex::::new(BitVec::repeat(false, build_config.bypass.len())); + + ( + BuildCheckCtx { + cargo_home, + bypasses, + diag_packs: parking_lot::Mutex::new(std::collections::BTreeMap::new()), + build_config, + }, + rx, + ) + }); + + let mut ws_duplicate_packs = Vec::new(); + + rayon::scope(|scope| { + scope.spawn(|_| { let last = ctx.krates.len() - 1; for (i, krate) in ctx.krates.krates().enumerate() { @@ -836,42 +872,87 @@ pub fn check( multi_detector.dupes.push(i); - if wildcards != LintLevel::Allow && !krate.is_git_source() { - let severity = match wildcards { - LintLevel::Warn => Severity::Warning, - LintLevel::Deny => Severity::Error, - LintLevel::Allow => unreachable!(), - }; + 'wildcards: { + if wildcards != LintLevel::Allow && !krate.is_git_source() { + let severity = match wildcards { + LintLevel::Warn => Severity::Warning, + LintLevel::Deny => Severity::Error, + LintLevel::Allow => unreachable!(), + }; + + let Some(manifest) = ctx.krate_spans.manifest(&krate.id) else { + break 'wildcards; + }; + let is_private = krate.is_private(&[]); + let mut labels = Vec::new(); + let mut pack = Pack::with_kid(Check::Bans, krate.id.clone()); - let mut wildcards: Vec<_> = krate - .deps - .iter() - .filter(|dep| dep.req == VersionReq::STAR) - .collect(); + for mdep in manifest.deps(false) { + if mdep.dep.req != VersionReq::STAR { + continue; + } - if allow_wildcard_paths { - let is_private = krate.is_private(&[]); + // Wildcards are allowed for path or git dependencies, if the krate + // is private, or it's only a dev-dependency + if allow_wildcard_paths + && !mdep.krate.is_registry() + && (is_private + || mdep.dep.kind == DependencyKind::Development) + { + continue; + } - wildcards.retain(|dep| { - let is_path_or_git = is_path_or_git_dependency(dep); - if is_private { - !is_path_or_git - } else { - let is_path_non_dev_dependency = is_path_or_git - && dep.kind != DependencyKind::Development; - is_path_non_dev_dependency || !is_path_or_git + labels.push( + crate::diag::Label::primary( + manifest.id, + mdep.version_req + .as_ref() + .map_or(mdep.value_span, |vr| vr.span), + ) + .with_message("wildcard dependency"), + ); + + // If the dependency is a workspace dependency we also want to show + // the bad version requirement for the workspace declaration + if let Some(workspace) = &mdep.workspace { + if !workspace.value { + continue; + } + + if let Some(ws_dep) = + ctx.krate_spans.workspace_span(&krate.id) + { + labels.push( + crate::diag::Label::secondary( + ctx.krate_spans.workspace_id.unwrap(), + ws_dep + .version + .as_ref() + .map_or(ws_dep.value, |vr| vr.span), + ) + .with_message("workspace dependency"), + ); + } else { + // This indicates a bug because we were unable to resolve the workspace dependency + // to an appropriate crate, even though cargo did + pack.push(diags::UnresolveWorkspaceDependency { + manifest, + dep: mdep, + }); + } } - }); - } + } - if !wildcards.is_empty() { - sink.push(diags::Wildcards { - krate, - severity, - wildcards, - allow_wildcard_paths, - cargo_spans: &cargo_spans, - }); + sink.push(pack); + + if !labels.is_empty() { + sink.push(diags::Wildcards { + krate, + severity, + labels, + allow_wildcard_paths, + }); + } } } } @@ -885,82 +966,82 @@ pub fn check( } drop(tx); - }, - || { - let (build_config, rx) = rx?; - - // Keep track of the individual crate configs so we can emit warnings - // if they're configured but not actually used - let bcv = - parking_lot::Mutex::::new(BitVec::repeat(false, build_config.bypass.len())); + }); - // Make all paths reported in build diagnostics be relative to cargo_home + scope.spawn(|scope| { + let Some((build_ctx, rx)) = &build_check_ctx else { + return; + }; + while let Ok((index, krate, mut pack)) = rx.recv() { + scope.spawn(move |_s| { + if let Some(bcc) = check_build( + ctx.cfg.file_id, + &build_ctx.build_config, + build_ctx.cargo_home.as_deref(), + krate, + ctx.krates, + &mut pack, + ) { + build_ctx.bypasses.lock().set(bcc, true); + } - let cargo_home = home::cargo_home() - .map_err(|err| { - log::error!("unable to locate $CARGO_HOME: {err}"); - err - }) - .ok() - .and_then(|pb| { - crate::PathBuf::from_path_buf(pb) - .map_err(|pb| { - log::error!("$CARGO_HOME path '{}' is not utf-8", pb.display()); - }) - .ok() + if !pack.is_empty() { + build_ctx.diag_packs.lock().insert(index, pack); + } }); + } + }); - let pq = parking_lot::Mutex::new(std::collections::BTreeMap::new()); - rayon::scope(|s| { - let bc = &build_config; - let pq = &pq; - let bcv = &bcv; - let home = cargo_home.as_deref(); - - while let Ok((index, krate, mut pack)) = rx.recv() { - s.spawn(move |_s| { - if let Some(bcc) = - check_build(ctx.cfg.file_id, bc, home, krate, ctx.krates, &mut pack) - { - bcv.lock().set(bcc, true); - } - - if !pack.is_empty() { - pq.lock().insert(index, pack); - } - }); - } + // Check the workspace to detect dependencies that are used more than once + // but don't use a shared [workspace.[dev-/build-]dependencies] declaration + if workspace_duplicates != LintLevel::Allow { + scope.spawn(|_| { + check_workspace_duplicates( + ctx.krates, + ctx.krate_spans, + workspace_duplicates, + &mut ws_duplicate_packs, + ); }); + } + }); - let unmatched_exe_configs = { - let mut pack = Pack::new(Check::Bans); + if let Some((bcc, _)) = build_check_ctx { + for bp in bcc.diag_packs.into_inner().into_values() { + sink.push(bp); + } - for ve in bcv - .into_inner() - .into_iter() - .zip(build_config.bypass.into_iter()) - .filter_map(|(hit, ve)| if !hit { Some(ve) } else { None }) - { - pack.push(diags::UnmatchedBypass { - unmatched: &ve, - file_id, - }); - } + let mut pack = Pack::new(Check::Bans); + for ve in bcc + .bypasses + .into_inner() + .into_iter() + .zip(bcc.build_config.bypass.into_iter()) + .filter_map(|(hit, ve)| if !hit { Some(ve) } else { None }) + { + pack.push(diags::UnmatchedBypass { + unmatched: &ve, + file_id, + }); + } - pack - }; + sink.push(pack); + } - Some( - pq.into_inner() - .into_values() - .chain(Some(unmatched_exe_configs)), - ) - }, - ); + for pack in ws_duplicate_packs { + sink.push(pack); + } - if let Some(bps) = build_packs { - for bp in bps { - sink.push(bp); + if unused_workspace_dependencies != LintLevel::Allow { + if let Some(id) = krate_spans + .workspace_id + .filter(|_id| !krate_spans.unused_workspace_deps.is_empty()) + { + sink.push(diags::UnusedWorkspaceDependencies { + id, + unused: &krate_spans.unused_workspace_deps, + level: unused_workspace_dependencies, + }); } } @@ -999,11 +1080,13 @@ pub fn check_build( krates: &Krates, pack: &mut Pack, ) -> Option { + use krates::cm::TargetKind; + let build_script_allowed = if let Some(allow_build_scripts) = &config.allow_build_scripts { let has_build_script = krate .targets .iter() - .any(|t| t.kind.iter().any(|k| *k == "custom-build")); + .any(|t| t.kind.iter().any(|k| *k == TargetKind::CustomBuild)); !has_build_script || allow_build_scripts @@ -1022,7 +1105,7 @@ pub fn check_build( krate.targets.iter().any(|t| { t.kind .iter() - .any(|k| *k == "custom-build" || *k == "proc-macro") + .any(|k| matches!(*k, TargetKind::CustomBuild | TargetKind::ProcMacro)) }) } @@ -1475,14 +1558,131 @@ fn validate_file_checksum(path: &crate::Path, expected: &cfg::Checksum) -> anyho Ok(()) } -/// Returns true if the dependency has a `path` or `git` source. -/// -/// TODO: Possibly what we actually care about, where this is used in the wildcard check, is -/// “is not using any registry source”. -fn is_path_or_git_dependency(dep: &krates::cm::Dependency) -> bool { - dep.path.is_some() - || dep - .source - .as_ref() - .is_some_and(|url| url.starts_with("git+")) +fn check_workspace_duplicates( + krates: &Krates, + krate_spans: &crate::diag::KrateSpans<'_>, + severity: LintLevel, + diags: &mut Vec, +) { + use crate::diag::Label; + + // Note this will ignore cases where a dependency is used as more than 1 kind etc, + // but this check is not really meant for such simple cases + if krates.workspace_members().count() <= 1 { + return; + } + + // Gather any direct dependencies that are declared more than once + let mut deps = std::collections::BTreeMap::< + _, + Vec<( + &crate::diag::ManifestDep<'_>, + &crate::Krate, + crate::diag::FileId, + )>, + >::new(); + + for wsm in krates.workspace_members() { + let krates::Node::Krate { id, krate, .. } = wsm else { + continue; /* unreachable */ + }; + + let Some(man) = krate_spans.manifest(id) else { + continue; + }; + + for mdep in man.deps(true) { + deps.entry(&mdep.krate.id) + .or_default() + .push((mdep, krate, man.id)); + } + } + + // Strip out any direct dependencies that aren't referenced multiple times in the workspace + // Note this will retain in cases where the dependency is only used + // by 1 crate, but as different dependency kinds, which is still useful + // to catch + deps.retain(|_, parents| parents.len() > 1); + + for (kid, parents) in deps { + let total = parents.len(); + let mut labels = Vec::new(); + + if let Some((ws_span, ws_id)) = krate_spans + .workspace_span(kid) + .zip(krate_spans.workspace_id) + { + labels.push(Label::primary(ws_id, ws_span.key).with_message(format!( + "{}workspace dependency", + if ws_span.patched.is_some() { + "patched " + } else { + "" + } + ))); + + if let Some(patched) = ws_span.patched { + labels.push( + Label::secondary(ws_id, patched) + .with_message("note this is the original dependency that is patched"), + ); + } + + if let Some(rename) = &ws_span.rename { + labels.push( + Label::secondary(ws_id, rename.span) + .with_message("note the workspace dependency is renamed"), + ); + } + } + + let llen = labels.len(); + let has_workspace_declaration = llen != 0; + + for (mdep, _parent, id) in parents { + // Unfortunately cargo doesn't allow `workspace = false`, so if + // there are situations where the user wants to explicitly opt out + // of the lint for a specific crate/crates/manifest they need to use + // [package.metadata.cargo-deny.workspace-duplicates] + if mdep.workspace.is_some() { + continue; + } + + labels.push(Label::secondary(id, mdep.key_span)); + + if let Some(rename) = &mdep.rename { + labels.push( + Label::secondary(id, rename.span) + .with_message("note the dependency is renamed"), + ); + } + } + + if llen >= labels.len() { + continue; + } + + let Some(duplicate) = krates.node_for_kid(kid) else { + log::error!("failed to find node for {kid}"); + continue; + }; + + let krates::Node::Krate { + krate: duplicate, .. + } = duplicate + else { + log::error!("{kid} pointed a crate feature, this should be impossible"); + continue; + }; + + let mut pack = Pack::with_kid(Check::Bans, duplicate.id.clone()); + pack.push(diags::WorkspaceDuplicate { + duplicate, + labels, + severity, + has_workspace_declaration, + total_uses: total, + }); + diags.push(pack); + } } diff --git a/src/bans/cfg.rs b/src/bans/cfg.rs index e892eda5c..27c8e0c42 100644 --- a/src/bans/cfg.rs +++ b/src/bans/cfg.rs @@ -333,6 +333,11 @@ pub struct Config { /// How to handle multiple versions of the same crate pub multiple_versions: LintLevel, pub multiple_versions_include_dev: bool, + /// How to handle workspace dependencies on the same crate that aren't declared + /// in `[workspace.dependencies]` + pub workspace_duplicates: LintLevel, + /// How to handle [`workspace.dependencies`] that are not used + pub unused_workspace_dependencies: LintLevel, /// How the duplicate graphs are highlighted pub highlight: GraphHighlight, /// The crates that will cause us to emit failures @@ -373,6 +378,8 @@ impl Default for Config { Self { multiple_versions: LintLevel::Warn, multiple_versions_include_dev: false, + workspace_duplicates: LintLevel::Warn, + unused_workspace_dependencies: LintLevel::Allow, highlight: GraphHighlight::All, deny: Vec::new(), allow: Vec::new(), @@ -397,6 +404,12 @@ impl<'de> Deserialize<'de> for Config { let multiple_versions_include_dev = th .optional("multiple-versions-include-dev") .unwrap_or_default(); + let workspace_duplicates = th + .optional("workspace-duplicates") + .unwrap_or(LintLevel::Warn); + let unused_workspace_dependencies = th + .optional("unused-workspace-dependencies") + .unwrap_or(LintLevel::Allow); let highlight = th.optional("highlight").unwrap_or_default(); let deny = th.optional("deny").unwrap_or_default(); let allow = th.optional("allow").unwrap_or_default(); @@ -415,6 +428,8 @@ impl<'de> Deserialize<'de> for Config { Ok(Self { multiple_versions, multiple_versions_include_dev, + workspace_duplicates, + unused_workspace_dependencies, highlight, deny, allow, @@ -722,6 +737,8 @@ impl crate::cfg::UnvalidatedConfig for Config { file_id: ctx.cfg_id, multiple_versions: self.multiple_versions, multiple_versions_include_dev: self.multiple_versions_include_dev, + workspace_duplicates: self.workspace_duplicates, + unused_workspace_dependencies: self.unused_workspace_dependencies, highlight: self.highlight, denied, denied_multiple_versions, @@ -894,6 +911,8 @@ pub struct ValidConfig { pub file_id: FileId, pub multiple_versions: LintLevel, pub multiple_versions_include_dev: bool, + pub workspace_duplicates: LintLevel, + pub unused_workspace_dependencies: LintLevel, pub highlight: GraphHighlight, pub(crate) denied: Vec, pub(crate) denied_multiple_versions: Vec, diff --git a/src/bans/diags.rs b/src/bans/diags.rs index 59a3f5a75..f7d982969 100644 --- a/src/bans/diags.rs +++ b/src/bans/diags.rs @@ -51,6 +51,9 @@ pub enum Code { UnmatchedPathBypass, UnmatchedGlob, UnusedWrapper, + WorkspaceDuplicate, + UnresolvedWorkspaceDependency, + UnusedWorkspaceDependency, } impl From for String { @@ -171,24 +174,13 @@ impl<'a> From> for Diag { pub(crate) struct Wildcards<'a> { pub(crate) krate: &'a Krate, pub(crate) severity: Severity, - pub(crate) wildcards: Vec<&'a krates::cm::Dependency>, + pub(crate) labels: Vec