Fix the dollar re-interpolation behavior in draft-2 [BA-4928] (broad…

…institute#5026)

backend/src/main/scala/cromwell/backend/ReadLikeFunctions.scala

@@ -8,7 +8,7 @@ import scala.concurrent.Future
 import scala.util.Try
 
 trait ReadLikeFunctions extends PathFactory with IoFunctionSet with AsyncIoFunctions {
-  
+
   override def readFile(path: String, maxBytes: Option[Int], failOnOverflow: Boolean): Future[String] =
     Future.fromTry(Try(buildPath(path))) flatMap { p => asyncIo.contentAsStringAsync(p, maxBytes, failOnOverflow) }
 

centaur/src/main/resources/standardTestCases/dollars_in_strings.test

@@ -0,0 +1,14 @@
+name: dollars_in_strings
+testFormat: workflowsuccess
+
+files {
+  workflow: dollars_in_strings/dollars_in_strings.wdl
+}
+
+metadata {
+  workflowName: read_dollared_strings
+  status: Succeeded
+  "outputs.read_dollared_strings.s1": "${BLAH}"
+  "outputs.read_dollared_strings.s2": "${BLAH}"
+  "outputs.read_dollared_strings.s3": "oops ${BLAH}"
+}

centaur/src/main/resources/standardTestCases/dollars_in_strings/dollars_in_strings.wdl

@@ -0,0 +1,32 @@
+workflow read_dollared_strings {
+
+  call dollars_in_strings
+
+  String dollar = "$"
+
+  output {
+    String s1 = "${dollar}{BLAH}"
+    String s2 = s1
+
+    String s3 = dollars_in_strings.s3
+  }
+}
+
+
+task dollars_in_strings {
+  String dollar = "$"
+  command <<<
+    cat > foo.txt << 'EOF'
+      oops ${dollar}{BLAH}
+    EOF
+  >>>
+
+  output {
+    File x = "foo.txt"
+    String s3 = read_string(x)
+  }
+
+  runtime {
+    docker: "ubuntu:latest"
+  }
+}

wdl/model/draft2/src/main/scala/wdl/draft2/model/expression/ValueEvaluator.scala

@@ -21,34 +21,34 @@ case class ValueEvaluator(override val lookup: String => WomValue, override val
 
   private val InterpolationTagPattern = "\\$\\{\\s*([^\\}]*)\\s*\\}".r
 
-  private def replaceInterpolationTag(string: Try[WomString], tag: String): Try[WomString] = {
-    val expr = WdlExpression.fromString(tag.substring(2, tag.length - 1))
-    (expr.evaluate(lookup, functions), string) match {
-      case (Success(value), Success(str)) =>
-        value match {
-          case s: WomString if InterpolationTagPattern.anchored.findFirstIn(s.valueString).isDefined =>
-            replaceInterpolationTag(Success(WomString(str.value.replace(tag, s.valueString))), s.valueString)
-          case v => Success(WomString(str.value.replace(tag, v.valueString)))
-        }
-      case (Failure(ex), _) => Failure(ex)
-      case (_, Failure(ex)) => Failure(ex)
+
+  private def interpolate(strToProcess: String, resultSoFar: Try[WomString] = Success(WomString(""))): Try[WomString] = {
+
+    def evaluateTag(tag: String): Try[WomString] = {
+      val expr = WdlExpression.fromString(tag.substring(2, tag.length - 1))
+      expr.evaluate(lookup, functions).map(result => WomString(result.valueString))
     }
-  }
 
-  private def interpolate(str: String): Try[WomString] = {
-    InterpolationTagPattern.findAllIn(str).foldLeft(Try(WomString(str)))(replaceInterpolationTag)
-  }
+    InterpolationTagPattern.findFirstIn(strToProcess) match {
+      case Some(tag) =>
+        val prefix = strToProcess.substring(0, strToProcess.indexOf(tag))
+        val suffix = strToProcess.substring(strToProcess.indexOf(tag) + tag.length, strToProcess.length)
+
+        val newResultSoFar = for {
+          rsf <- resultSoFar
+          interpolatedTag <- evaluateTag(tag)
+        } yield WomString(rsf.value + prefix + interpolatedTag.value)
 
-  private def interpolate(value: WomValue): Try[WomValue] = {
-    value match {
-      case s: WomString => interpolate(s.valueString)
-      case _ => Try(value)
+        interpolate(suffix, newResultSoFar)
+
+      case None => resultSoFar.map(ws => WomString(ws.value + strToProcess))
     }
+
   }
 
   override def evaluate(ast: AstNode): Try[WomValue] = {
     ast match {
-      case t: Terminal if t.getTerminalStr == "identifier" => Try(lookup(t.getSourceString)).flatMap(interpolate)
+      case t: Terminal if t.getTerminalStr == "identifier" => Try(lookup(t.getSourceString))
       case t: Terminal if t.getTerminalStr == "integer" => Success(WomInteger(t.getSourceString.toInt))
       case t: Terminal if t.getTerminalStr == "float" => Success(WomFloat(t.getSourceString.toDouble))
       case t: Terminal if t.getTerminalStr == "boolean" => Success(WomBoolean(t.getSourceString == "true"))
@@ -136,7 +136,7 @@ case class ValueEvaluator(override val lookup: String => WomValue, override val
         a.getAttribute("rhs") match {
           case rhs:Terminal if rhs.getTerminalStr == "identifier" =>
             val memberAccessAsString = s"${a.getAttribute("lhs").sourceString}.${a.getAttribute("rhs").sourceString}"
-            Try(lookup(memberAccessAsString)).flatMap(interpolate).recoverWith {
+            Try(lookup(memberAccessAsString)).recoverWith {
               case _ =>
                 evaluate(a.getAttribute("lhs")).flatMap {
                   case o: WomObjectLike =>

wdl/model/draft2/src/test/scala/wdl/AdvancedInterpolationSpec.scala

@@ -31,12 +31,21 @@ class AdvancedInterpolationSpec extends FlatSpec with Matchers {
 
   val namespace = WdlNamespaceWithWorkflow.load(wdl, Seq.empty).get
 
-  it should "be able to resolve interpolated strings within interpolated strings" in {
+  it should "not interpolate into input strings which are not WDL literals" in {
     val testCall = namespace.workflow.taskCalls.find(_.unqualifiedName == "test") getOrElse {
       fail("call 'test' not found")
     }
     val inputs = testCall.evaluateTaskInputs(Map("testWF.test.eval_this" -> WomString("${var}")), NoFunctions)
     val testCmd = testCall.task.instantiateCommand(inputs.get, NoFunctions).toTry.get.head
-    testCmd.commandString shouldEqual "echo 'inside inside inside'"
+    testCmd.commandString shouldEqual "echo '${var} ${var} inside'"
+  }
+
+  it should "interpolate into input values which are WDL literals" in {
+    val test2Call = namespace.workflow.taskCalls.find(_.unqualifiedName == "test2") getOrElse {
+      fail("call 'test2' not found")
+    }
+    val inputs2 = test2Call.evaluateTaskInputs(Map.empty, NoFunctions)
+    val test2Cmd = test2Call.task.instantiateCommand(inputs2.get, NoFunctions).toTry.get.head
+    test2Cmd.commandString shouldEqual "echo 'outside outside inside'"
   }
 }