feat: add non-matching output lint rule

Gauntlet.toml

@@ -20,7 +20,7 @@ commit_hash = "4536dd2c6719b86f4c50348f0ce354818260cadb"
 
 [repositories."broadinstitute/palantir-workflows"]
 identifier = "broadinstitute/palantir-workflows"
-commit_hash = "1e32078d3b57dcb2291534d0caa30f600d40967e"
+commit_hash = "4e7fdfa7992dd1bf4c4721e77c81e4d7b8ecf4fe"
 
 [repositories."broadinstitute/warp"]
 identifier = "broadinstitute/warp"
@@ -379,7 +379,7 @@ permalink = "https://github.com/biowdl/tasks/blob/2bf875300d90a3c9c8d670b3d99026
 [[diagnostics]]
 document = "broadinstitute/palantir-workflows:/HaplotypeMap/BuildHaplotypeMap.wdl"
 message = "BuildHaplotypeMap.wdl:17:1: error: a WDL document must start with a version statement"
-permalink = "https://github.com/broadinstitute/palantir-workflows/blob/1e32078d3b57dcb2291534d0caa30f600d40967e/HaplotypeMap/BuildHaplotypeMap.wdl/#L17"
+permalink = "https://github.com/broadinstitute/palantir-workflows/blob/4e7fdfa7992dd1bf4c4721e77c81e4d7b8ecf4fe/HaplotypeMap/BuildHaplotypeMap.wdl/#L17"
 
 [[diagnostics]]
 document = "broadinstitute/warp:/pipelines/skylab/scATAC/scATAC.wdl"

