Simplify check API

Author: benwebber

src/diagnostic.rs

@@ -4,7 +4,6 @@ use std::path::PathBuf;
 
 use serde::{Deserialize, Serialize};
 
-use crate::changelog::ParsedChangelog;
 use crate::rule::Rule;
 use crate::span::{Locator, Position, Ranged, Span};
 

src/linter.rs

@@ -4,7 +4,9 @@ use std::path::PathBuf;
 use crate::changelog::parsed;
 use crate::changelog::traits::*;
 use crate::diagnostic::Diagnostic;
+use crate::rule::Rule;
 use crate::ruleset::RuleSet;
+use crate::span::Span;
 
 #[macro_use]
 mod macros;
@@ -27,6 +29,17 @@ pub struct Linter<'a> {
     filename: Option<PathBuf>,
 }
 
+#[derive(Default)]
+pub(crate) struct Context {
+    diagnostics: Vec<Diagnostic>,
+}
+
+impl Context {
+    fn report(&mut self, rule: Rule, span: Option<Span>) {
+        self.diagnostics.push(Diagnostic::new(rule, span));
+    }
+}
+
 impl<'a> Linter<'a> {
     /// Create a new linter with the given ruleset.
     pub fn new(ruleset: &'a RuleSet) -> Self {
@@ -46,44 +59,42 @@ impl<'a> Linter<'a> {
 
     /// Lint a changelog.
     pub fn lint(&self, changelog: &parsed::ParsedChangelog) -> Vec<Diagnostic> {
-        let mut diagnostics: Vec<Diagnostic> = Vec::new();
         let mut checks: Vec<_> = checks()
             .into_iter()
             .filter(|check| self.ruleset.is_enabled(check.rule()))
             .collect();
+        let mut context = Context::default();
         for check in checks.iter_mut() {
-            check.visit_changelog(changelog);
+            check.visit_changelog(&mut context, changelog);
             if let Some(unreleased) = changelog.unreleased() {
-                check.visit_unreleased(unreleased);
+                check.visit_unreleased(&mut context, unreleased);
                 for changes in unreleased.changes() {
-                    check.visit_changes(changes);
+                    check.visit_changes(&mut context, changes);
                 }
             }
             for span in &changelog.invalid_spans {
-                check.visit_invalid_span(span);
+                check.visit_invalid_span(&mut context, span);
             }
         }
         for release in changelog.releases() {
             for check in checks.iter_mut() {
-                check.visit_release(release);
+                check.visit_release(&mut context, release);
                 for changes in release.changes() {
-                    check.visit_changes(changes);
+                    check.visit_changes(&mut context, changes);
                 }
             }
         }
         for check in checks.iter_mut() {
-            diagnostics.append(
-                &mut check
-                    .diagnostics()
-                    .into_iter()
-                    .map(|mut d| {
-                        d.path = self.filename.clone();
-                        d
-                    })
-                    .collect(),
-            );
+            check.finalize(&mut context);
         }
-        diagnostics
+        context
+            .diagnostics
+            .into_iter()
+            .map(|mut diagnostic| {
+                diagnostic.path = self.filename.clone();
+                diagnostic
+            })
+            .collect()
     }
 }
 

src/linter/check.rs

@@ -1,28 +1,18 @@
 use crate::changelog::parsed;
-use crate::diagnostic::Diagnostic;
 use crate::rule::Rule;
-use crate::span::Span;
+
+use super::Context;
 
 pub(crate) trait Check {
     /// Return the `Rule` this check evaluates.
     fn rule(&self) -> Rule;
 
-    /// Return spans that this check matched.
-    fn spans(&self) -> &[Span] {
-        &[]
+    fn visit_changelog(&mut self, _context: &mut Context, _changelog: &parsed::ParsedChangelog) {}
+    fn visit_unreleased(&mut self, _context: &mut Context, _unreleased: &parsed::ParsedUnreleased) {
     }
+    fn visit_release(&mut self, _context: &mut Context, _release: &parsed::ParsedRelease) {}
+    fn visit_changes(&mut self, _context: &mut Context, _changes: &parsed::ParsedChanges) {}
+    fn visit_invalid_span(&mut self, _context: &mut Context, _span: &parsed::InvalidSpan) {}
 
-    fn visit_changelog(&mut self, _changelog: &parsed::ParsedChangelog) {}
-    fn visit_unreleased(&mut self, _unreleased: &parsed::ParsedUnreleased) {}
-    fn visit_release(&mut self, _release: &parsed::ParsedRelease) {}
-    fn visit_changes(&mut self, _changes: &parsed::ParsedChanges) {}
-    fn visit_invalid_span(&mut self, _span: &parsed::InvalidSpan) {}
-
-    /// Publish diagnostics generated by this check.
-    fn diagnostics(&self) -> Vec<Diagnostic> {
-        self.spans()
-            .iter()
-            .map(|span| Diagnostic::new(self.rule(), Some(*span)))
-            .collect()
-    }
+    fn finalize(&mut self, _context: &mut Context) {}
 }

src/linter/checks/content.rs

@@ -3,67 +3,55 @@ use std::collections::HashSet;
 use crate::changelog::parsed;
 use crate::linter::Check;
 use crate::rule::Rule;
-use crate::span::Span;
+
+use super::preamble::*;
 
 invalid_span!(InvalidTitle);
 
 invalid_span!(InvalidSectionHeading);
 
 #[derive(Default)]
-pub struct EmptySection {
-    spans: Vec<Span>,
-}
+pub struct EmptySection;
 
 // TODO: Store better spans for these headings.
 impl Check for EmptySection {
     fn rule(&self) -> Rule {
         Rule::EmptySection
     }
 
-    fn spans(&self) -> &[Span] {
-        self.spans.as_slice()
-    }
-
-    fn visit_release(&mut self, release: &parsed::ParsedRelease) {
+    fn visit_release(&mut self, context: &mut Context, release: &parsed::ParsedRelease) {
         if release.changes.is_empty() {
-            self.spans.push(release.heading_span);
+            context.report(self.rule(), Some(release.heading_span));
         }
     }
 
-    fn visit_changes(&mut self, changes: &parsed::ParsedChanges) {
+    fn visit_changes(&mut self, context: &mut Context, changes: &parsed::ParsedChanges) {
         if changes.items.is_empty() {
-            self.spans.push(changes.heading_span)
+            context.report(self.rule(), Some(changes.heading_span));
         }
     }
 }
 
 #[derive(Default)]
-pub struct UnknownChangeType {
-    spans: Vec<Span>,
-}
+pub struct UnknownChangeType;
 
 impl Check for UnknownChangeType {
     fn rule(&self) -> Rule {
         Rule::UnknownChangeType
     }
 
-    fn spans(&self) -> &[Span] {
-        self.spans.as_slice()
-    }
-
-    fn visit_changes(&mut self, changes: &parsed::ParsedChanges) {
+    fn visit_changes(&mut self, context: &mut Context, changes: &parsed::ParsedChanges) {
         if !matches!(
             changes.kind.value,
             "Added" | "Changed" | "Deprecated" | "Fixed" | "Removed" | "Security"
         ) {
-            self.spans.push(changes.kind.span);
+            context.report(self.rule(), Some(changes.kind.span));
         }
     }
 }
 
 #[derive(Default)]
 pub struct DuplicateChangeType {
-    spans: Vec<Span>,
     seen: HashSet<String>,
 }
 
@@ -72,21 +60,17 @@ impl Check for DuplicateChangeType {
         Rule::DuplicateChangeType
     }
 
-    fn spans(&self) -> &[Span] {
-        self.spans.as_slice()
-    }
-
-    fn visit_unreleased(&mut self, _unreleased: &parsed::ParsedUnreleased) {
+    fn visit_unreleased(&mut self, _context: &mut Context, _unreleased: &parsed::ParsedUnreleased) {
         self.seen.clear();
     }
 
-    fn visit_release(&mut self, _unreleased: &parsed::ParsedRelease) {
+    fn visit_release(&mut self, _context: &mut Context, _unreleased: &parsed::ParsedRelease) {
         self.seen.clear();
     }
 
-    fn visit_changes(&mut self, changes: &parsed::ParsedChanges) {
+    fn visit_changes(&mut self, context: &mut Context, changes: &parsed::ParsedChanges) {
         if !self.seen.insert(changes.kind.value.to_string()) {
-            self.spans.push(changes.kind.span);
+            context.report(self.rule(), Some(changes.kind.span));
         }
     }
 }

src/linter/checks/links.rs

@@ -1,7 +1,8 @@
 use crate::changelog::parsed;
 use crate::linter::Check;
 use crate::rule::Rule;
-use crate::span::Span;
+
+use super::preamble::*;
 
 invalid_span!(UndefinedLinkReference);
 

src/linter/checks/preamble.rs

@@ -1,7 +1,6 @@
-pub(crate) use crate::diagnostic::Diagnostic;
+pub(crate) use crate::linter::Context;
 pub(crate) use crate::linter::check::Check;
 pub(crate) use crate::rule::Rule;
 pub(crate) use crate::span::Span;
 
 pub(crate) use crate::changelog::parsed;
-pub(crate) use crate::changelog::traits::*;