diff --git a/src/check_release.rs b/src/check_release.rs index a87b6c00..b219709d 100644 --- a/src/check_release.rs +++ b/src/check_release.rs @@ -10,6 +10,7 @@ use rayon::prelude::*; use trustfall::{FieldValue, TransparentValue}; use crate::data_generation::DataStorage; +use crate::query::LintMode; use crate::{ query::{ActualSemverUpdate, LintLevel, OverrideStack, RequiredSemverUpdate, SemverQuery}, CrateReport, GlobalConfig, ReleaseType, WitnessGeneration, @@ -215,8 +216,9 @@ pub(super) fn run_check_release( let (queries_to_run, queries_to_skip): (Vec<_>, _) = SemverQuery::all_queries().into_values().partition(|query| { - !version_change.supports_requirement(overrides.effective_required_update(query)) - && overrides.effective_lint_level(query) > LintLevel::Allow + overrides.effective_lint_mode(query) == LintMode::AlwaysRun + || !version_change.supports_requirement(overrides.effective_required_update(query)) + && overrides.effective_lint_level(query) > LintLevel::Allow }); let skipped_queries = queries_to_skip.len(); @@ -272,12 +274,19 @@ pub(super) fn run_check_release( let mut results_with_errors = vec![]; let mut results_with_warnings = vec![]; + let mut results_with_always_run_errors = vec![]; + let mut results_with_always_run_warnings = vec![]; + for (semver_query, time_to_decide, results) in all_results { config .log_verbose(|config| { - let category = match overrides.effective_required_update(semver_query) { - RequiredSemverUpdate::Major => "major", - RequiredSemverUpdate::Minor => "minor", + let category = match ( + overrides.effective_required_update(semver_query), + overrides.effective_lint_mode(semver_query), + ) { + (RequiredSemverUpdate::Major, LintMode::SemVer) => "major", + (RequiredSemverUpdate::Minor, LintMode::SemVer) => "minor", + (_, LintMode::AlwaysRun) => "always-run", }; let (status, status_color) = match ( @@ -309,24 +318,50 @@ pub(super) fn run_check_release( .expect("print failed"); if !results.is_empty() { - match overrides.effective_lint_level(semver_query) { - LintLevel::Deny => results_with_errors.push((semver_query, results)), - LintLevel::Warn => results_with_warnings.push((semver_query, results)), - LintLevel::Allow => unreachable!( + let lint_level = overrides.effective_lint_level(semver_query); + let lint_mode = overrides.effective_lint_mode(semver_query); + + match (lint_level, lint_mode) { + (LintLevel::Deny, LintMode::SemVer) => { + results_with_errors.push((semver_query, results)) + } + (LintLevel::Warn, LintMode::SemVer) => { + results_with_warnings.push((semver_query, results)) + } + (LintLevel::Warn, LintMode::AlwaysRun) => { + results_with_always_run_warnings.push((semver_query, results)) + } + (LintLevel::Deny, LintMode::AlwaysRun) => { + results_with_always_run_errors.push((semver_query, results)) + } + (LintLevel::Allow, _) => unreachable!( "`LintLevel::Allow` lint was unexpectedly not skipped: {semver_query:?}" ), }; } } - let produced_errors = !results_with_errors.is_empty(); - let produced_warnings = !results_with_warnings.is_empty(); - if produced_errors || produced_warnings { - let status_color = if produced_errors { + let produced_semver_errors = !results_with_errors.is_empty(); + let produced_semver_warnings = !results_with_warnings.is_empty(); + let produced_always_run_errors = !results_with_always_run_errors.is_empty(); + let produced_always_run_warnings = !results_with_always_run_warnings.is_empty(); + let has_issues = produced_semver_errors + || produced_semver_warnings + || produced_always_run_errors + || produced_always_run_warnings; + + if has_issues { + let status_color = if produced_semver_errors || produced_always_run_errors { AnsiColor::Red } else { AnsiColor::Yellow }; + + let total_always_run_issues = + results_with_always_run_errors.len() + results_with_always_run_warnings.len(); + let total_failures = results_with_errors.len() + results_with_always_run_errors.len(); + let total_warnings = results_with_warnings.len() + results_with_always_run_warnings.len(); + config .shell_print( "Checked", @@ -334,9 +369,9 @@ pub(super) fn run_check_release( "[{:>8.3}s] {} checks: {} pass, {} fail, {} warn, {} skip", queries_start_instant.elapsed().as_secs_f32(), queries_to_run.len(), - queries_to_run.len() - results_with_errors.len() - results_with_warnings.len(), - results_with_errors.len(), - results_with_warnings.len(), + queries_to_run.len() - total_failures - total_warnings, + total_failures, + total_warnings, skipped_queries, ), Color::Ansi(status_color), @@ -377,6 +412,35 @@ pub(super) fn run_check_release( print_triggered_lint(config, semver_query, results, witness_generation)?; } + for (semver_query, results) in results_with_always_run_errors { + config.log_info(|config| { + writeln!( + config.stdout(), + "\n--- risk failure {}: {} ---\n", + &semver_query.id, + &semver_query.human_readable_name + )?; + Ok(()) + })?; + + print_triggered_lint(config, semver_query, results, witness_generation)?; + } + + // Process AlwaysRun warnings + for (semver_query, results) in results_with_always_run_warnings { + config.log_info(|config| { + writeln!( + config.stdout(), + "\n--- risk warning {}: {} ---\n", + semver_query.id, + semver_query.human_readable_name + )?; + Ok(()) + })?; + + print_triggered_lint(config, semver_query, results, witness_generation)?; + } + let required_bump = required_versions.iter().max().copied(); let suggested_bump = suggested_versions.iter().max().copied(); @@ -399,7 +463,7 @@ pub(super) fn run_check_release( Color::Ansi(AnsiColor::Red), true, )?; - } else if produced_warnings { + } else if produced_semver_warnings { writeln!(config.stderr())?; config.shell_print( "Summary", @@ -407,8 +471,6 @@ pub(super) fn run_check_release( Color::Ansi(AnsiColor::Green), true, )?; - } else { - unreachable!("Expected either warnings or errors to be produced."); } if let Some(suggested_bump) = suggested_bump { @@ -439,9 +501,34 @@ pub(super) fn run_check_release( } } + if total_always_run_issues > 0 { + writeln!(config.stderr())?; + let label = if produced_always_run_errors { + "Risk Alert" + } else { + "Risk Notice" + }; + let color = if produced_always_run_errors { + AnsiColor::Red + } else { + AnsiColor::Yellow + }; + + config.shell_print( + label, + format_args!( + "{} potentially risky changes detected that require attention regardless of version bump", + total_always_run_issues + ), + Color::Ansi(color), + true, + )?; + } + Ok(CrateReport { required_bump: required_bump.map(ReleaseType::from), detected_bump: version_change, + has_always_run_issues: total_always_run_issues > 0, }) } else { config @@ -469,6 +556,7 @@ pub(super) fn run_check_release( Ok(CrateReport { detected_bump: version_change, required_bump: None, + has_always_run_issues: false, }) } } diff --git a/src/lib.rs b/src/lib.rs index c13027e6..9a642e06 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -653,12 +653,17 @@ pub struct CrateReport { /// For example, if the crate contains breaking changes, this is [`Some(ReleaseType::Major)`]. /// If no additional bump beyond the already-detected one is required, this is [`Option::None`]. required_bump: Option, + /// Whether the crate has breaking changes that require attention regardless of version bump. + has_always_run_issues: bool, } impl CrateReport { /// Check if the semver check was successful. /// `true` if required bump <= detected bump. pub fn success(&self) -> bool { + if self.has_always_run_issues { + return false; + } match self.required_bump { // If `None`, no additional bump is required. None => true, diff --git a/src/manifest.rs b/src/manifest.rs index 171ac22f..bafe1304 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -3,7 +3,7 @@ use std::collections::BTreeMap; use anyhow::Context; use serde::Deserialize; -use crate::{LintLevel, OverrideMap, QueryOverride, RequiredSemverUpdate}; +use crate::{query::LintMode, LintLevel, OverrideMap, QueryOverride, RequiredSemverUpdate}; #[derive(Debug, Clone)] pub(crate) struct Manifest { @@ -116,17 +116,20 @@ impl LintTable { QueryOverride { lint_level: Some(lint_level), required_update: None, + lint_mode: None, }, ), OverrideConfig::Both { level, required_update, + lint_mode, priority, } => ( priority, QueryOverride { lint_level: Some(level), required_update: Some(required_update), + lint_mode, }, ), OverrideConfig::LintLevel { level, priority } => ( @@ -134,6 +137,7 @@ impl LintTable { QueryOverride { lint_level: Some(level), required_update: None, + lint_mode: None, }, ), OverrideConfig::RequiredUpdate { @@ -144,6 +148,15 @@ impl LintTable { QueryOverride { lint_level: None, required_update: Some(required_update), + lint_mode: None, + }, + ), + OverrideConfig::LintMode { mode, priority } => ( + priority, + QueryOverride { + lint_level: None, + required_update: None, + lint_mode: Some(mode), }, ), }; @@ -165,12 +178,13 @@ impl LintTable { #[derive(Debug, Clone, Deserialize)] #[serde(untagged)] pub(crate) enum OverrideConfig { - /// Specify both lint level and required update by name, e.g. - /// `lint_name = { level = "deny", required-update = "major" } + /// Specify both lint level and required update by name, with optional lint mode. + /// `lint_name = { level = "deny", required-update = "major" }` #[serde(rename_all = "kebab-case")] Both { level: LintLevel, required_update: RequiredSemverUpdate, + lint_mode: Option, /// The priority for this configuration. If there are multiple entries that /// configure a lint (e.g., a lint group containing a lint and the lint itself), /// the configuration entry with the **lowest** priority takes precedence. @@ -179,7 +193,7 @@ pub(crate) enum OverrideConfig { priority: i64, }, /// Specify just lint level by name, with optional priority. - /// `lint_name = { level = "deny" } + /// `lint_name = { level = "deny" }` #[serde(rename_all = "kebab-case")] LintLevel { level: LintLevel, @@ -187,13 +201,21 @@ pub(crate) enum OverrideConfig { priority: i64, }, /// Specify just required update by name, with optional priority. - /// `lint_name = { required-update = "minor" } + /// `lint_name = { required-update = "minor" }` #[serde(rename_all = "kebab-case")] RequiredUpdate { required_update: RequiredSemverUpdate, #[serde(default)] priority: i64, }, + /// Specify just lint mode by name, with optional priority. + /// `lint_name = { mode = "always-run" }` + #[serde(rename_all = "kebab-case")] + LintMode { + mode: LintMode, + #[serde(default)] + priority: i64, + }, /// Shorthand for specifying just a lint level and leaving /// the other members (required_update and priority) as default: e.g., /// `lint_name = "deny"` @@ -301,6 +323,7 @@ mod tests { QueryOverride { lint_level: Some(Deny), required_update: None, + lint_mode: None, } ),]), OverrideMap::from_iter([( @@ -308,6 +331,7 @@ mod tests { QueryOverride { lint_level: Some(Allow), required_update: None, + lint_mode: None, } ),]), ] @@ -321,6 +345,7 @@ mod tests { QueryOverride { lint_level: Some(Warn), required_update: None, + lint_mode: None, } )]), OverrideMap::from_iter([ @@ -328,7 +353,8 @@ mod tests { "two".into(), QueryOverride { lint_level: Some(Deny), - required_update: None + required_update: None, + lint_mode: None, } ), ( @@ -336,6 +362,7 @@ mod tests { QueryOverride { lint_level: None, required_update: Some(Major), + lint_mode: None, } ), ]), @@ -344,6 +371,7 @@ mod tests { QueryOverride { lint_level: Some(Allow), required_update: Some(Minor), + lint_mode: None, } )]) ] diff --git a/src/query.rs b/src/query.rs index e33da14a..ef001341 100644 --- a/src/query.rs +++ b/src/query.rs @@ -57,6 +57,17 @@ impl LintLevel { } } +/// Whether to run this query in semver mode or always run mode. +#[non_exhaustive] +#[derive(Default, Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum LintMode { + #[serde(alias = "always-run")] + AlwaysRun, + #[default] + #[serde(alias = "semver")] + SemVer, +} + /// Kind of semver update. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ActualSemverUpdate { @@ -103,6 +114,10 @@ pub struct SemverQuery { /// The default lint level for when this lint occurs. pub lint_level: LintLevel, + /// Whether to run this query in semver mode or always run mode. + #[serde(default)] + pub lint_mode: LintMode, + #[serde(default)] pub reference: Option, @@ -181,6 +196,13 @@ pub struct QueryOverride { /// the effective lint level. #[serde(default)] pub lint_level: Option, + + /// Whether to run this query in semver mode or always run mode. + /// + /// If this is `None`, use the query's default `lint_mode` when calculating + /// the effective lint mode. + #[serde(default)] + pub lint_mode: Option, } /// A mapping of lint ids to configured values that override that lint's defaults. @@ -233,6 +255,18 @@ impl OverrideStack { .find_map(|x| x.get(&query.id).and_then(|y| y.required_update)) .unwrap_or(query.required_update) } + + /// Calculates the *effective* lint mode of this query, by searching for an override + /// mapped to this query's id from the top of the stack first, returning the query's default + /// lint mode if not overridden. + #[must_use] + pub fn effective_lint_mode(&self, query: &SemverQuery) -> LintMode { + self.0 + .iter() + .rev() + .find_map(|x| x.get(&query.id).and_then(|y| y.lint_mode)) + .unwrap_or(query.lint_mode) + } } /// Data for generating a **witness** from the results of a [`SemverQuery`]. @@ -862,6 +896,7 @@ mod tests { id, lint_level, required_update, + lint_mode: Default::default(), human_readable_name: String::new(), description: String::new(), reference: None, @@ -883,6 +918,7 @@ mod tests { QueryOverride { lint_level: Some(LintLevel::Allow), required_update: Some(RequiredSemverUpdate::Minor), + lint_mode: None, }, ), ( @@ -890,6 +926,7 @@ mod tests { QueryOverride { lint_level: None, required_update: Some(RequiredSemverUpdate::Minor), + lint_mode: None, }, ), ])); @@ -930,6 +967,7 @@ mod tests { QueryOverride { lint_level: Some(LintLevel::Allow), required_update: Some(RequiredSemverUpdate::Minor), + lint_mode: None, }, ), ( @@ -937,6 +975,7 @@ mod tests { QueryOverride { lint_level: None, required_update: Some(RequiredSemverUpdate::Minor), + lint_mode: None, }, ), ])); @@ -946,6 +985,7 @@ mod tests { QueryOverride { required_update: None, lint_level: Some(LintLevel::Warn), + lint_mode: None, }, )]));