add tests

juleswritescode · juleswritescode · commit eab166f26749 · 2025-07-04T16:23:43.000+02:00
diff --git a/crates/pgt_suppressions/src/lib.rs b/crates/pgt_suppressions/src/lib.rs
@@ -205,3 +205,138 @@ impl Suppressions {
             .unwrap_or_default()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use pgt_diagnostics::{Diagnostic, MessageAndDescription};
+    use pgt_text_size::TextRange;
+
+    use crate::suppression::SuppressionDiagnostic;
+
+    #[derive(Clone, Debug, Diagnostic)]
+    #[diagnostic(category = "lint", severity = Error)]
+    pub struct TestDiagnostic {
+        #[location(span)]
+        pub span: TextRange,
+    }
+
+    #[test]
+    fn correctly_suppresses_diagnostics_at_top_level() {
+        let doc = r#"
+        -- pgt-ignore-all lint
+
+        select 1;
+        "#;
+
+        let len_doc: u32 = doc.len().try_into().unwrap();
+
+        let suppressions = super::Suppressions::from(doc);
+
+        assert!(suppressions.is_suppressed(&TestDiagnostic {
+            span: TextRange::new((len_doc - 10).into(), len_doc.into()),
+        }));
+    }
+
+    #[test]
+    fn correctly_suppresses_diagnostics_at_line() {
+        let doc = r#"
+            select 2;
+
+            -- pgt-ignore lint
+            select 1;
+            "#;
+
+        let suppressions = super::Suppressions::from(doc);
+
+        assert!(suppressions.is_suppressed(&TestDiagnostic {
+            span: TextRange::new(89.into(), 98.into()),
+        }));
+    }
+
+    #[test]
+    fn correctly_suppresses_with_multiple_line_diagnostics() {
+        let doc = r#"
+            select 2;
+
+            -- pgt-ignore lint
+            -- pgt-ignore action
+            select 1;
+            "#;
+
+        let suppressions = super::Suppressions::from(doc);
+
+        assert!(suppressions.is_suppressed(&TestDiagnostic {
+            span: TextRange::new(100.into(), 109.into()),
+        }));
+    }
+
+    #[test]
+    fn correctly_suppresses_diagnostics_with_ranges() {
+        let doc = r#"
+            select 2;
+
+            -- pgt-ignore-start lint
+            select 1;
+            -- pgt-ignore-end lint
+            "#;
+
+        let suppressions = super::Suppressions::from(doc);
+
+        assert!(suppressions.is_suppressed(&TestDiagnostic {
+            span: TextRange::new(73.into(), 82.into()),
+        }));
+    }
+
+    #[test]
+    fn marks_disabled_rule_suppressions_as_errors() {
+        let doc = r#"
+            select 2;
+
+            -- pgt-ignore lint/safety/banDropTable
+            select 1;
+            "#;
+
+        let suppressions = super::Suppressions::from(doc)
+            .with_disabled_rules(&[pgt_analyse::RuleFilter::Group("safety")]);
+
+        assert!(!suppressions.is_suppressed(&TestDiagnostic {
+            span: TextRange::new(89.into(), 98.into()),
+        }));
+
+        assert_eq!(suppressions.diagnostics.len(), 1);
+
+        assert_eq!(
+            suppressions.diagnostics[0],
+            SuppressionDiagnostic {
+                span: TextRange::new(36.into(), 74.into()),
+                message: MessageAndDescription::from("This rule has been disabled via the configuration. The suppression has no effect.".to_string())
+            }
+        );
+    }
+
+    #[test]
+    fn marks_unused_suppressions_as_errors() {
+        let doc = r#"
+            select 2;
+
+            -- pgt-ignore lint
+            select 1;
+            "#;
+
+        // no diagnostics
+        let diagnostics: Vec<TestDiagnostic> = vec![];
+
+        let suppressions =
+            super::Suppressions::from(doc).with_unused_suppressions_as_errors(&diagnostics);
+
+        assert_eq!(suppressions.diagnostics.len(), 1);
+
+        assert_eq!(
+            suppressions.diagnostics[0],
+            SuppressionDiagnostic {
+                span: TextRange::new(36.into(), 54.into()),
+                message: MessageAndDescription::from("This suppression has no effect.".to_string())
+            }
+        );
+    }
+}
diff --git a/crates/pgt_suppressions/src/line_index.rs b/crates/pgt_suppressions/src/line_index.rs
@@ -31,7 +31,13 @@ impl LineIndex {
         self.line_offset
             .iter()
             .enumerate()
-            .find(|(_, line_offset)| **line_offset >= offset)
-            .map(|(i, _)| i)
+            .filter_map(|(i, line_offset)| {
+                if offset >= *line_offset {
+                    Some(i)
+                } else {
+                    None
+                }
+            })
+            .last()
     }
 }
