feat: body matches definition in #12 (#62)

a-frantz · adthrasher · web-flow · commit 16381e81bff0 · 2024-05-31T14:35:14.000-04:00
Co-authored-by: Andrew Thrasher &lt;adthrasher@gmail.com&gt;
diff --git a/RULES.md b/RULES.md
@@ -4,7 +4,7 @@ This table documents all implemented `wdl` rules (validation failures and lint
 warnings) implemented on the `main` branch of the `stjude-rust-labs/wdl`
 repository. Note that the information may be out of sync with released packages.
 
-# Validation Failures
+## Validation Failures
 
 | Name                       | Code       | Package                                 |
 |:---------------------------|:-----------|:----------------------------------------|
@@ -13,23 +13,23 @@ repository. Note that the information may be out of sync with released packages.
 | `duplicate_runtime_keys`   | `v1::E003` | [`wdl-grammar`][wdl-grammar-validation] |
 | `missing_literal_commas`   | `v1::E004` | [`wdl-grammar`][wdl-grammar-validation] |
 
-# Lint Warnings
+## Lint Warnings
 
-| Name                      | Code       | Group        | Package                            |
-|:--------------------------|:-----------|:-------------|:-----------------------------------|
-| `whitespace`              | `v1::W001` | Style        | [`wdl-grammar`][wdl-grammar-lints] |
-| `no_curly_commands`       | `v1::W002` | Pedantic     | [`wdl-grammar`][wdl-grammar-lints] |
-| `matching_parameter_meta` | `v1::W003` | Completeness | [`wdl-ast`][wdl-ast-lints]         |
-| `mixed_indentation`       | `v1::W004` | Style        | [`wdl-grammar`][wdl-grammar-lints] |
-| `missing_runtime_block`   | `v1::W005` | Completeness | [`wdl-grammar`][wdl-grammar-lints] |
-| `snake_case`              | `v1::W006` | Naming       | [`wdl-grammar`][wdl-grammar-lints] |
-| `newline_eof`             | `v1::W007` | Spacing      | [`wdl-grammar`][wdl-grammar-lints] |
-| `document_preamble`       | `v1::W009` | Spacing      | [`wdl-grammar`][wdl-grammar-lints] |
-| `preamble_comment`        | `v1::W010` | Style        | [`wdl-grammar`][wdl-grammar-lints] |
-| `one_empty_line`          | `v1::W011` | Spacing      | [`wdl-grammar`][wdl-grammar-lints] |
-| `double_quotes`           | `v1::W012` | Naming       | [`wdl-grammar`][wdl-grammar-lints] |
+| Name                      | Code       | Tags                      | Package                            |
+|:--------------------------|:-----------|:--------------------------|:-----------------------------------|
+| `whitespace`              | `v1::W001` | Spacing, Style            | [`wdl-grammar`][wdl-grammar-lints] |
+| `no_curly_commands`       | `v1::W002` | Clarity                   | [`wdl-grammar`][wdl-grammar-lints] |
+| `matching_parameter_meta` | `v1::W003` | Completeness              | [`wdl-ast`][wdl-ast-lints]         |
+| `mixed_indentation`       | `v1::W004` | Spacing, Style, Clarity   | [`wdl-grammar`][wdl-grammar-lints] |
+| `missing_runtime_block`   | `v1::W005` | Completeness, Portability | [`wdl-grammar`][wdl-grammar-lints] |
+| `snake_case`              | `v1::W006` | Naming, Style, Clarity    | [`wdl-grammar`][wdl-grammar-lints] |
+| `newline_eof`             | `v1::W007` | Spacing, Style            | [`wdl-grammar`][wdl-grammar-lints] |
+| `document_preamble`       | `v1::W009` | Spacing, Style            | [`wdl-grammar`][wdl-grammar-lints] |
+| `preamble_comment`        | `v1::W010` | Style                     | [`wdl-grammar`][wdl-grammar-lints] |
+| `one_empty_line`          | `v1::W011` | Spacing, Style            | [`wdl-grammar`][wdl-grammar-lints] |
+| `double_quotes`           | `v1::W012` | Style, Clarity            | [`wdl-grammar`][wdl-grammar-lints] |
 
 [wdl-ast-lints]: https://docs.rs/wdl-ast/latest/wdl_ast/v1/index.html#lint-rules
 [wdl-ast-validation]: https://docs.rs/wdl-ast/latest/wdl_ast/v1/index.html#validation-rules
 [wdl-grammar-lints]: https://docs.rs/wdl-grammar/latest/wdl_grammar/v1/index.html#lint-rules
