feat(rome_analyze): add a warning for unused suppression comments
MODIFY
crates/rome_analyze/src/lib.rs
crates/rome_analyze/src/lib.rs
@@ -162,6 +162,27 @@ where
}
}
+ for suppression in line_suppressions {
+ if suppression.did_suppress_signal {
+ continue;
+ }
+
+ let signal = DiagnosticSignal::new(|| {
+ let diag = SuppressionDiagnostic::new(
+ ctx.file_id,
+ category!("suppressions/unused"),
+ suppression.comment_span,
+ "Suppression comment is not being used",
+ );
+
+ AnalyzerDiagnostic::from_error(diag.into())
+ });
+
+ if let ControlFlow::Break(br) = (emit_signal)(&signal) {
+ return Some(br);
+ }
+ }
+
None
}
}
@@ -203,6 +224,8 @@ struct PhaseRunner<'analyzer, 'phase, L: Language, Matcher, Break> {
struct LineSuppression {
/// Line index this comment is suppressing lint rules for
line_index: usize,
+ /// Range of source text covered by the suppression comment
+ comment_span: TextRange,
/// Range of source text this comment is suppressing lint rules for
text_range: TextRange,
/// Set to true if this comment has set the `suppress_all` flag to true
@@ -211,6 +234,9 @@ struct LineSuppression {
/// List of all the rules this comment has started suppressing (must be
/// removed from the suppressed set on expiration)
suppressed_rules: Vec<RuleFilter<'static>>,
+ /// Set to `true` when a signal matching this suppression was emitted and
+ /// suppressed
+ did_suppress_signal: bool,
}
impl<'a, 'phase, L, Matcher, Break> PhaseRunner<'a, 'phase, L, Matcher, Break>
@@ -344,14 +370,14 @@ where
// if it matchs the current line index, otherwise perform a binary
// search over all the previously seen suppressions to find one
// with a matching range
- let suppression = self
- .line_suppressions
- .last()
- .filter(|suppression| {
+ let suppression = self.line_suppressions.last_mut().filter(|suppression| {
suppression.line_index == *self.line_index
&& suppression.text_range.start() <= start
- })
- .or_else(|| {
+ });
+
+ let suppression = match suppression {
+ Some(suppression) => Some(suppression),
+ None => {
let index = self.line_suppressions.binary_search_by(|suppression| {
if suppression.text_range.end() < entry.text_range.start() {
Ordering::Less
@@ -362,21 +388,26 @@ where
}
});
- Some(&self.line_suppressions[index.ok()?])
- });
+ index.ok().map(|index| &mut self.line_suppressions[index])
+ }
+ };
- let is_suppressed = suppression.map_or(false, |suppression| {
+ let suppression = suppression.filter(|suppression| {
if suppression.suppress_all {
return true;
}
+
suppression
.suppressed_rules
.iter()
.any(|filter| *filter == entry.rule)
});
- // Emit the signal if the rule that created it is not currently being suppressed
- if !is_suppressed {
+ // If the signal is being suppressed mark the line suppression as
+ // hit, otherwise emit the signal
+ if let Some(suppression) = suppression {
+ suppression.did_suppress_signal = true;
+ } else {
(self.emit_signal)(&*entry.signal)?;
}
@@ -479,9 +510,11 @@ where
let entry = LineSuppression {
line_index,
+ comment_span: range,
text_range: range,
suppress_all,
suppressed_rules: suppressions,
+ did_suppress_signal: false,
};
self.line_suppressions.push(entry);

MODIFY
crates/rome_analyze/src/matcher.rs
crates/rome_analyze/src/matcher.rs
@@ -211,7 +211,7 @@ mod tests {
let mut builder = RawSyntaxTreeBuilder::new();
builder.start_node(RawLanguageKind::ROOT);
- builder.start_node(RawLanguageKind::EXPRESSION_LIST);
+ builder.start_node(RawLanguageKind::SEPARATED_EXPRESSION_LIST);
builder.start_node(RawLanguageKind::LITERAL_EXPRESSION);
builder.token_with_trivia(
@@ -289,6 +289,16 @@ mod tests {
&[TriviaPiece::new(TriviaPieceKind::Newline, 1)],
);
+ builder.token_with_trivia(
+ RawLanguageKind::SEMICOLON_TOKEN,
+ "//group/rule\n;\n",
+ &[
+ TriviaPiece::new(TriviaPieceKind::SingleLineComment, 12),
+ TriviaPiece::new(TriviaPieceKind::Newline, 1),
+ ],
+ &[TriviaPiece::new(TriviaPieceKind::Newline, 1)],
+ );
+
builder.finish_node();
builder.finish_node();
@@ -356,6 +366,10 @@ mod tests {
category!("args/fileNotFound"),
TextRange::new(TextSize::from(97), TextSize::from(108))
),
+ (
+ category!("suppressions/unused"),
+ TextRange::new(TextSize::from(110), TextSize::from(122))
+ ),
]
);
}

MODIFY
crates/rome_diagnostics_categories/src/categories.rs
crates/rome_diagnostics_categories/src/categories.rs
@@ -100,6 +100,7 @@ define_dategories! {
"suppressions/unknownGroup",
"suppressions/unknownRule",
+ "suppressions/unused",
// Used in tests and examples
"args/fileNotFound",
"flags/invalid",

MODIFY
crates/rome_js_analyze/src/lib.rs
crates/rome_js_analyze/src/lib.rs
@@ -252,11 +252,12 @@ mod tests {
let options = AnalyzerOptions::default();
analyze(FileId::zero(), &parsed.tree(), filter, &options, |signal| {
- if let Some(mut diag) = signal.diagnostic() {
- diag.set_severity(Severity::Warning);
+ if let Some(diag) = signal.diagnostic() {
let code = diag.category().unwrap();
+ if code != category!("suppressions/unused") {
panic!("unexpected diagnostic {code:?}");
}
+ }
ControlFlow::<Never>::Continue(())
});

MODIFY
crates/rome_service/src/file_handlers/javascript.rs
crates/rome_service/src/file_handlers/javascript.rs
@@ -10,7 +10,7 @@ use rome_analyze::{
AnalysisFilter, AnalyzerOptions, ControlFlow, GroupCategory, Never, QueryMatch,
RegistryVisitor, RuleCategories, RuleCategory, RuleFilter, RuleGroup,
};
-use rome_diagnostics::{Applicability, CodeSuggestion};
+use rome_diagnostics::{v2::category, Applicability, CodeSuggestion};
use rome_formatter::{FormatError, Printed};
use rome_fs::RomePath;
use rome_js_analyze::{analyze, analyze_with_inspect_matcher, visit_registry, RuleError};
@@ -221,8 +221,15 @@ fn lint(params: LintParams) -> LintResults {
.iter()
.any(|diag| diag.severity() <= v2::Severity::Error);
+ let has_lint = params.filter.categories.contains(RuleCategories::LINT);
+
analyze(file_id, &tree, params.filter, &analyzer_options, |signal| {
if let Some(mut diagnostic) = signal.diagnostic() {
+ // Do not report unused suppression comment diagnostics if this is a syntax-only analyzer pass
+ if !has_lint && diagnostic.category() == Some(category!("suppressions/unused")) {
+ return ControlFlow::<Never>::Continue(());
+ }
+
diagnostic_count += 1;
// We do now check if the severity of the diagnostics should be changed.

MODIFY
npm/backend-jsonrpc/src/workspace.ts
npm/backend-jsonrpc/src/workspace.ts
@@ -405,6 +405,7 @@ export type Category =
| "parse/noSuperWithoutExtends"
| "suppressions/unknownGroup"
| "suppressions/unknownRule"
+ | "suppressions/unused"
| "args/fileNotFound"
| "flags/invalid"
| "semanticTests";

feat(rome_analyze): add a warning for unused suppression comments (#3718)