diff --git a/crates/pgt_suppressions/src/parser.rs b/crates/pgt_suppressions/src/parser.rs
@@ -51,7 +51,7 @@ impl<'a> SuppressionsParser<'a> {
             line_suppressions: parser.line_suppressions,
             range_suppressions: parser.range_suppressions,
             diagnostics: parser.diagnostics,
-            line_index: LineIndex::new(doc),
+            line_index: parser.line_index,
         }
     }
 
@@ -60,6 +60,11 @@ impl<'a> SuppressionsParser<'a> {
     /// suppression, this will stop.
     fn parse_file_suppressions(&mut self) {
         while let Some((_, preview)) = self.lines.peek() {
+            if preview.trim().is_empty() {
+                self.lines.next();
+                continue;
+            }
+
             if !preview.trim().starts_with("-- pgt-ignore-all") {
                 return;
             }
@@ -109,9 +114,16 @@ impl<'a> SuppressionsParser<'a> {
                 SuppressionKind::End => {
                     let matching_start_idx = self
                         .start_suppressions_stack
-                        .iter_mut()
-                        .rev()
-                        .position(|start| start.rule_specifier == suppr.rule_specifier);
+                        .iter()
+                        .enumerate()
+                        .filter_map(|(idx, s)| {
+                            if s.rule_specifier == suppr.rule_specifier {
+                                Some(idx)
+                            } else {
+                                None
+                            }
+                        })
+                        .last();
 
                     if let Some(start_idx) = matching_start_idx {
                         let start = self.start_suppressions_stack.remove(start_idx);
@@ -153,3 +165,191 @@ impl<'a> SuppressionsParser<'a> {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use pgt_analyse::RuleCategory;
+
+    use super::*;
+    use crate::suppression::{RuleSpecifier, SuppressionKind};
+
+    #[test]
+    fn test_parse_line_suppressions() {
+        let doc = r#"
+SELECT 1;
+-- pgt-ignore lint/safety/banDropColumn
+SELECT 2;
+"#;
+        let suppressions = SuppressionsParser::parse(doc);
+
+        // Should have a line suppression on line 1 (0-based index)
+        let suppression = suppressions
+            .line_suppressions
+            .get(&2)
+            .expect("no suppression found");
+
+        assert_eq!(suppression.kind, SuppressionKind::Line);
+        assert_eq!(
+            suppression.rule_specifier,
+            RuleSpecifier::Rule(
+                RuleCategory::Lint,
+                "safety".to_string(),
+                "banDropColumn".to_string()
+            )
+        );
+    }
+
+    #[test]
+    fn test_parse_multiple_line_suppressions() {
+        let doc = r#"
+SELECT 1;
+-- pgt-ignore lint/safety/banDropColumn
+-- pgt-ignore lint/safety/banDropTable
+-- pgt-ignore lint/safety/banDropSomething
+"#;
+
+        let suppressions = SuppressionsParser::parse(doc);
+
+        assert_eq!(suppressions.line_suppressions.len(), 3);
+
+        assert_eq!(
+            suppressions
+                .line_suppressions
+                .get(&2)
+                .unwrap()
+                .rule_specifier
+                .rule(),
+            Some("banDropColumn")
+        );
+
+        assert_eq!(
+            suppressions
+                .line_suppressions
+                .get(&3)
+                .unwrap()
+                .rule_specifier
+                .rule(),
+            Some("banDropTable")
+        );
+
+        assert_eq!(
+            suppressions
+                .line_suppressions
+                .get(&4)
+                .unwrap()
+                .rule_specifier
+                .rule(),
+            Some("banDropSomething")
+        );
+    }
+
+    #[test]
+    fn parses_file_level_suppressions() {
+        let doc = r#"
+-- pgt-ignore-all lint
+-- pgt-ignore-all action
+
+SELECT 1;
+-- pgt-ignore-all lint/safety
+"#;
+
+        let suppressions = SuppressionsParser::parse(doc);
+
+        assert_eq!(suppressions.diagnostics.len(), 1);
+        assert_eq!(suppressions.file_suppressions.len(), 2);
+
+        assert_eq!(
+            suppressions.file_suppressions[0].rule_specifier,
+            RuleSpecifier::Category(RuleCategory::Lint)
+        );
+        assert_eq!(
+            suppressions.file_suppressions[1].rule_specifier,
+            RuleSpecifier::Category(RuleCategory::Action)
+        );
+
+        assert_eq!(
+            suppressions.diagnostics[0].message.to_string(),
+            String::from("File suppressions should be at the top of the file.")
+        );
+    }
+
+    #[test]
+    fn parses_range_suppressions() {
+        let doc = r#"
+-- pgt-ignore-start lint/safety/banDropTable
+drop table users;
+drop table auth;
+drop table posts;
+-- pgt-ignore-end lint/safety/banDropTable
+"#;
+
+        let suppressions = SuppressionsParser::parse(doc);
+
+        assert_eq!(suppressions.range_suppressions.len(), 1);
+
+        assert_eq!(
+            suppressions.range_suppressions[0],
+            RangeSuppression {
+                suppressed_range: TextRange::new(1.into(), 141.into()),
+                start_suppression: Suppression {
+                    kind: SuppressionKind::Start,
+                    rule_specifier: RuleSpecifier::Rule(
+                        RuleCategory::Lint,
+                        "safety".to_string(),
+                        "banDropTable".to_string()
+                    ),
+                    suppression_range: TextRange::new(1.into(), 45.into()),
+                    explanation: None,
+                },
+            }
+        );
+    }
+
+    #[test]
+    fn parses_range_suppressions_with_errors() {
+        let doc = r#"
+-- pgt-ignore-start lint/safety/banDropTable
+drop table users;
+-- pgt-ignore-start lint/safety/banDropTable
+drop table auth;
+drop table posts;
+-- pgt-ignore-end lint/safety/banDropTable
+-- pgt-ignore-end lint/safety/banDropColumn
+"#;
+
+        let suppressions = SuppressionsParser::parse(doc);
+
+        assert_eq!(suppressions.range_suppressions.len(), 1);
+        assert_eq!(suppressions.diagnostics.len(), 2);
+
+        // the inner, nested start/end combination is recognized.
+        assert_eq!(
+            suppressions.range_suppressions[0],
+            RangeSuppression {
+                suppressed_range: TextRange::new(64.into(), 186.into()),
+                start_suppression: Suppression {
+                    kind: SuppressionKind::Start,
+                    rule_specifier: RuleSpecifier::Rule(
+                        RuleCategory::Lint,
+                        "safety".to_string(),
+                        "banDropTable".to_string()
+                    ),
+                    suppression_range: TextRange::new(64.into(), 108.into()),
+                    explanation: None,
+                },
+            }
+        );
+
+        // the outer end is an error
+        assert_eq!(
+            suppressions.diagnostics[0].message.to_string(),
+            String::from("This end suppression does not have a matching start.")
+        );
+
+        // the outer start is an error
+        assert_eq!(
+            suppressions.diagnostics[1].message.to_string(),
+            String::from("This start suppression does not have a matching end.")
+        );
+    }
+}
diff --git a/crates/pgt_suppressions/src/suppression.rs b/crates/pgt_suppressions/src/suppression.rs
