Skip to content

Commit

Permalink
[WX-1460] WDL 1.1 Struct Literal Parsing (#7391)
Browse files Browse the repository at this point in the history
Co-authored-by: Janet Gainer-Dewar <[email protected]>
  • Loading branch information
THWiseman and jgainerdewar authored Mar 29, 2024
1 parent 027de62 commit 2f8c46d
Show file tree
Hide file tree
Showing 42 changed files with 2,666 additions and 1,938 deletions.
12 changes: 12 additions & 0 deletions centaur/src/main/resources/standardTestCases/struct_literal.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
name: struct_literal
testFormat: workflowsuccess

files {
workflow: struct_literal/struct_literal.wdl
}

metadata {
workflowName: struct_literal
status: Succeeded
"outputs.struct_literal.out": 44
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
version development-1.1

struct Plant {
String color
Int id
}

struct Fungi {
File fungiFile
}


struct Animal {
Plant jacket
Fungi hat
}

task a {
input {
Plant in_plant_literal = Plant{color: "red", id: 44}
}

command {
echo "${in_plant_literal.id}"
}

output {
Animal out_animal = Animal{jacket: Plant{color: "green", id: 10}, hat: Fungi{fungiFile: stdout()}}
}

runtime {
docker: "ubuntu:latest"
}

meta {
volatile: true
}
}

task b {
input {
Animal in_animal
}

command {
cat ${in_animal.hat.fungiFile}
}

output {
Int out = read_int(stdout())
}

runtime {
docker: "ubuntu:latest"
}

meta {
volatile: true
}
}

workflow struct_literal {
call a
call b {input: in_animal=a.out_animal}
output {
Int out = b.out
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ object ExpressionElement {

final case class KvPair(key: String, value: ExpressionElement)
final case class ObjectLiteral(elements: Map[String, ExpressionElement]) extends ExpressionElement
final case class StructLiteral(structTypeName: String, elements: Map[String, ExpressionElement])
extends ExpressionElement
final case class ArrayLiteral(elements: Seq[ExpressionElement]) extends ExpressionElement
final case class MapLiteral(elements: Map[ExpressionElement, ExpressionElement]) extends ExpressionElement
final case class PairLiteral(left: ExpressionElement, right: ExpressionElement) extends ExpressionElement
Expand Down
1,944 changes: 983 additions & 961 deletions wdl/transforms/biscayne/src/main/java/wdl/biscayne/parser/WdlParser.java

Large diffs are not rendered by default.

13 changes: 12 additions & 1 deletion wdl/transforms/biscayne/src/main/resources/CHANGELOG.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
2023-03-16
Synced grammar from OpenWDL `development` version, which is actually development of 2.0. There is no 1.1 Hermes grammar, develop it here.
Changed version declaration to `development1_1`.
This disallows `version 1.1` workflows to run with incomplete support. Once development is finished, change to `1.1`.
This disallows `version 1.1` workflows to run with incomplete support. Once development is finished, change to `1.1`.

2024-02-28
When changing the grammar file, generate a new parser by:
- changing current working directory to cromwell/wdl/transforms/biscayne
- running: hermes generate src/main/resources/grammar.hgr \
--language=java \
--directory=src/main/java \
--name=wdl \
--java-package=wdl.biscayne.parser \
--java-use-apache-commons --java-imports=org.apache.commons.lang3.StringEscapeUtils \
--header
1 change: 1 addition & 0 deletions wdl/transforms/biscayne/src/main/resources/grammar.hgr
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ grammar {
(*:unary) $e = :not $e -> LogicalNot(expression=$1)
(-:unary) $e = :plus $e -> UnaryPlus(expression=$1)
(-:unary) $e = :dash $e -> UnaryNegation(expression=$1)
(*:left) $e = :identifier <=> :lbrace list($object_kv, :comma) :rbrace -> StructLiteral(name=$0, map=$2)
(*:left) $e = :identifier <=> :lparen list($e, :comma) :rparen -> FunctionCall(name=$0, params=$2)
(*:left) $e = $e <=> :lsquare $e :rsquare -> ArrayOrMapLookup(lhs=$0, rhs=$2)
(*:left) $e = $e <=> :dot :identifier -> MemberAccess(value=$0, member=$2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,14 @@ object BiscayneExpressionValueConsumers {
// None literals consume no values:
Set.empty[UnlinkedConsumedValueHook]
}

implicit val structLiteralExpressionValueConsumer: ExpressionValueConsumer[StructLiteral] =
new ExpressionValueConsumer[StructLiteral] {
override def expressionConsumedValueHooks(a: StructLiteral)(implicit
expressionValueConsumer: ExpressionValueConsumer[ExpressionElement]
): Set[UnlinkedConsumedValueHook] =
a.elements.values
.flatMap(element => expressionValueConsumer.expressionConsumedValueHooks(element)(expressionValueConsumer))
.toSet
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ package object consumed {

case a: StringExpression => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: ObjectLiteral => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: StructLiteral => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: PairLiteral => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: ArrayLiteral => a.expressionConsumedValueHooks(expressionValueConsumer)
case a: MapLiteral => a.expressionConsumedValueHooks(expressionValueConsumer)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package wdl.transforms.biscayne.linking.expression.files

import cats.implicits.{catsSyntaxValidatedId, toTraverseOps}
import common.validation.ErrorOr.ErrorOr
import wdl.model.draft3.elements.ExpressionElement
import wdl.model.draft3.elements.ExpressionElement.{
AsMap,
AsPairs,
Expand All @@ -10,16 +13,20 @@ import wdl.model.draft3.elements.ExpressionElement.{
Quote,
Sep,
SQuote,
StructLiteral,
SubPosix,
Suffix,
Unzip
}
import wdl.model.draft3.graph.expression.FileEvaluator
import wdl.model.draft3.graph.expression.{FileEvaluator, ValueEvaluator}
import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.{
threeParameterFunctionPassthroughFileEvaluator,
twoParameterFunctionPassthroughFileEvaluator
}
import wdl.transforms.base.linking.expression.files.EngineFunctionEvaluators.singleParameterPassthroughFileEvaluator
import wom.expression.IoFunctionSet
import wom.types.{WomCompositeType, WomType}
import wom.values.{WomFile, WomValue}

object BiscayneFileEvaluators {

Expand All @@ -39,4 +46,38 @@ object BiscayneFileEvaluators {
implicit val maxFunctionEvaluator: FileEvaluator[Max] = twoParameterFunctionPassthroughFileEvaluator[Max]

implicit val unzipFunctionEvaluator: FileEvaluator[Unzip] = singleParameterPassthroughFileEvaluator

implicit val structLiteralEvaluator: FileEvaluator[StructLiteral] = new FileEvaluator[StructLiteral] {
override def predictFilesNeededToEvaluate(a: StructLiteral,
inputs: Map[String, WomValue],
ioFunctionSet: IoFunctionSet,
coerceTo: WomType
)(implicit
fileEvaluator: FileEvaluator[ExpressionElement],
valueEvaluator: ValueEvaluator[ExpressionElement]
): ErrorOr[Set[WomFile]] = {
def filesInObjectField(fieldAndWomTypeTuple: (String, WomType)): ErrorOr[Set[WomFile]] = {
val (field, womType) = fieldAndWomTypeTuple
a.elements.get(field) match {
case Some(fieldElement) =>
fileEvaluator.predictFilesNeededToEvaluate(fieldElement, inputs, ioFunctionSet, womType)(fileEvaluator,
valueEvaluator
)
case None => s"Invalid assignment to struct. Required field $field was not specified.".invalidNel
}
}

coerceTo match {
case WomCompositeType(mapping, _) => mapping.toList.traverse(filesInObjectField).map(_.flatten.toSet)
case _ =>
a.elements.values.toList
.traverse(
fileEvaluator.evaluateFilesNeededToEvaluate(_, inputs, ioFunctionSet, coerceTo)(fileEvaluator,
valueEvaluator
)
)
.map(_.toSet.flatten)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import wom.expression.IoFunctionSet
import wom.types.WomType
import wom.values.{WomFile, WomValue}
import wdl.transforms.biscayne.linking.expression.files.BiscayneFileEvaluators._

package object files {

implicit val expressionFileEvaluator: FileEvaluator[ExpressionElement] = new FileEvaluator[ExpressionElement] {
Expand All @@ -37,6 +36,8 @@ package object files {
a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator)
case a: ObjectLiteral =>
a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator)
case a: StructLiteral =>
a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator)
case a: MapLiteral =>
a.predictFilesNeededToEvaluate(inputs, ioFunctionSet, coerceTo)(fileEvaluator, valueEvaluator)
case a: ArrayLiteral =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,4 +159,16 @@ object BiscayneTypeEvaluators {
case other => s"Cannot invoke 'unzip' on type '${other.stableName}'. Expected an array of pairs".invalidNel
}
}

implicit val structLiteralTypeEvaluator: TypeEvaluator[StructLiteral] = new TypeEvaluator[StructLiteral] {
override def evaluateType(a: StructLiteral, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle])(
implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement]
): ErrorOr[WomType] =
// This works fine, but is not yet a strong enough type check for the WDL 1.1 spec
// (i.e. users are able to instantiate struct literals with k/v pairs that aren't actually in the struct definition, without an error being thrown.)
// We want to add extra validation here, and return a WomCompositeType that matches the struct definition of everything is OK.
// Note that users are allowed to omit optional k/v pairs in their literal.
// This requires some extra work to be done in a subsequent PR.
WomObjectType.validNel
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ package object types {
case a: StringLiteral => a.evaluateType(linkedValues)(typeEvaluator)
case a: StringExpression => a.evaluateType(linkedValues)(typeEvaluator)
case a: ObjectLiteral => a.evaluateType(linkedValues)(typeEvaluator)
case a: StructLiteral => a.evaluateType(linkedValues)(typeEvaluator)
case a: MapLiteral => a.evaluateType(linkedValues)(typeEvaluator)
case a: ArrayLiteral => a.evaluateType(linkedValues)(typeEvaluator)
case a: PairLiteral => a.evaluateType(linkedValues)(typeEvaluator)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import wdl.transforms.base.linking.expression.values.EngineFunctionEvaluators.{
}
import wom.expression.IoFunctionSet
import wom.types._
import wom.values.{WomArray, WomFloat, WomInteger, WomMap, WomOptionalValue, WomPair, WomString, WomValue}
import wom.values.{WomArray, WomFloat, WomInteger, WomMap, WomObject, WomOptionalValue, WomPair, WomString, WomValue}
import wom.types.coercion.defaults._

object BiscayneValueEvaluators {
Expand Down Expand Up @@ -351,4 +351,29 @@ object BiscayneValueEvaluators {
s"Invalid call of 'unzip' on parameter of type '${other.womType.stableName}' (expected Array[Pair[X, Y]])".invalidNel
}
}

implicit val structLiteralValueEvaluator: ValueEvaluator[StructLiteral] = new ValueEvaluator[StructLiteral] {
// This works fine, but is missing a feature from the WDL 1.1 spec: users are allowed to omit optional values from their struct literal.
// This requires some extra work to be done in a subsequent PR.
// Specifically, make the known struct definitions available to this function so we can populate k/v pairs with None appropriately.
override def evaluateValue(a: StructLiteral,
inputs: Map[String, WomValue],
ioFunctionSet: IoFunctionSet,
forCommandInstantiationOptions: Option[ForCommandInstantiationOptions]
)(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[_ <: WomValue]] = {

val evaluated: ErrorOr[List[(String, EvaluatedValue[_])]] = a.elements.toList traverse {
case (key: String, value: ExpressionElement) =>
expressionValueEvaluator
.evaluateValue(value, inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
.map(key -> _)
}

evaluated map { mapping =>
val value = mapping.map(entry => entry._1 -> entry._2.value).toMap
val sideEffectFiles = mapping.flatMap(entry => entry._2.sideEffectFiles)
EvaluatedValue(WomObject(value), sideEffectFiles)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,14 @@ package object values {
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
case a: NoneLiteralElement.type =>
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)

case a: StringLiteral =>
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
case a: StringExpression =>
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
case a: ObjectLiteral =>
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
case a: StructLiteral =>
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
case a: MapLiteral =>
a.evaluateValue(inputs, ioFunctionSet, forCommandInstantiationOptions)(expressionValueEvaluator)
case a: ArrayLiteral =>
Expand Down
39 changes: 39 additions & 0 deletions wdl/transforms/biscayne/src/test/cases/struct_literal.wdl
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
version development-1.1

struct Plant {
String color
Boolean tasty
}

struct Animal {
String name
Boolean? isGood
}

task test_struct_parsing {
input {
Plant standard_plant_input
Animal standard_animal_input
}

runtime {
docker: "ubuntu:latest"
}

command {
echo "all dogs are good"
}

output {
Plant standard_plant_forwarded = standard_plant_input
Animal standard_animal_forwarded = standard_animal_input
Plant plant_output_literal = Plant{color: "red", tasty: true}
}
}

workflow struct_literal {
call test_struct_parsing {
input: standard_plant_input = Plant{color: "green", tasty: true},
standard_animal_input = Animal{name: "mittens", isGood: false}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import wdl.transforms.biscayne.ast2wdlom._
import wdl.transforms.biscayne.parsing.WdlBiscayneSyntaxErrorFormatter
import wom.callable.MetaValueElement.MetaValueElementInteger
import wom.types.WomIntegerType
import wom.values.WomInteger
import wom.values.{WomBoolean, WomInteger}

import scala.jdk.CollectionConverters._

Expand Down Expand Up @@ -126,4 +126,13 @@ class Ast2WdlomSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers {
val expr = fromString[ExpressionElement](str, parser.parse_e)
expr shouldBeValid (Unzip(IdentifierLookup("some_array_of_pairs")))
}

it should "parse a struct literal" in {
val str = """Dog{breed: "fluffy", isGood: true}"""
val expr = fromString[ExpressionElement](str, parser.parse_e)
expr shouldBeValid (StructLiteral(
"Dog",
Map("breed" -> StringLiteral("fluffy"), "isGood" -> PrimitiveLiteralExpressionElement(WomBoolean(true)))
))
}
}
Loading

0 comments on commit 2f8c46d

Please sign in to comment.