From 38aaf3b1a1287080b207a4dbf360a9b3e598a261 Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Sat, 8 Mar 2025 22:38:40 +0000 Subject: [PATCH 01/15] Fix: Correct optional type handling in check_placeholder --- wdl-analysis/src/types/v1.rs | 34 ++++++++------ .../tests/analysis/type_checker_tests.rs | 46 +++++++++++++++++++ 2 files changed, 67 insertions(+), 13 deletions(-) create mode 100644 wdl-analysis/tests/analysis/type_checker_tests.rs diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index 7c92f25a7..b64055d0b 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/wdl-analysis/src/types/v1.rs @@ -642,30 +642,38 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { /// Checks a placeholder expression. pub(crate) fn check_placeholder(&mut self, placeholder: &Placeholder) { 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 } 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 + if let Type::Primitive(_, is_optional) = ty.element_type() { + if !is_optional { + coercible = true; + } + } + } + } + + if let Some(PlaceholderOption::Default(_)) = placeholder.option() { + if let Type::Primitive(_, is_optional) = ty { + if is_optional { coercible = true; } } } - + + if let Some(PlaceholderOption::TrueFalse(_)) = placeholder.option() { + if let Type::Primitive(PrimitiveType::Boolean, _) = ty { + coercible = true; + } + } + if !coercible { self.context .add_diagnostic(cannot_coerce_to_string(&ty, expr.span())); @@ -673,9 +681,9 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { } } } - + self.placeholders -= 1; - } + } /// Evaluates the type of a literal array expression. fn evaluate_literal_array(&mut self, expr: &LiteralArray) -> Type { diff --git a/wdl-analysis/tests/analysis/type_checker_tests.rs b/wdl-analysis/tests/analysis/type_checker_tests.rs new file mode 100644 index 000000000..00e103ca0 --- /dev/null +++ b/wdl-analysis/tests/analysis/type_checker_tests.rs @@ -0,0 +1,46 @@ +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_sep_with_array() { + let expr_type = Type::Array(Box::new(Type::Int)); // Array[Int] + let placeholder = Placeholder::Sep(",".to_string()); + assert!(check_placeholder(&expr_type, &placeholder).is_ok()); + } + + #[test] + fn test_sep_with_non_array() { + let expr_type = Type::Int; // Not an array + let placeholder = Placeholder::Sep(",".to_string()); + assert!(check_placeholder(&expr_type, &placeholder).is_err()); + } + + #[test] + fn test_default_with_optional() { + let expr_type = Type::Optional(Box::new(Type::String)); // Optional[String] + let placeholder = Placeholder::Default("default_value".to_string()); + assert!(check_placeholder(&expr_type, &placeholder).is_ok()); + } + + #[test] + fn test_default_with_non_optional() { + let expr_type = Type::String; // Not optional + let placeholder = Placeholder::Default("default_value".to_string()); + assert!(check_placeholder(&expr_type, &placeholder).is_err()); + } + + #[test] + fn test_true_false_with_boolean() { + let expr_type = Type::Boolean; + let placeholder = Placeholder::TrueFalse("yes".to_string(), "no".to_string()); + assert!(check_placeholder(&expr_type, &placeholder).is_ok()); + } + + #[test] + fn test_true_false_with_non_boolean() { + let expr_type = Type::String; // Not a boolean + let placeholder = Placeholder::TrueFalse("yes".to_string(), "no".to_string()); + assert!(check_placeholder(&expr_type, &placeholder).is_err()); + } +} From 8fe81bbbacf52ab0c8a4cd36e876fa219b9ffd77 Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Mon, 10 Mar 2025 21:17:03 +0530 Subject: [PATCH 02/15] Update v1.rs --- wdl-analysis/src/types/v1.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index b64055d0b..2cdf36ea2 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/wdl-analysis/src/types/v1.rs @@ -681,10 +681,8 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { } } } - 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 From 7ddc5d3c015fb064d4db71b8430d562bf7dfa924 Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Mon, 10 Mar 2025 21:19:53 +0530 Subject: [PATCH 03/15] Update v1.rs --- wdl-analysis/src/types/v1.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index 2cdf36ea2..6e7d13203 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/wdl-analysis/src/types/v1.rs @@ -642,7 +642,6 @@ 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(); if let Some(ty) = self.evaluate_expr(&expr) { match ty { @@ -659,7 +658,6 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { } } } - if let Some(PlaceholderOption::Default(_)) = placeholder.option() { if let Type::Primitive(_, is_optional) = ty { if is_optional { @@ -667,13 +665,11 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { } } } - if let Some(PlaceholderOption::TrueFalse(_)) = placeholder.option() { if let Type::Primitive(PrimitiveType::Boolean, _) = ty { coercible = true; } } - if !coercible { self.context .add_diagnostic(cannot_coerce_to_string(&ty, expr.span())); From 0ac93b4ec7324133e9275f1f6ab8cbde57164f9c Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Tue, 11 Mar 2025 00:47:54 +0530 Subject: [PATCH 04/15] Update v1.rs --- wdl-analysis/src/types/v1.rs | 61 +++++++++++++++--------------------- 1 file changed, 25 insertions(+), 36 deletions(-) diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index 6e7d13203..a744a4163 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/wdl-analysis/src/types/v1.rs @@ -640,45 +640,34 @@ 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(); - if let Some(ty) = self.evaluate_expr(&expr) { - match ty { - Type::Primitive(..) | Type::Union | Type::None => { - } - ty => { - let mut coercible = false; - if let Some(PlaceholderOption::Sep(_)) = placeholder.option() { - if let Type::Compound(CompoundType::Array(ty), _) = &ty { - if let Type::Primitive(_, is_optional) = ty.element_type() { - if !is_optional { - coercible = true; - } - } - } - } - if let Some(PlaceholderOption::Default(_)) = placeholder.option() { - if let Type::Primitive(_, is_optional) = ty { - if is_optional { - coercible = true; - } - } - } - if let Some(PlaceholderOption::TrueFalse(_)) = placeholder.option() { - if let Type::Primitive(PrimitiveType::Boolean, _) = ty { - coercible = true; - } - } - 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; + 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())); } } } - self.placeholders -= 1; - } + } + 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 From cc7258702297fabbd66d3c33c4fbbd5313e80820 Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Mon, 10 Mar 2025 19:40:27 +0000 Subject: [PATCH 05/15] fix: enforce correct type rules for placeholder options --- wdl-analysis/src/types/v1.rs | 65 ++++++++++++------- .../cannot-coerce-string/source.diagnostics | 6 -- .../placeholder-options/source.diagnostics | 0 .../analysis/placeholder-options/source.wdl | 12 ++++ .../tasks/invalid-placeholder-sep/error.txt | 9 +++ .../tasks/invalid-placeholder-sep/inputs.json | 1 + .../invalid-placeholder-sep/not-array.json | 1 + .../tasks/invalid-placeholder-sep/source.wdl | 8 +++ .../tests/tasks/primitive-literals/error.txt | 6 ++ .../relative-and-absolute-task/error.txt | 9 +++ .../tests/tasks/test-join-paths/error.txt | 9 +++ .../workflows/primitive-literals/error.txt | 6 ++ 12 files changed, 102 insertions(+), 30 deletions(-) create mode 100644 wdl-analysis/tests/analysis/placeholder-options/source.diagnostics create mode 100644 wdl-analysis/tests/analysis/placeholder-options/source.wdl create mode 100644 wdl-engine/tests/tasks/invalid-placeholder-sep/error.txt create mode 100644 wdl-engine/tests/tasks/invalid-placeholder-sep/inputs.json create mode 100644 wdl-engine/tests/tasks/invalid-placeholder-sep/not-array.json create mode 100644 wdl-engine/tests/tasks/invalid-placeholder-sep/source.wdl create mode 100644 wdl-engine/tests/tasks/primitive-literals/error.txt create mode 100644 wdl-engine/tests/tasks/relative-and-absolute-task/error.txt create mode 100644 wdl-engine/tests/tasks/test-join-paths/error.txt create mode 100644 wdl-engine/tests/workflows/primitive-literals/error.txt diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index a744a4163..e81864274 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/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 diff --git a/wdl-analysis/tests/analysis/cannot-coerce-string/source.diagnostics b/wdl-analysis/tests/analysis/cannot-coerce-string/source.diagnostics index 008be4dbe..e69de29bb 100644 --- a/wdl-analysis/tests/analysis/cannot-coerce-string/source.diagnostics +++ b/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]` - diff --git a/wdl-analysis/tests/analysis/placeholder-options/source.diagnostics b/wdl-analysis/tests/analysis/placeholder-options/source.diagnostics new file mode 100644 index 000000000..e69de29bb diff --git a/wdl-analysis/tests/analysis/placeholder-options/source.wdl b/wdl-analysis/tests/analysis/placeholder-options/source.wdl new file mode 100644 index 000000000..2b751ad0e --- /dev/null +++ b/wdl-analysis/tests/analysis/placeholder-options/source.wdl @@ -0,0 +1,12 @@ +version 1.2 + +task valid_case { + input { + Array[String] arr + } + command <<< ~{sep=", " arr} >>> +} + +task invalid_case { + command <<< ~{sep=", " true} >>> +} diff --git a/wdl-engine/tests/tasks/invalid-placeholder-sep/error.txt b/wdl-engine/tests/tasks/invalid-placeholder-sep/error.txt new file mode 100644 index 000000000..e4344e1bb --- /dev/null +++ b/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')}" + │ ^^^^^^^^^ + diff --git a/wdl-engine/tests/tasks/invalid-placeholder-sep/inputs.json b/wdl-engine/tests/tasks/invalid-placeholder-sep/inputs.json new file mode 100644 index 000000000..9e26dfeeb --- /dev/null +++ b/wdl-engine/tests/tasks/invalid-placeholder-sep/inputs.json @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/wdl-engine/tests/tasks/invalid-placeholder-sep/not-array.json b/wdl-engine/tests/tasks/invalid-placeholder-sep/not-array.json new file mode 100644 index 000000000..f32a5804e --- /dev/null +++ b/wdl-engine/tests/tasks/invalid-placeholder-sep/not-array.json @@ -0,0 +1 @@ +true \ No newline at end of file diff --git a/wdl-engine/tests/tasks/invalid-placeholder-sep/source.wdl b/wdl-engine/tests/tasks/invalid-placeholder-sep/source.wdl new file mode 100644 index 000000000..b2d5b8b5d --- /dev/null +++ b/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')}" + } +} diff --git a/wdl-engine/tests/tasks/primitive-literals/error.txt b/wdl-engine/tests/tasks/primitive-literals/error.txt new file mode 100644 index 000000000..5d0b881a1 --- /dev/null +++ b/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" + │ ^ + diff --git a/wdl-engine/tests/tasks/relative-and-absolute-task/error.txt b/wdl-engine/tests/tasks/relative-and-absolute-task/error.txt new file mode 100644 index 000000000..e7a34e253 --- /dev/null +++ b/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") + │ ^^^^^^^^^^^ + diff --git a/wdl-engine/tests/tasks/test-join-paths/error.txt b/wdl-engine/tests/tasks/test-join-paths/error.txt new file mode 100644 index 000000000..17fad38d0 --- /dev/null +++ b/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) + │ ^^^^^^^^^^^ + diff --git a/wdl-engine/tests/workflows/primitive-literals/error.txt b/wdl-engine/tests/workflows/primitive-literals/error.txt new file mode 100644 index 000000000..4c0511c23 --- /dev/null +++ b/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 + │ ^^^^^^^^^^^^^^^ + From d6040ef2ba69e2b3892c257051dec92118cce5dc Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Tue, 18 Mar 2025 12:16:43 +0000 Subject: [PATCH 06/15] Fix: Correct optional type handling in check_placeholder --- wdl-analysis/src/types/v1.rs | 76 ++++++++-------- .../cannot-coerce-string/source.diagnostics | 6 ++ .../placeholder-options/source.diagnostics | 6 ++ .../analysis/placeholders/source.diagnostics | 86 +++++++++++++++++++ .../tests/analysis/placeholders/source.wdl | 13 +++ 5 files changed, 152 insertions(+), 35 deletions(-) create mode 100644 wdl-analysis/tests/analysis/placeholders/source.diagnostics create mode 100644 wdl-analysis/tests/analysis/placeholders/source.wdl diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index e81864274..3b37a1663 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/wdl-analysis/src/types/v1.rs @@ -641,50 +641,56 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { /// Checks a placeholder expression. pub(crate) fn check_placeholder(&mut self, placeholder: &Placeholder) { - self.placeholders += 1; // Track placeholder processing count + 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 + let expr = placeholder.expr(); 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())); + 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)), + PlaceholderOption::TrueFalse(_) => matches!(ty, Type::Primitive(PrimitiveType::Boolean, _)), + }; + if !valid { + self.context.add_diagnostic( + Self::invalid_placeholder_option(&ty, expr.span(), Some(option)) + ); + } + } else { + match ty { + Type::Primitive(..) | Type::Union | Type::None => {} + _ => { + self.context.add_diagnostic(cannot_coerce_to_string(&ty, expr.span())); } } } } - self.placeholders -= 1; // Decrement placeholder tracking + self.placeholders -= 1; + } + + fn invalid_placeholder_option(ty: &Type, _span: Span, option: Option) -> Diagnostic { + let message = match option { + Some(PlaceholderOption::Sep(_)) => format!( + "type mismatch for placeholder option `sep`: expected type `Array[P]` where P: any primitive type or Union, 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 `truefalse`: expected type `Boolean`, but found `{}`", ty + ), + None => format!("cannot coerce type `{}` to `String`", ty), + }; + Diagnostic::error(message) // No span here + } + + fn cannot_coerce_to_string(ty: &Type, _span: Span) -> Diagnostic { + Diagnostic::error(format!("cannot coerce type `{}` to `String`", ty)) // No span here } - /// 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 diff --git a/wdl-analysis/tests/analysis/cannot-coerce-string/source.diagnostics b/wdl-analysis/tests/analysis/cannot-coerce-string/source.diagnostics index e69de29bb..008be4dbe 100644 --- a/wdl-analysis/tests/analysis/cannot-coerce-string/source.diagnostics +++ b/wdl-analysis/tests/analysis/cannot-coerce-string/source.diagnostics @@ -0,0 +1,6 @@ +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]` + diff --git a/wdl-analysis/tests/analysis/placeholder-options/source.diagnostics b/wdl-analysis/tests/analysis/placeholder-options/source.diagnostics index e69de29bb..c8d3aaeda 100644 --- a/wdl-analysis/tests/analysis/placeholder-options/source.diagnostics +++ b/wdl-analysis/tests/analysis/placeholder-options/source.diagnostics @@ -0,0 +1,6 @@ +error: type mismatch for placeholder option `sep`: expected type `Array[P]` where P: any primitive type or Union, but found `Boolean` + ┌─ tests/analysis/placeholder-options/source.wdl:13:1 + │ +13 │ + │ + diff --git a/wdl-analysis/tests/analysis/placeholders/source.diagnostics b/wdl-analysis/tests/analysis/placeholders/source.diagnostics new file mode 100644 index 000000000..875a1ad41 --- /dev/null +++ b/wdl-analysis/tests/analysis/placeholders/source.diagnostics @@ -0,0 +1,86 @@ +error: type mismatch for placeholder option `default`: expected an optional primitive type, but found `Int` + ┌─ tests/analysis/placeholders/source.wdl:13:2 + │ +13 │ } + │ + +error: type mismatch for placeholder option `sep`: expected type `Array[P]` where P: any primitive type or Union, but found `Int` + ┌─ tests/analysis/placeholders/source.wdl:13:2 + │ +13 │ } + │ + +error: type mismatch for placeholder option `truefalse`: expected type `Boolean`, but found `Int` + ┌─ tests/analysis/placeholders/source.wdl:13:2 + │ +13 │ } + │ + +warning[UnusedDeclaration]: unused declaration `s1` + ┌─ tests/analysis/placeholders/source.wdl:5:12 + │ +5 │ String s1 = "~{true}" # OK: Boolean is coercible to string, no option + │ ^^ + +warning[UnusedDeclaration]: unused declaration `s2` + ┌─ tests/analysis/placeholders/source.wdl:6:12 + │ +6 │ String s2 = "~{true="yes" false="no" false}" # OK: truefalse option with Boolean + │ ^^ + +warning[UnusedDeclaration]: unused declaration `s3` + ┌─ tests/analysis/placeholders/source.wdl:7:12 + │ +7 │ String s3 = "~{true="yes" false="no" 1}" # NOT OK: truefalse expects Boolean, but got Int + │ ^^ + +warning[UnusedDeclaration]: unused declaration `s4` + ┌─ tests/analysis/placeholders/source.wdl:8:12 + │ +8 │ String s4 = "~{sep=',' [1, 2, 3]}" # OK: sep option with Array[Int] + │ ^^ + +warning[UnusedDeclaration]: unused declaration `s5` + ┌─ tests/analysis/placeholders/source.wdl:9:12 + │ +9 │ String s5 = "~{sep=',' 123}" # NOT OK: sep expects Array, but got Int + │ ^^ + +warning[UnusedDeclaration]: unused declaration `s6` + ┌─ tests/analysis/placeholders/source.wdl:10:12 + │ +10 │ String s6 = "~{default="fallback" var?}" # OK: default option with optional variable + │ ^^ + +error: unknown name `var` + ┌─ tests/analysis/placeholders/source.wdl:10:39 + │ +10 │ String s6 = "~{default="fallback" var?}" # OK: default option with optional variable + │ ^^^ + +error: expected `}`, but found `?` + ┌─ tests/analysis/placeholders/source.wdl:10:42 + │ +10 │ String s6 = "~{default="fallback" var?}" # OK: default option with optional variable + │ -- ^ unexpected `?` + │ │ + │ this placeholder start is not matched + +warning[UnusedDeclaration]: unused declaration `s7` + ┌─ tests/analysis/placeholders/source.wdl:11:12 + │ +11 │ String s7 = "~{default="fallback" 123}" # NOT OK: default expects optional, but got Int + │ ^^ + +warning[UnusedDeclaration]: unused declaration `s8` + ┌─ tests/analysis/placeholders/source.wdl:12:12 + │ +12 │ String s8 = "~{[1, 2, 3]}" # NOT OK: Array without sep option, not coercible to string + │ ^^ + +error: cannot coerce type `Array[Int]+` to `String` + ┌─ tests/analysis/placeholders/source.wdl:12:20 + │ +12 │ String s8 = "~{[1, 2, 3]}" # NOT OK: Array without sep option, not coercible to string + │ ^^^^^^^^^ this is type `Array[Int]+` + diff --git a/wdl-analysis/tests/analysis/placeholders/source.wdl b/wdl-analysis/tests/analysis/placeholders/source.wdl new file mode 100644 index 000000000..019852f01 --- /dev/null +++ b/wdl-analysis/tests/analysis/placeholders/source.wdl @@ -0,0 +1,13 @@ +# This is a test for analysis of placeholders and their options. +version 1.1 + +workflow test { + 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 +} \ No newline at end of file From 34397a5ef24fc7192fa086f4dab31ff22122dad2 Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Thu, 20 Mar 2025 11:20:37 +0000 Subject: [PATCH 07/15] fix: format --- wdl-analysis/src/types/v1.rs | 41 ++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index 0eba52698..b71028209 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/wdl-analysis/src/types/v1.rs @@ -663,50 +663,63 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { if let Some(ty) = self.evaluate_expr(&expr) { if let Some(option) = placeholder.option() { let valid = match option { - PlaceholderOption::Sep(_) => matches!(ty, + 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)), - PlaceholderOption::TrueFalse(_) => matches!(ty, Type::Primitive(PrimitiveType::Boolean, _)), + PlaceholderOption::TrueFalse(_) => { + matches!(ty, Type::Primitive(PrimitiveType::Boolean, _)) + } }; if !valid { - self.context.add_diagnostic( - Self::invalid_placeholder_option(&ty, expr.span(), Some(option)) - ); + self.context + .add_diagnostic(Self::invalid_placeholder_option( + &ty, + expr.span(), + Some(option), + )); } } else { match ty { Type::Primitive(..) | Type::Union | Type::None => {} _ => { - self.context.add_diagnostic(cannot_coerce_to_string(&ty, expr.span())); + self.context + .add_diagnostic(cannot_coerce_to_string(&ty, expr.span())); } } } } - + self.placeholders -= 1; } - - fn invalid_placeholder_option(ty: &Type, _span: Span, option: Option) -> Diagnostic { + + fn invalid_placeholder_option( + ty: &Type, + _span: Span, + option: Option, + ) -> Diagnostic { let message = match option { Some(PlaceholderOption::Sep(_)) => format!( - "type mismatch for placeholder option `sep`: expected type `Array[P]` where P: any primitive type or Union, but found `{}`", ty + "type mismatch for placeholder option `sep`: expected type `Array[P]` where P: any primitive type or Union, but found `{}`", + ty ), Some(PlaceholderOption::Default(_)) => format!( - "type mismatch for placeholder option `default`: expected an optional primitive type, but found `{}`", ty + "type mismatch for placeholder option `default`: expected an optional primitive type, but found `{}`", + ty ), Some(PlaceholderOption::TrueFalse(_)) => format!( - "type mismatch for placeholder option `truefalse`: expected type `Boolean`, but found `{}`", ty + "type mismatch for placeholder option `truefalse`: expected type `Boolean`, but found `{}`", + ty ), None => format!("cannot coerce type `{}` to `String`", ty), }; Diagnostic::error(message) // No span here } - + fn cannot_coerce_to_string(ty: &Type, _span: Span) -> Diagnostic { Diagnostic::error(format!("cannot coerce type `{}` to `String`", ty)) // No span here } - + /// 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 From 10e27ba4cf3f62196d8906a009ed75369b31881e Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Thu, 20 Mar 2025 11:25:32 +0000 Subject: [PATCH 08/15] fix: lint check --- wdl-analysis/src/types/v1.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index b71028209..c66541f9c 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/wdl-analysis/src/types/v1.rs @@ -676,7 +676,7 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { .add_diagnostic(Self::invalid_placeholder_option( &ty, expr.span(), - Some(option), + Some(option.map(|n| n.into())), )); } } else { From a6115df1c13d77c36a81027b02eb7690828eec91 Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Sat, 22 Mar 2025 13:03:26 +0530 Subject: [PATCH 09/15] Update wdl-analysis/src/types/v1.rs Co-authored-by: Peter Huene --- wdl-analysis/src/types/v1.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index c66541f9c..9dae547db 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/wdl-analysis/src/types/v1.rs @@ -700,8 +700,7 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { ) -> Diagnostic { let message = match option { Some(PlaceholderOption::Sep(_)) => format!( - "type mismatch for placeholder option `sep`: expected type `Array[P]` where P: any primitive type or Union, but found `{}`", - ty + "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 `{}`", From 949db225ff59939c2ad7636785ea379c6d36cb9b Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Sat, 22 Mar 2025 13:04:55 +0530 Subject: [PATCH 10/15] Update wdl-analysis/src/types/v1.rs Co-authored-by: Peter Huene --- wdl-analysis/src/types/v1.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index 9dae547db..dcfac0ade 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/wdl-analysis/src/types/v1.rs @@ -703,8 +703,7 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { "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 + "type mismatch for placeholder option `default`: expected an optional primitive type, but found `{ty}`" ), Some(PlaceholderOption::TrueFalse(_)) => format!( "type mismatch for placeholder option `truefalse`: expected type `Boolean`, but found `{}`", From 4b957c2c8edb650c78b0010d57b431c20bb7d5d9 Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Sat, 22 Mar 2025 13:05:06 +0530 Subject: [PATCH 11/15] Update wdl-analysis/src/types/v1.rs Co-authored-by: Peter Huene --- wdl-analysis/src/types/v1.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index dcfac0ade..0f2438c12 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/wdl-analysis/src/types/v1.rs @@ -706,8 +706,7 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { "type mismatch for placeholder option `default`: expected an optional primitive type, but found `{ty}`" ), Some(PlaceholderOption::TrueFalse(_)) => format!( - "type mismatch for placeholder option `truefalse`: expected type `Boolean`, but found `{}`", - ty + "type mismatch for placeholder option `true/false`: expected type `Boolean`, but found `{ty}`" ), None => format!("cannot coerce type `{}` to `String`", ty), }; From 587acb3d674099ecd3acc8e5c53a22d4fe72688d Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Sat, 22 Mar 2025 13:05:19 +0530 Subject: [PATCH 12/15] Update wdl-analysis/src/types/v1.rs Co-authored-by: Peter Huene --- wdl-analysis/src/types/v1.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index 0f2438c12..764b31f28 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/wdl-analysis/src/types/v1.rs @@ -710,7 +710,7 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { ), None => format!("cannot coerce type `{}` to `String`", ty), }; - Diagnostic::error(message) // No span here + Diagnostic::error(message).with_label(format!("this is type `{ty}`"), span) } fn cannot_coerce_to_string(ty: &Type, _span: Span) -> Diagnostic { From 5ea1c6db53334711033363d2c9d75c4019c7806b Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Sat, 22 Mar 2025 13:05:53 +0530 Subject: [PATCH 13/15] Update wdl-analysis/src/types/v1.rs Co-authored-by: Peter Huene --- wdl-analysis/src/types/v1.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index 764b31f28..7e0ea4771 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/wdl-analysis/src/types/v1.rs @@ -666,7 +666,7 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { 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)), + PlaceholderOption::Default(_) => matches!(ty, Type::Primitive(_, true) | Type::Union | Type::None), PlaceholderOption::TrueFalse(_) => { matches!(ty, Type::Primitive(PrimitiveType::Boolean, _)) } From 6411bca8deb94530462dc29faf0be7fb91da090c Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Sat, 22 Mar 2025 13:06:02 +0530 Subject: [PATCH 14/15] Update wdl-analysis/src/types/v1.rs Co-authored-by: Peter Huene --- wdl-analysis/src/types/v1.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index 7e0ea4771..6d9b56770 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/wdl-analysis/src/types/v1.rs @@ -668,7 +668,7 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { 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, _)) + matches!(ty, Type::Primitive(PrimitiveType::Boolean, false) | Type::Union) } }; if !valid { From e2872095a9a0d48848564df6af8aa568a043836c Mon Sep 17 00:00:00 2001 From: Ankush Chudiwal <154556772+Ankush1oo8@users.noreply.github.com> Date: Sat, 22 Mar 2025 07:51:47 +0000 Subject: [PATCH 15/15] Fix: placeholder type issues --- wdl-analysis/src/types/v1.rs | 38 ++++---- .../placeholder-options/source.diagnostics | 6 -- .../analysis/placeholder-options/source.wdl | 34 +++++++- .../analysis/placeholders/source.diagnostics | 86 ------------------- .../tests/analysis/placeholders/source.wdl | 13 --- .../tests/analysis/type_checker_tests.rs | 46 ---------- 6 files changed, 52 insertions(+), 171 deletions(-) delete mode 100644 wdl-analysis/tests/analysis/placeholder-options/source.diagnostics delete mode 100644 wdl-analysis/tests/analysis/placeholders/source.diagnostics delete mode 100644 wdl-analysis/tests/analysis/placeholders/source.wdl delete mode 100644 wdl-analysis/tests/analysis/type_checker_tests.rs diff --git a/wdl-analysis/src/types/v1.rs b/wdl-analysis/src/types/v1.rs index 6d9b56770..22dd9ec0d 100644 --- a/wdl-analysis/src/types/v1.rs +++ b/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,6 +659,7 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { /// Checks a placeholder expression. pub(crate) fn check_placeholder(&mut self, placeholder: &Placeholder) { self.placeholders += 1; + // Evaluate the placeholder expression and check that the resulting type is // coercible to string for interpolation let expr = placeholder.expr(); @@ -667,35 +670,31 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { 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) - } + PlaceholderOption::TrueFalse(_) => matches!(ty, Type::Primitive(PrimitiveType::Boolean, false) | Type::Union), }; + if !valid { self.context - .add_diagnostic(Self::invalid_placeholder_option( - &ty, - expr.span(), - Some(option.map(|n| n.into())), - )); + .add_diagnostic(diagnostics::invalid_placeholder_option(&ty, expr.span(), option)); } } 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; } + - fn invalid_placeholder_option( + pub fn invalid_placeholder_option( ty: &Type, - _span: Span, + span: Span, option: Option, ) -> Diagnostic { let message = match option { @@ -708,14 +707,17 @@ impl<'a, C: EvaluationContext> ExprTypeEvaluator<'a, C> { Some(PlaceholderOption::TrueFalse(_)) => format!( "type mismatch for placeholder option `true/false`: expected type `Boolean`, but found `{ty}`" ), - None => format!("cannot coerce type `{}` to `String`", ty), + None => format!("cannot coerce type `{ty}` to `String`"), }; - Diagnostic::error(message).with_label(format!("this is type `{ty}`"), span) - } - - fn cannot_coerce_to_string(ty: &Type, _span: Span) -> Diagnostic { - Diagnostic::error(format!("cannot coerce type `{}` to `String`", ty)) // No span here + + 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(&mut self, expr: &LiteralArray) -> Type { diff --git a/wdl-analysis/tests/analysis/placeholder-options/source.diagnostics b/wdl-analysis/tests/analysis/placeholder-options/source.diagnostics deleted file mode 100644 index c8d3aaeda..000000000 --- a/wdl-analysis/tests/analysis/placeholder-options/source.diagnostics +++ /dev/null @@ -1,6 +0,0 @@ -error: type mismatch for placeholder option `sep`: expected type `Array[P]` where P: any primitive type or Union, but found `Boolean` - ┌─ tests/analysis/placeholder-options/source.wdl:13:1 - │ -13 │ - │ - diff --git a/wdl-analysis/tests/analysis/placeholder-options/source.wdl b/wdl-analysis/tests/analysis/placeholder-options/source.wdl index 2b751ad0e..0deb9b2e6 100644 --- a/wdl-analysis/tests/analysis/placeholder-options/source.wdl +++ b/wdl-analysis/tests/analysis/placeholder-options/source.wdl @@ -1,12 +1,42 @@ version 1.2 +# Test cases for placeholder options analysis + task valid_case { input { Array[String] arr } - command <<< ~{sep=", " arr} >>> + command <<< ~{sep=", " arr} >>> # OK: sep option with an array } task invalid_case { - command <<< ~{sep=", " true} >>> + 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 } diff --git a/wdl-analysis/tests/analysis/placeholders/source.diagnostics b/wdl-analysis/tests/analysis/placeholders/source.diagnostics deleted file mode 100644 index 875a1ad41..000000000 --- a/wdl-analysis/tests/analysis/placeholders/source.diagnostics +++ /dev/null @@ -1,86 +0,0 @@ -error: type mismatch for placeholder option `default`: expected an optional primitive type, but found `Int` - ┌─ tests/analysis/placeholders/source.wdl:13:2 - │ -13 │ } - │ - -error: type mismatch for placeholder option `sep`: expected type `Array[P]` where P: any primitive type or Union, but found `Int` - ┌─ tests/analysis/placeholders/source.wdl:13:2 - │ -13 │ } - │ - -error: type mismatch for placeholder option `truefalse`: expected type `Boolean`, but found `Int` - ┌─ tests/analysis/placeholders/source.wdl:13:2 - │ -13 │ } - │ - -warning[UnusedDeclaration]: unused declaration `s1` - ┌─ tests/analysis/placeholders/source.wdl:5:12 - │ -5 │ String s1 = "~{true}" # OK: Boolean is coercible to string, no option - │ ^^ - -warning[UnusedDeclaration]: unused declaration `s2` - ┌─ tests/analysis/placeholders/source.wdl:6:12 - │ -6 │ String s2 = "~{true="yes" false="no" false}" # OK: truefalse option with Boolean - │ ^^ - -warning[UnusedDeclaration]: unused declaration `s3` - ┌─ tests/analysis/placeholders/source.wdl:7:12 - │ -7 │ String s3 = "~{true="yes" false="no" 1}" # NOT OK: truefalse expects Boolean, but got Int - │ ^^ - -warning[UnusedDeclaration]: unused declaration `s4` - ┌─ tests/analysis/placeholders/source.wdl:8:12 - │ -8 │ String s4 = "~{sep=',' [1, 2, 3]}" # OK: sep option with Array[Int] - │ ^^ - -warning[UnusedDeclaration]: unused declaration `s5` - ┌─ tests/analysis/placeholders/source.wdl:9:12 - │ -9 │ String s5 = "~{sep=',' 123}" # NOT OK: sep expects Array, but got Int - │ ^^ - -warning[UnusedDeclaration]: unused declaration `s6` - ┌─ tests/analysis/placeholders/source.wdl:10:12 - │ -10 │ String s6 = "~{default="fallback" var?}" # OK: default option with optional variable - │ ^^ - -error: unknown name `var` - ┌─ tests/analysis/placeholders/source.wdl:10:39 - │ -10 │ String s6 = "~{default="fallback" var?}" # OK: default option with optional variable - │ ^^^ - -error: expected `}`, but found `?` - ┌─ tests/analysis/placeholders/source.wdl:10:42 - │ -10 │ String s6 = "~{default="fallback" var?}" # OK: default option with optional variable - │ -- ^ unexpected `?` - │ │ - │ this placeholder start is not matched - -warning[UnusedDeclaration]: unused declaration `s7` - ┌─ tests/analysis/placeholders/source.wdl:11:12 - │ -11 │ String s7 = "~{default="fallback" 123}" # NOT OK: default expects optional, but got Int - │ ^^ - -warning[UnusedDeclaration]: unused declaration `s8` - ┌─ tests/analysis/placeholders/source.wdl:12:12 - │ -12 │ String s8 = "~{[1, 2, 3]}" # NOT OK: Array without sep option, not coercible to string - │ ^^ - -error: cannot coerce type `Array[Int]+` to `String` - ┌─ tests/analysis/placeholders/source.wdl:12:20 - │ -12 │ String s8 = "~{[1, 2, 3]}" # NOT OK: Array without sep option, not coercible to string - │ ^^^^^^^^^ this is type `Array[Int]+` - diff --git a/wdl-analysis/tests/analysis/placeholders/source.wdl b/wdl-analysis/tests/analysis/placeholders/source.wdl deleted file mode 100644 index 019852f01..000000000 --- a/wdl-analysis/tests/analysis/placeholders/source.wdl +++ /dev/null @@ -1,13 +0,0 @@ -# This is a test for analysis of placeholders and their options. -version 1.1 - -workflow test { - 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 -} \ No newline at end of file diff --git a/wdl-analysis/tests/analysis/type_checker_tests.rs b/wdl-analysis/tests/analysis/type_checker_tests.rs deleted file mode 100644 index 00e103ca0..000000000 --- a/wdl-analysis/tests/analysis/type_checker_tests.rs +++ /dev/null @@ -1,46 +0,0 @@ -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_sep_with_array() { - let expr_type = Type::Array(Box::new(Type::Int)); // Array[Int] - let placeholder = Placeholder::Sep(",".to_string()); - assert!(check_placeholder(&expr_type, &placeholder).is_ok()); - } - - #[test] - fn test_sep_with_non_array() { - let expr_type = Type::Int; // Not an array - let placeholder = Placeholder::Sep(",".to_string()); - assert!(check_placeholder(&expr_type, &placeholder).is_err()); - } - - #[test] - fn test_default_with_optional() { - let expr_type = Type::Optional(Box::new(Type::String)); // Optional[String] - let placeholder = Placeholder::Default("default_value".to_string()); - assert!(check_placeholder(&expr_type, &placeholder).is_ok()); - } - - #[test] - fn test_default_with_non_optional() { - let expr_type = Type::String; // Not optional - let placeholder = Placeholder::Default("default_value".to_string()); - assert!(check_placeholder(&expr_type, &placeholder).is_err()); - } - - #[test] - fn test_true_false_with_boolean() { - let expr_type = Type::Boolean; - let placeholder = Placeholder::TrueFalse("yes".to_string(), "no".to_string()); - assert!(check_placeholder(&expr_type, &placeholder).is_ok()); - } - - #[test] - fn test_true_false_with_non_boolean() { - let expr_type = Type::String; // Not a boolean - let placeholder = Placeholder::TrueFalse("yes".to_string(), "no".to_string()); - assert!(check_placeholder(&expr_type, &placeholder).is_err()); - } -}