-[wdl-grammar-validation]: https://docs.rs/wdl-grammar/latest/wdl_grammar/v1/index.html#validation-rules
+[wdl-grammar-validation]: https://docs.rs/wdl-grammar/latest/wdl_grammar/v1/index.html#validation-rules
diff --git a/wdl-ast/CHANGELOG.md b/wdl-ast/CHANGELOG.md
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+* Conform to definition of body as outlined in #12 (#62, contributed by @a-frantz)
 * Changes the singular `Group` feature of lint warnings to one or more `Tags` (#37, contributed by @a-frantz)
 
 ## 0.1.0 — 12-17-2023
diff --git a/wdl-ast/src/v1.rs b/wdl-ast/src/v1.rs
@@ -11,7 +11,7 @@
 //!
 //! The following abstract syntax tree linting rules are supported for WDL 1.x:
 //!
-//! | Name                      | Code       | Group        | Documentation                       |
+//! | Name                      | Code       | Tags         | Documentation                       |
 //! |:--------------------------|:-----------|:-------------|:-----------------------------------:|
 //! | `matching_parameter_meta` | `v1::W003` | Completeness | [Link](lint::MatchingParameterMeta) |
 
diff --git a/wdl-ast/src/v1/lint/matching_parameter_meta.rs b/wdl-ast/src/v1/lint/matching_parameter_meta.rs
@@ -62,11 +62,7 @@ impl<'a> MatchingParameterMeta {
                 "missing parameter meta within {}: {}",
                 context, parameter
             ))
