Skip to content

Commit

Permalink
Add allow list for dependencies that have build scripts (#432)
Browse files Browse the repository at this point in the history
* Implement allow-build-scripts feature

* Allow build scripts in example

* Add docs for new allow-build-scripts feature

* Address review comments

* Remove cfg contents from diagnostic

In large projects this could be very large and would bloat output with
little benefit.

Co-authored-by: Jake Shadle <jake.shadle@embark-studios.com>
  • Loading branch information
Stupremee and Jake-Shadle authored Jun 29, 2022
1 parent 2b0dacc commit 824c2d1
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 0 deletions.
4 changes: 4 additions & 0 deletions docs/src/checks/bans/cfg.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,7 @@ When dealing with duplicate versions, it's often the case that a particular crat
Note that by default, the `depth` is infinite.

**NOTE:** `skip-tree` is a very big hammer at the moment, and should be used with care.

### The `allow-build-scripts` field (optional)

Specifies all the crates that are allowed to have a build script. If this option is omitted, all crates are allowed to have a build script, and if this option is set to an empty list, no crate is allowed to have a build script.
4 changes: 4 additions & 0 deletions docs/src/checks/bans/diags.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,7 @@ A crate version in [`bans.skip-tree`](cfg.md#the-skip-tree-field-optional) was n
### `B011` - skipping crate due to root skip

A crate was skipped from being checked as a duplicate due to being transitively referenced by a crate version in [`bans.skip-tree`](cfg.md#the-skip-tree-field-optional).

### `B012` - crate has build script but is not allowed to have one

A crate which has been denied because it has a build script but is not part of the [`bans.allow-build-script`](cfg.md#the-allow-build-scripts-field-optional) list.
53 changes: 53 additions & 0 deletions examples/10_allow_build_script/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions examples/10_allow_build_script/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "allow-build-script"
version = "0.1.0"
edition = "2021"

[dependencies]
openssl-sys = "0.9.74"
5 changes: 5 additions & 0 deletions examples/10_allow_build_script/deny.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[bans]
allow-build-scripts = [
{ name = "openssl-sys" },
{ name = "libc" },
]
8 changes: 8 additions & 0 deletions examples/10_allow_build_script/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#[cfg(test)]
mod tests {
#[test]
fn it_works() {
let result = 2 + 2;
assert_eq!(result, 4);
}
}
18 changes: 18 additions & 0 deletions src/bans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ pub fn check(
highlight,
tree_skipped,
wildcards,
allow_build_scripts,
} = ctx.cfg;

let krate_spans = &ctx.krate_spans;
Expand Down Expand Up @@ -444,6 +445,23 @@ pub fn check(
}
}

if let Some(allow_build_scripts) = &allow_build_scripts {
let has_build_script = krate
.targets
.iter()
.any(|t| t.kind.iter().any(|k| *k == "custom-build"));

if has_build_script {
let allowed_build_script = allow_build_scripts.value.iter().any(|id| {
krate.name == id.name && crate::match_req(&krate.version, id.version.as_ref())
});

if !allowed_build_script {
pack.push(diags::BuildScriptNotAllowed { krate });
}
}
}

if !pack.is_empty() {
sink.push(pack);
}
Expand Down
16 changes: 16 additions & 0 deletions src/bans/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ pub struct Config {
/// How to handle wildcard dependencies
#[serde(default = "crate::lint_allow")]
pub wildcards: LintLevel,
/// List of crates that are allowed to have a build step.
pub allow_build_scripts: Option<Spanned<Vec<CrateId>>>,
}

impl Default for Config {
Expand All @@ -103,6 +105,7 @@ impl Default for Config {
skip: Vec::new(),
skip_tree: Vec::new(),
wildcards: LintLevel::Allow,
allow_build_scripts: None,
}
}
}
Expand Down Expand Up @@ -183,6 +186,18 @@ impl crate::cfg::UnvalidatedConfig for Config {
.into_iter()
.map(crate::Spanned::from)
.collect(),
allow_build_scripts: self.allow_build_scripts.map(|v| {
Spanned::new(
v.value
.into_iter()
.map(|id| KrateId {
name: id.name,
version: id.version,
})
.collect(),
v.span,
)
}),
}
}
}
Expand All @@ -209,6 +224,7 @@ pub struct ValidConfig {
pub(crate) skipped: Vec<Skrate>,
pub(crate) tree_skipped: Vec<Spanned<TreeSkip>>,
pub wildcards: LintLevel,
pub allow_build_scripts: Option<Spanned<Vec<KrateId>>>,
}

#[cfg(test)]
Expand Down
20 changes: 20 additions & 0 deletions src/bans/diags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,3 +221,23 @@ impl<'a> From<SkippedByRoot<'a>> for Diag {
.into()
}
}

pub(crate) struct BuildScriptNotAllowed<'a> {
pub(crate) krate: &'a Krate,
}

impl<'a> From<BuildScriptNotAllowed<'a>> for Diag {
fn from(bs: BuildScriptNotAllowed<'a>) -> Self {
Diagnostic::new(Severity::Error)
.with_message(format!(
"crate '{}' has a build script but is not allowed to have one",
bs.krate
))
.with_code("B012")
.with_notes(vec![
"the `bans.allow-build-scripts` field did not contain a match for the crate"
.to_owned(),
])
.into()
}
}

0 comments on commit 824c2d1

Please sign in to comment.