fix(SectionOrder): allow workflow conditionals (#129)

stjude-rust-labs · Jul 16, 2024 · 2da59bc · 2da59bc
1 parent d45f7f2
commit 2da59bc
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 70 deletions.
diff --git a/Arena.toml b/Arena.toml
@@ -1700,11 +1700,6 @@ document = "stjudecloud/workflows:/workflows/chipseq/chipseq-standard.wdl"
 message = "chipseq-standard.wdl:14:3: note[Todo]: remaining `TODO` item found"
 permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/chipseq/chipseq-standard.wdl/#L14"
 
-[[diagnostics]]
-document = "stjudecloud/workflows:/workflows/chipseq/chipseq-standard.wdl"
-message = "chipseq-standard.wdl:15:10: note[SectionOrdering]: sections are not in order for workflow `chipseq_standard`"
-permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/chipseq/chipseq-standard.wdl/#L15"
-
 [[diagnostics]]
 document = "stjudecloud/workflows:/workflows/dnaseq/dnaseq-core.wdl"
 message = "dnaseq-core.wdl:115:1: note[LineWidth]: line exceeds maximum width of 90"
@@ -1725,16 +1720,6 @@ document = "stjudecloud/workflows:/workflows/dnaseq/dnaseq-standard-fastq.wdl"
 message = "dnaseq-standard-fastq.wdl:51:1: note[LineWidth]: line exceeds maximum width of 90"
 permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/dnaseq/dnaseq-standard-fastq.wdl/#L51"
 
-[[diagnostics]]
-document = "stjudecloud/workflows:/workflows/dnaseq/dnaseq-standard-fastq.wdl"
-message = "dnaseq-standard-fastq.wdl:9:10: note[SectionOrdering]: sections are not in order for workflow `dnaseq_standard_fastq_experimental`"
-permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/dnaseq/dnaseq-standard-fastq.wdl/#L9"
-
-[[diagnostics]]
-document = "stjudecloud/workflows:/workflows/dnaseq/dnaseq-standard.wdl"
-message = "dnaseq-standard.wdl:11:10: note[SectionOrdering]: sections are not in order for workflow `dnaseq_standard_experimental`"
-permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/dnaseq/dnaseq-standard.wdl/#L11"
-
 [[diagnostics]]
 document = "stjudecloud/workflows:/workflows/dnaseq/dnaseq-standard.wdl"
 message = "dnaseq-standard.wdl:126:5: warning[RuntimeSectionKeys]: the following runtime key is not reserved in the WDL v1.1 specification: `disk`; therefore, its inclusion in the `runtime` section is deprecated"
@@ -1750,11 +1735,6 @@ document = "stjudecloud/workflows:/workflows/general/alignment-post.wdl"
 message = "alignment-post.wdl:78:7: note[Todo]: remaining `TODO` item found"
 permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/general/alignment-post.wdl/#L78"
 
-[[diagnostics]]
-document = "stjudecloud/workflows:/workflows/general/alignment-post.wdl"
-message = "alignment-post.wdl:8:10: note[SectionOrdering]: sections are not in order for workflow `alignment_post`"
-permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/general/alignment-post.wdl/#L8"
-
 [[diagnostics]]
 document = "stjudecloud/workflows:/workflows/general/samtools-merge.wdl"
 message = "samtools-merge.wdl:23:1: note[LineWidth]: line exceeds maximum width of 90"
@@ -1770,11 +1750,6 @@ document = "stjudecloud/workflows:/workflows/general/samtools-merge.wdl"
 message = "samtools-merge.wdl:35:1: note[LineWidth]: line exceeds maximum width of 90"
 permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/general/samtools-merge.wdl/#L35"
 
-[[diagnostics]]
-document = "stjudecloud/workflows:/workflows/general/samtools-merge.wdl"
-message = "samtools-merge.wdl:7:10: note[SectionOrdering]: sections are not in order for workflow `samtools_merge`"
-permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/general/samtools-merge.wdl/#L7"
-
 [[diagnostics]]
 document = "stjudecloud/workflows:/workflows/qc/quality-check-standard.wdl"
 message = "quality-check-standard.wdl:111:1: note[LineWidth]: line exceeds maximum width of 90"
@@ -1790,11 +1765,6 @@ document = "stjudecloud/workflows:/workflows/qc/quality-check-standard.wdl"
 message = "quality-check-standard.wdl:151:1: note[LineWidth]: line exceeds maximum width of 90"
 permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/qc/quality-check-standard.wdl/#L151"
 
-[[diagnostics]]
-document = "stjudecloud/workflows:/workflows/qc/quality-check-standard.wdl"
-message = "quality-check-standard.wdl:18:10: note[SectionOrdering]: sections are not in order for workflow `quality_check`"
-permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/qc/quality-check-standard.wdl/#L18"
-
 [[diagnostics]]
 document = "stjudecloud/workflows:/workflows/qc/quality-check-standard.wdl"
 message = "quality-check-standard.wdl:23:9: warning[NonmatchingOutput]: `outputs` section of `meta` for the workflow `quality_check` is out of order"
@@ -1825,16 +1795,6 @@ document = "stjudecloud/workflows:/workflows/qc/quality-check-standard.wdl"
 message = "quality-check-standard.wdl:558:3: note[Todo]: remaining `TODO` item found"
 permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/qc/quality-check-standard.wdl/#L558"
 
-[[diagnostics]]
-document = "stjudecloud/workflows:/workflows/reference/gatk-reference.wdl"
-message = "gatk-reference.wdl:7:10: note[SectionOrdering]: sections are not in order for workflow `gatk_reference`"
-permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/reference/gatk-reference.wdl/#L7"
-
-[[diagnostics]]
-document = "stjudecloud/workflows:/workflows/reference/make-qc-reference.wdl"
-message = "make-qc-reference.wdl:6:10: note[SectionOrdering]: sections are not in order for workflow `make_qc_reference`"
-permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/reference/make-qc-reference.wdl/#L6"
-
 [[diagnostics]]
 document = "stjudecloud/workflows:/workflows/rnaseq/rnaseq-core.wdl"
 message = "rnaseq-core.wdl:38:48: note[Todo]: remaining `TODO` item found"
@@ -1845,11 +1805,6 @@ document = "stjudecloud/workflows:/workflows/rnaseq/rnaseq-core.wdl"
 message = "rnaseq-core.wdl:87:34: note[Todo]: remaining `TODO` item found"
 permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/rnaseq/rnaseq-core.wdl/#L87"
 
-[[diagnostics]]
-document = "stjudecloud/workflows:/workflows/rnaseq/rnaseq-standard-fastq.wdl"
-message = "rnaseq-standard-fastq.wdl:25:10: note[SectionOrdering]: sections are not in order for workflow `rnaseq_standard_fastq`"
-permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/rnaseq/rnaseq-standard-fastq.wdl/#L25"
-
 [[diagnostics]]
 document = "stjudecloud/workflows:/workflows/rnaseq/rnaseq-standard-fastq.wdl"
 message = "rnaseq-standard-fastq.wdl:47:533: note[Todo]: remaining `TODO` item found"
@@ -1865,16 +1820,6 @@ document = "stjudecloud/workflows:/workflows/rnaseq/rnaseq-standard.wdl"
 message = "rnaseq-standard.wdl:94:10: note[Todo]: remaining `TODO` item found"
 permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/rnaseq/rnaseq-standard.wdl/#L94"
 
-[[diagnostics]]
-document = "stjudecloud/workflows:/workflows/rnaseq/rnaseq-standard.wdl"
-message = "rnaseq-standard.wdl:9:10: note[SectionOrdering]: sections are not in order for workflow `rnaseq_standard`"
-permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/rnaseq/rnaseq-standard.wdl/#L9"
-
-[[diagnostics]]
-document = "stjudecloud/workflows:/workflows/rnaseq/rnaseq-variant-calling.wdl"
-message = "rnaseq-variant-calling.wdl:6:10: note[SectionOrdering]: sections are not in order for workflow `rnaseq_variant_calling`"
-permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/rnaseq/rnaseq-variant-calling.wdl/#L6"
-
 [[diagnostics]]
 document = "stjudecloud/workflows:/workflows/scrnaseq/10x-bam-to-fastqs.wdl"
 message = "10x-bam-to-fastqs.wdl:130:5: warning[RuntimeSectionKeys]: the following runtime key is not reserved in the WDL v1.1 specification: `disk`; therefore, its inclusion in the `runtime` section is deprecated"
@@ -1950,11 +1895,6 @@ document = "stjudecloud/workflows:/workflows/scrnaseq/scrnaseq-standard.wdl"
 message = "scrnaseq-standard.wdl:28:1: note[LineWidth]: line exceeds maximum width of 90"
 permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/scrnaseq/scrnaseq-standard.wdl/#L28"
 
-[[diagnostics]]
-document = "stjudecloud/workflows:/workflows/scrnaseq/scrnaseq-standard.wdl"
-message = "scrnaseq-standard.wdl:40:10: note[SectionOrdering]: sections are not in order for workflow `scrnaseq_standard`"
-permalink = "https://github.com/stjudecloud/workflows/blob/833b97fb582fe7d4f6df9d29a645d0289f1fc92e/workflows/scrnaseq/scrnaseq-standard.wdl/#L40"
-
 [[diagnostics]]
 document = "stjudecloud/workflows:/workflows/scrnaseq/scrnaseq-standard.wdl"
 message = "scrnaseq-standard.wdl:5:1: note[LineWidth]: line exceeds maximum width of 90"

diff --git a/wdl-lint/CHANGELOG.md b/wdl-lint/CHANGELOG.md
@@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+* Fixed a bug in `SectionOrder` that caused false positives to fire
+  ([#129](https://github.com/stjude-rust-labs/wdl/pull/129))
 * Fixed a bug in the `PreambleWhitespace` rule that would cause it to fire if
   there is only a single blank line after the version statement remaining in
   the document ([#110](https://github.com/stjude-rust-labs/wdl/pull/110)).

diff --git a/wdl-lint/src/rules/section_order.rs b/wdl-lint/src/rules/section_order.rs
@@ -63,11 +63,11 @@ impl Rule for SectionOrderingRule {
     }
 
     fn explanation(&self) -> &'static str {
-        "For workflows, the following sections must be present and in this order: meta, \
+        "For workflows, if present, the following sections must be in this order: meta, \
          parameter_meta, input, (body), output. \"(body)\" represents all calls and declarations.
 
-        For tasks, the following sections must be present and in this order: meta, parameter_meta, \
-         input, (private declarations), command, output, requirements/runtime"
+        For tasks, if present, the following sections must be in this order: meta, parameter_meta, \
+         input, (private declarations), command, output, runtime, requirements, hints."
     }
 
     fn tags(&self) -> TagSet {
@@ -86,15 +86,19 @@ enum State {
     ParameterMeta,
     /// The input section.
     Input,
-    /// The declaration section. Overloaded to include call and scatter
-    /// statements in workflows.
+    /// The declaration section. Overloaded to include call, scatter,
+    /// and conditional statements in workflows.
     Decl,
     /// The command section.
     Command,
     /// The output section.
     Output,
-    /// The requirements/runtime section.
+    /// The runtime section (only in tasks).
+    Runtime,
+    /// The requirements section (only in tasks).
     Requirements,
+    /// The hints section.
+    Hints,
 }
 
 impl Visitor for SectionOrderingRule {
@@ -146,11 +150,15 @@ impl Visitor for SectionOrderingRule {
                 TaskItem::Output(_) if encountered <= State::Output => {
                     encountered = State::Output;
                 }
-                TaskItem::Requirements(_) | TaskItem::Runtime(_)
-                    if encountered <= State::Requirements =>
-                {
+                TaskItem::Runtime(_) if encountered <= State::Runtime => {
+                    encountered = State::Runtime;
+                }
+                TaskItem::Requirements(_) if encountered <= State::Requirements => {
                     encountered = State::Requirements;
                 }
+                TaskItem::Hints(_) if encountered <= State::Hints => {
+                    encountered = State::Hints;
+                }
                 _ => {
                     state.add(task_section_order(
                         task.name().span(),
@@ -185,14 +193,20 @@ impl Visitor for SectionOrderingRule {
                 WorkflowItem::Input(_) if encountered <= State::Input => {
                     encountered = State::Input;
                 }
-                WorkflowItem::Declaration(_) | WorkflowItem::Call(_) | WorkflowItem::Scatter(_)
+                WorkflowItem::Declaration(_)
+                | WorkflowItem::Call(_)
+                | WorkflowItem::Scatter(_)
+                | WorkflowItem::Conditional(_)
                     if encountered <= State::Decl =>
                 {
                     encountered = State::Decl;
                 }
                 WorkflowItem::Output(_) if encountered <= State::Output => {
                     encountered = State::Output;
                 }
+                WorkflowItem::Hints(_) if encountered <= State::Hints => {
+                    encountered = State::Hints;
+                }
                 _ => {
                     state.add(workflow_section_order(
                         workflow.name().span(),