-            .body(format!(
-                "Each input parameter within a {} should have an associated `parameter_meta` \
-                 entry with a detailed description of the input.",
-                context
-            ))
+            .body(self.body())
             .fix(
                 "Add a key to a `parameter_meta` block matching the parameter's exact name with a \
                  detailed description of the input.",
@@ -89,13 +85,13 @@ impl<'a> MatchingParameterMeta {
         lint::warning::Builder::default()
             .code(self.code())
             .level(lint::Level::Medium)
-            .tags(TagSet::new(&[lint::Tag::Completeness]))
+            .tags(self.tags())
             .push_location(location.clone())
             .subject(format!(
                 "extraneous parameter meta within {}: {}",
                 context, parameter
             ))
-            .body("A parameter meta entry with no corresponding input parameter was detected")
+            .body(self.body())
             .fix("Remove the parameter meta entry")
             .try_build()
             .unwrap()
@@ -112,6 +108,12 @@ impl<'a> Rule<&'a v1::Document> for MatchingParameterMeta {
         TagSet::new(&[lint::Tag::Completeness])
     }
 
+    fn body(&self) -> &'static str {
+        "Each input parameter within a task or workflow should have an associated `parameter_meta` \
+         entry with a detailed description of the input. Non-input keys are not permitted within \
+         the `parameter_meta` block."
+    }
+
     fn check(&self, tree: &'a v1::Document) -> lint::Result {
         let mut warnings = VecDeque::new();
 
diff --git a/wdl-core/CHANGELOG.md b/wdl-core/CHANGELOG.md
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+* Conform to definition of body as outlined in #12 (#62, contributed by @a-frantz)
 * Changes the singular `Group` feature of lint warnings to one or more `Tags` (#37, contributed by @a-frantz)
 
 ## 0.1.0 — 12-17-2023
diff --git a/wdl-core/src/concern/lint.rs b/wdl-core/src/concern/lint.rs
@@ -79,6 +79,9 @@ pub trait Rule<E>: std::fmt::Debug + Sync {
     /// Get the lint tags for this lint rule.
     fn tags(&self) -> TagSet;
 
+    /// Get the body of the lint rule.
+    fn body(&self) -> &'static str;
+
     /// Checks the tree according to the implemented lint rule.
     fn check(&self, tree: E) -> Result;
 }
diff --git a/wdl-grammar/CHANGELOG.md b/wdl-grammar/CHANGELOG.md
@@ -63,6 +63,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+* Conform to definition of body as outlined in #12 (#62, contributed by @a-frantz)
 * Changes the `preamble_comment` rule so that continuous blocks of comments are reported.
   Also permits triple+ pound sign comments outside of the preamble. (#55, contributed by @a-frantz)
 * Changes the `snake_case` rule so that lowercase letters can be adjacent to digits without triggering
diff --git a/wdl-grammar/src/v1.rs b/wdl-grammar/src/v1.rs
@@ -15,18 +15,18 @@
 //!
 //! The following parse tree linting rules are supported for WDL 1.x:
 //!
-//! | Name                    | Code       | Group        | Documentation                     |
-//! |:------------------------|:-----------|:-------------|:---------------------------------:|
-//! | `whitespace`            | `v1::W001` | Style        | [Link](lint::Whitespace)          |
-//! | `no_curly_commands`     | `v1::W002` | Pedantic     | [Link](lint::NoCurlyCommands)     |
-//! | `mixed_indentation`     | `v1::W004` | Style        | [Link](lint::MixedIndentation)    |
-//! | `missing_runtime_block` | `v1::W005` | Completeness | [Link](lint::MissingRuntimeBlock) |
-//! | `snake_case`            | `v1::W006` | Naming       | [Link](lint::SnakeCase)           |
-//! | `newline_eof`           | `v1::W007` | Spacing      | [Link](lint::NewlineEOF)          |
-//! | `document_preamble`     | `v1::W009` | Spacing      | [Link](lint::DocumentPreamble)    |
-//! | `preamble_comment`      | `v1::W010` | Style        | [Link](lint::PreambleComment)     |
-//! | `one_empty_line`        | `v1::W011` | Spacing      | [Link](lint::OneEmptyLine)        |
-//! | `double_quotes `        | `v1::W012` | Naming       | [Link](lint::DoubleQuotes)        |
+//! | Name                      | Code       | Tags                      | Documentation                     |
+//! |:--------------------------|:-----------|:--------------------------|:---------------------------------:|
+//! | `whitespace`              | `v1::W001` | Spacing, Style            | [Link](lint::Whitespace)          |
+//! | `no_curly_commands`       | `v1::W002` | Clarity                   | [Link](lint::NoCurlyCommands)     |
+//! | `mixed_indentation`       | `v1::W004` | Spacing, Style, Clarity   | [Link](lint::MixedIndentation)    |
+//! | `missing_runtime_block`   | `v1::W005` | Completeness, Portability | [Link](lint::MissingRuntimeBlock) |
+//! | `snake_case`              | `v1::W006` | Naming, Style, Clarity    | [Link](lint::SnakeCase)           |
+//! | `newline_eof`             | `v1::W007` | Spacing, Style            | [Link](lint::NewlineEOF)          |
+//! | `document_preamble`       | `v1::W009` | Spacing, Style            | [Link](lint::DocumentPreamble)    |
+//! | `preamble_comment`        | `v1::W010` | Style                     | [Link](lint::PreambleComment)     |
+//! | `one_empty_line`          | `v1::W011` | Spacing, Style            | [Link](lint::OneEmptyLine)        |
+//! | `double_quotes`           | `v1::W012` | Style, Clarity            | [Link](lint::DoubleQuotes)        |
 
 use pest::iterators::Pair;
 use pest::Parser as _;
diff --git a/wdl-grammar/src/v1/lint/document_preamble.rs b/wdl-grammar/src/v1/lint/document_preamble.rs
@@ -33,10 +33,7 @@ impl<'a> DocumentPreamble {
             .level(lint::Level::Low)
             .tags(self.tags())
             .subject("Improperly placed version declaration")
-            .body(
-                "The version declaration must be the first line of a WDL document or immediately \
-                 following exactly one blank line and any preamble comments.",
-            )
+            .body(self.body())
             .push_location(location)
             .fix(
                 "Move the version declaration to the first line of the WDL document or \
@@ -57,7 +54,7 @@ impl<'a> DocumentPreamble {
             .level(lint::Level::Low)
             .tags(self.tags())
             .subject("Leading whitespace detected")
-            .body("No whitespace is allowed at the beginning of the document")
+            .body(self.body())
             .push_location(location)
             .fix("Remove leading whitespace.")
             .try_build()
@@ -75,6 +72,14 @@ impl<'a> Rule<&Pair<'a, v1::Rule>> for DocumentPreamble {
         TagSet::new(&[Tag::Spacing, Tag::Style])
     }
 
+    fn body(&self) -> &'static str {
+        "The document preamble is defined as anything before the version declaration statement and \
+         the version declaration statement itself. Only comments and whitespace are permitted \
+         before the version declaration. If there are no comments, the version declaration must be \
+         the first line of the document. If there are comments, there must be exactly one blank \
+         line between the last comment and the version declaration."
+    }
+
     fn check(&self, tree: &Pair<'_, v1::Rule>) -> lint::Result {
         let mut warnings = VecDeque::new();
 
diff --git a/wdl-grammar/src/v1/lint/double_quotes.rs b/wdl-grammar/src/v1/lint/double_quotes.rs
@@ -31,7 +31,7 @@ impl<'a> DoubleQuotes {
             .tags(self.tags())
             .push_location(location)
             .subject("string defined with single quotes")
-            .body("All strings should be defined using double quotes.")
+            .body(self.body())
             .fix("Change the single quotes to double quotes.")
             .try_build()
             .unwrap()
@@ -48,6 +48,12 @@ impl<'a> Rule<&'a Pair<'a, v1::Rule>> for DoubleQuotes {
         TagSet::new(&[lint::Tag::Clarity, lint::Tag::Style])
     }
 
+    fn body(&self) -> &'static str {
+        "All strings should be defined using double quotes. There is no semantic difference \
+         between single and double quotes in WDL, but double quotes should be used exclusively to \
+         ensure consistency and avoid any confusion."
+    }
+
     fn check(&self, tree: &'a Pair<'_, v1::Rule>) -> lint::Result {
         let mut warnings = VecDeque::new();
 
diff --git a/wdl-grammar/src/v1/lint/missing_runtime_block.rs b/wdl-grammar/src/v1/lint/missing_runtime_block.rs
@@ -33,7 +33,7 @@ impl<'a> MissingRuntimeBlock {
             .tags(self.tags())
             .push_location(location)
             .subject("missing runtime block within a task")
-            .body("Tasks that don't declare runtime blocks are unlikely to be portable")
+            .body(self.body())
             .fix(
                 "Add a runtime block to the task with desired cpu, memory and container/docker \
                  requirements",
@@ -52,6 +52,10 @@ impl<'a> Rule<&'a Pair<'a, v1::Rule>> for MissingRuntimeBlock {
         TagSet::new(&[Tag::Completeness, Tag::Portability])
     }
 
+    fn body(&self) -> &'static str {
+        "Tasks that don't declare runtime blocks are unlikely to be portable."
+    }
+
     fn check(&self, tree: &'a Pair<'a, v1::Rule>) -> lint::Result {
         let mut warnings = VecDeque::new();
 
diff --git a/wdl-grammar/src/v1/lint/mixed_indentation.rs b/wdl-grammar/src/v1/lint/mixed_indentation.rs
@@ -33,10 +33,7 @@ impl<'a> MixedIndentation {
             .level(lint::Level::High)
             .tags(self.tags())
             .subject("mixed indentation characters")
-            .body(
-                "Mixed indentation characters were found within a command. This causes leading \
-                 whitespace stripping to be skipped.",
-            )
+            .body(self.body())
             .push_location(location)
             .fix("Use the same whitespace character within the command.")
             .try_build()
@@ -54,6 +51,12 @@ impl Rule<&Pair<'_, v1::Rule>> for MixedIndentation {
         TagSet::new(&[lint::Tag::Style, lint::Tag::Spacing, lint::Tag::Clarity])
     }
 
+    fn body(&self) -> &'static str {
+        "Mixing indentation characters (tab literals and spaces) within the command line causes \
+         leading whitespace stripping to be skipped. Commands may be whitespace sensitive, and \
+         skipping the whitespace stripping step may cause unexpected behavior."
+    }
+
     fn check(&self, tree: &Pair<'_, v1::Rule>) -> lint::Result {
         let mut warnings = VecDeque::new();
 
diff --git a/wdl-grammar/src/v1/lint/newline_eof.rs b/wdl-grammar/src/v1/lint/newline_eof.rs
@@ -31,7 +31,7 @@ impl<'a> NewlineEOF {
             .tags(self.tags())
             .push_location(location)
             .subject("missing newline at the end of the file")
-            .body("There should always be a newline at the end of a WDL file.")
+            .body(self.body())
             .fix("Add a newline at the end of the file.")
             .try_build()
             .unwrap()
@@ -66,6 +66,10 @@ impl<'a> Rule<&'a Pair<'a, v1::Rule>> for NewlineEOF {
         TagSet::new(&[lint::Tag::Spacing, lint::Tag::Style])
     }
 
+    fn body(&self) -> &'static str {
+        "The file should end with one and only one newline character. This is to conform to [POSIX standards](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_206)."
+    }
+
     fn check(&self, tree: &'a Pair<'_, v1::Rule>) -> lint::Result {
         let mut warnings: VecDeque<_> = VecDeque::new();
 
diff --git a/wdl-grammar/src/v1/lint/no_curly_commands.rs b/wdl-grammar/src/v1/lint/no_curly_commands.rs
@@ -35,7 +35,7 @@ impl<'a> NoCurlyCommands {
             .tags(self.tags())
             .push_location(location)
             .subject("curly command found")
-            .body("Command blocks using curly braces (`{}`) can create ambiguity with Bash syntax.")
+            .body(self.body())
             .fix("Replace the curly command block with a heredoc command block.")
             .try_build()
             .unwrap()
@@ -52,6 +52,12 @@ impl<'a> Rule<&'a Pair<'a, v1::Rule>> for NoCurlyCommands {
         TagSet::new(&[lint::Tag::Clarity])
     }
 
