diff --git a/crates/biome_analyze/src/lib.rs b/crates/biome_analyze/src/lib.rs index 04189553c7ff..9e57229da75b 100644 --- a/crates/biome_analyze/src/lib.rs +++ b/crates/biome_analyze/src/lib.rs @@ -786,6 +786,8 @@ pub struct SuppressionCommentEmitterPayload<'a, L: Language> { pub suppression_text: &'a str, /// The original range of the diagnostic where the rule was triggered pub diagnostic_text_range: &'a TextRange, + /// Explanation for the suppression to be used with `--suppress` and `--reason` + pub suppression_reason: &'a str, } type SignalHandler<'a, L, Break> = &'a mut dyn FnMut(&dyn AnalyzerSignal) -> ControlFlow; diff --git a/crates/biome_analyze/src/matcher.rs b/crates/biome_analyze/src/matcher.rs index a88858bcb53d..ac0f3e5694f7 100644 --- a/crates/biome_analyze/src/matcher.rs +++ b/crates/biome_analyze/src/matcher.rs @@ -380,6 +380,7 @@ mod tests { _: &mut BatchMutation, _: ApplySuppression, _: &str, + _: &str, ) { unreachable!("") } diff --git a/crates/biome_analyze/src/options.rs b/crates/biome_analyze/src/options.rs index 52c13a52978e..8d01cdf10767 100644 --- a/crates/biome_analyze/src/options.rs +++ b/crates/biome_analyze/src/options.rs @@ -77,6 +77,9 @@ pub struct AnalyzerOptions { /// The file that is being analyzed pub file_path: PathBuf, + + /// Suppression reason used when applying a suppression code action + pub suppression_reason: Option, } impl AnalyzerOptions { diff --git a/crates/biome_analyze/src/rule.rs b/crates/biome_analyze/src/rule.rs index 594c75327733..3e87b45128f3 100644 --- a/crates/biome_analyze/src/rule.rs +++ b/crates/biome_analyze/src/rule.rs @@ -881,6 +881,7 @@ pub trait Rule: RuleMeta + Sized { ctx: &RuleContext, text_range: &TextRange, suppression_action: &dyn SuppressionAction>, + suppression_reason: Option<&str>, ) -> Option>> where Self: 'static, @@ -901,6 +902,7 @@ pub trait Rule: RuleMeta + Sized { mutation: &mut mutation, token_offset: token, diagnostic_text_range: text_range, + suppression_reason: suppression_reason.unwrap_or(""), }); Some(SuppressAction { diff --git a/crates/biome_analyze/src/signals.rs b/crates/biome_analyze/src/signals.rs index d69e97b77f8b..93ab77a16469 100644 --- a/crates/biome_analyze/src/signals.rs +++ b/crates/biome_analyze/src/signals.rs @@ -326,7 +326,6 @@ where suppression_action: &'phase dyn SuppressionAction< Language = <::Query as Queryable>::Language, >, - options: &'phase AnalyzerOptions, ) -> Self { Self { @@ -402,9 +401,12 @@ where }); }; if let Some(text_range) = R::text_range(&ctx, &self.state) { - if let Some(suppression_action) = - R::suppress(&ctx, &text_range, self.suppression_action) - { + if let Some(suppression_action) = R::suppress( + &ctx, + &text_range, + self.suppression_action, + self.options.suppression_reason.as_deref(), + ) { let action = AnalyzerAction { rule_name: Some((::NAME, R::METADATA.name)), category: ActionCategory::Other(Cow::Borrowed(SUPPRESSION_ACTION_CATEGORY)), diff --git a/crates/biome_analyze/src/suppression_action.rs b/crates/biome_analyze/src/suppression_action.rs index 8ad8d022bd8f..77359a95f085 100644 --- a/crates/biome_analyze/src/suppression_action.rs +++ b/crates/biome_analyze/src/suppression_action.rs @@ -10,6 +10,7 @@ pub trait SuppressionAction { mutation, suppression_text, diagnostic_text_range, + suppression_reason, } = payload; // retrieve the most suited, most left token where the diagnostics was emitted @@ -22,7 +23,12 @@ pub trait SuppressionAction { }); if let Some(apply_suppression) = apply_suppression { - self.apply_suppression(mutation, apply_suppression, suppression_text); + self.apply_suppression( + mutation, + apply_suppression, + suppression_text, + suppression_reason, + ); } } @@ -72,6 +78,7 @@ pub trait SuppressionAction { mutation: &mut BatchMutation, apply_suppression: ApplySuppression, suppression_text: &str, + suppression_reason: &str, ); } diff --git a/crates/biome_analyze/src/syntax.rs b/crates/biome_analyze/src/syntax.rs index 87cb867f9e9b..9fbd2c8d736a 100644 --- a/crates/biome_analyze/src/syntax.rs +++ b/crates/biome_analyze/src/syntax.rs @@ -166,6 +166,7 @@ mod tests { _: &mut BatchMutation, _: ApplySuppression, _: &str, + _: &str, ) { unreachable!("") } diff --git a/crates/biome_cli/src/commands/check.rs b/crates/biome_cli/src/commands/check.rs index 74ba7472683f..7dce27be78d2 100644 --- a/crates/biome_cli/src/commands/check.rs +++ b/crates/biome_cli/src/commands/check.rs @@ -142,6 +142,7 @@ impl CommandRunner for CheckCommandPayload { apply_unsafe: self.apply_unsafe, write: self.write, suppress: false, + suppression_reason: None, fix: self.fix, unsafe_: self.unsafe_, }, diff --git a/crates/biome_cli/src/commands/lint.rs b/crates/biome_cli/src/commands/lint.rs index 32957c11e542..bacf55f92747 100644 --- a/crates/biome_cli/src/commands/lint.rs +++ b/crates/biome_cli/src/commands/lint.rs @@ -25,6 +25,7 @@ pub(crate) struct LintCommandPayload { pub(crate) fix: bool, pub(crate) unsafe_: bool, pub(crate) suppress: bool, + pub(crate) suppression_reason: Option, pub(crate) linter_configuration: Option, pub(crate) vcs_configuration: Option, pub(crate) files_configuration: Option, @@ -138,6 +139,7 @@ impl CommandRunner for LintCommandPayload { fix: self.fix, unsafe_: self.unsafe_, suppress: self.suppress, + suppression_reason: self.suppression_reason.clone(), }, console, )?; @@ -148,6 +150,7 @@ impl CommandRunner for LintCommandPayload { skip: self.skip.clone(), vcs_targeted: (self.staged, self.changed).into(), suppress: self.suppress, + suppression_reason: self.suppression_reason.clone(), }) .set_report(cli_options)) } diff --git a/crates/biome_cli/src/commands/migrate.rs b/crates/biome_cli/src/commands/migrate.rs index 85f3457141ea..ef60810e1299 100644 --- a/crates/biome_cli/src/commands/migrate.rs +++ b/crates/biome_cli/src/commands/migrate.rs @@ -85,6 +85,7 @@ impl CommandRunner for MigrateCommandPayload { fix: self.fix, unsafe_: false, suppress: false, + suppression_reason: None, }) } diff --git a/crates/biome_cli/src/commands/mod.rs b/crates/biome_cli/src/commands/mod.rs index bf9aa02dec16..1c79535d5600 100644 --- a/crates/biome_cli/src/commands/mod.rs +++ b/crates/biome_cli/src/commands/mod.rs @@ -187,10 +187,6 @@ pub enum BiomeCommand { #[bpaf(long("write"), switch)] write: bool, - /// Fix diagnostics with suppression comments if the language supports it. - #[bpaf(long("suppress"))] - suppress: bool, - /// Allow to do unsafe fixes, should be used with `--write` or `--fix` #[bpaf(long("unsafe"), switch)] unsafe_: bool, @@ -207,6 +203,14 @@ pub enum BiomeCommand { #[bpaf(long("apply-unsafe"), switch, hide_usage)] apply_unsafe: bool, + /// Fixes lint rule violations with a comment a suppression instead of using a rule code action (fix) + #[bpaf(long("suppress"))] + suppress: bool, + + /// Explanation for suppressing diagnostics with `--suppress` + #[bpaf(long("reason"), argument("STRING"))] + suppression_reason: Option, + #[bpaf(external(partial_linter_configuration), hide_usage, optional)] linter_configuration: Option, @@ -708,6 +712,7 @@ pub(crate) struct FixFileModeOptions { apply_unsafe: bool, write: bool, suppress: bool, + suppression_reason: Option, fix: bool, unsafe_: bool, } @@ -725,6 +730,7 @@ pub(crate) fn determine_fix_file_mode( write, fix, suppress, + suppression_reason: _, unsafe_, } = options; @@ -763,6 +769,7 @@ fn check_fix_incompatible_arguments(options: FixFileModeOptions) -> Result<(), C apply_unsafe, write, suppress, + suppression_reason, fix, unsafe_, } = options; @@ -795,7 +802,12 @@ fn check_fix_incompatible_arguments(options: FixFileModeOptions) -> Result<(), C )); } else if suppress && fix { return Err(CliDiagnostic::incompatible_arguments("--suppress", "--fix")); - } + } else if !suppress && suppression_reason.is_some() { + return Err(CliDiagnostic::unexpected_argument( + "--reason", + "`--reason` is only valid when `--suppress` is used.", + )); + }; Ok(()) } @@ -983,20 +995,21 @@ mod tests { #[test] fn incompatible_arguments() { - for (apply, apply_unsafe, write, suppress, fix, unsafe_) in [ - (true, true, false, false, false, false), // --apply --apply-unsafe - (true, false, true, false, false, false), // --apply --write - (true, false, false, false, true, false), // --apply --fix - (false, true, false, false, false, true), // --apply-unsafe --unsafe - (false, true, true, false, false, false), // --apply-unsafe --write - (false, true, false, false, true, false), // --apply-unsafe --fix - (false, false, true, false, true, false), // --write --fix + for (apply, apply_unsafe, write, suppress, suppression_reason, fix, unsafe_) in [ + (true, true, false, false, None, false, false), // --apply --apply-unsafe + (true, false, true, false, None, false, false), // --apply --write + (true, false, false, false, None, true, false), // --apply --fix + (false, true, false, false, None, false, true), // --apply-unsafe --unsafe + (false, true, true, false, None, false, false), // --apply-unsafe --write + (false, true, false, false, None, true, false), // --apply-unsafe --fix + (false, false, true, false, None, true, false), // --write --fix ] { assert!(check_fix_incompatible_arguments(FixFileModeOptions { apply, apply_unsafe, write, suppress, + suppression_reason, fix, unsafe_ }) @@ -1008,10 +1021,10 @@ mod tests { fn safe_fixes() { let mut console = BufferConsole::default(); - for (apply, apply_unsafe, write, suppress, fix, unsafe_) in [ - (true, false, false, false, false, false), // --apply - (false, false, true, false, false, false), // --write - (false, false, false, false, true, false), // --fix + for (apply, apply_unsafe, write, suppress, suppression_reason, fix, unsafe_) in [ + (true, false, false, false, None, false, false), // --apply + (false, false, true, false, None, false, false), // --write + (false, false, false, false, None, true, false), // --fix ] { assert_eq!( determine_fix_file_mode( @@ -1020,6 +1033,7 @@ mod tests { apply_unsafe, write, suppress, + suppression_reason, fix, unsafe_ }, @@ -1035,10 +1049,10 @@ mod tests { fn safe_and_unsafe_fixes() { let mut console = BufferConsole::default(); - for (apply, apply_unsafe, write, suppress, fix, unsafe_) in [ - (false, true, false, false, false, false), // --apply-unsafe - (false, false, true, false, false, true), // --write --unsafe - (false, false, false, false, true, true), // --fix --unsafe + for (apply, apply_unsafe, write, suppress, suppression_reason, fix, unsafe_) in [ + (false, true, false, false, None, false, false), // --apply-unsafe + (false, false, true, false, None, false, true), // --write --unsafe + (false, false, false, false, None, true, true), // --fix --unsafe ] { assert_eq!( determine_fix_file_mode( @@ -1047,6 +1061,7 @@ mod tests { apply_unsafe, write, suppress, + suppression_reason, fix, unsafe_ }, @@ -1062,8 +1077,8 @@ mod tests { fn no_fix() { let mut console = BufferConsole::default(); - let (apply, apply_unsafe, write, suppress, fix, unsafe_) = - (false, false, false, false, false, false); + let (apply, apply_unsafe, write, suppress, suppression_reason, fix, unsafe_) = + (false, false, false, false, None, false, false); assert_eq!( determine_fix_file_mode( FixFileModeOptions { @@ -1071,6 +1086,7 @@ mod tests { apply_unsafe, write, suppress, + suppression_reason, fix, unsafe_ }, diff --git a/crates/biome_cli/src/execute/mod.rs b/crates/biome_cli/src/execute/mod.rs index 1762c920b1cb..f77c70671796 100644 --- a/crates/biome_cli/src/execute/mod.rs +++ b/crates/biome_cli/src/execute/mod.rs @@ -159,6 +159,8 @@ pub enum TraversalMode { vcs_targeted: VcsTargeted, /// Supress existing diagnostics with a `// biome-ignore` comment suppress: bool, + /// Explanation for suppressing diagnostics with `--suppress` and `--reason` + suppression_reason: Option, }, /// This mode is enabled when running the command `biome ci` CI { diff --git a/crates/biome_cli/src/execute/process_file.rs b/crates/biome_cli/src/execute/process_file.rs index 79ea7dc3d871..c7bae642e1dd 100644 --- a/crates/biome_cli/src/execute/process_file.rs +++ b/crates/biome_cli/src/execute/process_file.rs @@ -209,9 +209,18 @@ pub(crate) fn process_file(ctx: &TraversalOptions, biome_path: &BiomePath) -> Fi let shared_context = &SharedTraversalOptions::new(ctx); match ctx.execution.traversal_mode { - TraversalMode::Lint { .. } => { + TraversalMode::Lint { + ref suppression_reason, + suppress, + .. + } => { // the unsupported case should be handled already at this point - lint(shared_context, biome_path) + lint( + shared_context, + biome_path, + suppress, + suppression_reason.as_deref(), + ) } TraversalMode::Format { .. } => { // the unsupported case should be handled already at this point diff --git a/crates/biome_cli/src/execute/process_file/assists.rs b/crates/biome_cli/src/execute/process_file/assists.rs index 6afe2c6cf1fb..43563ac702c9 100644 --- a/crates/biome_cli/src/execute/process_file/assists.rs +++ b/crates/biome_cli/src/execute/process_file/assists.rs @@ -28,6 +28,7 @@ pub(crate) fn assists_with_guard<'ctx>( RuleCategoriesBuilder::default().with_action().build(), only.clone(), skip.clone(), + None, ) .with_file_path_and_code( workspace_file.path.display().to_string(), diff --git a/crates/biome_cli/src/execute/process_file/check.rs b/crates/biome_cli/src/execute/process_file/check.rs index 0ecea599f71c..2981c79ee05a 100644 --- a/crates/biome_cli/src/execute/process_file/check.rs +++ b/crates/biome_cli/src/execute/process_file/check.rs @@ -18,7 +18,7 @@ pub(crate) fn check_file<'ctx>( tracing::info_span!("Process check", path =? workspace_file.path.display()).in_scope( move || { if file_features.supports_lint() { - let lint_result = lint_with_guard(ctx, &mut workspace_file); + let lint_result = lint_with_guard(ctx, &mut workspace_file, false, None); match lint_result { Ok(status) => { if status.is_changed() { diff --git a/crates/biome_cli/src/execute/process_file/lint.rs b/crates/biome_cli/src/execute/process_file/lint.rs index b5e4a7db823c..05ac42d9b5e9 100644 --- a/crates/biome_cli/src/execute/process_file/lint.rs +++ b/crates/biome_cli/src/execute/process_file/lint.rs @@ -11,14 +11,21 @@ use std::path::Path; use std::sync::atomic::Ordering; /// Lints a single file and returns a [FileResult] -pub(crate) fn lint<'ctx>(ctx: &'ctx SharedTraversalOptions<'ctx, '_>, path: &Path) -> FileResult { +pub(crate) fn lint<'ctx>( + ctx: &'ctx SharedTraversalOptions<'ctx, '_>, + path: &Path, + suppress: bool, + suppression_reason: Option<&str>, +) -> FileResult { let mut workspace_file = WorkspaceFile::new(ctx, path)?; - lint_with_guard(ctx, &mut workspace_file) + lint_with_guard(ctx, &mut workspace_file, suppress, suppression_reason) } pub(crate) fn lint_with_guard<'ctx>( ctx: &'ctx SharedTraversalOptions<'ctx, '_>, workspace_file: &mut WorkspaceFile, + suppress: bool, + suppression_reason: Option<&str>, ) -> FileResult { tracing::info_span!("Processes linting", path =? workspace_file.path.display()).in_scope( move || { @@ -31,6 +38,12 @@ pub(crate) fn lint_with_guard<'ctx>( (Vec::new(), Vec::new()) }; if let Some(fix_mode) = ctx.execution.as_fix_file_mode() { + let suppression_explanation = if suppress && suppression_reason.is_none() { + "ignored using `--suppress`" + } else { + suppression_reason.unwrap_or("") + }; + let fix_result = workspace_file .guard() .fix_file( @@ -42,6 +55,7 @@ pub(crate) fn lint_with_guard<'ctx>( .build(), only.clone(), skip.clone(), + Some(suppression_explanation.to_string()), ) .with_file_path_and_code( workspace_file.path.display().to_string(), diff --git a/crates/biome_cli/src/execute/std_in.rs b/crates/biome_cli/src/execute/std_in.rs index c516a6fa586d..81f0abb04546 100644 --- a/crates/biome_cli/src/execute/std_in.rs +++ b/crates/biome_cli/src/execute/std_in.rs @@ -121,6 +121,7 @@ pub(crate) fn run<'a>( should_format: mode.is_check() && file_features.supports_format(), only: only.clone(), skip: skip.clone(), + suppression_reason: None, rule_categories: RuleCategoriesBuilder::default() .with_syntax() .with_lint() diff --git a/crates/biome_cli/src/lib.rs b/crates/biome_cli/src/lib.rs index fab1c79db545..139936b68569 100644 --- a/crates/biome_cli/src/lib.rs +++ b/crates/biome_cli/src/lib.rs @@ -131,6 +131,7 @@ impl<'app> CliSession<'app> { apply_unsafe, write, suppress, + suppression_reason, fix, unsafe_, cli_options, @@ -156,6 +157,7 @@ impl<'app> CliSession<'app> { apply, write, suppress, + suppression_reason, fix, unsafe_, linter_configuration, diff --git a/crates/biome_cli/tests/cases/suppressions.rs b/crates/biome_cli/tests/cases/suppressions.rs index 1c861b0468b0..3167dda5a22d 100644 --- a/crates/biome_cli/tests/cases/suppressions.rs +++ b/crates/biome_cli/tests/cases/suppressions.rs @@ -9,7 +9,10 @@ use biome_service::DynRef; const SUPPRESS_BEFORE: &str = "(1 >= -0)"; const SUPPRESS_AFTER: &str = - "// biome-ignore lint/suspicious/noCompareNegZero: \n(1 >= -0)"; + "// biome-ignore lint/suspicious/noCompareNegZero: ignored using `--suppress`\n(1 >= -0)"; + +const SUPPRESS_WITH_REASON: &str = + "// biome-ignore lint/suspicious/noCompareNegZero: We love Biome\n(1 >= -0)"; #[test] fn ok() { @@ -231,3 +234,84 @@ fn suppress_skip_ok() { result, )); } + +#[test] +fn err_when_only_reason() { + let mut fs = MemoryFileSystem::default(); + let mut console = BufferConsole::default(); + + let file_path = Path::new("fix.js"); + fs.insert(file_path.into(), SUPPRESS_BEFORE.as_bytes()); + + let result = run_cli( + DynRef::Borrowed(&mut fs), + &mut console, + Args::from( + [ + ("lint"), + ("--reason"), + file_path.as_os_str().to_str().unwrap(), + ] + .as_slice(), + ), + ); + + assert!(result.is_err(), "run_cli returned {result:?}"); + + let mut buffer = String::new(); + fs.open(file_path) + .unwrap() + .read_to_string(&mut buffer) + .unwrap(); + + assert_eq!(buffer, SUPPRESS_BEFORE); + + assert_cli_snapshot(SnapshotPayload::new( + module_path!(), + "err_when_only_reason", + fs, + console, + result, + )); +} + +#[test] +fn custom_explanation_with_reason() { + let mut fs = MemoryFileSystem::default(); + let mut console = BufferConsole::default(); + + let file_path = Path::new("fix.js"); + fs.insert(file_path.into(), SUPPRESS_BEFORE.as_bytes()); + + let result = run_cli( + DynRef::Borrowed(&mut fs), + &mut console, + Args::from( + [ + ("lint"), + ("--suppress"), + ("--reason=We love Biome"), + file_path.as_os_str().to_str().unwrap(), + ] + .as_slice(), + ), + ); + + assert!(result.is_ok(), "run_cli returned {result:?}"); + + let mut buffer = String::new(); + fs.open(file_path) + .unwrap() + .read_to_string(&mut buffer) + .unwrap(); + + assert_eq!(buffer, SUPPRESS_WITH_REASON); + + assert_cli_snapshot(SnapshotPayload::new( + module_path!(), + "custom_explanation_with_reason", + fs, + console, + result, + )); +} diff --git a/crates/biome_cli/tests/snapshots/main_cases_suppressions/custom_explanation_with_reason.snap b/crates/biome_cli/tests/snapshots/main_cases_suppressions/custom_explanation_with_reason.snap new file mode 100644 index 000000000000..0480df2ce038 --- /dev/null +++ b/crates/biome_cli/tests/snapshots/main_cases_suppressions/custom_explanation_with_reason.snap @@ -0,0 +1,17 @@ +--- +source: crates/biome_cli/tests/snap_test.rs +assertion_line: 423 +expression: content +--- +## `fix.js` + +```js +// biome-ignore lint/suspicious/noCompareNegZero: We love Biome +(1 >= -0) +``` + +# Emitted Messages + +```block +Checked 1 file in