Update krates again (#590)

Resolves: EmbarkStudios/krates#69
Resolves: #405

deny.toml

@@ -31,8 +31,8 @@ deny = [
  { name = "libssh2-sys" },
  # There is literally never a reason to use this
  { name = "cmake" },
- # https://github.com/Byron/gitoxide/pull/1245
- { name = "windows", wrappers = ["gix-sec"] },
+ # Ditto
+ { name = "windows" },
 ]
 skip = [
  # strum_macros + maybe-async

src/bans.rs

@@ -214,8 +214,61 @@ pub fn check(
  sink.push(build_diags);
  }
 
- let (denied_ids, ban_wrappers): (Vec<_>, Vec<_>) =
- denied.into_iter().map(|kb| (kb.id, kb.wrappers)).unzip();
+ struct BanWrappers {
+ map: std::collections::BTreeMap<usize, (usize, Vec<crate::Spanned<String>>)>,
+ hits: BitVec,
+ }
+
+ impl BanWrappers {
+ fn new(
+ mut map: std::collections::BTreeMap<usize, (usize, Vec<crate::Spanned<String>>)>,
+ ) -> Self {
+ let hits = BitVec::repeat(
+ false,
+ map.values_mut().fold(0, |sum, v| {
+ v.0 = sum;
+ sum + v.1.len()
+ }),
+ );
+
+ Self { map, hits }
+ }
+
+ #[inline]
+ fn has_wrappers(&self, i: usize) -> bool {
+ self.map.contains_key(&i)
+ }
+
+ #[inline]
+ fn check(&mut self, i: usize, name: &str) -> Option<crate::diag::Span> {
+ let (offset, wrappers) = &self.map[&i];
+ if let Some(pos) = wrappers.iter().position(|wrapper| wrapper.value == name) {
+ self.hits.set(*offset + pos, true);
+ Some(wrappers[pos].span.clone())
+ } else {
+ None
+ }
+ }
+ }
+
+ let (denied_ids, mut ban_wrappers) = {
+ let mut bw = std::collections::BTreeMap::new();
+
+ (
+ denied
+ .into_iter()
+ .enumerate()
+ .map(|(i, kb)| {
+ if let Some(wrappers) = kb.wrappers.filter(|v| !v.is_empty()) {
+ bw.insert(i, (0, wrappers));
+ }
+
+ kb.id
+ })
+ .collect::<Vec<_>>(),
+ BanWrappers::new(bw),
+ )
+ };
 
  let (feature_ids, features): (Vec<_>, Vec<_>) =
  features.into_iter().map(|cf| (cf.id, cf.features)).unzip();
@@ -390,6 +443,7 @@ pub fn check(
  let (_, build_packs) = rayon::join(
  || {
  let last = ctx.krates.len() - 1;
+
  for (i, krate) in ctx.krates.krates().enumerate() {
  let mut pack = Pack::with_kid(Check::Bans, krate.id.clone());
 
@@ -401,24 +455,25 @@ pub fn check(
  span: rm.id.span.clone(),
  };
 
- // The crate is banned, but it might have be allowed if it's wrapped
- // by one or more particular crates
- let is_allowed_by_wrapper = if let Some(wrappers) =
- ban_wrappers.get(rm.index).and_then(|bw| bw.as_ref())
- {
+ // The crate is banned, but it might be allowed if it's
+ // wrapped by one or more particular crates
+ let is_allowed_by_wrapper = if ban_wrappers.has_wrappers(rm.index) {
  let nid = ctx.krates.nid_for_kid(&krate.id).unwrap();
 
  // Ensure that every single crate that has a direct dependency
- // on the banned crate is an allowed wrapper
- ctx.krates.direct_dependents(nid).into_iter().all(|src| {
+ // on the banned crate is an allowed wrapper, note we
+ // check every one even after a failure so we don't get
+ // extra warnings about unmatched wrappers
+ let mut all = true;
+ for src in ctx.krates.direct_dependents(nid) {
  let (diag, is_allowed): (Diag, _) =
- match wrappers.iter().find(|aw| aw.value == src.krate.name) {
- Some(aw) => (
+ match ban_wrappers.check(rm.index, &src.krate.name) {
+ Some(span) => (
  diags::BannedAllowedByWrapper {
  ban_cfg: ban_cfg.clone(),
  ban_exception_cfg: CfgCoord {
  file: file_id,
- span: aw.span.clone(),
+ span,
  },
  banned_krate: krate,
  wrapper_krate: src.krate,
@@ -438,8 +493,10 @@ pub fn check(
  };
 
  pack.push(diag);
- is_allowed
- })
+ all = all && is_allowed;
+ }
+
+ all
  } else {
  false
  };
@@ -846,7 +903,7 @@ pub fn check(
  for skip in skip_hit
  .into_iter()
  .zip(skipped.into_iter())
- .filter_map(|(hit, skip)| if !hit { Some(skip) } else { None })
+ .filter_map(|(hit, skip)| (!hit).then_some(skip))
  {
  pack.push(diags::UnmatchedSkip {
  skip_cfg: CfgCoord {
@@ -857,6 +914,20 @@ pub fn check(
  });
  }
 
+ for wrapper in ban_wrappers
+ .hits
+ .into_iter()
+ .zip(ban_wrappers.map.into_values().flat_map(|(_, w)| w))
+ .filter_map(|(hit, wrapper)| (!hit).then_some(wrapper))
+ {
+ pack.push(diags::UnusedWrapper {
+ wrapper_cfg: CfgCoord {
+ file: file_id,
+ span: wrapper.span,
+ },
+ });
+ }
+
  sink.push(pack);
 }
 

src/bans/diags.rs

@@ -50,6 +50,7 @@ pub enum Code {
  UnmatchedBypass,
  UnmatchedPathBypass,
  UnmatchedGlob,
+ UnusedWrapper,
 }
 
 impl From<Code> for String {
@@ -214,6 +215,23 @@ impl<'a> From<UnmatchedSkip<'a>> for Diag {
  }
 }
 
+pub(crate) struct UnusedWrapper {
+ pub(crate) wrapper_cfg: CfgCoord,
+}
+
+impl From<UnusedWrapper> for Diag {
+ fn from(us: UnusedWrapper) -> Self {
+ Diagnostic::new(Severity::Warning)
+ .with_message("wrapper for banned crate was not enountered")
+ .with_code(Code::UnusedWrapper)
+ .with_labels(vec![us
+ .wrapper_cfg
+ .into_label()
+ .with_message("unmatched wrapper")])
+ .into()
+ }
+}
+
 pub(crate) struct BannedAllowedByWrapper<'a> {
  pub(crate) ban_cfg: CfgCoord,
  pub(crate) banned_krate: &'a Krate,

tests/bans.rs

@@ -25,6 +25,21 @@ wrappers = ["safe-wrapper"]
  insta::assert_json_snapshot!(diags);
 }
 
+#[test]
+fn warns_on_unused_wrappers() {
+ let diags = gather_bans(
+ func_name!(),
+ KrateGather::new("allow_wrappers/maincrate"),
+ r#"
+[[deny]]
+name = "dangerous-dep"
+wrappers = ["safe-wrapper", "other-crate"]
+"#,
+ );
+
+ insta::assert_json_snapshot!(diags);
+}
+
 #[test]
 fn disallows_denied() {
  let diags = gather_bans(