Add option to allow wildcard path dependencies (#487)

* Add option to allow wildcard path dependencies Adds an `allow-wildcard-paths` option for the `bans` check which will allow path dependencies to use wildcard versions if the crate is private or the dependency is a dev-dependency. Closes issue #448 * Rustfmt * Fix clippy lints * Ignore atty advisory * Oops * Ignore wildcard paths in git sources Co-authored-by: Jake Shadle <[email protected]>
EmbarkStudios · Dec 20, 2022 · 05e1b34 · 05e1b34
1 parent 967450b
commit 05e1b34
Show file tree

Hide file tree

Showing 20 changed files with 1,591 additions and 14 deletions.
diff --git a/deny.toml b/deny.toml
@@ -17,6 +17,9 @@ ignore = [
     # ansi_term is unmaintained, but it does exactly what it needs to and no more
     # so no reason to change just for the sake of it
     "RUSTSEC-2021-0139",
+    # potential unaligned pointer read on windows. Doesn't happen in practice, don't
+    # care
+    "RUSTSEC-2021-0145",
 ]
 
 [bans]

diff --git a/docs/src/checks/bans/cfg.md b/docs/src/checks/bans/cfg.md
@@ -24,6 +24,16 @@ Determines what happens when a dependency is specified with the `*` (wildcard) v
 * `warn` (default) - Prints a warning for each crate with a wildcard version, but does not fail the check.
 * `allow` - Ignores all wildcard version specifications.
 
+### The `allow-wildcard-paths` field (optional)
+
+If specified, alters how the `wildcard` field behaves:
+
+  * [path](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-path-dependencies) `dependencies` in **private** crates will no longer emit a warning or error.
+  * path `dev-dependencies` in both public and private crates will no longer emit a warning or error.
+  * path `dependencies` and `build-dependencies` in **public** crates will continue to produce warnings and errors.
+
+Being limited to private crates is due to crates.io not allowing packages to be published with `path` dependencies except for `dev-dependencies`.
+
 ### The `highlight` field (optional)
 
 When multiple versions of the same crate are encountered and `multiple-versions` is set to `warn` or `deny`, using the `-g <dir>` option will print out a [dotgraph](https://www.graphviz.org/) of each of the versions and how they were included into the graph. This field determines how the graph is colored to help you quickly spot good candidates for removal or updating.

diff --git a/src/bans.rs b/src/bans.rs
@@ -9,6 +9,7 @@ use crate::{
 };
 use anyhow::Error;
 pub use diags::Code;
+use krates::cm::DependencyKind;
 use semver::VersionReq;
 use std::fmt;
 
@@ -201,6 +202,7 @@ pub fn check(
         highlight,
         tree_skipped,
         wildcards,
+        allow_wildcard_paths,
         allow_build_scripts,
     } = ctx.cfg;
 
@@ -645,24 +647,39 @@ pub fn check(
 
             multi_detector.dupes.push(i);
 
-            if wildcards != LintLevel::Allow {
+            if wildcards != LintLevel::Allow && !krate.is_git_source() {
                 let severity = match wildcards {
                     LintLevel::Warn => Severity::Warning,
                     LintLevel::Deny => Severity::Error,
                     LintLevel::Allow => unreachable!(),
                 };
 
-                let wildcards: Vec<_> = krate
+                let mut wildcards: Vec<_> = krate
                     .deps
                     .iter()
                     .filter(|dep| dep.req == VersionReq::STAR)
                     .collect();
 
+                if allow_wildcard_paths {
+                    let is_private = krate.is_private(&[]);
+
+                    wildcards.retain(|dep| {
+                        if is_private {
+                            dep.path.is_none()
+                        } else {
+                            let is_path_dev_dependency =
+                                dep.path.is_some() && dep.kind != DependencyKind::Development;
+                            is_path_dev_dependency || dep.path.is_none()
+                        }
+                    });
+                }
+
                 if !wildcards.is_empty() {
                     sink.push(diags::Wildcards {
                         krate,
                         severity,
                         wildcards,
+                        allow_wildcard_paths,
                         cargo_spans: &cargo_spans,
                     });
                 }

diff --git a/src/bans/cfg.rs b/src/bans/cfg.rs
@@ -118,6 +118,14 @@ pub struct Config {
     /// How to handle wildcard dependencies
     #[serde(default = "crate::lint_allow")]
     pub wildcards: LintLevel,
+    /// Wildcard dependencies defined using path attributes will be treated as
+    /// if they were [`LintLevel::Allow`] for private crates, but other wildcard
+    /// dependencies will be treated as [`LintLevel::Deny`].
+    ///
+    /// crates.io does not allow packages to be published with path dependencies,
+    /// thus this rule will not effect public packages.
+    #[serde(default)]
+    pub allow_wildcard_paths: bool,
     /// List of crates that are allowed to have a build step.
     pub allow_build_scripts: Option<Spanned<Vec<CrateId>>>,
 }
@@ -135,6 +143,7 @@ impl Default for Config {
             skip: Vec::new(),
             skip_tree: Vec::new(),
             wildcards: LintLevel::Allow,
+            allow_wildcard_paths: false,
             allow_build_scripts: None,
         }
     }
@@ -257,6 +266,7 @@ impl crate::cfg::UnvalidatedConfig for Config {
             workspace_default_features: self.workspace_default_features,
             skipped,
             wildcards: self.wildcards,
+            allow_wildcard_paths: self.allow_wildcard_paths,
             tree_skipped: self
                 .skip_tree
                 .into_iter()
@@ -316,6 +326,7 @@ pub struct ValidConfig {
     pub(crate) skipped: Vec<Skrate>,
     pub(crate) tree_skipped: Vec<Spanned<TreeSkip>>,
     pub wildcards: LintLevel,
+    pub allow_wildcard_paths: bool,
     pub allow_build_scripts: Option<Spanned<Vec<KrateId>>>,
 }
 
@@ -362,6 +373,10 @@ mod test {
 
         assert_eq!(validated.file_id, cd.id);
         assert_eq!(validated.multiple_versions, LintLevel::Deny);
+
+        assert_eq!(validated.wildcards, LintLevel::Deny);
+        assert!(validated.allow_wildcard_paths);
+
         assert_eq!(validated.highlight, GraphHighlight::SimplestPath);
         assert_eq!(
             validated.external_default_features.unwrap().value,

diff --git a/src/bans/diags.rs b/src/bans/diags.rs
@@ -132,6 +132,7 @@ pub(crate) struct Wildcards<'a> {
     pub(crate) krate: &'a Krate,
     pub(crate) severity: Severity,
     pub(crate) wildcards: Vec<&'a krates::cm::Dependency>,
+    pub(crate) allow_wildcard_paths: bool,
     pub(crate) cargo_spans: &'a crate::diag::CargoSpans,
 }
 
@@ -151,10 +152,15 @@ impl<'a> From<Wildcards<'a>> for Pack {
         let diag = Diag::new(
             Diagnostic::new(wc.severity)
                 .with_message(format!(
-                    "found {} wildcard dependenc{} for crate '{}'",
+                    "found {} wildcard dependenc{} for crate '{}'{}",
                     labels.len(),
                     if labels.len() == 1 { "y" } else { "ies" },
-                    wc.krate.name
+                    wc.krate.name,
+                    if wc.allow_wildcard_paths {
+                        ". allow-wildcard-paths is enabled, but does not apply to public crates as crates.io disallows path dependencies."
+                    } else {
+                        ""
+                    },
                 ))
                 .with_code(Code::Wildcard)
                 .with_labels(labels),

diff --git a/src/lib.rs b/src/lib.rs
@@ -193,6 +193,11 @@ impl Krate {
             url
         })
     }
+
+    #[inline]
+    pub(crate) fn is_git_source(&self) -> bool {
+        self.source.as_ref().map_or(false, |src| src.is_git())
+    }
 }
 
 impl fmt::Display for Krate {

diff --git a/src/licenses/gather.rs b/src/licenses/gather.rs
@@ -439,14 +439,9 @@ impl Gatherer {
         self
     }
 
+    #[inline]
     pub fn with_confidence_threshold(mut self, threshold: f32) -> Self {
-        self.threshold = if threshold > 1.0 {
-            1.0
-        } else if threshold < 0.0 {
-            0.0
-        } else {
-            threshold
-        };
+        self.threshold = threshold.clamp(0.0, 1.0);
         self
     }
 

diff --git a/src/test_utils.rs b/src/test_utils.rs
@@ -55,17 +55,22 @@ pub struct Config<C> {
     pub config: String,
 }
 
-impl<C> Config<C>
+impl<C> Default for Config<C>
 where
-    C: serde::de::DeserializeOwned + Default,
+    C: Default,
 {
-    pub fn default() -> Self {
+    fn default() -> Self {
         Self {
             deserialized: C::default(),
             config: "".to_owned(),
         }
     }
+}
 
+impl<C> Config<C>
+where
+    C: serde::de::DeserializeOwned,
+{
     pub fn new(config: impl Into<String>) -> Self {
         let config = config.into();
         Self {

diff --git a/tests/bans.rs b/tests/bans.rs
@@ -48,6 +48,39 @@ fn deny_wildcards() {
     insta::assert_json_snapshot!(diags);
 }
 
+/// Ensures that wildcard dependencies are still banned when
+/// allow-wildcard-paths is set to true but the package is public.
+#[test]
+fn allow_path_wildcards_public_package() {
+    let diags = gather_bans(
+        func_name!(),
+        KrateGather::new("wildcards/allow-paths-public"),
+        r#"
+multiple-versions = 'allow'
+wildcards = 'deny'
+allow-wildcard-paths = true
+"#,
+    );
+
+    insta::assert_json_snapshot!(diags);
+}
+
+/// Ensures that wildcard paths are allowed for private packages
+#[test]
+fn allow_path_wildcards_private_package() {
+    let diags = gather_bans(
+        func_name!(),
+        KrateGather::new("wildcards/allow-paths-private"),
+        r#"
+multiple-versions = 'allow'
+wildcards = 'deny'
+allow-wildcard-paths = true
+"#,
+    );
+
+    insta::assert_json_snapshot!(diags);
+}
+
 /// Ensures that multiple versions are always deterministically sorted by
 /// version number
 /// See <https://github.com/EmbarkStudios/cargo-deny/issues/384>

diff --git a/tests/cfg/bans.toml b/tests/cfg/bans.toml
@@ -1,6 +1,7 @@
 [bans]
 multiple-versions = "deny"
 wildcards = "deny"
+allow-wildcard-paths = true
 highlight = "simplest-path"
 workspace-default-features = "warn"
 external-default-features = "deny"

diff --git a/tests/snapshots/bans__allow_path_wildcards_private_package.snap b/tests/snapshots/bans__allow_path_wildcards_private_package.snap
@@ -0,0 +1,5 @@
+---
+source: tests/bans.rs
+expression: diags
+---
+[]
diff --git a/tests/snapshots/bans__allow_path_wildcards_public_package.snap b/tests/snapshots/bans__allow_path_wildcards_public_package.snap
@@ -0,0 +1,30 @@
+---
+source: tests/bans.rs
+expression: diags
+---
+[
+  {
+    "fields": {
+      "code": "wildcard",
+      "graphs": [
+        {
+          "Krate": {
+            "name": "wildcards-test-allow-paths-public",
+            "version": "0.1.0"
+          }
+        }
+      ],
+      "labels": [
+        {
+          "column": 1,
+          "line": 1,
+          "message": "wildcard crate entry",
+          "span": "wildcards-test-allow-paths-dependency = '*'"
+        }
+      ],
+      "message": "found 1 wildcard dependency for crate 'wildcards-test-allow-paths-public'. allow-wildcard-paths is enabled, but does not apply to public crates as crates.io disallows path dependencies.",
+      "severity": "error"
+    },
+    "type": "diagnostic"
+  }
+]
diff --git a/tests/test_data/wildcards/allow-paths-dependency/Cargo.toml b/tests/test_data/wildcards/allow-paths-dependency/Cargo.toml
@@ -0,0 +1,11 @@
+[package]
+name = "wildcards-test-allow-paths-dependency"
+version = "0.1.0"
+authors = []
+edition = "2018"
+license = "MIT"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+sqlx = "0.5"
diff --git a/tests/test_data/wildcards/allow-paths-dependency/src/lib.rs b/tests/test_data/wildcards/allow-paths-dependency/src/lib.rs
@@ -0,0 +1 @@
+fn main() {}