Skip to content

Commit

Permalink
Add option to allow wildcard path dependencies (#487)
Browse files Browse the repository at this point in the history
* 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 <jake.shadle@embark-studios.com>
  • Loading branch information
sribich and Jake-Shadle committed Dec 20, 2022
1 parent 967450b commit 05e1b34
Show file tree
Hide file tree
Showing 20 changed files with 1,591 additions and 14 deletions.
3 changes: 3 additions & 0 deletions deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
10 changes: 10 additions & 0 deletions docs/src/checks/bans/cfg.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
21 changes: 19 additions & 2 deletions src/bans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::{
};
use anyhow::Error;
pub use diags::Code;
use krates::cm::DependencyKind;
use semver::VersionReq;
use std::fmt;

Expand Down Expand Up @@ -201,6 +202,7 @@ pub fn check(
highlight,
tree_skipped,
wildcards,
allow_wildcard_paths,
allow_build_scripts,
} = ctx.cfg;

Expand Down Expand Up @@ -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,
});
}
Expand Down
15 changes: 15 additions & 0 deletions src/bans/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>>,
}
Expand All @@ -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,
}
}
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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>>>,
}

Expand Down Expand Up @@ -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,
Expand Down
10 changes: 8 additions & 2 deletions src/bans/diags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand All @@ -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),
Expand Down
5 changes: 5 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 2 additions & 7 deletions src/licenses/gather.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
11 changes: 8 additions & 3 deletions src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
33 changes: 33 additions & 0 deletions tests/bans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
1 change: 1 addition & 0 deletions tests/cfg/bans.toml
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: tests/bans.rs
expression: diags
---
[]
30 changes: 30 additions & 0 deletions tests/snapshots/bans__allow_path_wildcards_public_package.snap
Original file line number Diff line number Diff line change
@@ -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"
}
]
11 changes: 11 additions & 0 deletions tests/test_data/wildcards/allow-paths-dependency/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}
Loading

0 comments on commit 05e1b34

Please sign in to comment.