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..e05af2f8a2d 100644 --- a/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test +++ b/centaur/src/main/resources/standardTestCases/biscayne_new_engine_functions.test @@ -55,6 +55,8 @@ metadata { "outputs.biscayne_new_engine_functions.minMaxIntFloatComposition": 1.0 "outputs.biscayne_new_engine_functions.maxIntVsMaxFloat": 1.79769313E+308 + "outputs.biscayne_new_engine_functions.substituted": "WATtheWAT" + "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" 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..c4387134421 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", "sub", "suffix" ] } Map[String, Int] x_map_in = {"a": 1, "b": 2, "c": 3} @@ -51,6 +51,11 @@ workflow biscayne_new_engine_functions { Float minMaxIntFloatComposition = min(max(biggestInt, smallFloat), smallestInt) # 1.0 Float maxIntVsMaxFloat = max(maxInt, maxFloat) + # sub(): + # (Exists before Biscayne, but uses different regex flavor here) + # ================================================= + String substituted = sub("AtheZ", "[[:upper:]]", "WAT") + # suffix(): # ================================================= Array[String] with_suffixes = suffix("S", some_strings) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 6b1e3cc1740..30396519016 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -103,6 +103,7 @@ object Dependencies { private val postgresV = "42.4.4" private val pprintV = "0.7.3" private val rdf4jV = "3.7.1" + private val re2jV = "1.6" private val refinedV = "0.10.1" private val rhinoV = "1.7.14" @@ -517,7 +518,8 @@ object Dependencies { val wdlDependencies: List[ModuleID] = List( "commons-io" % "commons-io" % commonsIoV, "org.scala-graph" %% "graph-core" % scalaGraphV, - "com.chuusai" %% "shapeless" % shapelessV + "com.chuusai" %% "shapeless" % shapelessV, + "com.google.re2j" % "re2j" % re2jV, ) ++ betterFilesDependencies val languageFactoryDependencies = List( 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..23101adfa9e 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 @@ -180,6 +180,8 @@ object ExpressionElement { def arg2: ExpressionElement def arg3: ExpressionElement } + + // Pre-1.1 WDL versions have undefined regex flavor. Cromwell uses the standard Java flavor. final case class Sub(input: ExpressionElement, pattern: ExpressionElement, replace: ExpressionElement) extends ThreeParamFunctionCallElement { override def arg1: ExpressionElement = input @@ -187,6 +189,14 @@ object ExpressionElement { override def arg3: ExpressionElement = replace } + // As of WDL 1.1, WDL regular expressions are expected to be POSIX ERE flavor. + final case class SubPosix(input: ExpressionElement, pattern: ExpressionElement, replace: ExpressionElement) + extends ThreeParamFunctionCallElement { + override def arg1: ExpressionElement = input + override def arg2: ExpressionElement = pattern + override def arg3: ExpressionElement = replace + } + /** * A single identifier lookup expression, eg Int x = y */ 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..70ebd4879c3 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, Sep, SubPosix, Suffix} import wdl.transforms.base.ast2wdlom.AstNodeToExpressionElement object AstToNewExpressionElements { @@ -15,6 +15,7 @@ object AstToNewExpressionElements { "min" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Min, "min"), "max" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Max, "max"), "sep" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Sep, "sep"), + "sub" -> AstNodeToExpressionElement.validateThreeParamEngineFunction(SubPosix, "sub"), "suffix" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Suffix, "suffix"), "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..8c5e200e7ee 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 @@ -58,6 +58,16 @@ object BiscayneExpressionValueConsumers { expressionValueConsumer.expressionConsumedValueHooks(a.arg2)(expressionValueConsumer) } + implicit val subPosixExpressionValueConsumer: ExpressionValueConsumer[SubPosix] = + new ExpressionValueConsumer[SubPosix] { + override def expressionConsumedValueHooks(a: SubPosix)(implicit + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] + ): Set[UnlinkedConsumedValueHook] = + expressionValueConsumer.expressionConsumedValueHooks(a.arg1)(expressionValueConsumer) ++ + expressionValueConsumer.expressionConsumedValueHooks(a.arg2)(expressionValueConsumer) ++ + expressionValueConsumer.expressionConsumedValueHooks(a.arg3)(expressionValueConsumer) + } + implicit val suffixExpressionValueConsumer: ExpressionValueConsumer[Suffix] = new ExpressionValueConsumer[Suffix] { override def expressionConsumedValueHooks(a: Suffix)(implicit expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] 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..0016b0c7458 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 @@ -96,7 +96,7 @@ package object consumed { case a: Zip => a.expressionConsumedValueHooks(expressionValueConsumer) case a: Cross => a.expressionConsumedValueHooks(expressionValueConsumer) - case a: Sub => a.expressionConsumedValueHooks(expressionValueConsumer) + case a: SubPosix => a.expressionConsumedValueHooks(expressionValueConsumer) // New WDL biscayne expressions: case a: Keys => 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..6d83600ea2b 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,9 +1,12 @@ 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, Sep, 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.twoParameterFunctionPassthroughFileEvaluator +import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.{ + threeParameterFunctionPassthroughFileEvaluator, + twoParameterFunctionPassthroughFileEvaluator +} object BiscayneFileEvaluators { @@ -16,6 +19,8 @@ object BiscayneFileEvaluators { EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator implicit val sepFunctionEvaluator: FileEvaluator[Sep] = twoParameterFunctionPassthroughFileEvaluator[Sep] + implicit val subPosixFunctionEvaluator: FileEvaluator[SubPosix] = + threeParameterFunctionPassthroughFileEvaluator[SubPosix] implicit val suffixFunctionEvaluator: FileEvaluator[Suffix] = twoParameterFunctionPassthroughFileEvaluator[Suffix] implicit val minFunctionEvaluator: FileEvaluator[Min] = twoParameterFunctionPassthroughFileEvaluator[Min] 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..e8a8a195289 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 @@ -154,7 +154,8 @@ package object files { case a: Zip => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: Cross => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) - case a: Sub => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) + case a: SubPosix => + a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: Keys => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: AsMap => 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..73e01576abe 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 @@ -1,6 +1,6 @@ package wdl.transforms.biscayne.linking.expression.types -import cats.implicits.catsSyntaxTuple2Semigroupal +import cats.implicits.{catsSyntaxTuple2Semigroupal, catsSyntaxTuple3Semigroupal} import cats.syntax.validated._ import common.validation.ErrorOr._ import wdl.model.draft3.elements.ExpressionElement @@ -102,6 +102,16 @@ object BiscayneTypeEvaluators { } } + implicit val subPosixFunctionEvaluator: TypeEvaluator[SubPosix] = new TypeEvaluator[SubPosix] { + override def evaluateType(a: SubPosix, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit + expressionTypeEvaluator: TypeEvaluator[ExpressionElement] + ): ErrorOr[WomType] = + (validateParamType(a.input, linkedValues, WomSingleFileType), + validateParamType(a.pattern, linkedValues, WomSingleFileType), + validateParamType(a.replace, linkedValues, WomSingleFileType) + ) mapN { (_, _, _) => WomStringType } + } + implicit val suffixFunctionEvaluator: TypeEvaluator[Suffix] = new TypeEvaluator[Suffix] { override def evaluateType(a: Suffix, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] 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..fbcfcd6d339 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 @@ -98,7 +98,7 @@ package object types { case a: Zip => a.evaluateType(linkedValues)(typeEvaluator) case a: Cross => a.evaluateType(linkedValues)(typeEvaluator) - case a: Sub => a.evaluateType(linkedValues)(typeEvaluator) + case a: SubPosix => a.evaluateType(linkedValues)(typeEvaluator) case a: StdoutElement.type => a.evaluateType(linkedValues)(typeEvaluator) case a: StderrElement.type => 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..0e29f40b2fd 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 @@ -4,12 +4,15 @@ import cats.data.NonEmptyList import cats.syntax.validated._ import cats.syntax.traverse._ import cats.instances.list._ +import com.google.re2j.{Pattern => RE2JPattern} import common.validation.ErrorOr._ import common.collections.EnhancedCollections._ +import common.validation.ErrorOr import wdl.model.draft3.elements.ExpressionElement import wdl.model.draft3.elements.ExpressionElement._ import wdl.model.draft3.graph.expression.{EvaluatedValue, ForCommandInstantiationOptions, ValueEvaluator} import wdl.transforms.base.linking.expression.values.EngineFunctionEvaluators.{ + processThreeValidatedValues, processTwoValidatedValues, processValidatedSingleValue } @@ -218,6 +221,36 @@ object BiscayneValueEvaluators { } } + implicit val subPosixFunctionEvaluator: ValueEvaluator[SubPosix] = new ValueEvaluator[SubPosix] { + override def evaluateValue(a: SubPosix, + inputs: Map[String, WomValue], + ioFunctionSet: IoFunctionSet, + forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomString]] = + processThreeValidatedValues[WomString, WomString, WomString, WomString]( + expressionValueEvaluator.evaluateValue(a.input, inputs, ioFunctionSet, forCommandInstantiationOptions)( + expressionValueEvaluator + ), + expressionValueEvaluator.evaluateValue(a.pattern, inputs, ioFunctionSet, forCommandInstantiationOptions)( + expressionValueEvaluator + ), + expressionValueEvaluator.evaluateValue(a.replace, inputs, ioFunctionSet, forCommandInstantiationOptions)( + expressionValueEvaluator + ) + ) { (input, pattern, replace) => + ErrorOr( + EvaluatedValue(WomString( + RE2JPattern + .compile(pattern.valueString) + .matcher(input.valueString) + .replaceAll(replace.valueString) + ), + Seq.empty + ) + ) + } + } + implicit val suffixFunctionEvaluator: ValueEvaluator[Suffix] = new ValueEvaluator[Suffix] { override def evaluateValue(a: Suffix, inputs: Map[String, WomValue], 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..3a3ba825fe9 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 @@ -165,7 +165,8 @@ package object values { case a: Cross => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) - case a: Sub => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) + case a: SubPosix => + a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: Keys => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: AsMap => 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..b3a28b0d203 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 @@ -1,7 +1,6 @@ package wdl.transforms.biscayne import java.util - import common.Checked import common.assertion.ErrorOrAssertions._ import common.transforms.CheckedAtoB @@ -98,6 +97,12 @@ class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { expr shouldBeValid NoneLiteralElement } + it should "get the posix version when parsing the sub function" in { + val str = """sub("my input", "[A-Za-z]", "repl")""" + val expr = fromString[ExpressionElement](str, parser.parse_e) + expr shouldBeValid (SubPosix(StringLiteral("my input"), StringLiteral("[A-Za-z]"), StringLiteral("repl"))) + } + it should "parse the new suffix function" in { val str = "suffix(some_str, some_arr)" val expr = fromString[ExpressionElement](str, parser.parse_e) 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..841f283a512 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 @@ -58,6 +58,15 @@ class BiscayneExpressionValueConsumersSpec extends AnyFlatSpec with CromwellTime } } + it should "discover the variable lookups within a sub() call" in { + val str = """ sub(my_input, "^[A-Z]$", "0") """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_input"))) + } + } + it should "discover the variable lookups within a suffix() call" in { val str = """ suffix(my_suffix, ["a", "b", c]) """ val expr = fromString[ExpressionElement](str, parser.parse_e) 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..11affe50c66 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 @@ -46,6 +46,17 @@ class BiscayneFileEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit } } + it should "discover the file which would be required to evaluate a sub() function" in { + val str = """ sub(read_string("my_nice_file.txt"), "foo", "NEW_VAL") """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.predictFilesNeededToEvaluate(Map.empty, NoIoFunctionSet, WomStringType) shouldBeValid Set( + WomSingleFile("my_nice_file.txt") + ) + } + } + it should "discover the file which would be required to evaluate a suffix() function" in { val str = """ suffix(' # what a line', 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/types/BiscayneTypeEvaluatorSpec.scala b/wdl/transforms/biscayne/src/test/scala/wdl/transforms/biscayne/linking/expression/types/BiscayneTypeEvaluatorSpec.scala index c8372d36cd1..bea86f6d850 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 @@ -56,6 +56,15 @@ class BiscayneTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit } } + it should "evaluate the type of a sub() function as String" in { + val str = """ sub("input", "^pattern$", "s") """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomStringType + } + } + it should "evaluate the type of a suffix() function as Array[String]" in { val str = """ suffix('S', ["a", "b", "c"]) """ 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 1487543a838..66d07e02fe4 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 @@ -201,6 +201,29 @@ class BiscayneValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi } } + it should "evaluate a POSIX-flavor regex in a sub expression correctly" in { + val str = """ sub("aB", "[[:lower:]]", "9") """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedString: WomString = WomString("9B") + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedString, Seq.empty) + } + } + + it should "fail to evaluate a Java-flavor regex in a sub expression" in { + val str = """ sub("aB", "\\p{Lower}", "9") """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) + .shouldBeInvalid( + """error parsing regexp: invalid character class range: `\p{Lower}`""" + ) + } + } + it should "evaluate a suffix expression correctly" in { val str = """ suffix("S", ["a", "b", "c"]) """ 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 46cd8cb2e60..c1f36aae542 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, Sep, SubPosix, Suffix} import wdl.transforms.base.ast2wdlom.AstNodeToExpressionElement object AstToNewExpressionElements { @@ -15,6 +15,7 @@ object AstToNewExpressionElements { "min" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Min, "min"), "max" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Max, "max"), "sep" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Sep, "sep"), + "sub" -> AstNodeToExpressionElement.validateThreeParamEngineFunction(SubPosix, "sub"), "suffix" -> AstNodeToExpressionElement.validateTwoParamEngineFunction(Suffix, "suffix"), "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..332550dde47 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 @@ -58,6 +58,16 @@ object cascadesExpressionValueConsumers { expressionValueConsumer.expressionConsumedValueHooks(a.arg2)(expressionValueConsumer) } + implicit val subPosixExpressionValueConsumer: ExpressionValueConsumer[SubPosix] = + new ExpressionValueConsumer[SubPosix] { + override def expressionConsumedValueHooks(a: SubPosix)(implicit + expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] + ): Set[UnlinkedConsumedValueHook] = + expressionValueConsumer.expressionConsumedValueHooks(a.arg1)(expressionValueConsumer) ++ + expressionValueConsumer.expressionConsumedValueHooks(a.arg2)(expressionValueConsumer) ++ + expressionValueConsumer.expressionConsumedValueHooks(a.arg3)(expressionValueConsumer) + } + implicit val suffixExpressionValueConsumer: ExpressionValueConsumer[Suffix] = new ExpressionValueConsumer[Suffix] { override def expressionConsumedValueHooks(a: Suffix)(implicit expressionValueConsumer: ExpressionValueConsumer[ExpressionElement] 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..fa6df7e9c97 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 @@ -96,7 +96,7 @@ package object consumed { case a: Zip => a.expressionConsumedValueHooks(expressionValueConsumer) case a: Cross => a.expressionConsumedValueHooks(expressionValueConsumer) - case a: Sub => a.expressionConsumedValueHooks(expressionValueConsumer) + case a: SubPosix => a.expressionConsumedValueHooks(expressionValueConsumer) // New WDL biscayne expressions: case a: Keys => 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..f551359f215 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,9 +1,12 @@ 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, Sep, 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.twoParameterFunctionPassthroughFileEvaluator +import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.{ + threeParameterFunctionPassthroughFileEvaluator, + twoParameterFunctionPassthroughFileEvaluator +} object cascadesFileEvaluators { @@ -16,6 +19,8 @@ object cascadesFileEvaluators { EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator implicit val sepFunctionEvaluator: FileEvaluator[Sep] = twoParameterFunctionPassthroughFileEvaluator[Sep] + implicit val subPosixFunctionEvaluator: FileEvaluator[SubPosix] = + threeParameterFunctionPassthroughFileEvaluator[SubPosix] implicit val suffixFunctionEvaluator: FileEvaluator[Suffix] = twoParameterFunctionPassthroughFileEvaluator[Suffix] implicit val minFunctionEvaluator: FileEvaluator[Min] = twoParameterFunctionPassthroughFileEvaluator[Min] 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..f075f82a4ff 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 @@ -154,7 +154,8 @@ package object files { case a: Zip => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: Cross => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) - case a: Sub => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) + case a: SubPosix => + a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: Keys => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) case a: AsMap => a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator) 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..7b1b2e45dc5 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 @@ -1,6 +1,6 @@ package wdl.transforms.biscayne.linking.expression.types -import cats.implicits.catsSyntaxTuple2Semigroupal +import cats.implicits.{catsSyntaxTuple2Semigroupal, catsSyntaxTuple3Semigroupal} import cats.syntax.validated._ import common.validation.ErrorOr._ import wdl.model.draft3.elements.ExpressionElement @@ -102,6 +102,16 @@ object cascadesTypeEvaluators { } } + implicit val subPosixFunctionEvaluator: TypeEvaluator[SubPosix] = new TypeEvaluator[SubPosix] { + override def evaluateType(a: SubPosix, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit + expressionTypeEvaluator: TypeEvaluator[ExpressionElement] + ): ErrorOr[WomType] = + (validateParamType(a.input, linkedValues, WomSingleFileType), + validateParamType(a.pattern, linkedValues, WomSingleFileType), + validateParamType(a.replace, linkedValues, WomSingleFileType) + ) mapN { (_, _, _) => WomStringType } + } + implicit val suffixFunctionEvaluator: TypeEvaluator[Suffix] = new TypeEvaluator[Suffix] { override def evaluateType(a: Suffix, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] 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..70cc81ee395 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 @@ -98,7 +98,7 @@ package object types { case a: Zip => a.evaluateType(linkedValues)(typeEvaluator) case a: Cross => a.evaluateType(linkedValues)(typeEvaluator) - case a: Sub => a.evaluateType(linkedValues)(typeEvaluator) + case a: SubPosix => a.evaluateType(linkedValues)(typeEvaluator) case a: StdoutElement.type => a.evaluateType(linkedValues)(typeEvaluator) case a: StderrElement.type => 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..7489728980e 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 @@ -4,12 +4,15 @@ import cats.data.NonEmptyList import cats.syntax.validated._ import cats.syntax.traverse._ import cats.instances.list._ +import com.google.re2j.{Pattern => RE2JPattern} import common.validation.ErrorOr._ import common.collections.EnhancedCollections._ +import common.validation.ErrorOr import wdl.model.draft3.elements.ExpressionElement import wdl.model.draft3.elements.ExpressionElement._ import wdl.model.draft3.graph.expression.{EvaluatedValue, ForCommandInstantiationOptions, ValueEvaluator} import wdl.transforms.base.linking.expression.values.EngineFunctionEvaluators.{ + processThreeValidatedValues, processTwoValidatedValues, processValidatedSingleValue } @@ -218,6 +221,36 @@ object cascadesValueEvaluators { } } + implicit val subPosixFunctionEvaluator: ValueEvaluator[SubPosix] = new ValueEvaluator[SubPosix] { + override def evaluateValue(a: SubPosix, + inputs: Map[String, WomValue], + ioFunctionSet: IoFunctionSet, + forCommandInstantiationOptions: Option[ForCommandInstantiationOptions] + )(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomString]] = + processThreeValidatedValues[WomString, WomString, WomString, WomString]( + expressionValueEvaluator.evaluateValue(a.input, inputs, ioFunctionSet, forCommandInstantiationOptions)( + expressionValueEvaluator + ), + expressionValueEvaluator.evaluateValue(a.pattern, inputs, ioFunctionSet, forCommandInstantiationOptions)( + expressionValueEvaluator + ), + expressionValueEvaluator.evaluateValue(a.replace, inputs, ioFunctionSet, forCommandInstantiationOptions)( + expressionValueEvaluator + ) + ) { (input, pattern, replace) => + ErrorOr( + EvaluatedValue(WomString( + RE2JPattern + .compile(pattern.valueString) + .matcher(input.valueString) + .replaceAll(replace.valueString) + ), + Seq.empty + ) + ) + } + } + implicit val suffixFunctionEvaluator: ValueEvaluator[Suffix] = new ValueEvaluator[Suffix] { override def evaluateValue(a: Suffix, inputs: Map[String, WomValue], 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..d0e2fb8c155 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 @@ -165,7 +165,8 @@ package object values { case a: Cross => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) - case a: Sub => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) + case a: SubPosix => + a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: Keys => a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator) case a: AsMap => 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..49602617a42 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 @@ -58,6 +58,15 @@ class CascadesExpressionValueConsumersSpec extends AnyFlatSpec with CromwellTime } } + it should "discover the variable lookups within a sub() call" in { + val str = """ sub(my_input, "^[A-Z]$", "0") """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.expressionConsumedValueHooks should be(Set(UnlinkedIdentifierHook("my_input"))) + } + } + it should "discover the variable lookups within a suffix() call" in { val str = """ suffix(my_suffix, ["a", "b", c]) """ 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 5335f7d5f29..1733630c121 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 @@ -46,6 +46,17 @@ class CascadesFileEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit } } + it should "discover the file which would be required to evaluate a sub() function" in { + val str = """ sub(read_string("my_nice_file.txt"), "foo", "NEW_VAL") """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.predictFilesNeededToEvaluate(Map.empty, NoIoFunctionSet, WomStringType) shouldBeValid Set( + WomSingleFile("my_nice_file.txt") + ) + } + } + it should "discover the file which would be required to evaluate a suffix() function" in { val str = """ suffix(' # what a line', 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 954ef0f52f0..a84adccfa0f 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 @@ -56,6 +56,15 @@ class CascadesTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit } } + it should "evaluate the type of a sub() function as String" in { + val str = """ sub("input", "^pattern$", "s") """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.evaluateType(Map.empty) shouldBeValid WomStringType + } + } + it should "evaluate the type of a suffix() function as Array[String]" in { val str = """ suffix('S', ["a", "b", "c"]) """ val expr = fromString[ExpressionElement](str, 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 bd47a20ca68..06ab5818331 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 @@ -201,6 +201,29 @@ class CascadesValueEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wi } } + it should "evaluate a POSIX-flavor regex in a sub expression correctly" in { + val str = """ sub("aB", "[[:lower:]]", "9") """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedString: WomString = WomString("9B") + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedString, Seq.empty) + } + } + + it should "fail to evaluate a Java-flavor regex in a sub expression" in { + val str = """ sub("aB", "\\p{Lower}", "9") """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) + .shouldBeInvalid( + """error parsing regexp: invalid character class range: `\p{Lower}`""" + ) + } + } + it should "evaluate a suffix expression correctly" in { val str = """ suffix("S", ["a", "b", "c"]) """ val expr = fromString[ExpressionElement](str, parser.parse_e) 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..d99d3493b60 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 @@ -1,8 +1,8 @@ package wdl.draft3.transforms.ast2wdlom import cats.instances.either._ -import java.util +import java.util import common.Checked import common.assertion.CromwellTimeoutSpec import common.assertion.ErrorOrAssertions._ @@ -11,14 +11,15 @@ import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers import wdl.draft3.parser.WdlParser import wdl.draft3.parser.WdlParser.{ParseTree, SyntaxErrorFormatter} +import wdl.draft3.transforms.ast2wdlom.Ast2WdlomSpec.{fromString, parser} import wdl.draft3.transforms.parsing.WdlDraft3SyntaxErrorFormatter -import wdl.model.draft3.elements.ExpressionElement.IdentifierLookup +import wdl.model.draft3.elements.ExpressionElement.{IdentifierLookup, StringLiteral, Sub} import wdl.model.draft3.elements._ import wdl.transforms.base.ast2wdlom.GenericAstNode import scala.jdk.CollectionConverters._ -class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { +object Ast2WdlomSpec { val parser = new WdlParser() @@ -29,10 +30,15 @@ class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { val versionedExpression = "version 1.0\n" + expression // That "version 1.0" means we'll have 2 unwanted tokens at the start of the list, so drop 'em: val tokens = parser.lex(versionedExpression, "string").asScala.drop(2).asJava - val terminalMap = (tokens.asScala.toVector map { (_, expression) }).toMap + val terminalMap = (tokens.asScala.toVector map { + (_, expression) + }).toMap val parseTree = parseFunction(tokens, WdlDraft3SyntaxErrorFormatter(terminalMap)) (wrapAstNode andThen converter).run(parseTree.toAst) } +} + +class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { it should "not parse the new as_map function" in { val str = "as_map(some_pairs)" @@ -52,6 +58,12 @@ class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { expr shouldBeInvalid "Failed to parse expression (reason 1 of 1): Unknown engine function: 'collect_by_key'" } + it should "get the non-posix version when parsing the sub function" in { + val str = """sub("my input", "[A-Za-z]", "repl")""" + val expr = fromString[ExpressionElement](str, parser.parse_e) + expr shouldBeValid (Sub(StringLiteral("my input"), StringLiteral("[A-Za-z]"), StringLiteral("repl"))) + } + it should "not parse the new suffix function" in { val str = "suffix(aStr, anArray)" val expr = fromString[ExpressionElement](str, parser.parse_e) diff --git a/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/linking/expression/values/ValueEvaluatorsSpec.scala b/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/linking/expression/values/ValueEvaluatorsSpec.scala new file mode 100644 index 00000000000..013a45308e6 --- /dev/null +++ b/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/linking/expression/values/ValueEvaluatorsSpec.scala @@ -0,0 +1,42 @@ +package wdl.draft3.transforms.linking.expression.values + +import common.assertion.CromwellTimeoutSpec +import common.assertion.ErrorOrAssertions._ +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers +import wdl.draft3.transforms.ast2wdlom.Ast2WdlomSpec.{fromString, parser} +import wdl.draft3.transforms.ast2wdlom._ +import wdl.model.draft3.elements.ExpressionElement +import wdl.model.draft3.graph.expression.EvaluatedValue +import wdl.model.draft3.graph.expression.ValueEvaluator.ops._ +import wom.expression.NoIoFunctionSet +import wom.values.WomString + +class ValueEvaluatorsSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers { + + behavior of "base value evaluator" + + // In pre-1.1 WDL we use Java-flavored regex + + it should "fail to apply a POSIX-flavor sub expression" in { + val str = """ sub("aB", "[[:lower:]]", "9") """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedString: WomString = WomString("aB") // no substitution + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedString, Seq.empty) + } + } + + it should "evaluate a Java-flavor regex in a sub expression correctly" in { + val str = """ sub("aB", "\\p{Lower}", "9") """ + val expr = fromString[ExpressionElement](str, parser.parse_e) + + val expectedString: WomString = WomString("9B") + + expr.shouldBeValidPF { case e => + e.evaluateValue(Map.empty, NoIoFunctionSet, None) shouldBeValid EvaluatedValue(expectedString, Seq.empty) + } + } +} diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/ast2wdlom/AstNodeToExpressionElement.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/ast2wdlom/AstNodeToExpressionElement.scala index 2a773fd4e23..407d7f6f96c 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/ast2wdlom/AstNodeToExpressionElement.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/ast2wdlom/AstNodeToExpressionElement.scala @@ -231,7 +231,7 @@ object AstNodeToExpressionElement { s"Function $functionName expects exactly 2 arguments but got ${params.size}".invalidNel } - private def validateThreeParamEngineFunction( + def validateThreeParamEngineFunction( elementMaker: (ExpressionElement, ExpressionElement, ExpressionElement) => ExpressionElement, functionName: String )(params: Vector[ExpressionElement]): ErrorOr[ExpressionElement] = diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/files/EngineFunctionEvaluators.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/files/EngineFunctionEvaluators.scala index 11c687588a7..b8ecf9d764b 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/files/EngineFunctionEvaluators.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/files/EngineFunctionEvaluators.scala @@ -219,8 +219,8 @@ object EngineFunctionEvaluators { implicit val crossFunctionEvaluator: FileEvaluator[Cross] = twoParameterFunctionPassthroughFileEvaluator[Cross] implicit val prefixFunctionEvaluator: FileEvaluator[Prefix] = twoParameterFunctionPassthroughFileEvaluator[Prefix] - implicit val subFunctionEvaluator: FileEvaluator[Sub] = new FileEvaluator[Sub] { - override def predictFilesNeededToEvaluate(a: Sub, + def threeParameterFunctionPassthroughFileEvaluator[A <: ThreeParamFunctionCallElement] = new FileEvaluator[A] { + override def predictFilesNeededToEvaluate(a: A, inputs: Map[String, WomValue], ioFunctionSet: IoFunctionSet, coerceTo: WomType @@ -228,9 +228,13 @@ object EngineFunctionEvaluators { fileEvaluator: FileEvaluator[ExpressionElement], valueEvaluator: ValueEvaluator[ExpressionElement] ): ErrorOr[Set[WomFile]] = - (a.pattern.evaluateFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo), - a.input.evaluateFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo), - a.replace.evaluateFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo) - ) mapN { _ ++ _ ++ _ } + (a.arg1.evaluateFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo), + a.arg2.evaluateFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo), + a.arg3.evaluateFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo) + ) mapN { + _ ++ _ ++ _ + } } + + implicit val subFunctionEvaluator: FileEvaluator[Sub] = threeParameterFunctionPassthroughFileEvaluator[Sub] } diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/values/EngineFunctionEvaluators.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/values/EngineFunctionEvaluators.scala index ffaf16ad0d2..58c83c67edc 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/values/EngineFunctionEvaluators.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/values/EngineFunctionEvaluators.scala @@ -835,7 +835,7 @@ object EngineFunctionEvaluators { s"Expected (${coercerA.toDisplayString}, ${coercerB.toDisplayString}) argument but got (${otherA.value.womType.stableName}, ${otherB.value.womType.stableName})".invalidNel } - private def processThreeValidatedValues[A <: WomValue, B <: WomValue, C <: WomValue, R <: WomValue]( + def processThreeValidatedValues[A <: WomValue, B <: WomValue, C <: WomValue, R <: WomValue]( arg1: ErrorOr[EvaluatedValue[_ <: WomValue]], arg2: ErrorOr[EvaluatedValue[_ <: WomValue]], arg3: ErrorOr[EvaluatedValue[_ <: WomValue]] 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..1df05e48db1 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 @@ -401,6 +401,7 @@ object WdlWriterImpl { case a: OneOrTwoParamFunctionCallElement => a.toWdlV1 case a: TwoParamFunctionCallElement => a.toWdlV1 case a: Sub => s"sub(${a.input.toWdlV1}, ${a.pattern.toWdlV1}, ${a.replace.toWdlV1})" + case a: SubPosix => s"sub(${a.input.toWdlV1}, ${a.pattern.toWdlV1}, ${a.replace.toWdlV1})" } }