diff --git a/docs/src/checks/bans/cfg.md b/docs/src/checks/bans/cfg.md index 6e43270b..85c4938b 100644 --- a/docs/src/checks/bans/cfg.md +++ b/docs/src/checks/bans/cfg.md @@ -38,21 +38,36 @@ If specified, alters how the `wildcard` field behaves: Being limited to private crates is due to crates.io not allowing packages to be published with `path` or `git` dependencies except for `dev-dependencies`. -### The `workspace-duplicates` field (optional) +### The `workspace-dependencies` field (optional) -Determines what happens when a more than 1 direct workspace dependency is resolved to the same crate and 1 or more declarations do not use `workspace = true` +Used to configure how [`[workspace.dependencies]`](https://doc.rust-lang.org/cargo/reference/workspaces.html#the-dependencies-table) are treated. -* `deny` - Will emit an error for each dependency declaration that does not use `workspace = true` -* `warn` (default) - Will emit a warning for each dependency declaration that does not use `workspace = true`, but does not fail the check. -* `allow` - Ignores checking for `workspace = true` +```ini +[bans.workspace-dependencies] +duplicates = 'deny' +include-path-dependencies = true +unused = 'deny' +``` + +#### The `duplicates` field (optional) + +Determines what happens when more than 1 direct workspace dependency is resolved to the same crate and 1 or more declarations do not use `workspace = true` + +* `deny` (default) - Will emit an error for each dependency declaration that does not use `workspace = true` +* `warn` - Will emit a warning for each dependency declaration that does not use `workspace = true`, but does not fail the check. +* `allow` - Ignores checking for `workspace = true` for dependencies in workspace crates + +#### The `include-path-dependencies` field (optional) + +If true, path dependencies will be included in the duplication check, otherwise they are completely ignored. -### The `unused-workspace-dependencies` field (optional) +#### The `unused` field (optional) Determines what happens when a dependency in [`[workspace.dependencies]`](https://doc.rust-lang.org/cargo/reference/workspaces.html#the-dependencies-table) is not used in the workspace. -* `deny` - Will emit an error for each dependency that is not actually used in the workspace. +* `deny` (default) - Will emit an error for each dependency that is not actually used in the workspace. * `warn` - Will emit a warning for each dependency that is not actually used in the workspace, but does not fail the check. -* `allow` - (default) Ignores checking for unused workspace dependencies. +* `allow` - Ignores checking for unused workspace dependencies. ### The `highlight` field (optional) diff --git a/src/bans.rs b/src/bans.rs index 9d095662..b3e1cfdf 100644 --- a/src/bans.rs +++ b/src/bans.rs @@ -205,8 +205,7 @@ pub fn check( skipped, multiple_versions, multiple_versions_include_dev, - workspace_duplicates, - unused_workspace_dependencies, + workspace_dependencies, highlight, tree_skipped, wildcards, @@ -994,15 +993,17 @@ pub fn check( // 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, - ); - }); + if let Some(ws_deps) = &workspace_dependencies { + if ws_deps.duplicates != LintLevel::Allow { + scope.spawn(|_| { + check_workspace_duplicates( + ctx.krates, + ctx.krate_spans, + ws_deps, + &mut ws_duplicate_packs, + ); + }); + } } }); @@ -1032,16 +1033,18 @@ pub fn check( sink.push(pack); } - 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, - }); + if let Some(ws_deps) = workspace_dependencies { + if ws_deps.unused != 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: ws_deps.unused, + }); + } } } @@ -1561,7 +1564,7 @@ fn validate_file_checksum(path: &crate::Path, expected: &cfg::Checksum) -> anyho fn check_workspace_duplicates( krates: &Krates, krate_spans: &crate::diag::KrateSpans<'_>, - severity: LintLevel, + cfg: &cfg::WorkspaceDepsConfig, diags: &mut Vec, ) { use crate::diag::Label; @@ -1592,6 +1595,10 @@ fn check_workspace_duplicates( }; for mdep in man.deps(true) { + if mdep.dep.path.is_some() && !cfg.include_path_dependencies { + continue; + } + deps.entry(&mdep.krate.id) .or_default() .push((mdep, krate, man.id)); @@ -1612,7 +1619,7 @@ fn check_workspace_duplicates( .workspace_span(kid) .zip(krate_spans.workspace_id) { - labels.push(Label::primary(ws_id, ws_span.key).with_message(format!( + labels.push(Label::secondary(ws_id, ws_span.key).with_message(format!( "{}workspace dependency", if ws_span.patched.is_some() { "patched " @@ -1649,7 +1656,7 @@ fn check_workspace_duplicates( continue; } - labels.push(Label::secondary(id, mdep.key_span)); + labels.push(Label::primary(id, mdep.key_span)); if let Some(rename) = &mdep.rename { labels.push( @@ -1680,7 +1687,7 @@ fn check_workspace_duplicates( pack.push(diags::WorkspaceDuplicate { duplicate, labels, - severity, + severity: cfg.duplicates, has_workspace_declaration, total_uses: total, }); diff --git a/src/bans/cfg.rs b/src/bans/cfg.rs index 27c8e0c4..7f3fac73 100644 --- a/src/bans/cfg.rs +++ b/src/bans/cfg.rs @@ -329,15 +329,40 @@ pub type CrateAllow = PackageSpecOrExtended; pub type CrateSkip = PackageSpecOrExtended; pub type TreeSkip = PackageSpecOrExtended; +#[cfg_attr(test, derive(serde::Serialize))] +pub struct WorkspaceDepsConfig { + /// How to handle workspace dependencies on the same crate that aren't declared + /// in `[workspace.dependencies]` + pub duplicates: LintLevel, + /// Whether path dependencies are treated as duplicates + pub include_path_dependencies: bool, + /// How to handle [`workspace.dependencies`] that are not used + pub unused: LintLevel, +} + +impl<'de> Deserialize<'de> for WorkspaceDepsConfig { + fn deserialize(value: &mut Value<'de>) -> Result { + let mut th = TableHelper::new(value)?; + + let duplicates = th.optional("duplicates").unwrap_or(LintLevel::Deny); + let include_path_dependencies = th.optional("include-path-dependencies").unwrap_or(true); + let unused = th.optional("unused").unwrap_or(LintLevel::Deny); + + th.finalize(None)?; + + Ok(Self { + duplicates, + include_path_dependencies, + unused, + }) + } +} + 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, + pub workspace_dependencies: Option, /// How the duplicate graphs are highlighted pub highlight: GraphHighlight, /// The crates that will cause us to emit failures @@ -378,8 +403,7 @@ impl Default for Config { Self { multiple_versions: LintLevel::Warn, multiple_versions_include_dev: false, - workspace_duplicates: LintLevel::Warn, - unused_workspace_dependencies: LintLevel::Allow, + workspace_dependencies: None, highlight: GraphHighlight::All, deny: Vec::new(), allow: Vec::new(), @@ -404,12 +428,6 @@ 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(); @@ -423,13 +441,14 @@ impl<'de> Deserialize<'de> for Config { let allow_build_scripts = th.optional("allow-build-scripts"); let build = th.optional("build"); + let workspace_dependencies = th.optional("workspace-dependencies"); + th.finalize(None)?; Ok(Self { multiple_versions, multiple_versions_include_dev, - workspace_duplicates, - unused_workspace_dependencies, + workspace_dependencies, highlight, deny, allow, @@ -737,8 +756,7 @@ 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, + workspace_dependencies: self.workspace_dependencies, highlight: self.highlight, denied, denied_multiple_versions, @@ -911,8 +929,7 @@ 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 workspace_dependencies: Option, 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 f7d98296..984abdf0 100644 --- a/src/bans/diags.rs +++ b/src/bans/diags.rs @@ -860,7 +860,7 @@ impl<'k> From> for Diag { if wd.has_workspace_declaration { "but not all declarations use the shared workspace dependency" } else { - "and there was no shared workspace dependency for it" + "and there is no shared workspace dependency for it" } )) .with_code(Code::WorkspaceDuplicate) diff --git a/src/bans/snapshots/cargo_deny__bans__cfg__test__deserializes_ban_cfg.snap b/src/bans/snapshots/cargo_deny__bans__cfg__test__deserializes_ban_cfg.snap index d7cafb1f..270c177c 100644 --- a/src/bans/snapshots/cargo_deny__bans__cfg__test__deserializes_ban_cfg.snap +++ b/src/bans/snapshots/cargo_deny__bans__cfg__test__deserializes_ban_cfg.snap @@ -6,8 +6,11 @@ expression: validated "file_id": 0, "multiple_versions": "deny", "multiple_versions_include_dev": false, - "workspace_duplicates": "warn", - "unused_workspace_dependencies": "allow", + "workspace_dependencies": { + "duplicates": "allow", + "include_path_dependencies": false, + "unused": "allow" + }, "highlight": "SimplestPath", "denied": [ { diff --git a/tests/bans.rs b/tests/bans.rs index c12335a7..2c474013 100644 --- a/tests/bans.rs +++ b/tests/bans.rs @@ -284,8 +284,10 @@ fn deny_duplicate_workspace_items() { }, r#" multiple-versions = 'allow' -workspace-duplicates = 'deny' -unused-workspace-dependencies = 'warn' + +[workspace-dependencies] +include-path-dependencies = true +unused = 'warn' "#, ); diff --git a/tests/bans_build.rs b/tests/bans_build.rs index d353a21d..4999c8c3 100644 --- a/tests/bans_build.rs +++ b/tests/bans_build.rs @@ -1,12 +1,20 @@ -// Ignore these tests on macos since they are only run in CI, which is actually just -// a potato masquerading as a computer -#![cfg(not(target_os = "macos"))] - use cargo_deny::{field_eq, func_name, test_utils::*}; +macro_rules! ci_ignore { + () => { + #[allow(clippy::disallowed_macros)] + if std::env::var_os("CI").is_some() { + eprintln!("potato detected, ignoring test"); + return; + } + }; +} + /// Verifies we can detect and error on builtin globs #[test] fn detects_scripts_by_builtin_glob() { + ci_ignore!(); + let mut diags = gather_bans( func_name!(), KrateGather { @@ -36,6 +44,8 @@ include-dependencies = true /// Verifies we can detect and error on extensions provided by the user #[test] fn detects_scripts_by_user_extension() { + ci_ignore!(); + let mut diags = gather_bans( func_name!(), KrateGather { @@ -56,6 +66,8 @@ fn detects_scripts_by_user_extension() { /// Verifies we detect and error on scripts detected by shebang #[test] fn detects_scripts_by_shebang() { + ci_ignore!(); + let mut diags = gather_bans( func_name!(), KrateGather { @@ -79,6 +91,8 @@ fn detects_scripts_by_shebang() { /// Verifies we detect and error on native executables #[test] fn detects_native_executables() { + ci_ignore!(); + let mut diags = gather_bans( func_name!(), KrateGather { @@ -105,6 +119,8 @@ include-dependencies = true /// Verifies user provided builscript checksums are always validated correctly #[test] fn detects_build_script_mismatch() { + ci_ignore!(); + let mut diags = gather_bans( func_name!(), KrateGather { @@ -135,6 +151,8 @@ build-script = "00abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef00 /// skipped #[test] fn skips_matching_build_scripts() { + ci_ignore!(); + let diags = gather_bans( func_name!(), KrateGather { @@ -166,6 +184,8 @@ build-script = "1a850d791184374f614d01c86c8d6c9ba0500e64cb746edc9720ceaaa1cd8eaf /// Verifies that build scripts are denied if not allowed nor bypassed #[test] fn allows_build_scripts_or_bypass() { + ci_ignore!(); + let diags = gather_bans( func_name!(), KrateGather { @@ -194,6 +214,8 @@ build-script = "1a850d791184374f614d01c86c8d6c9ba0500e64cb746edc9720ceaaa1cd8eaf /// Verifies executables are allowed by glob patterns #[test] fn allows_by_glob() { + ci_ignore!(); + let diags = gather_bans( func_name!(), KrateGather { @@ -225,6 +247,8 @@ allow-globs = [ /// Verifies executables are allowed by path/checksum #[test] fn allows_by_path() { + ci_ignore!(); + let mut diags = gather_bans( func_name!(), KrateGather { @@ -261,6 +285,8 @@ allow = [ /// Verifies unmatched configs emit diagnostics #[test] fn emits_unmatched_warnings() { + ci_ignore!(); + let mut diags = gather_bans( func_name!(), KrateGather { diff --git a/tests/cfg/bans.toml b/tests/cfg/bans.toml index 35e6450c..50cc372c 100644 --- a/tests/cfg/bans.toml +++ b/tests/cfg/bans.toml @@ -1,6 +1,5 @@ [bans] multiple-versions = "deny" -workspace-duplicates = "warn" wildcards = "deny" allow-wildcard-paths = true highlight = "simplest-path" @@ -20,6 +19,11 @@ deny = [ ] skip-tree = [{ name = "blah", depth = 20 }] +[bans.workspace-dependencies] +duplicates = "allow" +include-path-dependencies = false +unused = "allow" + [[bans.skip]] name = "rand" version = "=0.6.5"