ok

Author: juleswritescode

crates/pgt_suppressions/src/lib.rs

@@ -1,10 +1,11 @@
 use std::{
+    collections::HashMap,
     iter::{Enumerate, Peekable},
     str::Lines,
 };
 
 use pgt_analyse::{RuleCategory, RuleFilter};
-use pgt_diagnostics::{Diagnostic, Location, MessageAndDescription};
+use pgt_diagnostics::{Diagnostic, MessageAndDescription};
 use pgt_text_size::{TextRange, TextSize};
 
 pub mod line_index;
@@ -68,8 +69,6 @@ impl RuleSpecifier {
         let group = self.group();
         let rule = self.rule();
 
-        tracing::error!("group and rule: {:#?} {:#?}", group, rule);
-
         disabled_rules.iter().any(|r| match r {
             RuleFilter::Group(gr) => group.is_some_and(|specifier_group| specifier_group == *gr),
             RuleFilter::Rule(gr, ru) => group.is_some_and(|specifier_group| {
@@ -235,9 +234,8 @@ pub struct Suppressions {
 }
 
 impl Suppressions {
+    #[must_use]
     pub fn considering_disabled_rules(mut self, disabled_rules: &[RuleFilter<'_>]) -> Self {
-        tracing::error!("disabled rules: {:#?}", disabled_rules);
-
         {
             let (enabled, disabled) = self
                 .file_suppressions
@@ -282,16 +280,64 @@ impl Suppressions {
         self
     }
 
-    pub fn is_suppressed<D: Diagnostic>(&self, diagnostic: &D) -> bool {
-        let location = diagnostic.location();
+    #[must_use]
+    pub fn with_unused_suppressions_as_errors<D: Diagnostic>(mut self, diagnostics: &[D]) -> Self {
+        let mut diagnostics_by_line: HashMap<usize, Vec<&D>> = HashMap::new();
+        for diag in diagnostics {
+            if let Some(line) = diag
+                .location()
+                .span
+                .and_then(|sp| self.line_index.line_for_offset(sp.start()))
+            {
+                let entry = diagnostics_by_line.entry(line);
+                entry
+                    .and_modify(|current| {
+                        current.push(diag);
+                    })
+                    .or_insert(vec![diag]);
+            }
+        }
 
+        for (line, suppr) in &self.line_suppressions {
+            let mut expected_diagnostic_line = line + 1;
+            while let Some(_) = self.line_suppressions.get(&expected_diagnostic_line) {
+                expected_diagnostic_line += 1;
+            }
+
+            if diagnostics_by_line
+                .get(&expected_diagnostic_line)
+                .is_some_and(|diags| {
+                    diags.iter().any(|d| {
+                        d.category()
+                            .is_some_and(|cat| match RuleSpecifier::try_from(cat.name()) {
+                                Ok(spec) => suppr.matches(&spec),
+                                Err(_) => false,
+                            })
+                    })
+                })
+            {
+                continue;
+            } else {
+                self.diagnostics.push(SuppressionDiagnostic {
+                    span: suppr.suppression_range,
+                    message: MessageAndDescription::from(format!(
+                        "This suppression has no effect."
+                    )),
+                })
+            }
+        }
+
+        self
+    }
+
+    pub fn is_suppressed<D: Diagnostic>(&self, diagnostic: &D) -> bool {
         diagnostic
             .category()
             .map(|c| match RuleSpecifier::try_from(c.name()) {
                 Ok(specifier) => {
                     self.by_file_suppression(&specifier)
-                        || self.by_range_suppression(location, &specifier)
-                        || self.by_line_suppression(location, &specifier)
+                        || self.by_range_suppression(diagnostic, &specifier)
+                        || self.by_line_suppression(diagnostic, &specifier)
                 }
                 Err(_) => false,
             })
@@ -302,33 +348,53 @@ impl Suppressions {
         self.file_suppressions.iter().any(|s| s.matches(specifier))
     }
 
-    fn by_line_suppression(&self, location: Location<'_>, specifier: &RuleSpecifier) -> bool {
-        location
+    fn by_line_suppression<D: Diagnostic>(
+        &self,
+        diagnostic: &D,
+        specifier: &RuleSpecifier,
+    ) -> bool {
+        self.get_eligible_line_suppressions_for_diagnostic(diagnostic)
+            .iter()
+            .any(|s| s.matches(specifier))
+    }
+
+    fn by_range_suppression<D: Diagnostic>(
+        &self,
+        diagnostic: &D,
+        specifier: &RuleSpecifier,
+    ) -> bool {
+        self.range_suppressions.iter().any(|range_suppr| {
+            range_suppr.start_suppression.matches(specifier)
+                && diagnostic
+                    .location()
+                    .span
+                    .is_some_and(|sp| range_suppr.suppressed_range.contains_range(sp))
+        })
+    }
+
+    fn get_eligible_line_suppressions_for_diagnostic<D: Diagnostic>(
+        &self,
+        diagnostic: &D,
+    ) -> Vec<&Suppression> {
+        diagnostic
+            .location()
             .span
             .and_then(|span| self.line_index.line_for_offset(span.start()))
             .filter(|line_no| *line_no > 0)
-            .is_some_and(|mut line_no| {
-                let mut eligible_suppressions = vec![];
+            .map(|mut line_no| {
+                let mut eligible = vec![];
 
                 // one-for-one, we're checking the lines above a diagnostic location
                 // until there are no more diagnostics
                 line_no -= 1;
                 while let Some(suppr) = self.line_suppressions.get(&line_no) {
-                    eligible_suppressions.push(suppr);
+                    eligible.push(suppr);
                     line_no -= 1;
                 }
 
-                eligible_suppressions.iter().any(|s| s.matches(specifier))
+                eligible
             })
-    }
-
-    fn by_range_suppression(&self, location: Location<'_>, specifier: &RuleSpecifier) -> bool {
-        self.range_suppressions.iter().any(|range_suppr| {
-            range_suppr.start_suppression.matches(specifier)
-                && location
-                    .span
-                    .is_some_and(|sp| range_suppr.suppressed_range.contains_range(sp))
-        })
+            .unwrap_or(vec![])
     }
 }
 

crates/pgt_workspace/src/workspace/server.rs

@@ -573,13 +573,13 @@ impl Workspace for WorkspaceServer {
         let suppressions = parser
             .document_suppressions()
             .clone()
-            .considering_disabled_rules(&disabled_rules);
+            .considering_disabled_rules(&disabled_rules)
+            .with_unused_suppressions_as_errors(&diagnostics);
 
         diagnostics.retain(|d| !suppressions.is_suppressed(d));
 
         let suppression_errors: Vec<Error> = suppressions
             .diagnostics
-            .clone()
             .into_iter()
             .map(Error::from)
             .collect::<Vec<pgt_diagnostics::Error>>();

crates/pgt_workspace/src/workspace/server/analyser.rs

@@ -70,8 +70,6 @@ impl<'a, 'b> LintVisitor<'a, 'b> {
         let has_only_filter = !self.only.is_empty();
 
         if !has_only_filter {
-            println!("does not have only filter!");
-
             let enabled_rules = self
                 .settings
                 .as_linter_rules()