+    fn body(&self) -> &'static str {
+        "Curly command blocks are no longer considered idiomatic WDL. Idiomatic WDL code uses \
+         heredoc command blocks instead. This is because curly command blocks create ambiguity \
+         with Bash syntax."
+    }
+
     fn check(&self, tree: &'a Pair<'_, v1::Rule>) -> lint::Result {
         let mut warnings = VecDeque::new();
 
diff --git a/wdl-grammar/src/v1/lint/one_empty_line.rs b/wdl-grammar/src/v1/lint/one_empty_line.rs
@@ -33,7 +33,7 @@ impl<'a> OneEmptyLine {
             .tags(self.tags())
             .push_location(location)
             .subject("more than one empty line")
-            .body("There should be at most one empty line in a row.")
+            .body(self.body())
             .fix("Remove the superfluous empty lines.")
             .try_build()
             .unwrap()
@@ -50,6 +50,11 @@ impl<'a> Rule<&'a Pair<'a, v1::Rule>> for OneEmptyLine {
         TagSet::new(&[lint::Tag::Spacing, lint::Tag::Style])
     }
 
+    fn body(&self) -> &'static str {
+        "There should be at most one empty line in a row. Superfluous empty lines make the code \
+         harder to read."
+    }
+
     fn check(&self, tree: &'a Pair<'_, v1::Rule>) -> lint::Result {
         let mut warnings: VecDeque<_> = VecDeque::new();
 
diff --git a/wdl-grammar/src/v1/lint/preamble_comment.rs b/wdl-grammar/src/v1/lint/preamble_comment.rs
diff --git a/wdl-grammar/src/v1/lint/snake_case.rs b/wdl-grammar/src/v1/lint/snake_case.rs
diff --git a/wdl-grammar/src/v1/lint/whitespace.rs b/wdl-grammar/src/v1/lint/whitespace.rs

Original file line number	Diff line number	Diff line change
`@@ -79,6 +79,9 @@ pub trait Rule<E>: std::fmt::Debug + Sync {`
`79`	`79`	`/// Get the lint tags for this lint rule.`
`80`	`80`	`fn tags(&self) -> TagSet;`
`81`	`81`
	`82`	`+ /// Get the body of the lint rule.`
	`83`	`+ fn body(&self) -> &'static str;`
	`84`	`+`
`82`	`85`	`/// Checks the tree according to the implemented lint rule.`
`83`	`86`	`fn check(&self, tree: E) -> Result;`
`84`	`87`	`}`