feat: implement the env declaration modifier in WDL 1.2.

wdl-analysis/CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+* Added analysis support for the WDL 1.2 `env` declaration modifier ([#296](https://github.com/stjude-rust-labs/wdl/pull/296)).
 * Fixed missing diagnostic for unknown local name when using the abbreviated
   syntax for specifying a call input ([#292](https://github.com/stjude-rust-labs/wdl/pull/292))
 * Added functions for getting type information of task requirements and hints ([#241](https://github.com/stjude-rust-labs/wdl/pull/241)).

wdl-analysis/src/document/v1.rs

@@ -19,7 +19,6 @@ use wdl_ast::Span;
 use wdl_ast::SupportedVersion;
 use wdl_ast::SyntaxNode;
 use wdl_ast::SyntaxNodeExt;
-use wdl_ast::ToSpan;
 use wdl_ast::TokenStrHash;
 use wdl_ast::Version;
 use wdl_ast::v1::Ast;
@@ -269,7 +268,7 @@ fn add_namespace(
     };
 
     // Check for conflicting namespaces
-    let span = import.uri().syntax().text_range().to_span();
+    let span = import.uri().span();
     let ns = match import.namespace() {
         Some((ns, span)) => {
             if let Some(prev) = document.namespaces.get(&ns) {
@@ -589,10 +588,12 @@ fn add_task(config: DiagnosticsConfig, document: &mut Document, definition: &Tas
                 // Check for unused input
                 if let Some(severity) = config.unused_input {
                     let name = decl.name();
-                    if graph
-                        .edges_directed(index, Direction::Outgoing)
-                        .next()
-                        .is_none()
+                    // Don't warn for environment variables as they are always implicitly used
+                    if decl.env().is_none()
+                        && graph
+                            .edges_directed(index, Direction::Outgoing)
+                            .next()
+                            .is_none()
                     {
                         // Determine if the input is really used based on its name and type
                         if is_input_used(name.as_str(), &task.inputs[name.as_str()].ty) {
@@ -621,10 +622,12 @@ fn add_task(config: DiagnosticsConfig, document: &mut Document, definition: &Tas
                 // Check for unused declaration
                 if let Some(severity) = config.unused_declaration {
                     let name = decl.name();
-                    if graph
-                        .edges_directed(index, Direction::Outgoing)
-                        .next()
-                        .is_none()
+                    // Don't warn for environment variables as they are always implicitly used
+                    if decl.env().is_none()
+                        && graph
+                            .edges_directed(index, Direction::Outgoing)
+                            .next()
+                            .is_none()
                         && !decl.syntax().is_rule_excepted(UNUSED_DECL_RULE_ID)
                     {
                         document.diagnostics.push(
@@ -1319,7 +1322,7 @@ fn resolve_import(
     importer_version: &Version,
 ) -> Result<(Arc<Url>, Arc<Document>), Option<Diagnostic>> {
     let uri = stmt.uri();
-    let span = uri.syntax().text_range().to_span();
+    let span = uri.span();
     let text = match uri.text() {
         Some(text) => text,
         None => {

wdl-analysis/src/eval/v1.rs

@@ -201,7 +201,8 @@ impl TaskGraphBuilder {
         // Add reference edges again, but only for the output declaration nodes
         self.add_reference_edges(version, Some(count), &mut graph, diagnostics);
 
-        // Finally, add edges from the command to runtime/requirements/hints
+        // Finally, add edges from the command to runtime/requirements/hints and
+        // environment variables
         if let Some(command) = self.command {
             if let Some(runtime) = self.runtime {
                 graph.update_edge(runtime, command, ());
@@ -214,6 +215,19 @@ impl TaskGraphBuilder {
             if let Some(hints) = self.hints {
                 graph.update_edge(hints, command, ());
             }
+
+            // As environment variables are implicitly used by commands, add edges from the
+            // command to the environment variable declarations
+            for index in self.names.values() {
+                match &graph[*index] {
+                    TaskGraphNode::Input(decl) | TaskGraphNode::Decl(decl)
+                        if decl.env().is_some() =>
+                    {
+                        graph.update_edge(*index, command, ());
+                    }
+                    _ => continue,
+                }
+            }
         }
 
         graph

wdl-analysis/tests/analysis/env-vars/source.diagnostics

@@ -0,0 +1,6 @@
+warning[UnusedDeclaration]: unused declaration `d`
+   ┌─ tests/analysis/env-vars/source.wdl:13:12
+   │
+13 │     String d = ""
+   │            ^
+

wdl-analysis/tests/analysis/env-vars/source.wdl

@@ -0,0 +1,24 @@
+## This is a test of using task environment variables in WDL 1.2
+
+version 1.2
+
+task test {
+    input {
+        String a
+        env String b
+        env String c = ""
+    }
+
+    # This is unused because it is not referenced
+    String d = ""
+
+    # This is *not* unused because it is an environment variable,
+    # regardless of whether or not it's referenced
+    env String e = ""
+
+    command <<<
+        ~{a}
+        ~{b}
+        $c
+    >>>
+}

wdl-ast/CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+* Added AST support for the WDL 1.2 `env` declaration modifier ([#296](https://github.com/stjude-rust-labs/wdl/pull/296)).
 * Added `braced_scope_span` and `heredoc_scope_span` methods to `AstNodeExt` ([#292](https://github.com/stjude-rust-labs/wdl/pull/292))
 * Added constants for the task variable fields, task requirement names, and
   task hint names ([#265](https://github.com/stjude-rust-labs/wdl/pull/265)).

wdl-ast/src/element.rs

@@ -447,6 +447,8 @@ pub enum Token {
     DoubleQuote(DoubleQuote),
     /// The `else` keyword.
     ElseKeyword(ElseKeyword),
+    /// The `env` keyword.
+    EnvKeyword(EnvKeyword),
     /// The `==` symbol.
     Equal(Equal),
     /// The `!` symbol.
@@ -585,6 +587,7 @@ ast_element_impl!(
         dot(): Dot => Dot => Dot,
         double_quote(): DoubleQuote => DoubleQuote => DoubleQuote,
         else_keyword(): ElseKeyword => ElseKeyword => ElseKeyword,
+        env_keyword(): EnvKeyword => EnvKeyword => EnvKeyword,
         equal(): Equal => Equal => Equal,
         exclaimation(): Exclamation => Exclamation => Exclamation,
         exponentiation(): Exponentiation => Exponentiation => Exponentiation,

wdl-ast/src/v1/decls.rs

@@ -2,6 +2,7 @@
 
 use std::fmt;
 
+use super::EnvKeyword;
 use super::Expr;
 use crate::AstNode;
 use crate::AstToken;
@@ -763,6 +764,13 @@ impl fmt::Display for Type {
 pub struct UnboundDecl(pub(crate) SyntaxNode);
 
 impl UnboundDecl {
+    /// Gets the `env` token, if present.
+    ///
+    /// This may only return a token for task inputs (WDL 1.2+).
+    pub fn env(&self) -> Option<EnvKeyword> {
+        token(&self.0)
+    }
+
     /// Gets the type of the declaration.
     pub fn ty(&self) -> Type {
         Type::child(&self.0).expect("unbound declaration should have a type")
@@ -804,6 +812,14 @@ impl AstNode for UnboundDecl {
 pub struct BoundDecl(pub(crate) SyntaxNode);
 
 impl BoundDecl {
+    /// Gets the `env` token, if present.
+    ///
+    /// This may only return a token for task inputs and private declarations
+    /// (WDL 1.2+).
+    pub fn env(&self) -> Option<EnvKeyword> {
+        token(&self.0)
+    }
+
     /// Gets the type of the declaration.
     pub fn ty(&self) -> Type {
         Type::child(&self.0).expect("bound declaration should have a type")
@@ -889,6 +905,17 @@ impl Decl {
         }
     }
 
+    /// Gets the `env` token, if present.
+    ///
+    /// This may only return a token for task inputs and private declarations
+    /// (WDL 1.2+).
+    pub fn env(&self) -> Option<EnvKeyword> {
+        match self {
+            Self::Bound(d) => d.env(),
+            Self::Unbound(d) => d.env(),
+        }
+    }
+
     /// Gets the type of the declaration.
     pub fn ty(&self) -> Type {
         match self {

wdl-ast/src/v1/import.rs

@@ -13,13 +13,13 @@ use super::ImportKeyword;
 use super::LiteralString;
 use crate::AstChildren;
 use crate::AstNode;
+use crate::AstNodeExt;
 use crate::AstToken;
 use crate::Ident;
 use crate::Span;
 use crate::SyntaxElement;
 use crate::SyntaxKind;
 use crate::SyntaxNode;
-use crate::ToSpan;
 use crate::WorkflowDescriptionLanguage;
 use crate::support::child;
 use crate::support::children;
@@ -92,7 +92,7 @@ impl ImportStatement {
             _ => return None,
         }
 
-        Some((stem.to_string(), uri.syntax().text_range().to_span()))
+        Some((stem.to_string(), uri.span()))
     }
 }
 

wdl-ast/src/v1/tokens.rs

@@ -82,6 +82,7 @@ define_token!(
 define_token!(Dot, "the `.` symbol", ".");
 define_token!(DoubleQuote, "the `\"` symbol", "\"");
 define_token!(ElseKeyword, "the `else` keyword", "else");
+define_token!(EnvKeyword, "the `env` keyword", "env");
 define_token!(Equal, "the `=` symbol", "=");
 define_token!(Exclamation, "the `!` symbol", "!");
 define_token!(Exponentiation, "the `**` symbol", "**");

wdl-ast/src/validation.rs

@@ -15,6 +15,7 @@ use crate::VersionStatement;
 use crate::Visitor;
 
 mod counts;
+mod env;
 mod exprs;
 mod imports;
 mod keys;
@@ -138,6 +139,7 @@ impl Default for Validator {
                 Box::<requirements::RequirementsVisitor>::default(),
                 Box::<exprs::ScopedExprVisitor>::default(),
                 Box::<imports::ImportsVisitor>::default(),
+                Box::<env::EnvVisitor>::default(),
             ],
         }
     }

wdl-ast/src/validation/env.rs

@@ -0,0 +1,109 @@
+//! Validation of `env` declarations.
+
+use crate::AstNodeExt;
+use crate::AstToken;
+use crate::Diagnostic;
+use crate::Diagnostics;
+use crate::Document;
+use crate::Span;
+use crate::SupportedVersion;
+use crate::VisitReason;
+use crate::Visitor;
+use crate::v1;
+use crate::version::V1;
+
+/// Creates an "env type not primitive" diagnostic.
+fn env_type_not_primitive(env_span: Span, ty: &v1::Type, ty_span: Span) -> Diagnostic {
+    Diagnostic::error("environment variable modifier can only be used on primitive types")
+        .with_label(
+            format!("type `{ty}` cannot be used as an environment variable"),
+            ty_span,
+        )
+        .with_label(
+            "declaration is an environment variable due to this modifier",
+            env_span,
+        )
+}
+
+/// Checks the type to see if it is legal as an environment variable.
+///
+/// Returns `None` if the type is legal otherwise it returns the span of the
+/// type.
+fn check_type(ty: &v1::Type) -> Option<Span> {
+    match ty {
+        v1::Type::Map(ty) => Some(ty.span()),
+        v1::Type::Array(ty) => Some(ty.span()),
+        v1::Type::Pair(ty) => Some(ty.span()),
+        v1::Type::Object(ty) => Some(ty.span()),
+        v1::Type::Ref(ty) => Some(ty.span()),
+        v1::Type::Primitive(_) => None,
+    }
+}
+
+/// An AST visitor that ensures that environment variable modifiers only exist
+/// on primitive type declarations.
+#[derive(Debug, Default)]
+pub struct EnvVisitor {
+    /// The version of the document we're currently visiting.
+    version: Option<SupportedVersion>,
+}
+
+impl Visitor for EnvVisitor {
+    type State = Diagnostics;
+
+    fn document(
+        &mut self,
+        _: &mut Self::State,
+        reason: VisitReason,
+        _: &Document,
+        version: SupportedVersion,
+    ) {
+        if reason == VisitReason::Exit {
+            return;
+        }
+
+        *self = Default::default();
+        self.version = Some(version);
+    }
+
+    fn bound_decl(&mut self, state: &mut Self::State, reason: VisitReason, decl: &v1::BoundDecl) {
+        // Only visit decls for WDL >=1.2
+        if self.version.expect("should have a version") < SupportedVersion::V1(V1::Two) {
+            return;
+        }
+
+        if reason == VisitReason::Exit {
+            return;
+        }
+
+        if let Some(env_span) = decl.env().map(|t| t.span()) {
+            let ty = decl.ty();
+            if let Some(span) = check_type(&ty) {
+                state.add(env_type_not_primitive(env_span, &ty, span));
+            }
+        }
+    }
+
+    fn unbound_decl(
+        &mut self,
+        state: &mut Self::State,
+        reason: VisitReason,
+        decl: &v1::UnboundDecl,
+    ) {
+        // Only visit decls for WDL >=1.2
+        if self.version.expect("should have a version") < SupportedVersion::V1(V1::Two) {
+            return;
+        }
+
+        if reason == VisitReason::Exit {
+            return;
+        }
+
+        if let Some(env_span) = decl.env().map(|t| t.span()) {
+            let ty = decl.ty();
+            if let Some(span) = check_type(&ty) {
+                state.add(env_type_not_primitive(env_span, &ty, span));
+            }
+        }
+    }
+}