diff --git a/src/check_release.rs b/src/check_release.rs index e92dbe183..8e59c8461 100644 --- a/src/check_release.rs +++ b/src/check_release.rs @@ -11,7 +11,7 @@ use trustfall::{FieldValue, TransparentValue}; use trustfall_rustdoc::{VersionedCrate, VersionedIndexedCrate, VersionedRustdocAdapter}; use crate::{ - query::{ActualSemverUpdate, LintLevel, RequiredSemverUpdate, SemverQuery}, + query::{ActualSemverUpdate, LintLevel, QueryOverrideList, RequiredSemverUpdate, SemverQuery}, CrateReport, GlobalConfig, ReleaseType, }; @@ -160,8 +160,9 @@ pub(super) fn run_check_release( current_crate: VersionedCrate, baseline_crate: VersionedCrate, release_type: Option, + overrides: QueryOverrideList, ) -> anyhow::Result { - let queries = config.all_queries()?; + let queries = SemverQuery::all_queries(); let current_version = current_crate.crate_version(); let baseline_version = baseline_crate.crate_version(); @@ -193,8 +194,8 @@ pub(super) fn run_check_release( let adapter = VersionedRustdocAdapter::new(¤t, Some(&previous))?; let (queries_to_run, queries_to_skip): (Vec<_>, _) = queries.values().partition(|query| { - !version_change.supports_requirement(query.required_update) - && query.lint_level >= LintLevel::Warn + !version_change.supports_requirement(overrides.effective_required_update(query)) + && overrides.effective_lint_level(query) >= LintLevel::Warn }); let skipped_queries = queries_to_skip.len(); @@ -253,7 +254,7 @@ pub(super) fn run_check_release( for (semver_query, time_to_decide, results) in all_results { config .log_verbose(|config| { - let category = match semver_query.required_update { + let category = match overrides.effective_required_update(semver_query) { RequiredSemverUpdate::Major => "major", RequiredSemverUpdate::Minor => "minor", }; @@ -288,7 +289,7 @@ pub(super) fn run_check_release( }) .expect("print failed"); if !results.is_empty() { - if semver_query.lint_level == LintLevel::Deny { + if overrides.effective_lint_level(semver_query) == LintLevel::Deny { error_results.push((semver_query, results)); } else { warning_results.push((semver_query, results)); @@ -317,26 +318,26 @@ pub(super) fn run_check_release( let mut warnings = BTreeMap::new(); let mut errors = BTreeMap::new(); // print errors before warnings like clippy does - for (semver_query, results) in warning_results { - warnings - .entry(semver_query.required_update) + for (semver_query, results) in error_results { + errors + .entry(overrides.effective_required_update(semver_query)) .and_modify(|e| *e += 1) .or_insert(1); - config.shell_warn(format!( + config.shell_error(format!( "{}: {}", &semver_query.id, &semver_query.human_readable_name ))?; print_lint_failure(config, semver_query, results)?; } - for (semver_query, results) in error_results { - errors - .entry(semver_query.required_update) + for (semver_query, results) in warning_results { + warnings + .entry(overrides.effective_required_update(semver_query)) .and_modify(|e| *e += 1) .or_insert(1); - config.shell_error(format!( + config.shell_warn(format!( "{}: {}", &semver_query.id, &semver_query.human_readable_name ))?; diff --git a/src/config.rs b/src/config.rs index ce412d5f0..2a2517293 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,8 +1,8 @@ use anstream::{AutoStream, ColorChoice}; use anstyle::{AnsiColor, Color, Reset, Style}; -use std::{collections::BTreeMap, io::Write}; +use std::io::Write; -use crate::{query::QueryOverride, templating::make_handlebars_registry, SemverQuery}; +use crate::templating::make_handlebars_registry; #[allow(dead_code)] pub struct GlobalConfig { @@ -14,9 +14,6 @@ pub struct GlobalConfig { minimum_rustc_version: semver::Version, stdout: AutoStream>, stderr: AutoStream>, - /// A mapping of lint names to values to override that lint's defaults, - /// such as its lint level and required semver bump. - query_overrides: BTreeMap, } impl Default for GlobalConfig { @@ -41,7 +38,6 @@ impl GlobalConfig { minimum_rustc_version: semver::Version::new(1, 74, 0), stdout: AutoStream::new(Box::new(std::io::stdout()), stdout_choice), stderr: AutoStream::new(Box::new(std::io::stderr()), stderr_choice), - query_overrides: BTreeMap::new(), } } @@ -288,31 +284,6 @@ impl GlobalConfig { ColorChoice::Never | ColorChoice::Auto => false, } } - - pub fn set_query_overrides( - &mut self, - query_overrides: BTreeMap, - ) -> &mut Self { - self.query_overrides = query_overrides; - self - } - - #[must_use] - pub fn query_overrides(&self) -> &BTreeMap { - &self.query_overrides - } - - pub fn all_queries(&self) -> anyhow::Result> { - let mut queries = SemverQuery::all_queries(); - for (name, overrides) in &self.query_overrides { - if let Some(query) = queries.get_mut(name) { - query.apply_override(overrides); - } else { - anyhow::bail!("Can't configure lint with unknown name `{name}`."); - } - } - Ok(queries) - } } #[cfg(test)] diff --git a/src/lib.rs b/src/lib.rs index 6d3e5a21a..b4c7cea6c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,10 +22,14 @@ use trustfall_rustdoc::{load_rustdoc, VersionedCrate}; use rustdoc_cmd::RustdocCommand; use std::collections::{BTreeMap, HashSet}; use std::path::{Path, PathBuf}; +use std::sync::Arc; use std::time::Instant; pub use config::GlobalConfig; -pub use query::{ActualSemverUpdate, LintLevel, QueryOverride, RequiredSemverUpdate, SemverQuery}; +pub use query::{ + ActualSemverUpdate, LintLevel, QueryOverride, QueryOverrideList, QueryOverrides, + RequiredSemverUpdate, SemverQuery, +}; /// Test a release for semver violations. #[non_exhaustive] @@ -40,6 +44,13 @@ pub struct Check { baseline_feature_config: rustdoc_gen::FeatureConfig, /// Which `--target` to use, if unset pass no flag build_target: Option, + /// Workspace-level configuration overrides to apply to all packages if + /// running in workspace scope. Package-level overrides for a given + /// crate in `package_overrides take precedence over this member if both are set. + workspace_overrides: Option>, + /// A mapping of package name to package-level configuration overrides for + /// that package. Takes precedence over `workspace_overrides` if both are set. + package_overrides: BTreeMap>, } /// The kind of release we're making. @@ -253,6 +264,8 @@ impl Check { current_feature_config: rustdoc_gen::FeatureConfig::default_for_current(), baseline_feature_config: rustdoc_gen::FeatureConfig::default_for_baseline(), build_target: None, + workspace_overrides: None, + package_overrides: BTreeMap::new(), } } @@ -372,6 +385,22 @@ impl Check { }) } + /// Helper function to get the configured workspace-level overrides if set and + /// running in workspace scope, otherwise return an empty list. + #[must_use] + fn workspace_overrides(&self) -> Vec> { + if let ScopeMode::DenyList(PackageSelection { + selection: ScopeSelection::Workspace, + .. + }) = self.scope.mode + { + if let Some(wksp) = &self.workspace_overrides { + return vec![Arc::clone(wksp)]; + } + } + vec![] + } + pub fn check_release(&self, config: &mut GlobalConfig) -> anyhow::Result { let rustdoc_cmd = RustdocCommand::new().deps(false).silence(config.is_info()); @@ -447,12 +476,19 @@ impl Check { }, )?; + let mut overrides = self.workspace_overrides(); + + if let Some(pkg) = self.package_overrides.get(&name) { + overrides.push(Arc::clone(pkg)); + } + let report = run_check_release( config, &name, current_crate, baseline_crate, self.release_type, + overrides.into(), )?; config.shell_status( "Finished", @@ -525,6 +561,11 @@ note: skipped the following crates since they have no library target: {skipped}" }, )?; + let mut overrides = self.workspace_overrides(); + if let Some(pkg) = self.package_overrides.get(&selected.name) { + overrides.push(Arc::clone(pkg)); + } + let result = Ok(( crate_name.clone(), Some(run_check_release( @@ -533,6 +574,7 @@ note: skipped the following crates since they have no library target: {skipped}" current_crate, baseline_crate, self.release_type, + overrides.into(), )?), )); config.shell_status( diff --git a/src/main.rs b/src/main.rs index fa8cee264..c642cb8b0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -29,7 +29,8 @@ fn main() { let mut config = GlobalConfig::new(); config.set_log_level(args.check_release.verbosity.log_level()); - let queries = config.all_queries()?; + // TODO: do we want this to respect configuration in the current directory? + let queries = SemverQuery::all_queries(); let mut rows = vec![ ["id", "type", "level", "description"], ["==", "====", "=====", "==========="], diff --git a/src/query.rs b/src/query.rs index 676ea6a72..2d3594627 100644 --- a/src/query.rs +++ b/src/query.rs @@ -1,4 +1,4 @@ -use std::collections::BTreeMap; +use std::{collections::BTreeMap, sync::Arc}; use serde::{Deserialize, Serialize}; use trustfall::TransparentValue; @@ -82,7 +82,7 @@ impl LintLevel { } /// Configured values for a [`SemverQuery`] that differ from the lint's default. -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct QueryOverride { /// The required version bump for this lint; see [`SemverQuery`].`required_update`. /// @@ -96,6 +96,59 @@ pub struct QueryOverride { pub lint_level: Option, } +/// A mapping of lint level name to values to override that lint's defaults with. +pub type QueryOverrides = BTreeMap; + +/// Stores a list of [`QueryOverride`] references. +/// +/// Items later in the list (with higher indices) have *higher* precedence, +/// so e.g., store package-level overrides at index 1 and workspace-level +/// overrides at index 0 for package-level overrides to take priority over +/// workspace-level ones if both are set. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct QueryOverrideList(Vec>); + +impl QueryOverrideList { + /// Creates a new empty [`QueryOverrideList`] instance. + #[must_use] + pub fn new() -> Self { + Self(Vec::new()) + } + + /// Inserts the given element at the end of the list. + /// The inserted parameter `overrides` will take precedence over all + /// previous overrides in the list, if both set. + pub fn push(&mut self, overrides: Arc) { + self.0.push(overrides); + } + + #[must_use] + pub fn effective_lint_level(&self, query: &SemverQuery) -> LintLevel { + self.0 + .iter() + .rev() + .filter_map(|x| x.get(&query.id).and_then(|y| y.lint_level)) + .next() + .unwrap_or(query.lint_level) + } + + #[must_use] + pub fn effective_required_update(&self, query: &SemverQuery) -> RequiredSemverUpdate { + self.0 + .iter() + .rev() + .filter_map(|x| x.get(&query.id).and_then(|y| y.required_update)) + .next() + .unwrap_or(query.required_update) + } +} + +impl From>> for QueryOverrideList { + fn from(value: Vec>) -> Self { + Self(value) + } +} + /// A query that can be executed on a pair of rustdoc output files, /// returning instances of a particular kind of semver violation. #[non_exhaustive] @@ -168,7 +221,7 @@ Failed to parse a query: {e} #[cfg(test)] mod tests { - use std::sync::OnceLock; + use std::sync::{Arc, OnceLock}; use std::{collections::BTreeMap, path::Path}; use anyhow::Context; @@ -177,7 +230,10 @@ mod tests { load_rustdoc, VersionedCrate, VersionedIndexedCrate, VersionedRustdocAdapter, }; - use crate::query::SemverQuery; + use super::{ + LintLevel, QueryOverride, QueryOverrideList, QueryOverrides, RequiredSemverUpdate, + SemverQuery, + }; use crate::templating::make_handlebars_registry; static TEST_CRATE_NAMES: OnceLock> = OnceLock::new(); @@ -498,6 +554,115 @@ mod tests { } } } + + #[must_use] + fn make_blank_query( + id: String, + lint_level: LintLevel, + required_update: RequiredSemverUpdate, + ) -> SemverQuery { + SemverQuery { + id, + lint_level, + required_update, + human_readable_name: String::new(), + description: String::new(), + reference: None, + reference_link: None, + query: String::new(), + arguments: BTreeMap::new(), + error_message: String::new(), + per_result_error_template: None, + } + } + + #[test] + fn test_overrides() { + let qo: QueryOverrides = [( + "query1".into(), + QueryOverride { + lint_level: Some(crate::LintLevel::Deny), + required_update: Some(crate::RequiredSemverUpdate::Major), + }, + )] + .into_iter() + .collect(); + + let mut list = QueryOverrideList::new(); + list.push(Arc::new(qo)); + + let query1 = make_blank_query( + "query1".into(), + LintLevel::Allow, + RequiredSemverUpdate::Minor, + ); + + let query2 = make_blank_query( + "query2".into(), + LintLevel::Allow, + RequiredSemverUpdate::Minor, + ); + + // Overrides should apply on query 1. + assert_eq!(list.effective_lint_level(&query1), LintLevel::Deny); + assert_eq!( + list.effective_required_update(&query1), + RequiredSemverUpdate::Major + ); + + // But query 2 does not have overrides, so it should return the defaults. + assert_eq!(list.effective_lint_level(&query2), LintLevel::Allow); + assert_eq!( + list.effective_required_update(&query2), + RequiredSemverUpdate::Minor + ); + } + + #[test] + fn test_override_precedence() { + let mut list = QueryOverrideList::new(); + + // Since this one is pushed first, it has lower precedence than the following set of overrides. + list.push(Arc::new( + [( + "query1".into(), + QueryOverride { + lint_level: Some(LintLevel::Warn), + required_update: Some(RequiredSemverUpdate::Major), + }, + )] + .into_iter() + .collect(), + )); + + list.push(Arc::new( + [( + "query1".into(), + QueryOverride { + lint_level: Some(LintLevel::Deny), + required_update: None, + }, + )] + .into_iter() + .collect(), + )); + + let query1 = make_blank_query( + "query1".into(), + LintLevel::Allow, + RequiredSemverUpdate::Minor, + ); + + // Since both overrides are set on its lint level, the later one should take precedence. + assert_eq!(list.effective_lint_level(&query1), LintLevel::Deny); + + // Since only the lower-precedence override is set for version, it should return that one, + // not the query's default. + assert_eq!( + list.effective_required_update(&query1), + RequiredSemverUpdate::Major + ); + } } macro_rules! add_lints {