TypedUnion used only for >= 2 types. EmptyUnion (Typed.empty) removed…

…, generated tests (#5457) * TypedUnion used only for >= 2 types. EmptyUnion (Typed.empty) removed * Generator-based tests for TypingResult + fix for array deserialization + Typed.record * NumberTypesPromotionStrategy used only locally for math operations, CommonSupertypeFinder doesn't use it anymore * methodInfo legacy cleanup
TouK · Jan 29, 2024 · 46c54b4 · 46c54b4
1 parent 251bc15
commit 46c54b4
Show file tree

Hide file tree

Showing 63 changed files with 966 additions and 654 deletions.
diff --git a/build.sbt b/build.sbt
@@ -1455,9 +1455,10 @@ lazy val componentsApi = (project in file("components-api"))
         "org.scala-lang.modules"        %% "scala-collection-compat"          % scalaCollectionsCompatV,
         "com.iheart"                    %% "ficus"                            % ficusV,
         "org.springframework"            % "spring-core"                      % springV,
-        "org.springframework"            % "spring-expression"                % springV % "test",
+        "org.springframework"            % "spring-expression"                % springV        % "test",
         "com.google.code.findbugs"       % "jsr305"                           % findBugsV,
-        "com.softwaremill.sttp.client3" %% "async-http-client-backend-future" % sttpV
+        "com.softwaremill.sttp.client3" %% "async-http-client-backend-future" % sttpV,
+        "org.scalatestplus"             %% s"scalacheck-$scalaCheckVshort"    % scalaTestPlusV % "test"
       )
     }
   )

diff --git a/...in/scala/pl/touk/nussknacker/engine/api/context/transformation/BaseDefinedParameter.scala b/...in/scala/pl/touk/nussknacker/engine/api/context/transformation/BaseDefinedParameter.scala
@@ -21,7 +21,8 @@ sealed trait DefinedBranchParameter extends BaseDefinedParameter {
 
   def expressionByBranchId: Map[String, TypedExpression]
 
-  final override def returnType: TypingResult = Typed(expressionByBranchId.values.map(_.returnType).toSet)
+  final override def returnType: TypingResult =
+    Typed.fromIterableOrUnknownIfEmpty(expressionByBranchId.values.map(_.returnType))
 
 }
 

