Skip to content

Commit

Permalink
feat: port the MatchingParameterMeta rule to wdl-lint.
Browse files Browse the repository at this point in the history
This commit ports the "matching parameter meta" rule to `wdl-lint`.
  • Loading branch information
peterhuene committed Jun 10, 2024
1 parent deea4c6 commit 2e92905
Show file tree
Hide file tree
Showing 6 changed files with 276 additions and 0 deletions.
26 changes: 26 additions & 0 deletions wdl-ast/src/experimental/v1/decls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,32 @@ pub enum Decl {
}

impl Decl {
/// Gets the type of the declaration.
pub fn ty(&self) -> Type {
match self {
Self::Bound(d) => d.ty(),
Self::Unbound(d) => d.ty(),
}
}

/// Gets the name of the declaration.
pub fn name(&self) -> Ident {
match self {
Self::Bound(d) => d.name(),
Self::Unbound(d) => d.name(),
}
}

/// Gets the expression of the declaration.
///
/// Returns `None` for unbound declarations.
pub fn expr(&self) -> Option<Expr> {
match self {
Self::Bound(d) => Some(d.expr()),
Self::Unbound(_) => None,
}
}

/// Unwraps the declaration into a bound declaration.
///
/// # Panics
Expand Down
1 change: 1 addition & 0 deletions wdl-lint/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

* Ported the `MatchingParameterMeta` rule to `wdl-lint` ([#73](https://github.com/stjude-rust-labs/wdl/pull/73))
* Ported the `PreambleWhitespace` and `PreambleComments` rules to `wdl-lint`
([#72](https://github.com/stjude-rust-labs/wdl/pull/72))
* Ported the `SnakeCase` rule to `wdl-lint` ([#71](https://github.com/stjude-rust-labs/wdl/pull/71)).
Expand Down
3 changes: 3 additions & 0 deletions wdl-lint/src/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::TagSet;

mod double_quotes;
mod ending_newline;
mod matching_parameter_meta;
mod missing_runtime;
mod no_curly_commands;
mod preamble_comments;
Expand All @@ -17,6 +18,7 @@ mod snake_case;

pub use double_quotes::*;
pub use ending_newline::*;
pub use matching_parameter_meta::*;
pub use missing_runtime::*;
pub use no_curly_commands::*;
pub use preamble_comments::*;
Expand Down Expand Up @@ -61,6 +63,7 @@ pub fn rules() -> Vec<Box<dyn Rule>> {
Box::new(EndingNewlineRule),
Box::new(PreambleWhitespaceRule),
Box::new(PreambleCommentsRule),
Box::new(MatchingParameterMetaRule),
];

// Ensure all the rule ids are unique and pascal case
Expand Down
180 changes: 180 additions & 0 deletions wdl-lint/src/v1/matching_parameter_meta.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
//! A lint rule for matching parameter metadata.
use std::collections::HashMap;

use wdl_ast::experimental::v1::InputSection;
use wdl_ast::experimental::v1::ParameterMetadataSection;
use wdl_ast::experimental::v1::TaskDefinition;
use wdl_ast::experimental::v1::TaskOrWorkflow;
use wdl_ast::experimental::v1::Visitor;
use wdl_ast::experimental::v1::WorkflowDefinition;
use wdl_ast::experimental::AstToken;
use wdl_ast::experimental::Diagnostic;
use wdl_ast::experimental::Diagnostics;
use wdl_ast::experimental::Span;
use wdl_ast::experimental::VisitReason;

use super::Rule;
use crate::Tag;
use crate::TagSet;

/// The identifier for the matching parameter meta rule.
const ID: &str = "MatchingParameterMeta";

/// Creates a "missing param meta" diagnostic.
fn missing_param_meta(parent: &TaskOrWorkflow, missing: &str, span: Span) -> Diagnostic {
let (context, parent) = match parent {
TaskOrWorkflow::Task(t) => ("task", t.name()),
TaskOrWorkflow::Workflow(w) => ("workflow", w.name()),
};

Diagnostic::warning(format!(
"{context} `{parent}` is missing a parameter metadata key for input `{missing}`",
parent = parent.as_str(),
))
.with_rule(ID)
.with_label(
"this input does not have an entry in the parameter metadata section",
span,
)
.with_fix(format!(
"add a `{missing}` key to the `parameter_meta` section with a detailed description of the \
input.",
))
}

/// Creates an "extra param meta" diagnostic.
fn extra_param_meta(parent: &TaskOrWorkflow, extra: &str, span: Span) -> Diagnostic {
let (context, parent) = match parent {
TaskOrWorkflow::Task(t) => ("task", t.name()),
TaskOrWorkflow::Workflow(w) => ("workflow", w.name()),
};

Diagnostic::warning(format!(
"{context} `{parent}` has an extraneous parameter metadata key named `{extra}`",
parent = parent.as_str(),
))
.with_rule(ID)
.with_label(
"this key does not correspond to any input declaration",
span,
)
.with_fix("remove the extraneous parameter metadata entry")
}

/// Detects missing or extraneous entries in a `parameter_meta` section.
#[derive(Debug, Clone, Copy)]
pub struct MatchingParameterMetaRule;

impl Rule for MatchingParameterMetaRule {
fn id(&self) -> &'static str {
ID
}

fn description(&self) -> &'static str {
"Ensures that inputs have a matching entry in a `parameter_meta` section."
}

fn explanation(&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 tags(&self) -> TagSet {
TagSet::new(&[Tag::Completeness])
}

fn visitor(&self) -> Box<dyn Visitor<State = Diagnostics>> {
Box::new(MatchingParameterMetaVisitor)
}
}

/// Checks for both missing and extra items in a `parameter_meta` section.
fn check_parameter_meta(
parent: TaskOrWorkflow,
inputs: Option<InputSection>,
param_meta: Option<ParameterMetadataSection>,
diagnostics: &mut Diagnostics,
) {
let expected: HashMap<_, _> = inputs
.iter()
.flat_map(|i| {
i.declarations().map(|d| {
let name = d.name();
(name.as_str().to_string(), name.span())
})
})
.collect();

let actual: HashMap<_, _> = param_meta
.iter()
.flat_map(|m| {
m.items().map(|i| {
let name = i.name();
(name.as_str().to_string(), name.span())
})
})
.collect();

for (name, span) in &expected {
if !actual.contains_key(name) {
diagnostics.add(missing_param_meta(&parent, name, *span));
}
}

for (name, span) in &actual {
if !expected.contains_key(name) {
diagnostics.add(extra_param_meta(&parent, name, *span));
}
}
}

/// Implements the visitor for the matching parameter meta rule.
struct MatchingParameterMetaVisitor;

impl Visitor for MatchingParameterMetaVisitor {
type State = Diagnostics;

fn task_definition(
&mut self,
state: &mut Self::State,
reason: VisitReason,
task: &TaskDefinition,
) {
if reason == VisitReason::Exit {
return;
}

// Check the parameter metadata of the task
// Note that only the first input and parameter_meta sections are checked as any
// additional sections is considered a validation error
check_parameter_meta(
TaskOrWorkflow::Task(task.clone()),
task.inputs().next(),
task.parameter_metadata().next(),
state,
);
}

fn workflow_definition(
&mut self,
state: &mut Self::State,
reason: VisitReason,
workflow: &WorkflowDefinition,
) {
if reason == VisitReason::Exit {
return;
}

// Check the parameter metadata of the workflow
// Note that only the first input and parameter_meta sections are checked as any
// additional sections is considered a validation error
check_parameter_meta(
TaskOrWorkflow::Workflow(workflow.clone()),
workflow.inputs().next(),
workflow.parameter_metadata().next(),
state,
);
}
}
32 changes: 32 additions & 0 deletions wdl-lint/tests/lints/matching-param-meta/source.errors
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
warning: task `t` is missing a parameter metadata key for input `does_not_exist` [rule: MatchingParameterMeta]
┌─ tests/lints/matching-param-meta/source.wdl:8:16
8 │ String does_not_exist
│ ^^^^^^^^^^^^^^ this input does not have an entry in the parameter metadata section
= fix: add a `does_not_exist` key to the `parameter_meta` section with a detailed description of the input.

warning: task `t` has an extraneous parameter metadata key named `extra` [rule: MatchingParameterMeta]
┌─ tests/lints/matching-param-meta/source.wdl:15:9
15 │ extra: "this should not be here"
│ ^^^^^ this key does not correspond to any input declaration
= fix: remove the extraneous parameter metadata entry

warning: workflow `w` is missing a parameter metadata key for input `does_not_exist` [rule: MatchingParameterMeta]
┌─ tests/lints/matching-param-meta/source.wdl:25:16
25 │ String does_not_exist
│ ^^^^^^^^^^^^^^ this input does not have an entry in the parameter metadata section
= fix: add a `does_not_exist` key to the `parameter_meta` section with a detailed description of the input.

warning: workflow `w` has an extraneous parameter metadata key named `extra` [rule: MatchingParameterMeta]
┌─ tests/lints/matching-param-meta/source.wdl:32:9
32 │ extra: "this should not be here"
│ ^^^^^ this key does not correspond to any input declaration
= fix: remove the extraneous parameter metadata entry

34 changes: 34 additions & 0 deletions wdl-lint/tests/lints/matching-param-meta/source.wdl
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
## This is a test for checking for missing and extraneous entries in a `parameter_meta` section.
version 1.1

task t {
input {
String matching
String does_not_exist
}

parameter_meta {
matching: {
help: "a matching parameter!"
}
extra: "this should not be here"
}

runtime {}
command <<<>>>
}

workflow w {
input {
String matching
String does_not_exist
}

parameter_meta {
matching: {
help: "a matching parameter!"
}
extra: "this should not be here"
}
}

0 comments on commit 2e92905

Please sign in to comment.