fix: enforce correct type rules for placeholder options

Author: Ankush1oo8

wdl-analysis/src/types/v1.rs

@@ -640,34 +640,51 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> {
     }
 
     /// Checks a placeholder expression.
-  pub(crate) fn check_placeholder(&mut self, placeholder: &Placeholder) {
-    self.placeholders += 1;
-    let expr = placeholder.expr();
-     /// Evaluate the placeholder expression and check that the resulting type is
-    /// coercible to string for interpolation
-    if let Some(ty) = self.evaluate_expr(&expr) {
-        match ty {
-            Type::Primitive(..) | Type::Union | Type::None => { /* Already coercible */ }
-            _ => {
-                let coercible = match placeholder.option() {
-                    Some(PlaceholderOption::Sep(_)) => matches!(ty, Type::Compound(CompoundType::Array(element_type), _) 
-                        if matches!(element_type.as_ref(), Type::Primitive(_, false))),
-                    
-                    Some(PlaceholderOption::Default(_)) => matches!(ty, Type::Primitive(_, true)),
-
-                    Some(PlaceholderOption::TrueFalse(_)) => matches!(ty, Type::Primitive(PrimitiveType::Boolean, _)),
-
-                    None => true, // Default case: Always coercible
-                };
-                if !coercible {
-                    self.context
-                        .add_diagnostic(cannot_coerce_to_string(&ty, expr.span()));
+    pub(crate) fn check_placeholder(&mut self, placeholder: &Placeholder) {
+        self.placeholders += 1;  // Track placeholder processing count
+    
+        let expr = placeholder.expr(); // Extract expression inside placeholder
+    
+        // Evaluate the placeholder expression and check that the resulting type is
+        // coercible to string for interpolation
+        if let Some(ty) = self.evaluate_expr(&expr) {
+            match ty {
+                // If the type is already coercible, do nothing
+                Type::Primitive(..) | Type::Union | Type::None => {}
+    
+                // Otherwise, check placeholder options
+                _ => {
+                    let coercible = match placeholder.option() {
+                        // `Sep` option: Only `Array[P]` where `P` is a non-optional primitive is valid
+                        Some(PlaceholderOption::Sep(_)) => matches!(ty, 
+                            Type::Compound(CompoundType::Array(ref element_type), _) 
+                            if matches!(element_type.element_type(), Type::Primitive(_, false))),
+    
+                        // `Default` option: Only optional primitive types are valid
+                        Some(PlaceholderOption::Default(_)) => matches!(ty, 
+                            Type::Primitive(_, true)),
+    
+                        // `TrueFalse` option: Only Boolean type is valid
+                        Some(PlaceholderOption::TrueFalse(_)) => matches!(ty, 
+                            Type::Primitive(PrimitiveType::Boolean, _)),
+    
+                        // If no option is specified, allow all types
+                        None => true, 
+                    };
+    
+                    // If coercion is not possible, add a diagnostic error
+                    if !coercible {
+                        self.context
+                            .add_diagnostic(cannot_coerce_to_string(&ty, expr.span()));
+                    }
                 }
             }
         }
+    
+        self.placeholders -= 1; // Decrement placeholder tracking
     }
-    self.placeholders -= 1;
-}
+    
+
     /// Evaluates the type of a literal array expression.
     fn evaluate_literal_array(&mut self, expr: &LiteralArray) -> Type {
         // Look at the first array element to determine the element type

wdl-analysis/tests/analysis/cannot-coerce-string/source.diagnostics

@@ -1,6 +0,0 @@
-error: cannot coerce type `Array[String]` to `String`
-   ┌─ tests/analysis/cannot-coerce-string/source.wdl:10:23
-   │
-10 │     String x = "foo ${a}"
-   │                       ^ this is type `Array[String]`
-

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

@@ -0,0 +1,12 @@
+version 1.2
+
+task valid_case {
+    input {
+        Array[String] arr
+    }
+    command <<< ~{sep=", " arr} >>>
+}
+
+task invalid_case {
+    command <<< ~{sep=", " true} >>>
+}

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

@@ -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/error.txt

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

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

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

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

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

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

@@ -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/primitive-literals/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/relative-and-absolute-task/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/tasks/test-join-paths/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
+   │        ^^^^^^^^^^^^^^^
+