From 9ffff90bcb6d95a0e19462c129d8612ed7c9e12f Mon Sep 17 00:00:00 2001 From: Tom Wiseman Date: Wed, 10 Apr 2024 15:42:39 -0400 Subject: [PATCH] finish //todos and copy to cascades --- .../types/BiscayneTypeEvaluators.scala | 28 ++-- .../types/CascadesTypeEvaluators.scala | 145 +++++++++++++++--- .../types/CascadesTypeEvaluatorSpec.scala | 61 +++++++- .../types/EngineFunctionEvaluators.scala | 85 +++++----- 4 files changed, 245 insertions(+), 74 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 0fa041637d1..34ea1090d50 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 @@ -19,7 +19,7 @@ object BiscayneTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomMapType(WomAnyType, WomAnyType)) flatMap { + validateParamType(a.param, linkedValues, WomMapType(WomAnyType, WomAnyType), typeAliases) flatMap { case WomMapType(keyType, _) => WomArrayType(keyType).validNel case other => s"Cannot invoke 'keys' on type '${other.stableName}'. Expected a map".invalidNel } @@ -32,7 +32,7 @@ object BiscayneTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType))) flatMap { + validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType)), typeAliases) flatMap { case WomArrayType(WomPairType(x: WomPrimitiveType, y)) => WomMapType(x, y).validNel case other @ WomArrayType(WomPairType(x, _)) => s"Cannot invoke 'as_map' on type ${other.stableName}. Map keys must be primitive but got '${x.stableName}'".invalidNel @@ -47,7 +47,7 @@ object BiscayneTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomMapType(WomAnyType, WomAnyType)) flatMap { + validateParamType(a.param, linkedValues, WomMapType(WomAnyType, WomAnyType), typeAliases) flatMap { case WomMapType(x, y) => WomArrayType(WomPairType(x, y)).validNel case other => s"Cannot invoke 'as_pairs' on type '${other.stableName}'. Expected a map".invalidNel } @@ -60,7 +60,7 @@ object BiscayneTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType))) flatMap { + validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType)), typeAliases) flatMap { case WomArrayType(WomPairType(x: WomPrimitiveType, y)) => WomMapType(x, WomArrayType(y)).validNel case other @ WomArrayType(WomPairType(x, _)) => s"Cannot invoke 'collect_by_key' on type ${other.stableName}. Map keys must be primitive but got '${x.stableName}'".invalidNel @@ -114,7 +114,7 @@ object BiscayneTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.arg2, linkedValues, WomArrayType(WomAnyType)) flatMap { + validateParamType(a.arg2, linkedValues, WomArrayType(WomAnyType), typeAliases) flatMap { case WomArrayType(WomArrayType(_)) => s"Cannot invoke 'sep' on type 'Array[Array[_]]'. Expected an Array[String].".invalidNel case WomArrayType(_) => WomStringType.validNel @@ -131,9 +131,9 @@ object BiscayneTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - (validateParamType(a.input, linkedValues, WomSingleFileType), - validateParamType(a.pattern, linkedValues, WomSingleFileType), - validateParamType(a.replace, linkedValues, WomSingleFileType) + (validateParamType(a.input, linkedValues, WomSingleFileType, typeAliases), + validateParamType(a.pattern, linkedValues, WomSingleFileType, typeAliases), + validateParamType(a.replace, linkedValues, WomSingleFileType, typeAliases) ) mapN { (_, _, _) => WomStringType } } @@ -144,8 +144,8 @@ object BiscayneTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - (validateParamType(a.suffix, linkedValues, WomStringType), - validateParamType(a.array, linkedValues, WomArrayType(WomStringType)) + (validateParamType(a.suffix, linkedValues, WomStringType, typeAliases), + validateParamType(a.array, linkedValues, WomArrayType(WomStringType), typeAliases) ) mapN { (_, _) => WomArrayType(WomStringType) } } @@ -156,7 +156,7 @@ object BiscayneTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { + validateParamType(a.param, linkedValues, WomArrayType(WomAnyType), typeAliases) flatMap { case WomArrayType(WomNothingType) => WomArrayType(WomNothingType).validNel case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel case other @ WomArrayType(_) => @@ -173,7 +173,7 @@ object BiscayneTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { + validateParamType(a.param, linkedValues, WomArrayType(WomAnyType), typeAliases) flatMap { case WomArrayType(WomNothingType) => WomArrayType(WomNothingType).validNel case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel case other @ WomArrayType(_) => @@ -190,7 +190,7 @@ object BiscayneTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType))) flatMap { + validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType)), typeAliases) flatMap { case WomArrayType(WomNothingType) => WomPairType(WomArrayType(WomAnyType), WomArrayType(WomAnyType)).validNel case WomArrayType(WomPairType(x, y)) => WomPairType(WomArrayType(x), WomArrayType(y)).validNel case other => s"Cannot invoke 'unzip' on type '${other.stableName}'. Expected an array of pairs".invalidNel @@ -240,6 +240,8 @@ object BiscayneTypeEvaluators { structDefinition: WomCompositeType, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle], typeAliases: Map[String, WomType] + )(implicit + expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomCompositeType] = { val checkedMembers: Map[String, ErrorOr[WomType]] = a.elements.map { case (memberKey, memberExpressionElement) => val evaluatedType = 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 369cc8ba918..7a49804abb6 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,5 +1,6 @@ package wdl.transforms.biscayne.linking.expression.types +import cats.data.Validated.{Invalid, Valid} import cats.implicits.{catsSyntaxTuple2Semigroupal, catsSyntaxTuple3Semigroupal} import cats.syntax.validated._ import common.validation.ErrorOr._ @@ -18,7 +19,7 @@ object cascadesTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomMapType(WomAnyType, WomAnyType)) flatMap { + validateParamType(a.param, linkedValues, WomMapType(WomAnyType, WomAnyType), typeAliases) flatMap { case WomMapType(keyType, _) => WomArrayType(keyType).validNel case other => s"Cannot invoke 'keys' on type '${other.stableName}'. Expected a map".invalidNel } @@ -31,7 +32,7 @@ object cascadesTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType))) flatMap { + validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType)), typeAliases) flatMap { case WomArrayType(WomPairType(x: WomPrimitiveType, y)) => WomMapType(x, y).validNel case other @ WomArrayType(WomPairType(x, _)) => s"Cannot invoke 'as_map' on type ${other.stableName}. Map keys must be primitive but got '${x.stableName}'".invalidNel @@ -46,7 +47,7 @@ object cascadesTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomMapType(WomAnyType, WomAnyType)) flatMap { + validateParamType(a.param, linkedValues, WomMapType(WomAnyType, WomAnyType), typeAliases) flatMap { case WomMapType(x, y) => WomArrayType(WomPairType(x, y)).validNel case other => s"Cannot invoke 'as_pairs' on type '${other.stableName}'. Expected a map".invalidNel } @@ -59,7 +60,7 @@ object cascadesTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType))) flatMap { + validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType)), typeAliases) flatMap { case WomArrayType(WomPairType(x: WomPrimitiveType, y)) => WomMapType(x, WomArrayType(y)).validNel case other @ WomArrayType(WomPairType(x, _)) => s"Cannot invoke 'collect_by_key' on type ${other.stableName}. Map keys must be primitive but got '${x.stableName}'".invalidNel @@ -113,7 +114,7 @@ object cascadesTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.arg2, linkedValues, WomArrayType(WomAnyType)) flatMap { + validateParamType(a.arg2, linkedValues, WomArrayType(WomAnyType), typeAliases) flatMap { case WomArrayType(WomArrayType(_)) => s"Cannot invoke 'sep' on type 'Array[Array[_]]'. Expected an Array[String].".invalidNel case WomArrayType(_) => WomStringType.validNel @@ -130,9 +131,9 @@ object cascadesTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - (validateParamType(a.input, linkedValues, WomSingleFileType), - validateParamType(a.pattern, linkedValues, WomSingleFileType), - validateParamType(a.replace, linkedValues, WomSingleFileType) + (validateParamType(a.input, linkedValues, WomSingleFileType, typeAliases), + validateParamType(a.pattern, linkedValues, WomSingleFileType, typeAliases), + validateParamType(a.replace, linkedValues, WomSingleFileType, typeAliases) ) mapN { (_, _, _) => WomStringType } } @@ -143,8 +144,8 @@ object cascadesTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - (validateParamType(a.suffix, linkedValues, WomStringType), - validateParamType(a.array, linkedValues, WomArrayType(WomStringType)) + (validateParamType(a.suffix, linkedValues, WomStringType, typeAliases), + validateParamType(a.array, linkedValues, WomArrayType(WomStringType), typeAliases) ) mapN { (_, _) => WomArrayType(WomStringType) } } @@ -155,7 +156,7 @@ object cascadesTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { + validateParamType(a.param, linkedValues, WomArrayType(WomAnyType), typeAliases) flatMap { case WomArrayType(WomNothingType) => WomArrayType(WomNothingType).validNel case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel case other @ WomArrayType(_) => @@ -172,7 +173,7 @@ object cascadesTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)) flatMap { + validateParamType(a.param, linkedValues, WomArrayType(WomAnyType), typeAliases) flatMap { case WomArrayType(WomNothingType) => WomArrayType(WomNothingType).validNel case WomArrayType(_: WomPrimitiveType) => WomArrayType(WomStringType).validNel case other @ WomArrayType(_) => @@ -189,7 +190,7 @@ object cascadesTypeEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType))) flatMap { + validateParamType(a.param, linkedValues, WomArrayType(WomPairType(WomAnyType, WomAnyType)), typeAliases) flatMap { case WomArrayType(WomNothingType) => WomPairType(WomArrayType(WomAnyType), WomArrayType(WomAnyType)).validNel case WomArrayType(WomPairType(x, y)) => WomPairType(WomArrayType(x), WomArrayType(y)).validNel case other => s"Cannot invoke 'unzip' on type '${other.stableName}'. Expected an array of pairs".invalidNel @@ -197,17 +198,123 @@ object cascadesTypeEvaluators { } implicit val structLiteralTypeEvaluator: TypeEvaluator[StructLiteral] = new TypeEvaluator[StructLiteral] { + + /** + * Is the evaluated type allowed to be assigned to the expectedType? + */ + def areTypesAssignable(evaluatedType: WomType, expectedType: WomType): Boolean = + // NB: This check is a little looser than we'd like it to be. + // For example, String is coercible to Int (Int i = "1" is OK) + // It's not until we actually evaluate the value of the string that we can know if that coercion succeeded or not. (Int i = "orange" will fail) + // We don't know whether the user has provided "1" or "orange" at this stage. + // This is OK as-is because the value evaluators do the coercing and throw meaningful errors if the coercion fails. + expectedType.isCoerceableFrom(evaluatedType) + + /** + * Helper method to check if something (maybe) found in the struct literal against something (maybe) found in the struct definition. + * @return The WomType of the evaluated member. Error if either is not present, or if the types aren't compatible. + */ + def checkIfMemberIsValid(typeName: String, + memberName: String, + evaluatedType: Option[WomType], + expectedType: Option[WomType] + ): ErrorOr[WomType] = + evaluatedType match { + case Some(evaluated) => + expectedType match { + case Some(expected) => + if (areTypesAssignable(evaluated, expected)) evaluated.validNel + else + s"$typeName.$memberName expected to be ${expected.friendlyName}. Found ${evaluated.friendlyName}.".invalidNel + case None => s"Type $typeName does not have a member called $memberName.".invalidNel + } + case None => s"Error evaluating the type of ${typeName}.${memberName}.".invalidNel + } + + /** + * For each member in the literal, check that it exists in the struct definition and is the expected type. + * @return The WomCompositeType of the struct literal, as determined from evaluating each member. + * This might not *exactly* match the struct definition due to permitted type coercions. + */ + def checkMembersAgainstDefinition(a: StructLiteral, + structDefinition: WomCompositeType, + linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle], + typeAliases: Map[String, WomType] + )(implicit + expressionTypeEvaluator: TypeEvaluator[ExpressionElement] + ): ErrorOr[WomCompositeType] = { + val checkedMembers: Map[String, ErrorOr[WomType]] = a.elements.map { case (memberKey, memberExpressionElement) => + val evaluatedType = + expressionTypeEvaluator.evaluateType(memberExpressionElement, linkedValues, typeAliases).toOption + val expectedType = structDefinition.typeMap.get(memberKey) + (memberKey, checkIfMemberIsValid(a.structTypeName, memberKey, evaluatedType, expectedType)) + } + + val (errors, validatedTypes) = checkedMembers.partition { case (_, errorOr) => + errorOr match { + case Invalid(_) => true + case Valid(_) => false + } + } + + if (errors.nonEmpty) { + errors.collect { case (_, Invalid(e)) => e.toList.mkString(", ") }.toList.mkString("[ ", ", ", " ]").invalidNel + } else { + val types = validatedTypes.collect { case (key, Valid(v)) => (key, v) } + WomCompositeType(types, Some(a.structTypeName)).validNel + } + } + + /** + * For each member in the struct definition, if that member isn't optional, confirm that it is also in the struct literal. + */ + def checkForMissingMembers(foundMembers: Map[String, WomType], + structDefinition: WomCompositeType + ): ErrorOr[Unit] = { + val errors: Iterable[String] = structDefinition.typeMap flatMap { case (memberName, memberType) => + memberType match { + case WomOptionalType(_) => None + case _ => + if (!foundMembers.contains(memberName)) Some(s"Expected member ${memberName} not found. ") + else None + } + } + if (errors.nonEmpty) errors.mkString.invalidNel else ().validNel + } + + /** + * Returns the type of a struct literal, assuming it is compatible with an existing struct definition. + * It is required that: + * - a struct definition exist for the literal (looked up the via name of the struct type). This is defined elsewhere in the WDL and is not part of the literal. + * - the struct definition be a WomCompositeType (it's programmer error if it's not) + * - each member provided in the literal is also in the definition, and is coercible to the defined type. + * - all non-optional members of the definition are present in the literal. + * @param a The literal to evaluate + * @param linkedValues Used by the expression type evaluator. + * @param typeAliases A map containing available struct definitions + * @param expressionTypeEvaluator An object capable of evaluating the types of ExpressionElements. Used to evaluate the type of each member provided in the literal. + * @return The type of the struct definition (as found in typeAliases) if all goes well. A list of errors otherwise. + */ override def evaluateType(a: StructLiteral, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle], typeAliases: Map[String, WomType] )(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 + typeAliases.get(a.structTypeName) match { + case Some(definition) => + definition match { + case compositeType: WomCompositeType => + checkMembersAgainstDefinition(a, compositeType, linkedValues, typeAliases).flatMap { foundMembers => + checkForMissingMembers(foundMembers.typeMap, compositeType) match { + case Invalid(error) => error.invalid + case _ => compositeType.validNel + } + } + case _ => + s"Programmer error: Expected the struct definition of ${a.structTypeName} to be a WomCompositeType".invalidNel + } + case None => s"Could not find Struct Definition for type ${a.structTypeName}".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 ebc85e82696..b9274d7e866 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 @@ -22,9 +22,14 @@ class CascadesTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit "hat" -> WomCompositeType(plantTypeMap, Some("Plant")) ) + val bacteriumTypeMap: Map[String, WomType] = Map( + "myFile" -> WomSingleFileType + ) + val typeAliases: Map[String, WomType] = Map( "Plant" -> WomCompositeType(plantTypeMap, Some("Plant")), - "Animal" -> WomCompositeType(animalTypeMap, Some("Animal")) + "Animal" -> WomCompositeType(animalTypeMap, Some("Animal")), + "Bacterium" -> WomCompositeType(bacteriumTypeMap, Some("Bacterium")) ) it should "return nothing from static integer addition" in { @@ -155,12 +160,60 @@ class CascadesTypeEvaluatorSpec extends AnyFlatSpec with CromwellTimeoutSpec wit } it should "evaluate the type of a struct literal" in { - // NB: This is not yet strict enough type checking for the WDL 1.1 spec. - // In a subsequent branch, we will make this be a WomCompositeType that matches the struct definition. + val structLiteral = """ Plant{isTasty: true, count: 42} """ + val structExpr = fromString[ExpressionElement](structLiteral, parser.parse_e) + structExpr.shouldBeValidPF { case e => + e.evaluateType(Map.empty, typeAliases) shouldBeValid WomCompositeType(plantTypeMap, Some("Plant")) + } + } + + it should "evaluate the type of a struct literal with a nested struct literal" in { + val structLiteral = """ Animal{isMaybeGood: true, hat: Plant{isTasty: true, count: 42}} """ + val structExpr = fromString[ExpressionElement](structLiteral, parser.parse_e) + structExpr.shouldBeValidPF { case e => + e.evaluateType(Map.empty, typeAliases) shouldBeValid WomCompositeType(animalTypeMap, Some("Animal")) + } + } + + it should "fail to evaluate the type of a struct literal with incorrect members" in { val structLiteral = """ Animal{fur: "fuzzy", isGood: true} """ val structExpr = fromString[ExpressionElement](structLiteral, parser.parse_e) structExpr.shouldBeValidPF { case e => - e.evaluateType(Map.empty, typeAliases) shouldBeValid WomObjectType + e.evaluateType(Map.empty, + typeAliases + ) shouldBeInvalid "[ Type Animal does not have a member called fur., Type Animal does not have a member called isGood. ]" + } + } + + it should "fail to evaluate the type of a struct literal with members that are the wrong type" in { + val structLiteral = """ Plant{isTasty: true, count: (0, 1)} """ + val structExpr = fromString[ExpressionElement](structLiteral, parser.parse_e) + structExpr.shouldBeValidPF { case e => + e.evaluateType(Map.empty, typeAliases) shouldBeInvalid "[ Plant.count expected to be Int. Found Pair[Int, Int]. ]" + } + } + + it should "fail if a struct literal member is missing" in { + val structLiteral = """ Plant{count: 4} """ + val structExpr = fromString[ExpressionElement](structLiteral, parser.parse_e) + structExpr.shouldBeValidPF { case e => + e.evaluateType(Map.empty, typeAliases) shouldBeInvalid "Expected member isTasty not found. " + } + } + + it should "tolerate a missing struct literal optional member" in { + val structLiteral = """ Animal{hat: Plant{isTasty: true, count: 42}} """ + val structExpr = fromString[ExpressionElement](structLiteral, parser.parse_e) + structExpr.shouldBeValidPF { case e => + e.evaluateType(Map.empty, typeAliases) shouldBeValid WomCompositeType(animalTypeMap, Some("Animal")) + } + } + + it should "work with file paths in struct literal" in { + val structLiteral = """ Bacterium{myFile: "/my/file/path"} """ + val structExpr = fromString[ExpressionElement](structLiteral, parser.parse_e) + structExpr.shouldBeValidPF { case e => + e.evaluateType(Map.empty, typeAliases) shouldBeValid WomCompositeType(bacteriumTypeMap, Some("Bacterium")) } } } diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/types/EngineFunctionEvaluators.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/types/EngineFunctionEvaluators.scala index a1ad06230b8..56d9c59ff72 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/types/EngineFunctionEvaluators.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/types/EngineFunctionEvaluators.scala @@ -42,7 +42,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomSingleFileType).map(_ => WomArrayType(WomStringType)) + validateParamType(a.param, linkedValues, WomSingleFileType, typeAliases).map(_ => WomArrayType(WomStringType)) } implicit val readTsvFunctionEvaluator: TypeEvaluator[ReadTsv] = new TypeEvaluator[ReadTsv] { @@ -52,7 +52,9 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomSingleFileType).map(_ => WomArrayType(WomArrayType(WomStringType))) + validateParamType(a.param, linkedValues, WomSingleFileType, typeAliases).map(_ => + WomArrayType(WomArrayType(WomStringType)) + ) } implicit val readMapFunctionEvaluator: TypeEvaluator[ReadMap] = new TypeEvaluator[ReadMap] { @@ -62,7 +64,9 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomSingleFileType).map(_ => WomMapType(WomStringType, WomStringType)) + validateParamType(a.param, linkedValues, WomSingleFileType, typeAliases).map(_ => + WomMapType(WomStringType, WomStringType) + ) } implicit val readObjectFunctionEvaluator: TypeEvaluator[ReadObject] = new TypeEvaluator[ReadObject] { @@ -72,7 +76,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomSingleFileType).map(_ => WomObjectType) + validateParamType(a.param, linkedValues, WomSingleFileType, typeAliases).map(_ => WomObjectType) } implicit val readObjectsFunctionEvaluator: TypeEvaluator[ReadObjects] = new TypeEvaluator[ReadObjects] { @@ -82,7 +86,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomSingleFileType).map(_ => WomArrayType(WomObjectType)) + validateParamType(a.param, linkedValues, WomSingleFileType, typeAliases).map(_ => WomArrayType(WomObjectType)) } implicit val readJsonFunctionEvaluator: TypeEvaluator[ReadJson] = new TypeEvaluator[ReadJson] { @@ -93,7 +97,7 @@ object EngineFunctionEvaluators { expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = // we can't figure out the WomType of data without reading the file hence evaluate it to `WomAnyType` - validateParamType(a.param, linkedValues, WomSingleFileType).map(_ => WomAnyType) + validateParamType(a.param, linkedValues, WomSingleFileType, typeAliases).map(_ => WomAnyType) } implicit val readIntFunctionEvaluator: TypeEvaluator[ReadInt] = new TypeEvaluator[ReadInt] { @@ -103,7 +107,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomSingleFileType).map(_ => WomIntegerType) + validateParamType(a.param, linkedValues, WomSingleFileType, typeAliases).map(_ => WomIntegerType) } implicit val readStringFunctionEvaluator: TypeEvaluator[ReadString] = new TypeEvaluator[ReadString] { @@ -113,7 +117,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomSingleFileType).map(_ => WomStringType) + validateParamType(a.param, linkedValues, WomSingleFileType, typeAliases).map(_ => WomStringType) } implicit val readFloatFunctionEvaluator: TypeEvaluator[ReadFloat] = new TypeEvaluator[ReadFloat] { @@ -123,7 +127,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomSingleFileType).map(_ => WomFloatType) + validateParamType(a.param, linkedValues, WomSingleFileType, typeAliases).map(_ => WomFloatType) } implicit val readBooleanFunctionEvaluator: TypeEvaluator[ReadBoolean] = new TypeEvaluator[ReadBoolean] { @@ -133,7 +137,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomSingleFileType).map(_ => WomBooleanType) + validateParamType(a.param, linkedValues, WomSingleFileType, typeAliases).map(_ => WomBooleanType) } implicit val writeLinesFunctionEvaluator: TypeEvaluator[WriteLines] = new TypeEvaluator[WriteLines] { @@ -143,7 +147,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomArrayType(WomStringType)).map(_ => WomSingleFileType) + validateParamType(a.param, linkedValues, WomArrayType(WomStringType), typeAliases).map(_ => WomSingleFileType) } implicit val writeTsvFunctionEvaluator: TypeEvaluator[WriteTsv] = new TypeEvaluator[WriteTsv] { @@ -153,7 +157,9 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomArrayType(WomArrayType(WomStringType))).map(_ => WomSingleFileType) + validateParamType(a.param, linkedValues, WomArrayType(WomArrayType(WomStringType)), typeAliases).map(_ => + WomSingleFileType + ) } implicit val writeMapFunctionEvaluator: TypeEvaluator[WriteMap] = new TypeEvaluator[WriteMap] { @@ -163,7 +169,9 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomMapType(WomAnyType, WomAnyType)).map(_ => WomSingleFileType) + validateParamType(a.param, linkedValues, WomMapType(WomAnyType, WomAnyType), typeAliases).map(_ => + WomSingleFileType + ) } implicit val writeObjectFunctionEvaluator: TypeEvaluator[WriteObject] = new TypeEvaluator[WriteObject] { @@ -173,7 +181,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomObjectType).map(_ => WomSingleFileType) + validateParamType(a.param, linkedValues, WomObjectType, typeAliases).map(_ => WomSingleFileType) } implicit val writeObjectsFunctionEvaluator: TypeEvaluator[WriteObjects] = new TypeEvaluator[WriteObjects] { @@ -183,7 +191,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomArrayType(WomObjectType)).map(_ => WomSingleFileType) + validateParamType(a.param, linkedValues, WomArrayType(WomObjectType), typeAliases).map(_ => WomSingleFileType) } implicit val writeJsonFunctionEvaluator: TypeEvaluator[WriteJson] = new TypeEvaluator[WriteJson] { @@ -216,7 +224,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomIntegerType).map(_ => WomArrayType(WomIntegerType)) + validateParamType(a.param, linkedValues, WomIntegerType, typeAliases).map(_ => WomArrayType(WomIntegerType)) } implicit val transposeFunctionEvaluator: TypeEvaluator[Transpose] = new TypeEvaluator[Transpose] { @@ -240,7 +248,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomArrayType(WomAnyType)).map(_ => WomIntegerType) + validateParamType(a.param, linkedValues, WomArrayType(WomAnyType), typeAliases).map(_ => WomIntegerType) } implicit val flattenFunctionEvaluator: TypeEvaluator[Flatten] = new TypeEvaluator[Flatten] { @@ -294,7 +302,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomOptionalType(WomAnyType)).map(_ => WomBooleanType) + validateParamType(a.param, linkedValues, WomOptionalType(WomAnyType), typeAliases).map(_ => WomBooleanType) } implicit val floorFunctionEvaluator: TypeEvaluator[Floor] = new TypeEvaluator[Floor] { @@ -304,7 +312,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomFloatType).map(_ => WomIntegerType) + validateParamType(a.param, linkedValues, WomFloatType, typeAliases).map(_ => WomIntegerType) } implicit val ceilFunctionEvaluator: TypeEvaluator[Ceil] = new TypeEvaluator[Ceil] { @@ -314,7 +322,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomFloatType).map(_ => WomIntegerType) + validateParamType(a.param, linkedValues, WomFloatType, typeAliases).map(_ => WomIntegerType) } implicit val roundFunctionEvaluator: TypeEvaluator[Round] = new TypeEvaluator[Round] { @@ -324,7 +332,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomFloatType).map(_ => WomIntegerType) + validateParamType(a.param, linkedValues, WomFloatType, typeAliases).map(_ => WomIntegerType) } implicit val globFunctionTypeEvaluator: TypeEvaluator[Glob] = new TypeEvaluator[Glob] { @@ -334,7 +342,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - validateParamType(a.param, linkedValues, WomStringType).map(_ => WomArrayType(WomSingleFileType)) + validateParamType(a.param, linkedValues, WomStringType, typeAliases).map(_ => WomArrayType(WomSingleFileType)) } implicit val sizeFunctionEvaluator: TypeEvaluator[Size] = new TypeEvaluator[Size] { @@ -353,7 +361,7 @@ object EngineFunctionEvaluators { ): ErrorOr[WomType] = { val validatedSecondArg: ErrorOr[Unit] = a.secondParam match { case None => ().validNel - case Some(arg) => validateParamType(arg, linkedValues, WomStringType).void + case Some(arg) => validateParamType(arg, linkedValues, WomStringType, typeAliases).void } val validatedFirstArg: ErrorOr[Unit] = a.firstParam.evaluateType(linkedValues, typeAliases).flatMap { case t if suitableSizeType(t) => ().validNel @@ -373,21 +381,22 @@ object EngineFunctionEvaluators { ): ErrorOr[WomType] = { val validatedSecondArg: ErrorOr[Unit] = a.secondParam match { case None => ().validNel - case Some(arg) => validateParamType(arg, linkedValues, WomStringType).void + case Some(arg) => validateParamType(arg, linkedValues, WomStringType, typeAliases).void } - (validateParamType(a.firstParam, linkedValues, WomSingleFileType), validatedSecondArg) mapN { (_, _) => - WomStringType + (validateParamType(a.firstParam, linkedValues, WomSingleFileType, typeAliases), validatedSecondArg) mapN { + (_, _) => + WomStringType } } } private def crossOrZipType(arg1: ExpressionElement, arg2: ExpressionElement, - linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle] + linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle], + typeAliases: Map[String, WomType] )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement]): ErrorOr[WomType] = - // TODO: can/should we pipe type aliases through here? - (arg1.evaluateType(linkedValues, Map()), arg2.evaluateType(linkedValues, Map())) match { + (arg1.evaluateType(linkedValues, typeAliases), arg2.evaluateType(linkedValues, typeAliases)) match { case (Valid(WomArrayType(left)), Valid(WomArrayType(right))) => WomArrayType(WomPairType(left, right)).validNel case (Valid(otherLeft), Valid(WomArrayType(_))) => @@ -407,7 +416,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - crossOrZipType(a.arg1, a.arg2, linkedValues) + crossOrZipType(a.arg1, a.arg2, linkedValues, typeAliases) } implicit val crossFunctionEvaluator: TypeEvaluator[Cross] = new TypeEvaluator[Cross] { override def evaluateType(a: Cross, @@ -416,7 +425,7 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - crossOrZipType(a.arg1, a.arg2, linkedValues) + crossOrZipType(a.arg1, a.arg2, linkedValues, typeAliases) } implicit val prefixFunctionEvaluator: TypeEvaluator[Prefix] = new TypeEvaluator[Prefix] { @@ -426,8 +435,8 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - (validateParamType(a.prefix, linkedValues, WomStringType), - validateParamType(a.array, linkedValues, WomArrayType(WomStringType)) + (validateParamType(a.prefix, linkedValues, WomStringType, typeAliases), + validateParamType(a.array, linkedValues, WomArrayType(WomStringType), typeAliases) ) mapN { (_, _) => WomArrayType(WomStringType) } } @@ -438,17 +447,17 @@ object EngineFunctionEvaluators { )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement] ): ErrorOr[WomType] = - (validateParamType(a.input, linkedValues, WomSingleFileType), - validateParamType(a.pattern, linkedValues, WomSingleFileType), - validateParamType(a.replace, linkedValues, WomSingleFileType) + (validateParamType(a.input, linkedValues, WomSingleFileType, typeAliases), + validateParamType(a.pattern, linkedValues, WomSingleFileType, typeAliases), + validateParamType(a.replace, linkedValues, WomSingleFileType, typeAliases) ) mapN { (_, _, _) => WomStringType } } def validateParamType(param: ExpressionElement, linkedValues: Map[UnlinkedConsumedValueHook, GeneratedValueHandle], - expectedType: WomType + expectedType: WomType, + typeAliases: Map[String, WomType] )(implicit expressionTypeEvaluator: TypeEvaluator[ExpressionElement]): ErrorOr[WomType] = - // TODO: pipe type aliases through here param.evaluateType(linkedValues, Map()).flatMap { foundType => if (expectedType.isCoerceableFrom(foundType)) { foundType.validNel } else {