Skip to content

Commit

Permalink
Deny multiple versions for specific crates (#504)
Browse files Browse the repository at this point in the history
* Allow banning multiple versions of a specific crate

* Add test

---------

Co-authored-by: leops <[email protected]>
  • Loading branch information
Jake-Shadle and leops authored Apr 6, 2023
1 parent c92752f commit e21ce16
Show file tree
Hide file tree
Showing 5 changed files with 471 additions and 10 deletions.
15 changes: 12 additions & 3 deletions docs/src/checks/bans/cfg.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ Determines what happens when a dependency is specified with the `*` (wildcard) v

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.
* [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`.

Expand Down Expand Up @@ -76,6 +76,15 @@ deny = [{ name = "crate-you-don't-want", version = "<= 0.7.0", wrappers = ["this

This field allows specific crates to have a direct dependency on the banned crate but denies all transitive dependencies on it.

#### The `deny-multiple-versions` field (optional)

```ini
multiple-versions = 'allow'
deny = [{ name = "crate-you-want-only-one-version-of", deny-multiple-versions = true }]
```

This field allows specific crates to deny multiple versions of themselves, but allowing or warning on multiple versions for all other crates. This field cannot be set simultaneously with `wrappers`.

### The `allow` field (optional)

Determines specific crates that are allowed. If the `allow` list has one or more entries, then any crate not in that list will be denied, so use with care.
Expand Down
16 changes: 13 additions & 3 deletions src/bans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ pub fn check(
let ValidConfig {
file_id,
denied,
denied_multiple_versions,
allowed,
features,
workspace_default_features,
Expand Down Expand Up @@ -236,14 +237,23 @@ pub fn check(
};

let report_duplicates = |multi_detector: &MultiDetector<'_>, sink: &mut diag::ErrorSink| {
if multi_detector.dupes.len() <= 1 || multiple_versions == LintLevel::Allow {
if multi_detector.dupes.len() <= 1 {
return;
}

let severity = match multiple_versions {
let lint_level = if multi_detector.dupes.iter().any(|kindex| {
let krate = &ctx.krates[*kindex];
matches(&denied_multiple_versions, krate).is_some()
}) {
LintLevel::Deny
} else {
multiple_versions
};

let severity = match lint_level {
LintLevel::Warn => Severity::Warning,
LintLevel::Deny => Severity::Error,
LintLevel::Allow => unreachable!(),
LintLevel::Allow => return,
};

let mut all_start = std::usize::MAX;
Expand Down
51 changes: 47 additions & 4 deletions src/bans/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ pub struct CrateBan {
pub version: Option<VersionReq>,
/// One or more crates that will allow this crate to be used if it is a
/// direct dependency
pub wrappers: Option<Vec<Spanned<String>>>,
pub wrappers: Option<Spanned<Vec<Spanned<String>>>>,
/// Setting this to true will only emit an error if multiple
// versions of the crate are found
pub deny_multiple_versions: Option<Spanned<bool>>,
}

#[derive(Deserialize, Clone)]
Expand Down Expand Up @@ -163,8 +166,14 @@ impl crate::cfg::UnvalidatedConfig for Config {
)
};

let denied: Vec<_> = self
.deny
let (deny_multiple_versions, deny): (Vec<_>, Vec<_>) =
self.deny.into_iter().partition(|kb| {
kb.deny_multiple_versions
.as_ref()
.map_or(false, |spanned| spanned.value)
});

let denied: Vec<_> = deny
.into_iter()
.map(|cb| KrateBan {
id: Skrate::new(
Expand All @@ -174,7 +183,39 @@ impl crate::cfg::UnvalidatedConfig for Config {
},
cb.name.span,
),
wrappers: cb.wrappers,
wrappers: cb.wrappers.map(|spanned| spanned.value),
})
.collect();

let denied_multiple_versions: Vec<_> = deny_multiple_versions
.into_iter()
.map(|cb| {
let wrappers = cb.wrappers.filter(|spanned| !spanned.value.is_empty());
if let Some(wrappers) = wrappers {
// cb.multiple_versions is guaranteed to be Some(_) by the
// earlier call to `partition`
let multiple_versions = cb.deny_multiple_versions.unwrap();
diags.push(
Diagnostic::error()
.with_message(
"a crate ban was specified with both `wrappers` and `multiple-versions`",
)
.with_labels(vec![
Label::secondary(cfg_file, wrappers.span)
.with_message("has one or more `wrappers`"),
Label::secondary(cfg_file, multiple_versions.span)
.with_message("has `multiple-versions` set to true"),
]),
);
}

Skrate::new(
KrateId {
name: cb.name.value,
version: cb.version,
},
cb.name.span,
)
})
.collect();

Expand Down Expand Up @@ -260,6 +301,7 @@ impl crate::cfg::UnvalidatedConfig for Config {
multiple_versions: self.multiple_versions,
highlight: self.highlight,
denied,
denied_multiple_versions,
allowed,
features,
external_default_features: self.external_default_features,
Expand Down Expand Up @@ -319,6 +361,7 @@ pub struct ValidConfig {
pub multiple_versions: LintLevel,
pub highlight: GraphHighlight,
pub(crate) denied: Vec<KrateBan>,
pub(crate) denied_multiple_versions: Vec<Skrate>,
pub(crate) allowed: Vec<Skrate>,
pub(crate) features: Vec<KrateFeatures>,
pub external_default_features: Option<Spanned<LintLevel>>,
Expand Down
19 changes: 19 additions & 0 deletions tests/bans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,3 +120,22 @@ fn duplicate_graphs() {

insta::assert_debug_snapshot!(dup_graphs.lock().unwrap());
}

/// Ensures that we can allow duplicates generally, but deny them for specific
/// crates
#[test]
fn deny_multiple_versions_for_specific_krates() {
let diags = gather_bans(
func_name!(),
KrateGather::new("duplicates"),
r#"
multiple-versions = 'allow'
deny = [
{ name = 'block-buffer', deny-multiple-versions = true },
{ name = 'generic-array', deny-multiple-versions = true },
]
"#,
);

insta::assert_json_snapshot!(diags);
}
Loading

0 comments on commit e21ce16

Please sign in to comment.