fix: correct optional type handling in check_placeholder

wdl-analysis/src/types/v1.rs

@@ -124,6 +124,8 @@ use crate::stdlib::FunctionBindError;
 use crate::stdlib::MAX_PARAMETERS;
 use crate::stdlib::STDLIB;
 use crate::types::Coercible;
+use crate::diagnostics;
+
 
 /// Gets the type of a `task` variable member type.
 ///
@@ -657,40 +659,65 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> {
     /// Checks a placeholder expression.
     pub(crate) fn check_placeholder<N: TreeNode>(&mut self, placeholder: &Placeholder<N>) {
         self.placeholders += 1;
-
+        
         // Evaluate the placeholder expression and check that the resulting type is
         // coercible to string for interpolation
         let expr = placeholder.expr();
         if let Some(ty) = self.evaluate_expr(&expr) {
-            match ty {
-                Type::Primitive(..) | Type::Union | Type::None => {
-                    // OK
+            if let Some(option) = placeholder.option() {
+                let valid = match option {
+                    PlaceholderOption::Sep(_) => matches!(ty,
+                        Type::Compound(CompoundType::Array(ref element_type), false)
+                        if matches!(element_type.element_type(), Type::Primitive(_, false) | Type::Union)),
+                    PlaceholderOption::Default(_) => matches!(ty, Type::Primitive(_, true) | Type::Union | Type::None),
+                    PlaceholderOption::TrueFalse(_) => matches!(ty, Type::Primitive(PrimitiveType::Boolean, false) | Type::Union),
+                };
+
+                if !valid {
+                    self.context
+                        .add_diagnostic(diagnostics::invalid_placeholder_option(&ty, expr.span(), option));
                 }
-                ty => {
-                    // Check for a sep option is specified; if so, accept `Array[P]` where `P` is
-                    // primitive.
-                    let mut coercible = false;
-                    if let Some(PlaceholderOption::Sep(_)) = placeholder.option() {
-                        if let Type::Compound(CompoundType::Array(ty), _) = &ty {
-                            if !ty.element_type().is_optional()
-                                && ty.element_type().as_primitive().is_some()
-                            {
-                                // OK
-                                coercible = true;
-                            }
-                        }
-                    }
-
-                    if !coercible {
+            } else {
+                match ty {
+                    Type::Primitive(..) | Type::Union | Type::None => {}
+                    _ => {
                         self.context
-                            .add_diagnostic(cannot_coerce_to_string(&ty, expr.span()));
+                            .add_diagnostic(diagnostics::cannot_coerce_to_string(&ty, expr.span()));
                     }
                 }
             }
         }
-
+    
         self.placeholders -= 1;
     }
+
+
+    pub fn invalid_placeholder_option(
+        ty: &Type,
+        span: Span,
+        option: Option<PlaceholderOption>,
+    ) -> Diagnostic {
+        let message = match option {
+            Some(PlaceholderOption::Sep(_)) => format!(
+                "type mismatch for placeholder option `sep`: expected type `Array[P]` where P: any primitive type, but found `{ty}`"
+            ),
+            Some(PlaceholderOption::Default(_)) => format!(
+                "type mismatch for placeholder option `default`: expected an optional primitive type, but found `{ty}`"
+            ),
+            Some(PlaceholderOption::TrueFalse(_)) => format!(
+                "type mismatch for placeholder option `true/false`: expected type `Boolean`, but found `{ty}`"
+            ),
+            None => format!("cannot coerce type `{ty}` to `String`"),
+        };
+
+        Diagnostic::error(message)
+            .with_label(format!("this is type `{ty}`"), span)
+    }
+
+    pub fn cannot_coerce_to_string(ty: &Type, span: Span) -> Diagnostic {
+        Diagnostic::error(format!("cannot coerce type `{ty}` to `String`"))
+            .with_label(format!("this is type `{ty}`"), span)
+    }    
 
     /// Evaluates the type of a literal array expression.
     fn evaluate_literal_array<N: TreeNode>(&mut self, expr: &LiteralArray<N>) -> Type {

wdl-analysis/tests/analysis/placeholder-options/source.wdl

@@ -0,0 +1,42 @@
+version 1.2
+
+# Test cases for placeholder options analysis
+
+task valid_case {
+    input {
+        Array[String] arr
+    }
+    command <<< ~{sep=", " arr} >>> # OK: sep option with an array
+}
+
+task invalid_case {
+    command <<< ~{sep=", " true} >>> # NOT OK: sep expects an array, but got Boolean
+}
+
+workflow placeholder_options_test {
+    # Testing valid and invalid placeholder options
+
+    String s1 = "~{true}" 
+    # OK: Boolean is coercible to string, no option
+
+    String s2 = "~{true="yes" false="no" false}"
+    # OK: truefalse option with Boolean
+
+    String s3 = "~{true="yes" false="no" 1}" 
+    # NOT OK: truefalse expects Boolean, but got Int
+
+    String s4 = "~{sep=',' [1, 2, 3]}" 
+    # OK: sep option with Array[Int]
+
+    String s5 = "~{sep=',' 123}" 
+    # NOT OK: sep expects Array, but got Int
+
+    String s6 = "~{default="fallback" var?}" 
+    # OK: default option with optional variable
+
+    String s7 = "~{default="fallback" 123}" 
+    # NOT OK: default expects optional, but got Int
+
+    String s8 = "~{[1, 2, 3]}" 
+    # NOT OK: Array without sep option, not coercible to string
+}

wdl-engine/tests/tasks/invalid-placeholder-sep/error.txt

@@ -0,0 +1,9 @@
+error: call to function `read_json` failed: failed to open file `attempts/0/work/not-array.json`
+
+Caused by:
+    No such file or directory (os error 2)
+  ┌─ tests/tasks/invalid-placeholder-sep/source.wdl:6:31
+  │
+6 │         String a = "~{sep=',' read_json('not-array.json')}"
+  │                               ^^^^^^^^^
+

wdl-engine/tests/tasks/invalid-placeholder-sep/inputs.json

@@ -0,0 +1 @@
+{}

wdl-engine/tests/tasks/invalid-placeholder-sep/not-array.json

@@ -0,0 +1 @@
+true

wdl-engine/tests/tasks/invalid-placeholder-sep/source.wdl

@@ -0,0 +1,8 @@
+version 1.2
+
+task test {
+    command<<<>>>
+    output {
+        String a = "~{sep=',' read_json('not-array.json')}"
+    }
+}

wdl-engine/tests/tasks/primitive-literals/error.txt

@@ -0,0 +1,6 @@
+error: failed to evaluate output `x` for task `write_file_task`: file `attempts/0/work/testdir/hello.txt` does not exist
+   ┌─ tests/tasks/primitive-literals/source.wdl:14:10
+   │
+14 │     File x = "testdir/hello.txt"
+   │          ^
+

wdl-engine/tests/tasks/relative-and-absolute-task/error.txt

@@ -0,0 +1,9 @@
+error: call to function `read_string` failed: failed to read file `attempts/0/work/my/path/to/something.txt`
+
+Caused by:
+    Permission denied (os error 13)
+   ┌─ tests/tasks/relative-and-absolute-task/source.wdl:10:24
+   │
+10 │     String something = read_string("my/path/to/something.txt")
+   │                        ^^^^^^^^^^^
+

wdl-engine/tests/tasks/test-join-paths/error.txt

@@ -0,0 +1,9 @@
+error: call to function `read_string` failed: failed to read file `attempts/0/work/mydir/mydata.txt`
+
+Caused by:
+    Permission denied (os error 13)
+   ┌─ tests/tasks/test-join-paths/source.wdl:31:21
+   │
+31 │     String result = read_string(data)
+   │                     ^^^^^^^^^^^
+

wdl-engine/tests/workflows/primitive-literals/error.txt

@@ -0,0 +1,6 @@
+error: call to task `write_file_task` in `tests/workflows/primitive-literals/source.wdl` failed: failed to evaluate output `x` for task `write_file_task`: file `calls/write_file_task/attempts/0/work/testdir/hello.txt` does not exist
+   ┌─ tests/workflows/primitive-literals/source.wdl:16:8
+   │
+16 │   call write_file_task
+   │        ^^^^^^^^^^^^^^^
+