Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Always run lint mode #1169

Closed
wants to merge 4 commits into from
Closed

feat: Always run lint mode #1169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
95203f1
add `lint_mode` to `SemverQuery`
Frank-III Mar 3, 2025
9df8812
small fix
Frank-III Mar 3, 2025
2710e3c
fix check display
Frank-III Mar 4, 2025
b9cd4c9
minor fix
Frank-III Mar 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 107 additions & 19 deletions src/check_release.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -309,34 +318,60 @@ 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",
format_args!(
"[{:>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),
Expand Down Expand Up @@ -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();

Expand All @@ -399,16 +463,14 @@ 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",
"no semver update required",
Color::Ansi(AnsiColor::Green),
true,
)?;
} else {
unreachable!("Expected either warnings or errors to be produced.");
}

if let Some(suggested_bump) = suggested_bump {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -469,6 +556,7 @@ pub(super) fn run_check_release(
Ok(CrateReport {
detected_bump: version_change,
required_bump: None,
has_always_run_issues: false,
})
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![forbid(unsafe_code)]

Check warning on line 1 in src/lib.rs

View workflow job for this annotation

GitHub Actions / Ensure test crates pass "cargo check" 

the feature `unsafe_extern_blocks` has been stable since 1.82.0 and no longer requires an attribute to enable

Check warning on line 1 in src/lib.rs

View workflow job for this annotation

GitHub Actions / Ensure test crates pass "cargo check" 

the feature `unsafe_extern_blocks` has been stable since 1.82.0 and no longer requires an attribute to enable

mod callbacks;
mod check_release;
Expand Down Expand Up @@ -653,12 +653,17 @@
/// 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<ReleaseType>,
/// 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,
Expand Down
40 changes: 34 additions & 6 deletions src/manifest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -116,24 +116,28 @@ 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 } => (
priority,
QueryOverride {
lint_level: Some(level),
required_update: None,
lint_mode: None,
},
),
OverrideConfig::RequiredUpdate {
Expand All @@ -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),
},
),
};
Expand All @@ -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<LintMode>,
/// 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.
Expand All @@ -179,21 +193,29 @@ 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,
#[serde(default)]
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"`
Expand Down Expand Up @@ -301,13 +323,15 @@ mod tests {
QueryOverride {
lint_level: Some(Deny),
required_update: None,
lint_mode: None,
}
),]),
OverrideMap::from_iter([(
"six".into(),
QueryOverride {
lint_level: Some(Allow),
required_update: None,
lint_mode: None,
}
),]),
]
Expand All @@ -321,21 +345,24 @@ mod tests {
QueryOverride {
lint_level: Some(Warn),
required_update: None,
lint_mode: None,
}
)]),
OverrideMap::from_iter([
(
"two".into(),
QueryOverride {
lint_level: Some(Deny),
required_update: None
required_update: None,
lint_mode: None,
}
),
(
"four".into(),
QueryOverride {
lint_level: None,
required_update: Some(Major),
lint_mode: None,
}
),
]),
Expand All @@ -344,6 +371,7 @@ mod tests {
QueryOverride {
lint_level: Some(Allow),
required_update: Some(Minor),
lint_mode: None,
}
)])
]
Expand Down
Loading
Loading