From 309f13ec8477deef7b807e64b70962eea2b355ae Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Mon, 18 Oct 2021 15:55:53 +0100 Subject: [PATCH] Extend dependency status box to report more issues (#121) * Extend dependency status box to report more issues - replace render_dev_dependency_box with an extended render_dependency_box - reports insecure dev dependencies, outdated main dependencies, and outdated dev dependencies - handle pluralization in dependency count message - change methods in AnalyzeDependenciesOutcome - add count_outdated - remove any_dev_issues - remove AnalyzedDependencies::any_dev_issues * Format status.rs * Simplify AnalyzeDependenciesOutcome method impls - match ergonomics lint * Use bullet point list * Tweak dependency box again - only use list items if there is more than one dependency kind * Fix outdated dependency count - `count_outdated` already counts only main dependencies * Tweak dependency box to no longer assume non-zero issues - check for when all dependency component counts are zero, render nothing - always call `render_dependency_box` if it finds no security issues Co-authored-by: Cecile Tonglet --- .../styles/bulma/elements/notification.sass | 3 + src/engine/mod.rs | 19 +++--- src/models/crates.rs | 7 -- src/server/views/html/status.rs | 66 +++++++++++++++---- 4 files changed, 67 insertions(+), 28 deletions(-) diff --git a/assets/styles/bulma/elements/notification.sass b/assets/styles/bulma/elements/notification.sass index f5c6021..9e21e1b 100644 --- a/assets/styles/bulma/elements/notification.sass +++ b/assets/styles/bulma/elements/notification.sass @@ -50,3 +50,6 @@ $notification-colors: $colors !default &.is-light background-color: $color-light color: $color-dark + ul + list-style: disc + padding: 0 40 diff --git a/src/engine/mod.rs b/src/engine/mod.rs index 771f579..e564727 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -97,7 +97,7 @@ pub struct AnalyzeDependenciesOutcome { impl AnalyzeDependenciesOutcome { pub fn any_outdated(&self) -> bool { - self.crates.iter().any(|&(_, ref deps)| deps.any_outdated()) + self.crates.iter().any(|(_, deps)| deps.any_outdated()) } // TODO(feliix42): Why is this different from the any_outdated() function above? @@ -105,28 +105,29 @@ impl AnalyzeDependenciesOutcome { pub fn any_insecure(&self) -> bool { self.crates .iter() - .any(|&(_, ref deps)| deps.count_insecure() > 0) + .any(|(_, deps)| deps.count_insecure() > 0) } /// Checks if any always insecure main or build dependencies exist in the scanned crates pub fn any_always_insecure(&self) -> bool { self.crates .iter() - .any(|&(_, ref deps)| deps.count_always_insecure() > 0) + .any(|(_, deps)| deps.count_always_insecure() > 0) } - /// Checks if any dev-dependencies in the scanned crates are either outdated or insecure - pub fn any_dev_issues(&self) -> bool { + /// Returns the number of outdated main and dev dependencies + pub fn count_outdated(&self) -> usize { self.crates .iter() - .any(|&(_, ref deps)| deps.any_dev_issues()) + .map(|(_, deps)| deps.count_outdated()) + .sum() } /// Returns the number of outdated dev-dependencies pub fn count_dev_outdated(&self) -> usize { self.crates .iter() - .map(|&(_, ref deps)| deps.count_dev_outdated()) + .map(|(_, deps)| deps.count_dev_outdated()) .sum() } @@ -134,7 +135,7 @@ impl AnalyzeDependenciesOutcome { pub fn count_dev_insecure(&self) -> usize { self.crates .iter() - .map(|&(_, ref deps)| deps.count_dev_insecure()) + .map(|(_, deps)| deps.count_dev_insecure()) .sum() } @@ -142,7 +143,7 @@ impl AnalyzeDependenciesOutcome { pub fn outdated_ratio(&self) -> (usize, usize) { self.crates .iter() - .fold((0, 0), |(outdated, total), &(_, ref deps)| { + .fold((0, 0), |(outdated, total), (_, deps)| { (outdated + deps.count_outdated(), total + deps.count_total()) }) } diff --git a/src/models/crates.rs b/src/models/crates.rs index 3af5cdf..894ae40 100644 --- a/src/models/crates.rs +++ b/src/models/crates.rs @@ -255,13 +255,6 @@ impl AnalyzedDependencies { .filter(|&(_, dep)| dep.is_insecure()) .count() } - - /// Returns `true` if any dev-dependencies are either insecure or outdated. - pub fn any_dev_issues(&self) -> bool { - self.dev - .iter() - .any(|(_, dep)| dep.is_outdated() || dep.is_insecure()) - } } #[derive(Clone, Debug)] diff --git a/src/server/views/html/status.rs b/src/server/views/html/status.rs index 7e21506..7c974ef 100644 --- a/src/server/views/html/status.rs +++ b/src/server/views/html/status.rs @@ -158,19 +158,61 @@ fn render_title(subject_path: &SubjectPath) -> Markup { } } -fn render_dev_dependency_box(outcome: &AnalyzeDependenciesOutcome) -> Markup { - let insecure = outcome.count_dev_insecure(); - let outdated = outcome.count_dev_outdated(); - let text = if insecure > 0 { - format!("{} insecure development dependencies", insecure) +fn dependencies_pluralized(count: usize) -> &'static str { + if count == 1 { + "dependency" } else { - format!("{} outdated development dependencies", outdated) - }; + "dependencies" + } +} - html! { - div class="notification is-warning" { - p { "This project contains " b { (text) } "." } +/// Renders a warning with the numbers of outdated dependencies (of both kinds) +/// or insecure dev-dependencies. +/// +/// The function assumes that there are no insecure main dependencies. +/// If there is more than one kind of dependency with issues, +/// an unordered list is rendered. +/// Renders nothing if the counts of all three components are zero. +fn render_dependency_box(outcome: &AnalyzeDependenciesOutcome) -> Markup { + // assuming zero insecure main dependencies + let insecure_dev = outcome.count_dev_insecure(); + let outdated_dev = outcome.count_dev_outdated(); + let outdated = outcome.count_outdated(); + + let components = [ + ("insecure development", insecure_dev), + ("outdated main", outdated), + ("outdated development", outdated_dev), + ] + .iter() + .copied() + .filter(|&(_, c)| c > 0) + .map(|(kind, c)| { + let pluralized = dependencies_pluralized(c); + (c, kind, pluralized) + }) + .collect::>(); + + match components.len() { + 0 => html! {}, + 1 => { + let (c, kind, dep) = components[0]; + html! { + div class="notification is-warning" { + p { "This project contains " b { (c) " " (kind) " " (dep) } "." } + } + } } + _ => html! { + div class="notification is-warning" { + p { "This project contains:" } + ul { + @for (c, kind, dep) in components { + li { b { (c) " " (kind) " " (dep) } } + } + } + } + }, } } @@ -351,8 +393,8 @@ fn render_success( a href="#vulnerabilities" { "bottom"} "." } } - } @else if analysis_outcome.any_dev_issues() { - (render_dev_dependency_box(&analysis_outcome)) + } @else { + (render_dependency_box(&analysis_outcome)) } @for (crate_name, deps) in &analysis_outcome.crates { (dependency_tables(crate_name, deps))