wdl-lint/CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * Added the `SectionOrdering` lint rule ([#109](https://github.com/stjude-rust-labs/wdl/pull/109)).
 * Added the `DeprecatedObject` lint rule ([#112](https://github.com/stjude-rust-labs/wdl/pull/112)).
 * Added the `DescriptionMissing` lint rule ([#113](https://github.com/stjude-rust-labs/wdl/pull/113)).
+* Added the `NonmatchingOutput` lint rule ([#114](https://github.com/stjude-rust-labs/wdl/pull/114)).
 
 ### Fixed
 

wdl-lint/src/lib.rs

@@ -92,6 +92,7 @@ pub fn rules() -> Vec<Box<dyn Rule>> {
         Box::new(rules::SectionOrderingRule),
         Box::new(rules::DeprecatedObjectRule),
         Box::new(rules::DescriptionMissingRule),
+        Box::new(rules::NonmatchingOutputRule),
     ];
 
     // Ensure all the rule ids are unique and pascal case

wdl-lint/src/rules.rs

@@ -17,6 +17,7 @@ mod missing_metas;
 mod missing_output;
 mod missing_runtime;
 mod no_curly_commands;
+mod nonmatching_output;
 mod pascal_case;
 mod preamble_comments;
 mod preamble_whitespace;
@@ -41,6 +42,7 @@ pub use missing_metas::*;
 pub use missing_output::*;
 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::*;

wdl-lint/src/rules/nonmatching_output.rs

@@ -0,0 +1,249 @@
+//! A lint rule to ensure each output is documented in `meta`.
+
+use indexmap::IndexMap;
+use wdl_ast::v1::MetadataSection;
+use wdl_ast::v1::OutputSection;
+use wdl_ast::v1::TaskDefinition;
+use wdl_ast::v1::TaskOrWorkflow;
+use wdl_ast::v1::WorkflowDefinition;
+use wdl_ast::AstNode;
+use wdl_ast::AstToken;
+use wdl_ast::Diagnostic;
+use wdl_ast::Diagnostics;
+use wdl_ast::Document;
+use wdl_ast::Span;
+use wdl_ast::ToSpan;
+use wdl_ast::VisitReason;
+use wdl_ast::Visitor;
+
+use crate::Rule;
+use crate::Tag;
+use crate::TagSet;
+
+/// The identifier for the non-matching output rule.
+const ID: &str = "NonmatchingOutput";
+
+/// Creates a "non-matching output" diagnostic.
+fn nonmatching_output(span: Span, name: &str, context: &TaskOrWorkflow) -> Diagnostic {
+    let (ty, item_name) = match context {
+        TaskOrWorkflow::Task(t) => ("task", t.name().as_str().to_string()),
+        TaskOrWorkflow::Workflow(w) => ("workflow", w.name().as_str().to_string()),
+    };
+
+    Diagnostic::warning(format!(
+        "output `{name}` is missing from `meta` section in {ty} `{item_name}`"
+    ))
+    .with_rule(ID)
+    .with_highlight(span)
+    .with_fix(format!(
+        "add output (`{name}`) key to `outputs` documentation in `meta` section"
+    ))
+}
+
+/// Creates a missing outputs in meta diagnostic.
+fn missing_outputs_in_meta(span: Span, context: &TaskOrWorkflow) -> Diagnostic {
+    let (ty, name) = match context {
+        TaskOrWorkflow::Task(t) => ("task", t.name().as_str().to_string()),
+        TaskOrWorkflow::Workflow(w) => ("workflow", w.name().as_str().to_string()),
+    };
+
+    Diagnostic::warning(format!(
+        "`outputs` key missing in `meta` section in {ty} `{name}`"
+    ))
+    .with_rule(ID)
+    .with_highlight(span)
+    .with_fix("add `outputs` key to `meta` section")
+}
+
+/// Creates a diagnostic for extra `meta` entries.
+fn extra_output_in_meta(span: Span, name: &str, context: &TaskOrWorkflow) -> Diagnostic {
+    let (ty, item_name) = match context {
+        TaskOrWorkflow::Task(t) => ("task", t.name().as_str().to_string()),
+        TaskOrWorkflow::Workflow(w) => ("workflow", w.name().as_str().to_string()),
+    };
+
+    Diagnostic::warning(format!(
+        "`{name}` appears in `outputs` section of {ty} `{item_name}` but is not a declared \
+         `output`"
+    ))
+    .with_rule(ID)
+    .with_highlight(span)
+    .with_fix(format!(
+        "remove `{name}` from `outputs` key in `meta` section"
+    ))
+}
+
+/// Creates a diagnostic for out-of-order entries.
+fn out_of_order(span: Span, output_span: Span, context: &TaskOrWorkflow) -> Diagnostic {
+    let (ty, item_name) = match context {
+        TaskOrWorkflow::Task(t) => ("task", t.name().as_str().to_string()),
+        TaskOrWorkflow::Workflow(w) => ("workflow", w.name().as_str().to_string()),
+    };
+
+    Diagnostic::warning(format!(
+        "`outputs` section of `meta` for {ty} `{item_name}` is out of order"
+    ))
+    .with_rule(ID)
+    .with_highlight(span)
+    .with_highlight(output_span)
+    .with_fix("the `outputs` keys of `meta` section must match the order in `output`")
+}
+
+/// Detects non-matching outputs.
+#[derive(Default, Debug, Clone, Copy)]
+pub struct NonmatchingOutputRule;
+
+impl Rule for NonmatchingOutputRule {
+    fn id(&self) -> &'static str {
+        ID
+    }
+
+    fn description(&self) -> &'static str {
+        "Ensures that each output field is documented in the meta section."
+    }
+
+    fn explanation(&self) -> &'static str {
+        "The meta section should have an output key and keys with descriptions for each output of \
+         the task/workflow. These must match exactly. i.e. for each named output of a task or \
+         workflow, there should be an entry under meta.output with that same name. Additionally, \
+         these entries should be in the same order (that order is up to the developer to decide). \
+         No extraneous output entries are allowed."
+    }
+
+    fn tags(&self) -> TagSet {
+        TagSet::new(&[Tag::Completeness])
+    }
+}
+
+/// Check each output key exists in the outputs key within the `meta` section.
+fn check_output_meta(
+    state: &mut Diagnostics,
+    meta: &MetadataSection,
+    outputs: &OutputSection,
+    context: TaskOrWorkflow,
+) {
+    // Get the output section from the meta section.
+    if let Some(meta_outputs_key) = meta
+        .items()
+        .find(|entry| entry.name().syntax().to_string() == "outputs")
+    {
+        let actual: IndexMap<_, _> = meta_outputs_key
+            .value()
+            .unwrap_object()
+            .items()
+            .map(|entry| {
+                (
+                    entry.name().syntax().to_string(),
+                    entry.syntax().text_range().to_span(),
+                )
+            })
+            .collect();
+
+        // Get the declared outputs.
+        let expected: IndexMap<_, _> = outputs
+            .declarations()
+            .map(|d| {
+                (
+                    d.name().syntax().to_string(),
+                    d.syntax().text_range().to_span(),
+                )
+            })
+            .collect();
+
+        let mut extra_found = false;
+
+        // Check for entries missing from `meta`.
+        for (name, span) in &expected {
+            if !actual.contains_key(name) {
+                if !extra_found {
+                    extra_found = true;
+                }
+                state.add(nonmatching_output(*span, name, &context));
+            }
+        }
+
+        // Check for extra entries in `meta`.
+        for (name, span) in &actual {
+            if !expected.contains_key(name) {
+                if !extra_found {
+                    extra_found = true;
+                }
+                state.add(extra_output_in_meta(*span, name, &context));
+            }
+        }
+
+        // Check for out-of-order entries.
+        if !extra_found && !actual.keys().eq(expected.keys()) {
+            state.add(out_of_order(
+                meta_outputs_key.syntax().text_range().to_span(),
+                outputs.syntax().text_range().to_span(),
+                &context,
+            ));
+        }
+    } else {
+        state.add(missing_outputs_in_meta(
+            meta.syntax().first_token().unwrap().text_range().to_span(),
+            &context,
+        ));
+    }
+}
+
+impl Visitor for NonmatchingOutputRule {
+    type State = Diagnostics;
+
+    fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) {
+        if reason == VisitReason::Exit {
+            return;
+        }
+
+        // Reset the visitor upon document entry
+        *self = Default::default();
+    }
+
+    fn workflow_definition(
+        &mut self,
+        state: &mut Self::State,
+        reason: VisitReason,
+        workflow: &WorkflowDefinition,
+    ) {
+        if reason == VisitReason::Exit {
+            return;
+        }
+
+        // If the `meta` section is missing, the MissingMeta rule will handle it.
+        if let Some(meta) = workflow.metadata().next() {
+            // If the `output` section is missing, the MissingOutput rule will handle it.
+            if let Some(output) = workflow.outputs().next() {
+                if output.declarations().count() > 0 {
+                    check_output_meta(
+                        state,
+                        &meta,
+                        &output,
+                        TaskOrWorkflow::Workflow(workflow.clone()),
+                    );
+                }
+            }
+        }
+    }
+
+    fn task_definition(
+        &mut self,
+        state: &mut Self::State,
+        reason: VisitReason,
+        task: &TaskDefinition,
+    ) {
+        if reason == VisitReason::Exit {
+            return;
+        }
+
+        // If the `meta` section is missing, the MissingMeta rule will handle it.
+        if let Some(meta) = task.metadata().next() {
+            // If the `output` section is missing, the MissingOutput rule will handle it.
+            if let Some(output) = task.outputs().next() {
+                if output.declarations().count() > 0 {
+                    check_output_meta(state, &meta, &output, TaskOrWorkflow::Task(task.clone()));
+                }
+            }
+        }
+    }
+}

wdl-lint/tests/lints/command-mixed-line-cont/source.wdl

@@ -1,4 +1,4 @@
-#@ except: NoCurlyCommands, DescriptionMissing
+#@ except: DescriptionMissing, NoCurlyCommands, NonmatchingOutput
 ## This is a test of having mixed indentation in a line continuation.
 
 version 1.1

wdl-lint/tests/lints/command-mixed-spaces-first/source.wdl

@@ -1,4 +1,4 @@
-#@ except: NoCurlyCommands, DescriptionMissing
+#@ except: DescriptionMissing, NoCurlyCommands
 ## This is a test of having spaces before tabs in command sections.
 
 version 1.1

wdl-lint/tests/lints/command-mixed-tabs-first/source.wdl

@@ -1,4 +1,4 @@
-#@ except: NoCurlyCommands, DescriptionMissing
+#@ except: DescriptionMissing, NoCurlyCommands
 ## This is a test of having tabs before spaces in command sections.
 
 version 1.1

wdl-lint/tests/lints/command-mixed-trailing/source.wdl

@@ -1,4 +1,4 @@
-#@ except: NoCurlyCommands, DescriptionMissing
+#@ except: DescriptionMissing, NoCurlyCommands
 ## This is a test of having mixed _trailing_ indentation in command sections.
 ## There should be no warnings from the `CommandSectionMixedIndentation` rule.
 

wdl-lint/tests/lints/command-mixed-ws-ok/source.wdl

@@ -1,4 +1,4 @@
-#@ except: NoCurlyCommands, DescriptionMissing
+#@ except: DescriptionMissing, NoCurlyCommands
 ## This is a test of having mixed indentation inside of a placeholder.
 ## This should not cause a warning for the `CommandSectionMixedIndentation` rule.
 

wdl-lint/tests/lints/deprecated-object/source.wdl

@@ -1,4 +1,4 @@
-#@ except: DescriptionMissing, MissingMetas, SectionOrdering
+#@ except: DescriptionMissing, MissingMetas, NonmatchingOutput, SectionOrdering
 ## This is a test of the `DeprecatedObject` lint
 
 version 1.1

wdl-lint/tests/lints/missing-preamble-ws/source.errors

@@ -0,0 +1,10 @@
+note[PreambleWhitespace]: expected a blank line before the version statement
+  ┌─ tests/lints/missing-preamble-ws/source.wdl:2:50
+  │  
+2 │   ## This is a test of missing preamble whitespace.
+  │ ╭─────────────────────────────────────────────────^
+3 │ │ version 1.1
+  │ ╰^
+  │  
+  = fix: add a blank line between the last preamble comment and the version statement
+

wdl-lint/tests/lints/missing-preamble-ws/source.wdl

@@ -1,6 +1,5 @@
 #@ except: DescriptionMissing
 ## This is a test of missing preamble whitespace.
-
 version 1.1
 
 workflow text {

wdl-lint/tests/lints/nonmatching-output/source.errors

@@ -0,0 +1,41 @@
+warning[NonmatchingOutput]: `outputs` key missing in `meta` section in task `bar`
+   ┌─ tests/lints/nonmatching-output/source.wdl:18:5
+   │
+18 │     meta {}
+   │     ^^^^
+   │
+   = fix: add `outputs` key to `meta` section
+
+warning[NonmatchingOutput]: output `t` is missing from `meta` section in task `baz`
+   ┌─ tests/lints/nonmatching-output/source.wdl:34:9
+   │
+34 │         String t = "world"
+   │         ^^^^^^^^^^^^^^^^^^
+   │
+   = fix: add output (`t`) key to `outputs` documentation in `meta` section
+
+warning[NonmatchingOutput]: `outputs` section of `meta` for task `qux` is out of order
+   ┌─ tests/lints/nonmatching-output/source.wdl:40:9
+   │    
+40 │ ╭           outputs: {
+41 │ │               t: "t",
+42 │ │               s: "s",
+43 │ │           }
+   │ ╰───────────^
+   · │  
+46 │   ╭     output {
+47 │   │         String s = "hello"
+48 │   │         String t = "world"
+49 │   │     }
+   │   ╰─────'
+   │    
+   = fix: the `outputs` keys of `meta` section must match the order in `output`
+
+warning[NonmatchingOutput]: `v` appears in `outputs` section of task `quux` but is not a declared `output`
+   ┌─ tests/lints/nonmatching-output/source.wdl:57:13
+   │
+57 │             v: "v"
+   │             ^^^^^^
+   │
+   = fix: remove `v` from `outputs` key in `meta` section
+