From 9a2915025b92facc85bc03fd9630074ab17a3da3 Mon Sep 17 00:00:00 2001 From: Ryan Saperstein Date: Mon, 26 Feb 2024 11:00:05 -0500 Subject: [PATCH 01/11] Implements the quote() and squote() engine functions. --- .../biscayne_new_engine_functions.test | 16 +++ .../biscayne_new_engine_functions.wdl | 14 +- .../draft3/elements/ExpressionElement.scala | 2 + .../AstToNewExpressionElements.scala | 4 +- .../BiscayneExpressionValueConsumers.scala | 14 ++ .../expression/consumed/consumed.scala | 2 + .../files/BiscayneFileEvaluators.scala | 6 +- .../linking/expression/files/files.scala | 3 +- .../types/BiscayneTypeEvaluators.scala | 28 +++- .../linking/expression/types/types.scala | 3 +- .../values/BiscayneValueEvaluators.scala | 42 ++++++ .../linking/expression/values/values.scala | 4 + .../transforms/biscayne/Ast2WdlomSpec.scala | 12 ++ ...BiscayneExpressionValueConsumersSpec.scala | 18 +++ .../files/BiscayneFileEvaluatorSpec.scala | 22 ++++ .../types/BiscayneTypeEvaluatorSpec.scala | 18 +++ .../values/BiscayneValueEvaluatorSpec.scala | 124 ++++++++++++++++++ .../AstToNewExpressionElements.scala | 4 +- .../CascadesExpressionValueConsumers.scala | 14 ++ .../expression/consumed/consumed.scala | 2 + .../files/CascadesFileEvaluators.scala | 6 +- .../linking/expression/files/files.scala | 2 + .../types/CascadesTypeEvaluators.scala | 26 ++++ .../linking/expression/types/types.scala | 2 + .../values/CascadesValueEvaluators.scala | 42 ++++++ .../linking/expression/values/values.scala | 4 + .../transforms/cascades/Ast2WdlomSpec.scala | 12 ++ ...CascadesExpressionValueConsumersSpec.scala | 18 +++ .../files/CascadesFileEvaluatorSpec.scala | 22 ++++ .../types/CascadesTypeEvaluatorSpec.scala | 18 +++ .../values/CascadesValueEvaluatorSpec.scala | 124 ++++++++++++++++++ .../transforms/ast2wdlom/Ast2WdlomSpec.scala | 12 ++ .../base/wdlom2wdl/WdlWriterImpl.scala | 2 + 33 files changed, 634 insertions(+), 8 deletions(-) diff --git a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test index 6998d4af7e7..9c65a41e6ed 100644 --- a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test +++ b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test @@ -58,4 +58,20 @@ metadata { "outputs.biscayne_new_engine_functions.with_suffixes.0": "aaaS" "outputs.biscayne_new_engine_functions.with_suffixes.1": "bbbS" "outputs.biscayne_new_engine_functions.with_suffixes.2": "cccS" + + "outputs.biscayne_new_engine_functions.with_quotes.0": "\"1\"" + "outputs.biscayne_new_engine_functions.with_quotes.1": "\"2\"" + "outputs.biscayne_new_engine_functions.with_quotes.2": "\"3\"" + + "outputs.biscayne_new_engine_functions.string_with_quotes.0": "\"aaa\"" + "outputs.biscayne_new_engine_functions.string_with_quotes.1": "\"bbb\"" + "outputs.biscayne_new_engine_functions.string_with_quotes.2": "\"ccc\"" + + "outputs.biscayne_new_engine_functions.with_squotes.0": "\'1\'" + "outputs.biscayne_new_engine_functions.with_squotes.1": "\'2\'" + "outputs.biscayne_new_engine_functions.with_squotes.2": "\'3\'" + + "outputs.biscayne_new_engine_functions.string_with_squotes.0": "\'aaa\'" + "outputs.biscayne_new_engine_functions.string_with_squotes.1": "\'bbb\'" + "outputs.biscayne_new_engine_functions.string_with_squotes.2": "\'ccc\'" } diff --git a/centaur/src/main/resources/standardTestCases/wdl_biscayne/biscayne_new_engine_functions/biscayne_new_engine_functions.wdl b/centaur/src/main/resources/standardTestCases/wdl_biscayne/biscayne_new_engine_functions/biscayne_new_engine_functions.wdl index f7b3d0de7fd..38eb21c1b83 100644 --- a/centaur/src/main/resources/standardTestCases/wdl_biscayne/biscayne_new_engine_functions/biscayne_new_engine_functions.wdl +++ b/centaur/src/main/resources/standardTestCases/wdl_biscayne/biscayne_new_engine_functions/biscayne_new_engine_functions.wdl @@ -4,7 +4,7 @@ workflow biscayne_new_engine_functions { meta { description: "This test makes sure that these functions work in a real workflow" - functions_under_test: [ "keys", "as_map", "as_pairs", "collect_by_key", "suffix" ] + functions_under_test: [ "keys", "as_map", "as_pairs", "collect_by_key", "suffix", "quote", 'squote' ] } Map[String, Int] x_map_in = {"a": 1, "b": 2, "c": 3} @@ -17,6 +17,8 @@ workflow biscayne_new_engine_functions { Array[String] some_strings = ["aaa", "bbb", "ccc"] + Array[Int] some_ints = [1, 2, 3] + Int smallestInt = 1 Float smallFloat = 2.718 Float bigFloat = 3.141 @@ -54,6 +56,16 @@ workflow biscayne_new_engine_functions { # suffix(): # ================================================= Array[String] with_suffixes = suffix("S", some_strings) + + # quote(): + # ================================================= + Array[String] with_quotes = quote(some_ints) + Array[String] string_with_quotes = quote(some_strings) + + # squote(): + # ================================================= + Array[String] with_squotes = squote(some_ints) + Array[String] string_with_squotes = squote(some_strings) } } diff --git a/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/ExpressionElement.scala b/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/ExpressionElement.scala index 91f34d064ae..96f51d806bd 100644 --- a/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/ExpressionElement.scala +++ b/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/ExpressionElement.scala @@ -136,6 +136,8 @@ object ExpressionElement { final case class Ceil(param: ExpressionElement) extends OneParamFunctionCallElement final case class Round(param: ExpressionElement) extends OneParamFunctionCallElement final case class Glob(param: ExpressionElement) extends OneParamFunctionCallElement + final case class Quote(param: ExpressionElement) extends OneParamFunctionCallElement + final case class SQuote(param: ExpressionElement) extends OneParamFunctionCallElement // 1- or 2-param functions: sealed trait OneOrTwoParamFunctionCallElement extends FunctionCallElement { diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala index 5c0c01859c2..35fb5b4d741 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala @@ -3,7 +3,7 @@ package wdl.transforms.biscayne.ast2wdlom import cats.syntax.validated._ import common.validation.ErrorOr.ErrorOr import wdl.model.draft3.elements.ExpressionElement -import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, Suffix} +import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Quote, Sep, Suffix, SQuote} import wdl.transforms.base.ast2wdlom.AstNodeToExpressionElement object AstToNewExpressionElements { @@ -16,6 +16,8 @@ object AstToNewExpressionElements { "max" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Max, "max"), "sep" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Sep, "sep"), "suffix" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Suffix, "suffix"), + "quote" -> AstNodeToExpressionElement.validateOneParamEngineFunction(Quote, "quote"), + "squote" -> AstNodeToExpressionElement.validateOneParamEngineFunction(SQuote, "squote"), "read_object" -> (_ => "read_object is no longer available in this WDL version. Consider using read_json instead".invalidNel ), diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala index 3c73f8b55cd..e75da86ab3d 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala @@ -66,6 +66,20 @@ object BiscayneExpressionValueConsumers { expressionValueConsumer.expressionConsumedValueHooks(a.arg2)(expressionValueConsumer) } + implicit val quoteExpressionValueConsumer: ExpressionValueConsumer[Quote] = new ExpressionValueConsumer[Quote] { + override def expressionConsumedValueHooks(a: Quote)(implicit + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] + ): Set[UnlinkedConsumedValueHook] = + expressionValueConsumer.expressionConsumedValueHooks(a.param)(expressionValueConsumer) + } + + implicit val sQuoteExpressionValueConsumer: ExpressionValueConsumer[SQuote] = new ExpressionValueConsumer[SQuote] { + override def expressionConsumedValueHooks(a: SQuote)(implicit + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] + ): Set[UnlinkedConsumedValueHook] = + expressionValueConsumer.expressionConsumedValueHooks(a.param)(expressionValueConsumer) + } + implicit val noneLiteralExpressionValueConsumer: ExpressionValueConsumer[NoneLiteralElement.type] = new ExpressionValueConsumer[NoneLiteralElement.type] { override def expressionConsumedValueHooks(a: NoneLiteralElement.type)(implicit diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/consumed.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/consumed.scala index e1121e1d549..0778995ba43 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/consumed.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/consumed.scala @@ -104,6 +104,8 @@ package object consumed { case a: AsPairs => a.expressionConsumedValueHooks(expressionValueConsumer) case a: CollectByKey => a.expressionConsumedValueHooks(expressionValueConsumer) case a: Sep => sepExpressionValueConsumer.expressionConsumedValueHooks(a)(expressionValueConsumer) + case a: Quote => a.expressionConsumedValueHooks(expressionValueConsumer) + case a: SQuote => a.expressionConsumedValueHooks(expressionValueConsumer) case a: Min => a.expressionConsumedValueHooks(expressionValueConsumer) case a: Max => a.expressionConsumedValueHooks(expressionValueConsumer) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala index 6a79d2b3948..51057b77197 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala @@ -1,6 +1,6 @@ package wdl.transforms.biscayne.linking.expression.files -import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, Suffix} +import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Quote, Sep, Suffix, SQuote} import wdl.model.draft3.graph.expression.FileEvaluator import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.twoParameterFunctionPassthroughFileEvaluator @@ -17,6 +17,10 @@ object BiscayneFileEvaluators { implicit val sepFunctionEvaluator: FileEvaluator[Sep] = twoParameterFunctionPassthroughFileEvaluator[Sep] implicit val suffixFunctionEvaluator: FileEvaluator[Suffix] = twoParameterFunctionPassthroughFileEvaluator[Suffix] + implicit val quoteFunctionEvaluator: FileEvaluator[Quote] = + EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator + implicit val sQuoteFunctionEvaluator: FileEvaluator[SQuote] = + EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator implicit val minFunctionEvaluator: FileEvaluator[Min] = twoParameterFunctionPassthroughFileEvaluator[Min] implicit val maxFunctionEvaluator: FileEvaluator[Max] = twoParameterFunctionPassthroughFileEvaluator[Max] diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/files.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/files.scala index 9dd95e9a0ec..f87f3f466ef 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/files.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/files.scala @@ -146,7 +146,8 @@ package object files { case a: Ceil => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: Round => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: Glob => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) - + case a: Quote => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) + case a: SQuote => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: Size => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: Basename => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala index 1ff5bf71ba4..d633dfed970 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala @@ -110,4 +110,30 @@ object BiscayneTypeEvaluators { validateParamType(a.array, linkedValues, WomArrayType(WomStringType)) ) mapN { (_, _) => WomArrayType(WomStringType) } } -} + + implicit val quoteFunctionEvaluator: TypeEvaluator[Quote] = new TypeEvaluator[Quote] { + override def evaluateType(a: Quote, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit + expressionTypeEvaluator: TypeEvaluator[ExpressionElement] + ): ErrorOr[WomType] = + validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { + case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel + case other @ WomArrayType(_) => + s"Cannot invoke quote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel + case other => + s"Cannot invoke quote on type ${other.stableName}. Expected an Array[Primitive type]".invalidNel + } + } + + implicit val sQuoteFunctionEvaluator: TypeEvaluator[SQuote] = new TypeEvaluator[SQuote] { + override def evaluateType(a: SQuote, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit + expressionTypeEvaluator: TypeEvaluator[ExpressionElement] + ): ErrorOr[WomType] = + validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { + case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel + case other @ WomArrayType(_) => + s"Cannot invoke squote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel + case other => + s"Cannot invoke squote on type ${other.stableName}. Expected an Array[Primitive type]".invalidNel + } + } +} \ No newline at end of file diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/types.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/types.scala index dec2deeb24f..39819575fe3 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/types.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/types.scala @@ -91,7 +91,8 @@ package object types { case a: Ceil => a.evaluateType(linkedValues)(typeEvaluator) case a: Round => a.evaluateType(linkedValues)(typeEvaluator) case a: Glob => a.evaluateType(linkedValues)(typeEvaluator) - + case a: Quote => a.evaluateType(linkedValues)(typeEvaluator) + case a: SQuote => a.evaluateType(linkedValues)(typeEvaluator) case a: Size => a.evaluateType(linkedValues)(typeEvaluator) case a: Basename => a.evaluateType(linkedValues)(typeEvaluator) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala index 0c4805f0e68..48b3d61818f 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala @@ -235,4 +235,46 @@ object BiscayneValueEvaluators { EvaluatedValue(WomArray(arr.value.map(v => WomString(v.valueString + suffix.value))), Seq.empty).validNel } } + + /** + * Quote: Given an array of primitives, produce a new array in which all elements of the original are in quotes ("). + * https://github.com/openwdl/wdl/blob/main/versions/1.1/SPEC.md#-arraystring-quotearrayp + * input: Array[Primitive] + * output: Array[String] + */ + implicit val quoteFunctionEvaluator: ValueEvaluator[Quote] = new ValueEvaluator[Quote] { + override def evaluateValue(a: Quote, + inputs: Map[String, WomValue], + ioFunctionSet: IoFunctionSet, + forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = + processValidatedSingleValue[WomArray, WomArray]( + expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( + expressionValueEvaluator + ), + ) { arr => + EvaluatedValue(WomArray(arr.value.map(v => WomString("\"" + v.valueString + "\""))), Seq.empty).validNel + } + } + + /** + * SQuote: Given an array of primitives, produce a new array in which all elements of the original are in single quotes ('). + * https://github.com/openwdl/wdl/blob/main/versions/1.1/SPEC.md#-arraystring-squotearrayp + * input: Array[Primitive] + * output: Array[String] + */ + implicit val sQuoteFunctionEvaluator: ValueEvaluator[SQuote] = new ValueEvaluator[SQuote] { + override def evaluateValue(a: SQuote, + inputs: Map[String, WomValue], + ioFunctionSet: IoFunctionSet, + forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = + processValidatedSingleValue[WomArray, WomArray]( + expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( + expressionValueEvaluator + ), + ) { arr => + EvaluatedValue(WomArray(arr.value.map(v => WomString("\'" + v.valueString + "\'"))), Seq.empty).validNel + } + } } diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/values.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/values.scala index 3106ceba428..90aa0301951 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/values.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/values.scala @@ -156,6 +156,10 @@ package object values { case a: Round => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: Glob => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) + case a: Quote => + a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) + case a: SQuote => + a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: Size => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: Basename => diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/Ast2WdlomSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/Ast2WdlomSpec.scala index a0953cd8397..4d7bf162648 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/Ast2WdlomSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/Ast2WdlomSpec.scala @@ -103,4 +103,16 @@ class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { val expr = fromString[ExpressionElement](str, parser.parse_e) expr shouldBeValid (Suffix(IdentifierLookup("some_str"), IdentifierLookup("some_arr"))) } + + it should "parse the new quote function" in { + val str = "quote(some_arr)" + val expr = fromString[ExpressionElement](str, parser.parse_e) + expr shouldBeValid (Quote(IdentifierLookup("some_arr"))) + } + + it should "parse the new squote function" in { + val str = "squote(some_arr)" + val expr = fromString[ExpressionElement](str, parser.parse_e) + expr shouldBeValid (SQuote(IdentifierLookup("some_arr"))) + } } diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumersSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumersSpec.scala index 76f4a0338e7..b2f7b364c5c 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumersSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumersSpec.scala @@ -75,4 +75,22 @@ class BiscayneExpressionValueConsumersSpec extends AnyFlatSpec with CromwellTime e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_array"))) } } + + it should "discover an array variable lookup within a quote() call" in { + val str = """ quote(my_array) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_array"))) + } + } + + it should "discover an array variable lookup within a squote() call" in { + val str = """ squote(my_array) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_array"))) + } + } } diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluatorSpec.scala index 4a3ed6b63b9..f33d8b81445 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluatorSpec.scala @@ -56,4 +56,26 @@ class BiscayneFileEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit ) } } + + it should "discover the file which would be required to evaluate a quote() function" in { + val str = """ quote(read_lines("foo.txt")) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.predictFilesNeededToEvaluate(Map.empty, NoIoFunctionSet, WomStringType) shouldBeValid Set( + WomSingleFile("foo.txt") + ) + } + } + + it should "discover the file which would be required to evaluate a squote() function" in { + val str = """ squote(read_lines("foo.txt")) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.predictFilesNeededToEvaluate(Map.empty, NoIoFunctionSet, WomStringType) shouldBeValid Set( + WomSingleFile("foo.txt") + ) + } + } } diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala index c8372d36cd1..dda27a06186 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala @@ -64,4 +64,22 @@ class BiscayneTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomStringType) } } + + it should "evaluate the type of a quote() function as Array[String]" in { + val str = """ quote([1, 2, 3]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomStringType) + } + } + + it should "evaluate the type of a squote() function as Array[String]" in { + val str = """ squote([1, 2, 3]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomStringType) + } + } } diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala index 1487543a838..087f2f84b22 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala @@ -217,4 +217,128 @@ class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) } } + + it should "evaluate a quote expression correctly with an empty array" in { + val str = """ quote([]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray(Seq()) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a quote expression correctly with an array of integers" in { + val str = """ quote([1, 2, 3]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\"1\""), + WomString("\"2\""), + WomString("\"3\"") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a quote expression correctly with an array of strings" in { + val str = """ quote(["a", "b", "c"]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\"a\""), + WomString("\"b\""), + WomString("\"c\"") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a quote expression correctly with an array of strings that are already in quotes" in { + val str = """ quote(["\"a\"", "\"b", "c\""]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\"\"a\"\""), + WomString("\"\"b\""), + WomString("\"c\"\"") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a squote expression correctly with an empty array" in { + val str = """ squote([]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray(Seq()) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a squote expression correctly with an array of integers" in { + val str = """ squote([1, 2, 3]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\'1\'"), + WomString("\'2\'"), + WomString("\'3\'") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a squote expression correctly with an array of strings" in { + val str = """ squote(["a", "b", "c"]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\'a\'"), + WomString("\'b\'"), + WomString("\'c\'") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a squote expression correctly with an array of strings that are already in quotes" in { + val str = """ squote(["\'a\'", "\'b", "c\'"]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\'\'a\'\'"), + WomString("\'\'b\'"), + WomString("\'c\'\'") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } } diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala index 46cd8cb2e60..c132709da94 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala @@ -3,7 +3,7 @@ package wdl.transforms.cascades.ast2wdlom import cats.syntax.validated._ import common.validation.ErrorOr.ErrorOr import wdl.model.draft3.elements.ExpressionElement -import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, Suffix} +import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Quote, Sep, Suffix, SQuote} import wdl.transforms.base.ast2wdlom.AstNodeToExpressionElement object AstToNewExpressionElements { @@ -16,6 +16,8 @@ object AstToNewExpressionElements { "max" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Max, "max"), "sep" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Sep, "sep"), "suffix" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Suffix, "suffix"), + "quote" -> AstNodeToExpressionElement.validateOneParamEngineFunction(Quote, "quote"), + "squote" -> AstNodeToExpressionElement.validateOneParamEngineFunction(SQuote, "squote"), "read_object" -> (_ => "read_object is no longer available in this WDL version. Consider using read_json instead".invalidNel ), diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala index ebc6d48fba1..ea677ff7601 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala @@ -66,6 +66,20 @@ object cascadesExpressionValueConsumers { expressionValueConsumer.expressionConsumedValueHooks(a.arg2)(expressionValueConsumer) } + implicit val quoteExpressionValueConsumer: ExpressionValueConsumer[Quote] = new ExpressionValueConsumer[Quote] { + override def expressionConsumedValueHooks(a: Quote)(implicit + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] + ): Set[UnlinkedConsumedValueHook] = + expressionValueConsumer.expressionConsumedValueHooks(a.param)(expressionValueConsumer) + } + + implicit val sQuoteExpressionValueConsumer: ExpressionValueConsumer[SQuote] = new ExpressionValueConsumer[SQuote] { + override def expressionConsumedValueHooks(a: SQuote)(implicit + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] + ): Set[UnlinkedConsumedValueHook] = + expressionValueConsumer.expressionConsumedValueHooks(a.param)(expressionValueConsumer) + } + implicit val noneLiteralExpressionValueConsumer: ExpressionValueConsumer[NoneLiteralElement.type] = new ExpressionValueConsumer[NoneLiteralElement.type] { override def expressionConsumedValueHooks(a: NoneLiteralElement.type)(implicit diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/consumed.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/consumed.scala index 2026aaec010..1df871495e1 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/consumed.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/consumed.scala @@ -82,6 +82,8 @@ package object consumed { case a: Flatten => a.expressionConsumedValueHooks(expressionValueConsumer) case a: Prefix => a.expressionConsumedValueHooks(expressionValueConsumer) case a: Suffix => a.expressionConsumedValueHooks(expressionValueConsumer) + case a: Quote => a.expressionConsumedValueHooks(expressionValueConsumer) + case a: SQuote => a.expressionConsumedValueHooks(expressionValueConsumer) case a: SelectFirst => a.expressionConsumedValueHooks(expressionValueConsumer) case a: SelectAll => a.expressionConsumedValueHooks(expressionValueConsumer) case a: Defined => a.expressionConsumedValueHooks(expressionValueConsumer) diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala index 32ea447b584..d508c7bf713 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala @@ -1,6 +1,6 @@ package wdl.transforms.cascades.linking.expression.files -import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Sep, Suffix} +import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Quote, Sep, Suffix, SQuote} import wdl.model.draft3.graph.expression.FileEvaluator import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.twoParameterFunctionPassthroughFileEvaluator @@ -17,6 +17,10 @@ object cascadesFileEvaluators { implicit val sepFunctionEvaluator: FileEvaluator[Sep] = twoParameterFunctionPassthroughFileEvaluator[Sep] implicit val suffixFunctionEvaluator: FileEvaluator[Suffix] = twoParameterFunctionPassthroughFileEvaluator[Suffix] + implicit val quoteFunctionEvaluator: FileEvaluator[Quote] = + EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator + implicit val sQuoteFunctionEvaluator: FileEvaluator[SQuote] = + EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator implicit val minFunctionEvaluator: FileEvaluator[Min] = twoParameterFunctionPassthroughFileEvaluator[Min] implicit val maxFunctionEvaluator: FileEvaluator[Max] = twoParameterFunctionPassthroughFileEvaluator[Max] diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/files.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/files.scala index a53c85b9b89..d629afbd0e2 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/files.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/files.scala @@ -136,6 +136,8 @@ package object files { a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: Prefix => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: Suffix => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) + case a: Quote => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) + case a: SQuote => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: SelectFirst => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: SelectAll => diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala index 180af9eb0d8..e30a7f1bca8 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala @@ -110,4 +110,30 @@ object cascadesTypeEvaluators { validateParamType(a.array, linkedValues, WomArrayType(WomStringType)) ) mapN { (_, _) => WomArrayType(WomStringType) } } + + implicit val quoteFunctionEvaluator: TypeEvaluator[Quote] = new TypeEvaluator[Quote] { + override def evaluateType(a: Quote, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit + expressionTypeEvaluator: TypeEvaluator[ExpressionElement] + ): ErrorOr[WomType] = + validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { + case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel + case other @ WomArrayType(_) => + s"Cannot invoke quote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel + case other => + s"Cannot invoke quote on type ${other.stableName}. Expected an Array[Primitive type]".invalidNel + } + } + + implicit val sQuoteFunctionEvaluator: TypeEvaluator[SQuote] = new TypeEvaluator[SQuote] { + override def evaluateType(a: SQuote, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit + expressionTypeEvaluator: TypeEvaluator[ExpressionElement] + ): ErrorOr[WomType] = + validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { + case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel + case other @ WomArrayType(_) => + s"Cannot invoke quote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel + case other => + s"Cannot invoke quote on type ${other.stableName}. Expected an Array[Primitive type]".invalidNel + } + } } diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/types.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/types.scala index 4dd7c528e8b..9b95cd88b74 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/types.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/types.scala @@ -84,6 +84,8 @@ package object types { case a: Flatten => a.evaluateType(linkedValues)(typeEvaluator) case a: Prefix => a.evaluateType(linkedValues)(typeEvaluator) case a: Suffix => a.evaluateType(linkedValues)(typeEvaluator) + case a: Quote => a.evaluateType(linkedValues)(typeEvaluator) + case a: SQuote => a.evaluateType(linkedValues)(typeEvaluator) case a: SelectFirst => a.evaluateType(linkedValues)(typeEvaluator) case a: SelectAll => a.evaluateType(linkedValues)(typeEvaluator) case a: Defined => a.evaluateType(linkedValues)(typeEvaluator) diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala index db9df85bf06..97d06b5b3cc 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala @@ -235,4 +235,46 @@ object cascadesValueEvaluators { EvaluatedValue(WomArray(arr.value.map(v => WomString(v.valueString + suffix.value))), Seq.empty).validNel } } + + /** + * Quote: Given an array of primitives, produce a new array in which all elements of the original are in quotes ("). + * https://github.com/openwdl/wdl/blob/main/versions/1.1/SPEC.md#-arraystring-quotearrayp + * input: Array[Primitive] + * output: Array[String] + */ + implicit val quoteFunctionEvaluator: ValueEvaluator[Quote] = new ValueEvaluator[Quote] { + override def evaluateValue(a: Quote, + inputs: Map[String, WomValue], + ioFunctionSet: IoFunctionSet, + forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = + processValidatedSingleValue[WomArray, WomArray]( + expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( + expressionValueEvaluator + ), + ) { arr => + EvaluatedValue(WomArray(arr.value.map(v => WomString("\"" + v.valueString + "\""))), Seq.empty).validNel + } + } + + /** + * SQuote: Given an array of primitives, produce a new array in which all elements of the original are in single quotes ('). + * https://github.com/openwdl/wdl/blob/main/versions/1.1/SPEC.md#-arraystring-squotearrayp + * input: Array[Primitive] + * output: Array[String] + */ + implicit val sQuoteFunctionEvaluator: ValueEvaluator[SQuote] = new ValueEvaluator[SQuote] { + override def evaluateValue(a: SQuote, + inputs: Map[String, WomValue], + ioFunctionSet: IoFunctionSet, + forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = + processValidatedSingleValue[WomArray, WomArray]( + expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( + expressionValueEvaluator + ), + ) { arr => + EvaluatedValue(WomArray(arr.value.map(v => WomString("\'" + v.valueString + "\'"))), Seq.empty).validNel + } + } } diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/values.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/values.scala index 8850d4371df..89b2729e896 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/values.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/values.scala @@ -144,6 +144,10 @@ package object values { a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: Suffix => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) + case a: Quote => + a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) + case a: SQuote => + a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: SelectFirst => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: SelectAll => diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/Ast2WdlomSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/Ast2WdlomSpec.scala index 88b7aff6578..d49a81210d4 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/Ast2WdlomSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/Ast2WdlomSpec.scala @@ -103,4 +103,16 @@ class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { val expr = fromString[ExpressionElement](str, parser.parse_e) expr shouldBeValid (Suffix(IdentifierLookup("some_str"), IdentifierLookup("some_arr"))) } + + it should "parse the new quote function" in { + val str = "quote(some_arr)" + val expr = fromString[ExpressionElement](str, parser.parse_e) + expr shouldBeValid (Quote(IdentifierLookup("some_arr"))) + } + + it should "parse the new squote function" in { + val str = "squote(some_arr)" + val expr = fromString[ExpressionElement](str, parser.parse_e) + expr shouldBeValid (SQuote(IdentifierLookup("some_arr"))) + } } diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala index 57beb7a12a1..7f10a2ae223 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala @@ -75,4 +75,22 @@ class CascadesExpressionValueConsumersSpec extends AnyFlatSpec with CromwellTime e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_array"))) } } + + it should "discover an array variable lookup within a quote() call" in { + val str = """ quote(my_array) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_array"))) + } + } + + it should "discover an array variable lookup within a squote() call" in { + val str = """ squote(my_array) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_array"))) + } + } } diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluatorSpec.scala index 5335f7d5f29..9f6f773fd6b 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluatorSpec.scala @@ -56,4 +56,26 @@ class CascadesFileEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit ) } } + + it should "discover the file which would be required to evaluate a quote() function" in { + val str = """ quote(read_lines("foo.txt")) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.predictFilesNeededToEvaluate(Map.empty, NoIoFunctionSet, WomStringType) shouldBeValid Set( + WomSingleFile("foo.txt") + ) + } + } + + it should "discover the file which would be required to evaluate a squote() function" in { + val str = """ squote(read_lines("foo.txt")) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.predictFilesNeededToEvaluate(Map.empty, NoIoFunctionSet, WomStringType) shouldBeValid Set( + WomSingleFile("foo.txt") + ) + } + } } diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala index 954ef0f52f0..b3c6ceb30af 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala @@ -64,4 +64,22 @@ class CascadesTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomStringType) } } + + it should "evaluate the type of a quote() function as Array[String]" in { + val str = """ quote([1, 2, 3]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomStringType) + } + } + + it should "evaluate the type of a squote() function as Array[String]" in { + val str = """ squote([1, 2, 3]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomStringType) + } + } } diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala index bd47a20ca68..83e09de366d 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala @@ -217,4 +217,128 @@ class CascadesValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) } } + + it should "evaluate a quote expression correctly with an empty array" in { + val str = """ quote([]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray(Seq()) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a quote expression correctly with an array of integers" in { + val str = """ quote([1, 2, 3]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\"1\""), + WomString("\"2\""), + WomString("\"3\"") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a quote expression correctly with an array of strings" in { + val str = """ quote(["a", "b", "c"]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\"a\""), + WomString("\"b\""), + WomString("\"c\"") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a quote expression correctly with an array of strings that are already in quotes" in { + val str = """ quote(["\"a\"", "\"b", "c\""]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\"\"a\"\""), + WomString("\"\"b\""), + WomString("\"c\"\"") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a squote expression correctly with an empty array" in { + val str = """ squote([]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray(Seq()) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a squote expression correctly with an array of integers" in { + val str = """ squote([1, 2, 3]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\'1\'"), + WomString("\'2\'"), + WomString("\'3\'") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a squote expression correctly with an array of strings" in { + val str = """ squote(["a", "b", "c"]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\'a\'"), + WomString("\'b\'"), + WomString("\'c\'") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a squote expression correctly with an array of strings that are already in quotes" in { + val str = """ squote(["\'a\'", "\'b", "c\'"]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\'\'a\'\'"), + WomString("\'\'b\'"), + WomString("\'c\'\'") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } } diff --git a/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/ast2wdlom/Ast2WdlomSpec.scala b/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/ast2wdlom/Ast2WdlomSpec.scala index 664696ee86d..791ea57cf2b 100644 --- a/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/ast2wdlom/Ast2WdlomSpec.scala +++ b/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/ast2wdlom/Ast2WdlomSpec.scala @@ -58,6 +58,18 @@ class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { expr shouldBeInvalid "Failed to parse expression (reason 1 of 1): Unknown engine function: 'suffix'" } + it should "not parse the new quote function" in { + val str = "quote(anArray)" + val expr = fromString[ExpressionElement](str, parser.parse_e) + expr shouldBeInvalid "Failed to parse expression (reason 1 of 1): Unknown engine function: 'quote'" + } + + it should "not parse the new squote function" in { + val str = "squote(anArray)" + val expr = fromString[ExpressionElement](str, parser.parse_e) + expr shouldBeInvalid "Failed to parse expression (reason 1 of 1): Unknown engine function: 'squote'" + } + it should "parse the (biscayne) None keyword as a plain old identifier" in { val str = "None" val expr = fromString[ExpressionElement](str, parser.parse_e) diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wdl/WdlWriterImpl.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wdl/WdlWriterImpl.scala index a85f3c34420..fc0061c2a10 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wdl/WdlWriterImpl.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wdl/WdlWriterImpl.scala @@ -435,6 +435,8 @@ object WdlWriterImpl { case _: Ceil => "ceil" case _: Round => "round" case _: Glob => "glob" + case _: Quote => "quote" + case _: SQuote => "squote" case _: Keys => "keys" case _: AsMap => "as_map" case _: AsPairs => "as_pairs" From b55424cb0a4486c756efc0d419a76b49cce4c6d2 Mon Sep 17 00:00:00 2001 From: Ryan Saperstein Date: Mon, 26 Feb 2024 11:13:53 -0500 Subject: [PATCH 02/11] scalafmt --- .../ast2wdlom/AstToNewExpressionElements.scala | 13 ++++++++++++- .../consumed/BiscayneExpressionValueConsumers.scala | 4 ++-- .../expression/files/BiscayneFileEvaluators.scala | 13 ++++++++++++- .../expression/types/BiscayneTypeEvaluators.scala | 2 +- .../expression/values/BiscayneValueEvaluators.scala | 8 ++++---- .../ast2wdlom/AstToNewExpressionElements.scala | 13 ++++++++++++- .../consumed/CascadesExpressionValueConsumers.scala | 4 ++-- .../expression/files/CascadesFileEvaluators.scala | 13 ++++++++++++- .../expression/types/CascadesTypeEvaluators.scala | 2 +- .../expression/values/CascadesValueEvaluators.scala | 8 ++++---- 10 files changed, 62 insertions(+), 18 deletions(-) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala index 35fb5b4d741..26554602b49 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala @@ -3,7 +3,18 @@ package wdl.transforms.biscayne.ast2wdlom import cats.syntax.validated._ import common.validation.ErrorOr.ErrorOr import wdl.model.draft3.elements.ExpressionElement -import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Quote, Sep, Suffix, SQuote} +import wdl.model.draft3.elements.ExpressionElement.{ + AsMap, + AsPairs, + CollectByKey, + Keys, + Max, + Min, + Quote, + Sep, + SQuote, + Suffix +} import wdl.transforms.base.ast2wdlom.AstNodeToExpressionElement object AstToNewExpressionElements { diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala index e75da86ab3d..3895eedca60 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala @@ -68,14 +68,14 @@ object BiscayneExpressionValueConsumers { implicit val quoteExpressionValueConsumer: ExpressionValueConsumer[Quote] = new ExpressionValueConsumer[Quote] { override def expressionConsumedValueHooks(a: Quote)(implicit - expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] ): Set[UnlinkedConsumedValueHook] = expressionValueConsumer.expressionConsumedValueHooks(a.param)(expressionValueConsumer) } implicit val sQuoteExpressionValueConsumer: ExpressionValueConsumer[SQuote] = new ExpressionValueConsumer[SQuote] { override def expressionConsumedValueHooks(a: SQuote)(implicit - expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] ): Set[UnlinkedConsumedValueHook] = expressionValueConsumer.expressionConsumedValueHooks(a.param)(expressionValueConsumer) } diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala index 51057b77197..70b7dd0f30b 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala @@ -1,6 +1,17 @@ package wdl.transforms.biscayne.linking.expression.files -import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Quote, Sep, Suffix, SQuote} +import wdl.model.draft3.elements.ExpressionElement.{ + AsMap, + AsPairs, + CollectByKey, + Keys, + Max, + Min, + Quote, + Sep, + SQuote, + Suffix +} import wdl.model.draft3.graph.expression.FileEvaluator import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.twoParameterFunctionPassthroughFileEvaluator diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala index d633dfed970..98a331f8f75 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala @@ -126,7 +126,7 @@ object BiscayneTypeEvaluators { implicit val sQuoteFunctionEvaluator: TypeEvaluator[SQuote] = new TypeEvaluator[SQuote] { override def evaluateType(a: SQuote, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit - expressionTypeEvaluator: TypeEvaluator[ExpressionElement] + expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala index 48b3d61818f..213e1062387 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala @@ -247,11 +247,11 @@ object BiscayneValueEvaluators { inputs: Map[String, WomValue], ioFunctionSet: IoFunctionSet, forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] - )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = processValidatedSingleValue[WomArray, WomArray]( expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( expressionValueEvaluator - ), + ) ) { arr => EvaluatedValue(WomArray(arr.value.map(v => WomString("\"" + v.valueString + "\""))), Seq.empty).validNel } @@ -268,11 +268,11 @@ object BiscayneValueEvaluators { inputs: Map[String, WomValue], ioFunctionSet: IoFunctionSet, forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] - )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = processValidatedSingleValue[WomArray, WomArray]( expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( expressionValueEvaluator - ), + ) ) { arr => EvaluatedValue(WomArray(arr.value.map(v => WomString("\'" + v.valueString + "\'"))), Seq.empty).validNel } diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala index c132709da94..22e58be8958 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala @@ -3,7 +3,18 @@ package wdl.transforms.cascades.ast2wdlom import cats.syntax.validated._ import common.validation.ErrorOr.ErrorOr import wdl.model.draft3.elements.ExpressionElement -import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Quote, Sep, Suffix, SQuote} +import wdl.model.draft3.elements.ExpressionElement.{ + AsMap, + AsPairs, + CollectByKey, + Keys, + Max, + Min, + Quote, + Sep, + SQuote, + Suffix +} import wdl.transforms.base.ast2wdlom.AstNodeToExpressionElement object AstToNewExpressionElements { diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala index ea677ff7601..7eb1541fb5f 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumers.scala @@ -68,14 +68,14 @@ object cascadesExpressionValueConsumers { implicit val quoteExpressionValueConsumer: ExpressionValueConsumer[Quote] = new ExpressionValueConsumer[Quote] { override def expressionConsumedValueHooks(a: Quote)(implicit - expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] ): Set[UnlinkedConsumedValueHook] = expressionValueConsumer.expressionConsumedValueHooks(a.param)(expressionValueConsumer) } implicit val sQuoteExpressionValueConsumer: ExpressionValueConsumer[SQuote] = new ExpressionValueConsumer[SQuote] { override def expressionConsumedValueHooks(a: SQuote)(implicit - expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] ): Set[UnlinkedConsumedValueHook] = expressionValueConsumer.expressionConsumedValueHooks(a.param)(expressionValueConsumer) } diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala index d508c7bf713..ed581fa595a 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala @@ -1,6 +1,17 @@ package wdl.transforms.cascades.linking.expression.files -import wdl.model.draft3.elements.ExpressionElement.{AsMap, AsPairs, CollectByKey, Keys, Max, Min, Quote, Sep, Suffix, SQuote} +import wdl.model.draft3.elements.ExpressionElement.{ + AsMap, + AsPairs, + CollectByKey, + Keys, + Max, + Min, + Quote, + Sep, + SQuote, + Suffix +} import wdl.model.draft3.graph.expression.FileEvaluator import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.twoParameterFunctionPassthroughFileEvaluator diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala index e30a7f1bca8..6efdc3d4a8a 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala @@ -126,7 +126,7 @@ object cascadesTypeEvaluators { implicit val sQuoteFunctionEvaluator: TypeEvaluator[SQuote] = new TypeEvaluator[SQuote] { override def evaluateType(a: SQuote, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit - expressionTypeEvaluator: TypeEvaluator[ExpressionElement] + expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala index 97d06b5b3cc..569c0bd13fd 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala @@ -247,11 +247,11 @@ object cascadesValueEvaluators { inputs: Map[String, WomValue], ioFunctionSet: IoFunctionSet, forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] - )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = processValidatedSingleValue[WomArray, WomArray]( expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( expressionValueEvaluator - ), + ) ) { arr => EvaluatedValue(WomArray(arr.value.map(v => WomString("\"" + v.valueString + "\""))), Seq.empty).validNel } @@ -268,11 +268,11 @@ object cascadesValueEvaluators { inputs: Map[String, WomValue], ioFunctionSet: IoFunctionSet, forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] - )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = processValidatedSingleValue[WomArray, WomArray]( expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( expressionValueEvaluator - ), + ) ) { arr => EvaluatedValue(WomArray(arr.value.map(v => WomString("\'" + v.valueString + "\'"))), Seq.empty).validNel } From 1c48340c5bb06a58320e3ba2fd66ef11ee3b6059 Mon Sep 17 00:00:00 2001 From: Ryan Saperstein Date: Mon, 26 Feb 2024 11:19:13 -0500 Subject: [PATCH 03/11] scalafmt --- .../linking/expression/types/BiscayneTypeEvaluators.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala index 98a331f8f75..0ddd5184cf4 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala @@ -136,4 +136,4 @@ object BiscayneTypeEvaluators { s"Cannot invoke squote on type ${other.stableName}. Expected an Array[Primitive type]".invalidNel } } -} \ No newline at end of file +} From 52385a21af8eaf0011b836535f5c3a7c893bf9e3 Mon Sep 17 00:00:00 2001 From: Ryan Saperstein Date: Mon, 26 Feb 2024 12:20:14 -0500 Subject: [PATCH 04/11] Fixed failing tests. --- .../biscayne_new_engine_functions.test | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test index 9c65a41e6ed..14ef63b3b7e 100644 --- a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test +++ b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test @@ -67,11 +67,11 @@ metadata { "outputs.biscayne_new_engine_functions.string_with_quotes.1": "\"bbb\"" "outputs.biscayne_new_engine_functions.string_with_quotes.2": "\"ccc\"" - "outputs.biscayne_new_engine_functions.with_squotes.0": "\'1\'" - "outputs.biscayne_new_engine_functions.with_squotes.1": "\'2\'" - "outputs.biscayne_new_engine_functions.with_squotes.2": "\'3\'" + "outputs.biscayne_new_engine_functions.with_squotes.0": "'1'" + "outputs.biscayne_new_engine_functions.with_squotes.1": "'2'" + "outputs.biscayne_new_engine_functions.with_squotes.2": "'3'" - "outputs.biscayne_new_engine_functions.string_with_squotes.0": "\'aaa\'" - "outputs.biscayne_new_engine_functions.string_with_squotes.1": "\'bbb\'" - "outputs.biscayne_new_engine_functions.string_with_squotes.2": "\'ccc\'" + "outputs.biscayne_new_engine_functions.string_with_squotes.0": "'aaa'" + "outputs.biscayne_new_engine_functions.string_with_squotes.1": "'bbb'" + "outputs.biscayne_new_engine_functions.string_with_squotes.2": "'ccc'" } From e3f950d222bfc7e3b8b7d59ad79bf56e8929d9b2 Mon Sep 17 00:00:00 2001 From: Ryan Saperstein Date: Tue, 27 Feb 2024 14:42:24 -0500 Subject: [PATCH 05/11] Addressed review comments. --- .../files/BiscayneFileEvaluators.scala | 27 ++--- .../values/BiscayneValueEvaluators.scala | 10 +- .../values/BiscayneValueEvaluatorSpec.scala | 110 +++++++++++++++++- 3 files changed, 124 insertions(+), 23 deletions(-) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala index df076a6eed7..7e079679cc9 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala @@ -9,35 +9,28 @@ import wdl.model.draft3.elements.ExpressionElement.{ Min, Quote, Sep, - SubPosix SQuote, + SubPosix, Suffix } import wdl.model.draft3.graph.expression.FileEvaluator -import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators -import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.{ - threeParameterFunctionPassthroughFileEvaluator, - twoParameterFunctionPassthroughFileEvaluator -} +import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator +import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.twoParameterFunctionPassthroughFileEvaluator +import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.threeParameterFunctionPassthroughFileEvaluator object BiscayneFileEvaluators { - implicit val keysFileEvaluator: FileEvaluator[Keys] = EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator - implicit val asMapFileEvaluator: FileEvaluator[AsMap] = - EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator - implicit val asPairsFileEvaluator: FileEvaluator[AsPairs] = - EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator - implicit val collectByKeyFileEvaluator: FileEvaluator[CollectByKey] = - EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator + implicit val keysFileEvaluator: FileEvaluator[Keys] = singleParameterPassthroughFileEvaluator + implicit val asMapFileEvaluator: FileEvaluator[AsMap] = singleParameterPassthroughFileEvaluator + implicit val asPairsFileEvaluator: FileEvaluator[AsPairs] = singleParameterPassthroughFileEvaluator + implicit val collectByKeyFileEvaluator: FileEvaluator[CollectByKey] = singleParameterPassthroughFileEvaluator implicit val sepFunctionEvaluator: FileEvaluator[Sep] = twoParameterFunctionPassthroughFileEvaluator[Sep] implicit val subPosixFunctionEvaluator: FileEvaluator[SubPosix] = threeParameterFunctionPassthroughFileEvaluator[SubPosix] implicit val suffixFunctionEvaluator: FileEvaluator[Suffix] = twoParameterFunctionPassthroughFileEvaluator[Suffix] - implicit val quoteFunctionEvaluator: FileEvaluator[Quote] = - EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator - implicit val sQuoteFunctionEvaluator: FileEvaluator[SQuote] = - EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator + implicit val quoteFunctionEvaluator: FileEvaluator[Quote] = singleParameterPassthroughFileEvaluator + implicit val sQuoteFunctionEvaluator: FileEvaluator[SQuote] = singleParameterPassthroughFileEvaluator implicit val minFunctionEvaluator: FileEvaluator[Min] = twoParameterFunctionPassthroughFileEvaluator[Min] implicit val maxFunctionEvaluator: FileEvaluator[Max] = twoParameterFunctionPassthroughFileEvaluator[Max] diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala index c4e7e083375..2b90fb65967 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala @@ -286,7 +286,10 @@ object BiscayneValueEvaluators { expressionValueEvaluator ) ) { arr => - EvaluatedValue(WomArray(arr.value.map(v => WomString("\"" + v.valueString + "\""))), Seq.empty).validNel + EvaluatedValue( + WomArray(arr.value.map(v => WomString("\"" + v.valueString.replaceAll(""""""", "\"") + "\""))), + Seq.empty + ).validNel } } @@ -307,7 +310,10 @@ object BiscayneValueEvaluators { expressionValueEvaluator ) ) { arr => - EvaluatedValue(WomArray(arr.value.map(v => WomString("\'" + v.valueString + "\'"))), Seq.empty).validNel + EvaluatedValue( + WomArray(arr.value.map(v => WomString("\'" + v.valueString.replaceAll("""'""", "\'") + "\'"))), + Seq.empty + ).validNel } } } diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala index d4abe150ff7..ad0b4804b10 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala @@ -269,6 +269,57 @@ class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi } } + it should "evaluate a quote expression correctly with an array of booleans" in { + val str = """ quote([true, false, true]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\"true\""), + WomString("\"false\""), + WomString("\"true\"") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a quote expression correctly with an array of floats" in { + val str = """ quote([1.1, 2.2, 3]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\"1.1\""), + WomString("\"2.2\""), + WomString("\"3.0\"") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a quote expression correctly with an array of files" in { + val str = """ quote(["/home/someFile.txt", "/rootFile.exe", "./anotherFile.py"]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\"/home/someFile.txt\""), + WomString("\"/rootFile.exe\""), + WomString("\"./anotherFile.py\"") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + it should "evaluate a quote expression correctly with an array of strings" in { val str = """ quote(["a", "b", "c"]) """ val expr = fromString[ExpressionElement](str, parser.parse_e) @@ -331,6 +382,57 @@ class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi } } + it should "evaluate a squote expression correctly with an array of booleans" in { + val str = """ squote([true, false, true]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\'true\'"), + WomString("\'false\'"), + WomString("\'true\'") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a squote expression correctly with an array of floats" in { + val str = """ squote([1.1, 2.2, 3]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\'1.1\'"), + WomString("\'2.2\'"), + WomString("\'3.0\'") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + + it should "evaluate a squote expression correctly with an array of files" in { + val str = """ squote(["/home/someFile.txt", "/rootFile.exe", "./anotherFile.py"]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedArray: WomArray = WomArray( + Seq( + WomString("\'/home/someFile.txt\'"), + WomString("\'/rootFile.exe\'"), + WomString("\'./anotherFile.py\'") + ) + ) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) + } + } + it should "evaluate a squote expression correctly with an array of strings" in { val str = """ squote(["a", "b", "c"]) """ val expr = fromString[ExpressionElement](str, parser.parse_e) @@ -349,14 +451,14 @@ class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi } it should "evaluate a squote expression correctly with an array of strings that are already in quotes" in { - val str = """ squote(["\'a\'", "\'b", "c\'"]) """ + val str = """ squote(["'a'", "'b", "c'"]) """ val expr = fromString[ExpressionElement](str, parser.parse_e) val expectedArray: WomArray = WomArray( Seq( - WomString("\'\'a\'\'"), - WomString("\'\'b\'"), - WomString("\'c\'\'") + WomString("""''a''"""), + WomString("""''b'"""), + WomString("""'c''""") ) ) From cd7a54c820ad830fde47f0521860a5191c7c86b7 Mon Sep 17 00:00:00 2001 From: Ryan Saperstein Date: Tue, 27 Feb 2024 16:27:15 -0500 Subject: [PATCH 06/11] Fixed merge mistakes --- .../biscayne/ast2wdlom/AstToNewExpressionElements.scala | 2 +- .../test/scala/wdl/transforms/biscayne/Ast2WdlomSpec.scala | 3 ++- .../cascades/ast2wdlom/AstToNewExpressionElements.scala | 3 +-- .../linking/expression/files/CascadesFileEvaluators.scala | 2 +- .../linking/expression/types/CascadesTypeEvaluators.scala | 6 ++++-- .../linking/expression/values/CascadesValueEvaluators.scala | 6 ++++-- .../test/scala/wdl/transforms/cascades/Ast2WdlomSpec.scala | 1 + .../consumed/CascadesExpressionValueConsumersSpec.scala | 2 ++ 8 files changed, 16 insertions(+), 9 deletions(-) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala index d856b597f3f..fdbfd26dd52 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/ast2wdlom/AstToNewExpressionElements.scala @@ -14,7 +14,7 @@ import wdl.model.draft3.elements.ExpressionElement.{ Sep, SQuote, SubPosix, - Suffix + Suffix, Unzip } import wdl.transforms.base.ast2wdlom.AstNodeToExpressionElement diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/Ast2WdlomSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/Ast2WdlomSpec.scala index afe17a6d53f..dae69cfdcf8 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/Ast2WdlomSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/Ast2WdlomSpec.scala @@ -119,7 +119,8 @@ class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { val str = "squote(some_arr)" val expr = fromString[ExpressionElement](str, parser.parse_e) expr shouldBeValid (SQuote(IdentifierLookup("some_arr"))) - + } + it should "parse the new unzip function" in { val str = "unzip(some_array_of_pairs)" val expr = fromString[ExpressionElement](str, parser.parse_e) diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala index f3cf471de94..f4d65d2538b 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/ast2wdlom/AstToNewExpressionElements.scala @@ -14,7 +14,7 @@ import wdl.model.draft3.elements.ExpressionElement.{ Sep, SQuote, SubPosix, - Suffix + Suffix, Unzip } import wdl.transforms.base.ast2wdlom.AstNodeToExpressionElement @@ -33,7 +33,6 @@ object AstToNewExpressionElements { "quote" -> AstNodeToExpressionElement.validateOneParamEngineFunction(Quote, "quote"), "squote" -> AstNodeToExpressionElement.validateOneParamEngineFunction(SQuote, "squote"), "unzip" -> AstNodeToExpressionElement.validateOneParamEngineFunction(Unzip, "unzip"), - "read_object" -> (_ => "read_object is no longer available in this WDL version. Consider using read_json instead".invalidNel ), diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala index cffa750d804..2ce3d01b0de 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluators.scala @@ -11,7 +11,7 @@ import wdl.model.draft3.elements.ExpressionElement.{ Sep, SQuote, SubPosix, - Suffix + Suffix, Unzip } import wdl.model.draft3.graph.expression.FileEvaluator diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala index eaa67a652f6..a7ac56291eb 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala @@ -136,14 +136,16 @@ object cascadesTypeEvaluators { implicit val sQuoteFunctionEvaluator: TypeEvaluator[SQuote] = new TypeEvaluator[SQuote] { override def evaluateType(a: SQuote, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit - expressionTypeEvaluator: TypeEvaluator[ExpressionElement] + expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel - case other @ WomArrayType(_) => + case other@WomArrayType(_) => s"Cannot invoke quote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel case other => s"Cannot invoke quote on type ${other.stableName}. Expected an Array[Primitive type]".invalidNel + } + } implicit val unzipFunctionEvaluator: TypeEvaluator[Unzip] = new TypeEvaluator[Unzip] { override def evaluateType(a: Unzip, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala index d2bd10bd4ee..6890312f10f 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala @@ -301,14 +301,16 @@ object cascadesValueEvaluators { inputs: Map[String, WomValue], ioFunctionSet: IoFunctionSet, forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] - )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = processValidatedSingleValue[WomArray, WomArray]( expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( expressionValueEvaluator ) ) { arr => EvaluatedValue(WomArray(arr.value.map(v => WomString("\'" + v.valueString + "\'"))), Seq.empty).validNel - + } + } + /** * Unzip: Creates a pair of arrays, the first containing the elements from the left members of an array of pairs, * and the second containing the right members. This is the inverse of the zip function. * https://github.com/openwdl/wdl/blob/main/versions/1.1/SPEC.md#-pairarrayx-arrayy-unziparraypairx-y diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/Ast2WdlomSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/Ast2WdlomSpec.scala index e4bde35dd61..ffc6d3bc131 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/Ast2WdlomSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/Ast2WdlomSpec.scala @@ -114,6 +114,7 @@ class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { val str = "squote(some_arr)" val expr = fromString[ExpressionElement](str, parser.parse_e) expr shouldBeValid (SQuote(IdentifierLookup("some_arr"))) + } it should "parse the new unzip function" in { val str = "unzip(some_array_of_pairs)" diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala index caf82840020..d2e19d66727 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala @@ -100,6 +100,8 @@ class CascadesExpressionValueConsumersSpec extends AnyFlatSpec with CromwellTime expr.shouldBeValidPF { case e => e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_array"))) + } + } it should "discover an array variable lookup within a unzip() call" in { val str = """ unzip(my_array_of_pairs) """ From 36ceff77aeffdff87868932b03e4190cfc9e058f Mon Sep 17 00:00:00 2001 From: Ryan Saperstein Date: Wed, 28 Feb 2024 09:56:32 -0500 Subject: [PATCH 07/11] Fixed merge issues. --- .../consumed/BiscayneExpressionValueConsumers.scala | 4 ++++ .../linking/expression/files/BiscayneFileEvaluators.scala | 2 +- .../linking/expression/types/BiscayneTypeEvaluators.scala | 2 ++ .../expression/values/BiscayneValueEvaluators.scala | 7 +++++-- 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala index 7fc1f85cd4b..dd909356ff8 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumers.scala @@ -85,6 +85,10 @@ object BiscayneExpressionValueConsumers { implicit val sQuoteExpressionValueConsumer: ExpressionValueConsumer[SQuote] = new ExpressionValueConsumer[SQuote] { override def expressionConsumedValueHooks(a: SQuote)(implicit + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] + ): Set[UnlinkedConsumedValueHook] = + expressionValueConsumer.expressionConsumedValueHooks(a.param)(expressionValueConsumer) + } implicit val unzipExpressionValueConsumer: ExpressionValueConsumer[Unzip] = new ExpressionValueConsumer[Unzip] { override def expressionConsumedValueHooks(a: Unzip)(implicit diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala index 05571bec08f..4993ab16bcb 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala @@ -11,7 +11,7 @@ import wdl.model.draft3.elements.ExpressionElement.{ Sep, SQuote, SubPosix, - Suffix + Suffix, Unzip } import wdl.model.draft3.graph.expression.FileEvaluator diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala index b423edea5b8..ff8779bcb23 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala @@ -144,6 +144,8 @@ object BiscayneTypeEvaluators { s"Cannot invoke squote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel case other => s"Cannot invoke squote on type ${other.stableName}. Expected an Array[Primitive type]".invalidNel + } + } implicit val unzipFunctionEvaluator: TypeEvaluator[Unzip] = new TypeEvaluator[Unzip] { override def evaluateType(a: Unzip, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala index 795446cd5ce..1a063ad6e5c 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala @@ -304,7 +304,7 @@ object BiscayneValueEvaluators { inputs: Map[String, WomValue], ioFunctionSet: IoFunctionSet, forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] - )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = processValidatedSingleValue[WomArray, WomArray]( expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( expressionValueEvaluator @@ -314,7 +314,10 @@ object BiscayneValueEvaluators { WomArray(arr.value.map(v => WomString("\'" + v.valueString.replaceAll("""'""", "\'") + "\'"))), Seq.empty ).validNel - + } + } + + /** * Unzip: Creates a pair of arrays, the first containing the elements from the left members of an array of pairs, * and the second containing the right members. This is the inverse of the zip function. * https://github.com/openwdl/wdl/blob/main/versions/1.1/SPEC.md#-pairarrayx-arrayy-unziparraypairx-y From 05ddb489ff6e66089488416632733304b95bb82b Mon Sep 17 00:00:00 2001 From: Ryan Saperstein Date: Wed, 28 Feb 2024 10:10:09 -0500 Subject: [PATCH 08/11] Fixed merge issues --- .../linking/expression/files/BiscayneFileEvaluators.scala | 2 +- .../consumed/BiscayneExpressionValueConsumersSpec.scala | 4 +++- .../linking/expression/types/BiscayneTypeEvaluatorSpec.scala | 3 +-- .../expression/values/BiscayneValueEvaluatorSpec.scala | 1 + 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala index 4993ab16bcb..b702e091cf3 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala @@ -39,5 +39,5 @@ object BiscayneFileEvaluators { implicit val maxFunctionEvaluator: FileEvaluator[Max] = twoParameterFunctionPassthroughFileEvaluator[Max] implicit val unzipFunctionEvaluator: FileEvaluator[Unzip] = - EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator + singleParameterPassthroughFileEvaluator } diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumersSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumersSpec.scala index 4ac57ef2104..b20b844b02e 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumersSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/consumed/BiscayneExpressionValueConsumersSpec.scala @@ -100,7 +100,9 @@ class BiscayneExpressionValueConsumersSpec extends AnyFlatSpec with CromwellTime expr.shouldBeValidPF { case e => e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_array"))) - + } + } + it should "discover an array variable lookup within a unzip() call" in { val str = """ unzip(my_array_of_pairs) """ val expr = fromString[ExpressionElement](str, parser.parse_e) diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala index 8aa50fabd36..6281ac40649 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala @@ -91,14 +91,13 @@ class BiscayneTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomStringType) } } - + it should "evaluate the type of an unzip() function as Pair[Array[X], Array[Y]]" in { val string_and_int = """ unzip([("one", 1),("two", 2),("three", 3)]) """ val string_and_int_expr = fromString[ExpressionElement](string_and_int, parser.parse_e) string_and_int_expr.shouldBeValidPF { case e => e.evaluateType(Map.empty) shouldBeValid WomPairType(WomArrayType(WomStringType), WomArrayType(WomIntegerType)) } - } val int_and_int = """ unzip([(1,2),(3,4),(5,6)]) """ val int_and_int_expr = fromString[ExpressionElement](int_and_int, parser.parse_e) diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala index 26bb871b576..631d88f4da2 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala @@ -465,6 +465,7 @@ class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi expr.shouldBeValidPF { case e => e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) } + } it should "evaluate an unzip expression correctly" in { val str = """ unzip([("one", 1),("two", 2),("three", 3)]) """ From 20611b100408a9d90e257e98fb5f467b6adc4bd9 Mon Sep 17 00:00:00 2001 From: Ryan Saperstein Date: Wed, 28 Feb 2024 10:18:45 -0500 Subject: [PATCH 09/11] scalafmt --- .../linking/expression/files/BiscayneFileEvaluators.scala | 3 +-- .../linking/expression/values/BiscayneValueEvaluators.scala | 2 +- .../linking/expression/files/BiscayneFileEvaluatorSpec.scala | 2 +- .../expression/values/BiscayneValueEvaluatorSpec.scala | 2 +- .../linking/expression/types/CascadesTypeEvaluators.scala | 4 ++-- .../linking/expression/values/CascadesValueEvaluators.scala | 3 ++- .../consumed/CascadesExpressionValueConsumersSpec.scala | 2 +- .../linking/expression/files/CascadesFileEvaluatorSpec.scala | 2 +- .../linking/expression/types/CascadesTypeEvaluatorSpec.scala | 2 +- .../expression/values/CascadesValueEvaluatorSpec.scala | 2 +- 10 files changed, 12 insertions(+), 12 deletions(-) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala index b702e091cf3..77de240732e 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluators.scala @@ -38,6 +38,5 @@ object BiscayneFileEvaluators { implicit val minFunctionEvaluator: FileEvaluator[Min] = twoParameterFunctionPassthroughFileEvaluator[Min] implicit val maxFunctionEvaluator: FileEvaluator[Max] = twoParameterFunctionPassthroughFileEvaluator[Max] - implicit val unzipFunctionEvaluator: FileEvaluator[Unzip] = - singleParameterPassthroughFileEvaluator + implicit val unzipFunctionEvaluator: FileEvaluator[Unzip] = singleParameterPassthroughFileEvaluator } diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala index 1a063ad6e5c..9903313bf00 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluators.scala @@ -304,7 +304,7 @@ object BiscayneValueEvaluators { inputs: Map[String, WomValue], ioFunctionSet: IoFunctionSet, forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] - )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = processValidatedSingleValue[WomArray, WomArray]( expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( expressionValueEvaluator diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluatorSpec.scala index 37c88087ead..69ac8a9cfca 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/files/BiscayneFileEvaluatorSpec.scala @@ -89,7 +89,7 @@ class BiscayneFileEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit ) } } - + it should "discover the file which would be required to evaluate a unzip() function" in { val str = """ unzip(read_lines("foo.txt")) """ val expr = fromString[ExpressionElement](str, parser.parse_e) diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala index 631d88f4da2..a6d3066b5e9 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/values/BiscayneValueEvaluatorSpec.scala @@ -466,7 +466,7 @@ class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) } } - + it should "evaluate an unzip expression correctly" in { val str = """ unzip([("one", 1),("two", 2),("three", 3)]) """ val expr = fromString[ExpressionElement](str, parser.parse_e) diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala index a7ac56291eb..704b7b32fed 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala @@ -136,11 +136,11 @@ object cascadesTypeEvaluators { implicit val sQuoteFunctionEvaluator: TypeEvaluator[SQuote] = new TypeEvaluator[SQuote] { override def evaluateType(a: SQuote, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit - expressionTypeEvaluator: TypeEvaluator[ExpressionElement] + expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel - case other@WomArrayType(_) => + case other @ WomArrayType(_) => s"Cannot invoke quote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel case other => s"Cannot invoke quote on type ${other.stableName}. Expected an Array[Primitive type]".invalidNel diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala index 6890312f10f..ee106f2ef6a 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluators.scala @@ -301,7 +301,7 @@ object cascadesValueEvaluators { inputs: Map[String, WomValue], ioFunctionSet: IoFunctionSet, forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] - )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomArray]] = processValidatedSingleValue[WomArray, WomArray]( expressionValueEvaluator.evaluateValue(a.param, inputs, ioFunctionSet, forCommandInstantiationOptions)( expressionValueEvaluator @@ -310,6 +310,7 @@ object cascadesValueEvaluators { EvaluatedValue(WomArray(arr.value.map(v => WomString("\'" + v.valueString + "\'"))), Seq.empty).validNel } } + /** * Unzip: Creates a pair of arrays, the first containing the elements from the left members of an array of pairs, * and the second containing the right members. This is the inverse of the zip function. diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala index d2e19d66727..9e47424d6c5 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/consumed/CascadesExpressionValueConsumersSpec.scala @@ -102,7 +102,7 @@ class CascadesExpressionValueConsumersSpec extends AnyFlatSpec with CromwellTime e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_array"))) } } - + it should "discover an array variable lookup within a unzip() call" in { val str = """ unzip(my_array_of_pairs) """ val expr = fromString[ExpressionElement](str, parser.parse_e) diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluatorSpec.scala index 8d49683021c..afc73a56a32 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/files/CascadesFileEvaluatorSpec.scala @@ -89,7 +89,7 @@ class CascadesFileEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit ) } } - + it should "discover the file which would be required to evaluate a unzip() function" in { val str = """ unzip(read_lines("foo.txt")) """ val expr = fromString[ExpressionElement](str, parser.parse_e) diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala index 133af42d2a4..a4b3072e5d0 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala @@ -91,7 +91,7 @@ class CascadesTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomStringType) } } - + it should "evaluate the type of an unzip() function as Pair[Array[X], Array[Y]]" in { val string_and_int = """ unzip([("one", 1),("two", 2),("three", 3)]) """ val string_and_int_expr = fromString[ExpressionElement](string_and_int, parser.parse_e) diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala index a02908725d5..60215046d05 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/values/CascadesValueEvaluatorSpec.scala @@ -364,7 +364,7 @@ class CascadesValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedArray, Seq.empty) } } - + it should "evaluate an unzip expression correctly" in { val str = """ unzip([("one", 1),("two", 2),("three", 3)]) """ val expr = fromString[ExpressionElement](str, parser.parse_e) From 7cf708890ad51bd9ded38fd479774600bfffa6a1 Mon Sep 17 00:00:00 2001 From: Ryan Saperstein Date: Wed, 28 Feb 2024 15:41:49 -0500 Subject: [PATCH 10/11] Addressed review comment. --- .../types/BiscayneTypeEvaluators.scala | 2 ++ .../types/BiscayneTypeEvaluatorSpec.scala | 18 ++++++++++++++++++ .../types/CascadesTypeEvaluators.scala | 2 ++ .../types/CascadesTypeEvaluatorSpec.scala | 18 ++++++++++++++++++ 4 files changed, 40 insertions(+) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala index ff8779bcb23..788c2c81c37 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala @@ -126,6 +126,7 @@ object BiscayneTypeEvaluators { expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { + case WomArrayType(WomNothingType) => WomArrayType(WomNothingType).validNel case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel case other @ WomArrayType(_) => s"Cannot invoke quote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel @@ -139,6 +140,7 @@ object BiscayneTypeEvaluators { expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { + case WomArrayType(WomNothingType) => WomArrayType(WomNothingType).validNel case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel case other @ WomArrayType(_) => s"Cannot invoke squote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel diff --git a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala index 6281ac40649..a38a80079e5 100644 --- a/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala +++ b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala @@ -83,6 +83,15 @@ class BiscayneTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit } } + it should "evaluate the type of a quote() function with an empty array as Array[Nothing]" in { + val str = """ quote([]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomNothingType) + } + } + it should "evaluate the type of a squote() function as Array[String]" in { val str = """ squote([1, 2, 3]) """ val expr = fromString[ExpressionElement](str, parser.parse_e) @@ -92,6 +101,15 @@ class BiscayneTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit } } + it should "evaluate the type of an squote() function with an empty array as Array[Nothing]" in { + val str = """ squote([]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomNothingType) + } + } + it should "evaluate the type of an unzip() function as Pair[Array[X], Array[Y]]" in { val string_and_int = """ unzip([("one", 1),("two", 2),("three", 3)]) """ val string_and_int_expr = fromString[ExpressionElement](string_and_int, parser.parse_e) diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala index 704b7b32fed..e785c5fb557 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala @@ -126,6 +126,7 @@ object cascadesTypeEvaluators { expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { + case WomArrayType(WomNothingType) => WomArrayType(WomNothingType).validNel case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel case other @ WomArrayType(_) => s"Cannot invoke quote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel @@ -139,6 +140,7 @@ object cascadesTypeEvaluators { expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { + case WomArrayType(WomNothingType) => WomArrayType(WomNothingType).validNel case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel case other @ WomArrayType(_) => s"Cannot invoke quote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel diff --git a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala index a4b3072e5d0..d874fb7e6dd 100644 --- a/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala +++ b/wdl/transforms/cascades/src/test/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluatorSpec.scala @@ -83,6 +83,15 @@ class CascadesTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit } } + it should "evaluate the type of a quote() function with an empty array as Array[Nothing]" in { + val str = """ quote([]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomNothingType) + } + } + it should "evaluate the type of a squote() function as Array[String]" in { val str = """ squote([1, 2, 3]) """ val expr = fromString[ExpressionElement](str, parser.parse_e) @@ -92,6 +101,15 @@ class CascadesTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit } } + it should "evaluate the type of an squote() function with an empty array as Array[Nothing]" in { + val str = """ squote([]) """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomArrayType(WomNothingType) + } + } + it should "evaluate the type of an unzip() function as Pair[Array[X], Array[Y]]" in { val string_and_int = """ unzip([("one", 1),("two", 2),("three", 3)]) """ val string_and_int_expr = fromString[ExpressionElement](string_and_int, parser.parse_e) From 8fd1cbe09ff6a5eefc5d74f84b021882159edef9 Mon Sep 17 00:00:00 2001 From: Ryan Saperstein Date: Thu, 29 Feb 2024 10:17:45 -0500 Subject: [PATCH 11/11] Responded to review comment. --- .../linking/expression/types/BiscayneTypeEvaluators.scala | 8 ++++---- .../linking/expression/types/CascadesTypeEvaluators.scala | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala index 788c2c81c37..21f2e9bcc8c 100644 --- a/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala +++ b/wdl/transforms/biscayne/src/main/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluators.scala @@ -129,9 +129,9 @@ object BiscayneTypeEvaluators { case WomArrayType(WomNothingType) => WomArrayType(WomNothingType).validNel case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel case other @ WomArrayType(_) => - s"Cannot invoke quote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel + s"Cannot invoke quote on type Array[${other.stableName}]. Expected an Array of primitive type".invalidNel case other => - s"Cannot invoke quote on type ${other.stableName}. Expected an Array[Primitive type]".invalidNel + s"Cannot invoke quote on type ${other.stableName}. Expected an Array of primitive type".invalidNel } } @@ -143,9 +143,9 @@ object BiscayneTypeEvaluators { case WomArrayType(WomNothingType) => WomArrayType(WomNothingType).validNel case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel case other @ WomArrayType(_) => - s"Cannot invoke squote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel + s"Cannot invoke squote on type Array[${other.stableName}]. Expected an Array of primitive type".invalidNel case other => - s"Cannot invoke squote on type ${other.stableName}. Expected an Array[Primitive type]".invalidNel + s"Cannot invoke squote on type ${other.stableName}. Expected an Array of primitive type".invalidNel } } diff --git a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala index e785c5fb557..ba7e9ae8dff 100644 --- a/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala +++ b/wdl/transforms/cascades/src/main/scala/wdl/transforms/cascades/linking/expression/types/CascadesTypeEvaluators.scala @@ -129,9 +129,9 @@ object cascadesTypeEvaluators { case WomArrayType(WomNothingType) => WomArrayType(WomNothingType).validNel case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel case other @ WomArrayType(_) => - s"Cannot invoke quote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel + s"Cannot invoke quote on type Array[${other.stableName}]. Expected an Array of primitive type".invalidNel case other => - s"Cannot invoke quote on type ${other.stableName}. Expected an Array[Primitive type]".invalidNel + s"Cannot invoke quote on type ${other.stableName}. Expected an Array of primitive type".invalidNel } } @@ -143,9 +143,9 @@ object cascadesTypeEvaluators { case WomArrayType(WomNothingType) => WomArrayType(WomNothingType).validNel case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel case other @ WomArrayType(_) => - s"Cannot invoke quote on type Array[${other.stableName}]. Expected an Array[Primitive type]".invalidNel + s"Cannot invoke quote on type Array[${other.stableName}]. Expected an Array of primitive type".invalidNel case other => - s"Cannot invoke quote on type ${other.stableName}. Expected an Array[Primitive type]".invalidNel + s"Cannot invoke quote on type ${other.stableName}. Expected an Array of primitive type".invalidNel } }