diff --git a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/generics/MethodTypeInfo.scala b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/generics/MethodTypeInfo.scala
@@ -22,8 +22,6 @@ object MethodTypeInfo {
   def withoutVarargs(params: List[Parameter], result: TypingResult): MethodTypeInfo =
     MethodTypeInfo(params, None, result)
 
-  def withoutParameters(result: TypingResult): MethodTypeInfo =
-    MethodTypeInfo(Nil, None, result)
 }
 
 case class MethodTypeInfo(noVarArgs: List[Parameter], varArg: Option[Parameter], result: TypingResult) {

diff --git a/...nts-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/CanBeSubclassDeterminer.scala b/...nts-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/CanBeSubclassDeterminer.scala
@@ -1,7 +1,7 @@
 package pl.touk.nussknacker.engine.api.typed
 
 import cats.data.Validated._
-import cats.data.ValidatedNel
+import cats.data.{NonEmptyList, NonEmptySet, ValidatedNel}
 import cats.implicits.{catsSyntaxValidatedId, _}
 import org.apache.commons.lang3.ClassUtils
 import pl.touk.nussknacker.engine.api.typed.typing._
@@ -26,8 +26,10 @@ trait CanBeSubclassDeterminer {
       case (Unknown, _)       => ().validNel
       case (TypedNull, other) => canNullBeSubclassOf(other)
       case (_, TypedNull)     => s"No type can be subclass of ${TypedNull.display}".invalidNel
-      case (given: SingleTypingResult, superclass: TypedUnion) => canBeSubclassOf(Set(given), superclass.possibleTypes)
-      case (given: TypedUnion, superclass: SingleTypingResult) => canBeSubclassOf(given.possibleTypes, Set(superclass))
+      case (given: SingleTypingResult, superclass: TypedUnion) =>
+        canBeSubclassOf(NonEmptyList.one(given), superclass.possibleTypes)
+      case (given: TypedUnion, superclass: SingleTypingResult) =>
+        canBeSubclassOf(given.possibleTypes, NonEmptyList.one(superclass))
       case (given: SingleTypingResult, superclass: SingleTypingResult) => singleCanBeSubclassOf(given, superclass)
       case (given: TypedUnion, superclass: TypedUnion) => canBeSubclassOf(given.possibleTypes, superclass.possibleTypes)
     }
@@ -151,8 +153,8 @@ trait CanBeSubclassDeterminer {
   }
 
   private def canBeSubclassOf(
-      givenTypes: Set[SingleTypingResult],
-      superclassCandidates: Set[SingleTypingResult]
+      givenTypes: NonEmptyList[SingleTypingResult],
+      superclassCandidates: NonEmptyList[SingleTypingResult]
   ): ValidatedNel[String, Unit] = {
     // Would be more safety to do givenTypes.forAll(... superclassCandidates.exists ...) - we wil protect against
     // e.g. (String | Int).canBeSubclassOf(String) which can fail in runtime for Int, but on the other hand we can't block user's intended action.
@@ -162,9 +164,9 @@ trait CanBeSubclassDeterminer {
       givenTypes.exists(given => superclassCandidates.exists(singleCanBeSubclassOf(given, _).isValid)),
       (),
       s"""None of the following types:
-         |${givenTypes.map(" - " + _.display).mkString(",\n")}
+         |${givenTypes.map(" - " + _.display).toList.mkString(",\n")}
          |can be a subclass of any of:
-         |${superclassCandidates.map(" - " + _.display).mkString(",\n")}""".stripMargin
+         |${superclassCandidates.map(" - " + _.display).toList.mkString(",\n")}""".stripMargin
     )
   }
 

diff --git a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/TypeEncoders.scala b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/TypeEncoders.scala
@@ -1,5 +1,6 @@
 package pl.touk.nussknacker.engine.api.typed
 
+import cats.data.NonEmptyList
 import io.circe.Json._
 import io.circe._
 import pl.touk.nussknacker.engine.api.typed.TypeEncoders.typeField
@@ -31,8 +32,10 @@ object TypeEncoders {
       case single: SingleTypingResult => encodeSingleTypingResult(single)
       case typing.Unknown             => encodeUnknown
       case typing.TypedNull           => encodeNull
-      case TypedUnion(classes) =>
-        JsonObject("union" -> fromValues(classes.map(typ => fromJsonObject(encodeTypingResult(typ))).toList))
+      case union: TypedUnion =>
+        JsonObject(
+          "union" -> fromValues(union.possibleTypes.map(typ => fromJsonObject(encodeTypingResult(typ))).toList)
+        )
     })
       .+:(typeField -> fromString(TypingType.forType(result).toString))
       .+:("display" -> fromString(result.display))
@@ -144,8 +147,19 @@ class TypingResultDecoder(loadClass: String => Class[_]) {
     } yield TypedDict(id, valueType)
   }
 
+  // This implementation is lax. We are not warranting that the result will be a TypedUnion. We use Typed.apply
+  // under the hood which can e.g. produce a single element for a NEL.one. In general, it can produce the other
+  // number of elements than it is passed. We could be strict but we decided that being lax gives more benefits
+  // than risks
   private def typedUnion(obj: HCursor): Decoder.Result[TypingResult] = {
-    obj.downField("union").as[Set[SingleTypingResult]].map(TypedUnion)
+    obj.downField("union").as[List[SingleTypingResult]].flatMap { list =>
+      NonEmptyList
+        .fromList(list)
+        .map(nel => Right(Typed(nel)))
+        .getOrElse(
+          Left(DecodingFailure(s"Union should has at least 2 elements but it has ${list.size} elements", obj.history))
+        )
+    }
   }
 
   private def typedClass(obj: HCursor): Decoder.Result[TypedClass] = {
@@ -157,9 +171,14 @@ class TypingResultDecoder(loadClass: String => Class[_]) {
   }
 
   private def tryToLoadClass(name: String, obj: HCursor): Decoder.Result[Class[_]] = {
-    Try(loadClass(name)) match {
-      case Success(value) => Right(value)
-      case Failure(thr)   => Left(DecodingFailure(s"Failed to load class $name with ${thr.getMessage}", obj.history))
+    if (name == Typed.KlassForArrays.getName) {
+      // loading of array class causes Failed to load class [Ljava.lang.Object; with [Ljava.lang.Object;
+      Right(Typed.KlassForArrays)
+    } else {
+      Try(loadClass(name)) match {
+        case Success(value) => Right(value)
+        case Failure(thr)   => Left(DecodingFailure(s"Failed to load class $name with ${thr.getMessage}", obj.history))
+      }
     }
   }