revise: preamble and version stmt lint rules

wdl-lint/src/lib.rs

@@ -80,8 +80,7 @@ pub fn rules() -> Vec<Box<dyn Rule>> {
         Box::<rules::SnakeCaseRule>::default(),
         Box::<rules::MissingRuntimeRule>::default(),
         Box::<rules::EndingNewlineRule>::default(),
-        Box::<rules::PreambleWhitespaceRule>::default(),
-        Box::<rules::PreambleCommentsRule>::default(),
+        Box::<rules::PreambleFormattingRule>::default(),
         Box::<rules::MatchingParameterMetaRule>::default(),
         Box::<rules::WhitespaceRule>::default(),
         Box::<rules::CommandSectionMixedIndentationRule>::default(),
@@ -112,7 +111,9 @@ pub fn rules() -> Vec<Box<dyn Rule>> {
         Box::<rules::ContainerValue>::default(),
         Box::<rules::MissingRequirementsRule>::default(),
         Box::<rules::UnknownRule>::default(),
-        Box::<rules::MisplacedLintDirective>::default(),
+        Box::<rules::MisplacedLintDirectiveRule>::default(),
+        Box::<rules::VersionFormattingRule>::default(),
+        Box::<rules::PreambleCommentAfterVersionRule>::default(),
     ];
 
     // Ensure all the rule ids are unique and pascal case

wdl-lint/src/rules.rs

@@ -29,14 +29,15 @@ mod missing_runtime;
 mod no_curly_commands;
 mod nonmatching_output;
 mod pascal_case;
-mod preamble_comments;
-mod preamble_whitespace;
+mod preamble_comment_after_version;
+mod preamble_formatting;
 mod runtime_section_keys;
 mod section_order;
 mod snake_case;
 mod todo;
 mod trailing_comma;
 mod unknown_rule;
+mod version_formatting;
 mod whitespace;
 
 pub use blank_lines_between_elements::*;
@@ -68,12 +69,13 @@ pub use missing_runtime::*;
 pub use no_curly_commands::*;
 pub use nonmatching_output::*;
 pub use pascal_case::*;
-pub use preamble_comments::*;
-pub use preamble_whitespace::*;
+pub use preamble_comment_after_version::*;
+pub use preamble_formatting::*;
 pub use runtime_section_keys::*;
 pub use section_order::*;
 pub use snake_case::*;
 pub use todo::*;
 pub use trailing_comma::*;
 pub use unknown_rule::*;
+pub use version_formatting::*;
 pub use whitespace::*;

wdl-lint/src/rules/deprecated_placeholder_option.rs

@@ -25,7 +25,7 @@ const ID: &str = "DeprecatedPlaceholderOption";
 /// Creates a diagnostic for the use of the deprecated `default` placeholder
 /// option.
 fn deprecated_default_placeholder_option(span: Span) -> Diagnostic {
-    Diagnostic::warning(String::from(
+    Diagnostic::note(String::from(
         "use of the deprecated `default` placeholder option",
     ))
     .with_rule(ID)
@@ -38,7 +38,7 @@ fn deprecated_default_placeholder_option(span: Span) -> Diagnostic {
 
 /// Creates a diagnostic for the use of the deprecated `sep` placeholder option.
 fn deprecated_sep_placeholder_option(span: Span) -> Diagnostic {
-    Diagnostic::warning(String::from(
+    Diagnostic::note(String::from(
         "use of the deprecated `sep` placeholder option",
     ))
     .with_rule(ID)
@@ -51,7 +51,7 @@ fn deprecated_sep_placeholder_option(span: Span) -> Diagnostic {
 /// Creates a diagnostic for the use of the deprecated `true`/`false`
 /// placeholder option.
 fn deprecated_true_false_placeholder_option(span: Span) -> Diagnostic {
-    Diagnostic::warning(String::from(
+    Diagnostic::note(String::from(
         "use of the deprecated `true`/`false` placeholder option",
     ))
     .with_rule(ID)

wdl-lint/src/rules/double_quotes.rs

@@ -24,7 +24,7 @@ const ID: &str = "DoubleQuotes";
 
 /// Creates a "use double quotes" diagnostic.
 fn use_double_quotes(span: Span) -> Diagnostic {
-    Diagnostic::warning("string defined with single quotes")
+    Diagnostic::note("string defined with single quotes")
         .with_rule(ID)
         .with_highlight(span)
         .with_fix("change the single quotes to double quotes")

wdl-lint/src/rules/ending_newline.rs

@@ -21,15 +21,15 @@ const ID: &str = "EndingNewline";
 
 /// Creates a "missing ending newline" diagnostic.
 fn missing_ending_newline(span: Span) -> Diagnostic {
-    Diagnostic::warning("missing newline at the end of the file")
+    Diagnostic::note("missing newline at the end of the file")
         .with_rule(ID)
         .with_label("expected a newline to follow this", span)
         .with_fix("add an empty line at the end of the file")
 }
 
 /// Creates a "multiple ending newline" diagnostic.
 fn multiple_ending_newline(span: Span, count: usize) -> Diagnostic {
-    Diagnostic::warning("multiple empty lines at the end of file")
+    Diagnostic::note("multiple empty lines at the end of file")
         .with_rule(ID)
         .with_label(
             if count > 1 {

wdl-lint/src/rules/input_not_sorted.rs

@@ -26,7 +26,7 @@ const ID: &str = "InputSorting";
 
 /// Creates a "input not sorted" diagnostic.
 fn input_not_sorted(span: Span, sorted_inputs: String) -> Diagnostic {
-    Diagnostic::warning("input not sorted")
+    Diagnostic::note("input not sorted")
         .with_rule(ID)
         .with_label("input section must be sorted".to_string(), span)
         .with_fix(format!("sort input statements as: \n{}", sorted_inputs))

wdl-lint/src/rules/misplaced_lint_directive.rs

@@ -65,9 +65,9 @@ pub static RULE_MAP: LazyLock<HashMap<&'static str, Option<&'static [SyntaxKind]
 
 /// Detects unknown rules within lint directives.
 #[derive(Default, Debug, Clone, Copy)]
-pub struct MisplacedLintDirective;
+pub struct MisplacedLintDirectiveRule;
 
-impl Rule for MisplacedLintDirective {
+impl Rule for MisplacedLintDirectiveRule {
     fn id(&self) -> &'static str {
         ID
     }
@@ -91,7 +91,7 @@ impl Rule for MisplacedLintDirective {
     }
 }
 
-impl Visitor for MisplacedLintDirective {
+impl Visitor for MisplacedLintDirectiveRule {
     type State = Diagnostics;
 
     fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document, _: SupportedVersion) {

wdl-lint/src/rules/missing_requirements.rs

@@ -24,7 +24,7 @@ const ID: &str = "MissingRequirements";
 
 /// Creates a "deprecated runtime section" diagnostic.
 fn deprecated_runtime_section(task: &str, span: Span) -> Diagnostic {
-    Diagnostic::warning(format!(
+    Diagnostic::note(format!(
         "task `{task}` contains a deprecated `runtime` section"
     ))
     .with_rule(ID)

wdl-lint/src/rules/nonmatching_output.rs

@@ -63,7 +63,7 @@ fn extra_output_in_meta(span: Span, name: &str, item_name: &str, ty: &str) -> Di
 
 /// Creates a diagnostic for out-of-order entries.
 fn out_of_order(span: Span, output_span: Span, item_name: &str, ty: &str) -> Diagnostic {
-    Diagnostic::warning(format!(
+    Diagnostic::note(format!(
         "`outputs` section of `meta` for the {ty} `{item_name}` is out of order"
     ))
     .with_rule(ID)
@@ -76,6 +76,7 @@ fn out_of_order(span: Span, output_span: Span, item_name: &str, ty: &str) -> Dia
 
 /// Creates a diagnostic for non-object `meta.outputs` entries.
 fn non_object_meta_outputs(span: Span, item_name: &str, ty: &str) -> Diagnostic {
+    // TODO: this message is way too long
     Diagnostic::warning(format!(
         "`outputs` key in `meta` section is reserved for an object with keys corresponding to \
          declared `output` values. {ty} `{item_name}` has a `meta.outputs` key that is not an \

wdl-lint/src/rules/preamble_comment_after_version.rs

@@ -0,0 +1,143 @@
+//! A lint rule for flagging preamble comments which are outside the preamble.
+
+use wdl_ast::AstToken;
+use wdl_ast::Comment;
+use wdl_ast::Diagnostic;
+use wdl_ast::Diagnostics;
+use wdl_ast::Document;
+use wdl_ast::Span;
+use wdl_ast::SyntaxElement;
+use wdl_ast::SyntaxKind;
+use wdl_ast::VisitReason;
+use wdl_ast::Visitor;
+
+use crate::Rule;
+use crate::Tag;
+use crate::TagSet;
+
+/// The identifier for the rule.
+const ID: &str = "PreambleCommentAfterVersion";
+
+/// Creates a diagnostic for a comment outside the preamble.
+fn preamble_comment_outside_preamble(span: Span) -> Diagnostic {
+    Diagnostic::error("preamble comment after the version statement")
+        .with_rule(ID)
+        .with_highlight(span)
+}
+
+/// A lint rule for flagging preamble comments which are outside the preamble.
+#[derive(Default, Debug, Clone)]
+pub struct PreambleCommentAfterVersionRule {
+    /// Exited the preamble.
+    exited_preamble: bool,
+    /// The number of comment tokens to skip.
+    ///
+    /// This is used when consolidating multiple comments into a single
+    /// diagnositc.
+    skip_count: usize,
+}
+
+impl Rule for PreambleCommentAfterVersionRule {
+    fn id(&self) -> &'static str {
+        ID
+    }
+
+    fn description(&self) -> &'static str {
+        "Ensures that preamble comments are inside the preamble."
+    }
+
+    fn explanation(&self) -> &'static str {
+        "Preamble comments should be inside the preamble."
+    }
+
+    fn tags(&self) -> TagSet {
+        TagSet::new(&[Tag::Clarity])
+    }
+
+    fn exceptable_nodes(&self) -> Option<&'static [SyntaxKind]> {
+        None
+    }
+}
+
+impl Visitor for PreambleCommentAfterVersionRule {
+    type State = Diagnostics;
+
+    fn document(
+        &mut self,
+        _: &mut Self::State,
+        reason: VisitReason,
+        _: &Document,
+        _: wdl_ast::SupportedVersion,
+    ) {
+        if reason == VisitReason::Exit {
+            return;
+        }
+
+        // Reset the visitor upon document entry.
+        *self = Default::default();
+    }
+
+    fn version_statement(
+        &mut self,
+        _state: &mut Self::State,
+        reason: VisitReason,
+        _stmt: &wdl_ast::VersionStatement,
+    ) {
+        if reason == VisitReason::Exit {
+            self.exited_preamble = true;
+        }
+    }
+
+    fn comment(&mut self, state: &mut Self::State, comment: &Comment) {
+        if !self.exited_preamble {
+            return;
+        }
+
+        if self.skip_count > 0 {
+            self.skip_count -= 1;
+            return;
+        }
+
+        if !comment.as_str().starts_with("## ") {
+            return;
+        }
+
+        let mut span = comment.span();
+        let mut current = comment.syntax().next_sibling_or_token();
+        while let Some(sibling) = current {
+            match sibling.kind() {
+                SyntaxKind::Comment => {
+                    self.skip_count += 1;
+
+                    if !sibling
+                        .as_token()
+                        .expect("expected a token")
+                        .text()
+                        .starts_with("## ")
+                    {
+                        // The sibling comment is valid, so we can break.
+                        break;
+                    }
+
+                    // Not valid, update the span
+                    span = Span::new(
+                        span.start(),
+                        usize::from(sibling.text_range().end()) - span.start(),
+                    )
+                }
+                SyntaxKind::Whitespace => {
+                    // Skip whitespace
+                }
+                _ => break,
+            }
+
+            current = sibling.next_sibling_or_token();
+        }
+
+        state.exceptable_add(
+            preamble_comment_outside_preamble(span),
+            SyntaxElement::from(comment.syntax().clone()),
+            &self.exceptable_nodes(),
+        );
+    }
+}