Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to allow wildcard path dependencies #487

Merged
merged 6 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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