rename for pr

TouK · Nov 18, 2024 · e4724a2 · e4724a2
1 parent 209ab0c
commit e4724a2
Show file tree

Hide file tree

Showing 29 changed files with 201 additions and 154 deletions.
diff --git a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/NumberTypeUtils.scala b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/NumberTypeUtils.scala
@@ -15,9 +15,9 @@ object NumberTypeUtils {
     else if (typ == Typed[java.lang.Double]) java.lang.Double.valueOf(0)
     else if (typ == Typed[java.math.BigDecimal]) java.math.BigDecimal.ZERO
     // in case of some unions
-    else if (typ.canBeImplicitlyConvertedTo(Typed[java.lang.Integer])) java.lang.Integer.valueOf(0)
+    else if (typ.canBeConvertedTo(Typed[java.lang.Integer])) java.lang.Integer.valueOf(0)
     // double is quite safe - it can be converted to any Number
-    else if (typ.canBeImplicitlyConvertedTo(Typed[Number])) java.lang.Double.valueOf(0)
+    else if (typ.canBeConvertedTo(Typed[Number])) java.lang.Double.valueOf(0)
     else throw new IllegalArgumentException(s"Not expected type: ${typ.display}, should be Number")
   }
 

diff --git a/...-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/StrictConversionDeterminer.scala b/...-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/StrictConversionDeterminer.scala
@@ -1,18 +1,49 @@
 package pl.touk.nussknacker.engine.api.typed
 
 import cats.data.Validated.condNel
-import cats.data.ValidatedNel
+import cats.data.{NonEmptyList, Validated, ValidatedNel}
+import cats.implicits.catsSyntaxValidatedId
 import org.apache.commons.lang3.ClassUtils
 import pl.touk.nussknacker.engine.api.typed.supertype.NumberTypesPromotionStrategy.AllNumbers
-import pl.touk.nussknacker.engine.api.typed.typing.{SingleTypingResult, TypedClass, TypingResult}
+import pl.touk.nussknacker.engine.api.typed.typing._
 
 object StrictConversionDeterminer {
 
   def canBeConvertedTo(givenType: TypingResult, superclassCandidate: TypingResult): ValidatedNel[String, Unit] = {
-  ImplicitConversionDeterminer.canBeConvertedTo(givenType, superclassCandidate)
+    (givenType, superclassCandidate) match {
+      case (_, Unknown)       => ().validNel
+      case (Unknown, _)       => ().validNel
+      case (TypedNull, other) => canNullBeConvertedTo(other)
+      case (_, TypedNull)     => s"No type can be subclass of ${TypedNull.display}".invalidNel
+      case (given: SingleTypingResult, superclass: TypedUnion) =>
+        canBeConvertedTo(NonEmptyList.one(given), superclass.possibleTypes)
+      case (given: TypedUnion, superclass: SingleTypingResult) =>
+        canBeConvertedTo(given.possibleTypes, NonEmptyList.one(superclass))
+      case (given: SingleTypingResult, superclass: SingleTypingResult) => singleCanBeConvertedTo(given, superclass)
+      case (given: TypedUnion, superclass: TypedUnion) =>
+        canBeConvertedTo(given.possibleTypes, superclass.possibleTypes)
+    }
+  }
+
+  def canBeConvertedTo(
+      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.
+    // He/she could be sure that in this type, only String will appear. He/she also can't easily downcast (String | Int) to String so leaving here
+    // "double exists" looks like a good tradeoff
+    condNel(
+      givenTypes.exists(given => superclassCandidates.exists(singleCanBeConvertedTo(given, _).isValid)),
+      (),
+      s"""None of the following types:
+         |${givenTypes.map(" - " + _.display).toList.mkString(",\n")}
+         |can be a subclass of any of:
+         |${superclassCandidates.map(" - " + _.display).toList.mkString(",\n")}""".stripMargin
+    )
   }
 
-    def singleCanBeConvertedTo(
+  def singleCanBeConvertedTo(
       givenType: SingleTypingResult,
       superclassCandidate: SingleTypingResult
   ): ValidatedNel[String, Unit] = {
@@ -22,11 +53,26 @@ object StrictConversionDeterminer {
     isStrictSubclass(givenClass, givenSuperclas)
   }
 
-  def canBeStrictSubclassOf(givenType: TypingResult, superclassCandidate: TypingResult): ValidatedNel[String, Unit] = {
-    this.canBeConvertedTo(givenType, superclassCandidate)
+  private def canNullBeConvertedTo(result: TypingResult): ValidatedNel[String, Unit] = result match {
+    // TODO: Null should not be subclass of typed map that has all values assigned.
+    case TypedObjectWithValue(_, _) => s"${TypedNull.display} cannot be subclass of type with value".invalidNel
+    case _                          => ().validNel
+  }
+
+  def isStrictSubclass(givenClass: TypedClass, givenSuperclass: TypedClass): Validated[NonEmptyList[String], Unit] = {
+    condNel(
+      givenClass == givenSuperclass,
+      (),
+      f"${givenClass.display} and ${givenSuperclass.display} are not the same"
+    ) orElse
+      condNel(
+        isAssignable(givenClass.klass, givenSuperclass.klass),
+        (),
+        s"${givenClass.klass} is not assignable from ${givenSuperclass.klass}"
+      )
   }
 
-  override def isAssignable(from: Class[_], to: Class[_]): Boolean = {
+  def isAssignable(from: Class[_], to: Class[_]): Boolean = {
     (from, to) match {
       case (f, t) if ClassUtils.isAssignable(f, t, true) => true
       // Number double check by hand because lang3 can incorrectly throw false when dealing with java types

diff --git a/...nents-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/TypeConversionHandler.scala b/...nents-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/TypeConversionHandler.scala
@@ -108,7 +108,7 @@ object TypeConversionHandler {
         ClassUtils.isAssignable(boxedGivenClass, classOf[Number], true)
 
       case candidate if isDecimalNumber(candidate) =>
-        SubclassDeterminer.isAssignable(boxedGivenClass, candidate)
+        StrictConversionDeterminer.isAssignable(boxedGivenClass, candidate)
 
       case _ => false
     }

diff --git a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/typing.scala b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/typing.scala
@@ -27,11 +27,18 @@ object typing {
   // TODO: Rename to Typed, maybe NuType?
   sealed trait TypingResult {
 
-    final def canBeImplicitlyConvertedTo(typingResult: TypingResult): Boolean =
+    /**
+     * Checks if there exists a conversion to a given typingResult, with possible loss of precision, e.g. long to int.
+     * If you need to retain conversion precision, use canBeStrictlyConvertedTo
+     */
+    final def canBeConvertedTo(typingResult: TypingResult): Boolean =
       ImplicitConversionDeterminer.canBeConvertedTo(this, typingResult).isValid
 
-    def canBeStrictSubclassOf(typingResult: TypingResult): Boolean =
-      SubclassDeterminer.canBeStrictSubclassOf(this, typingResult).isValid
+    /**
+     * Checks if the conversion to a given typingResult can be made without loss of precision
+     */
+    final def canBeStrictlyConvertedTo(typingResult: TypingResult): Boolean =
+      StrictConversionDeterminer.canBeConvertedTo(this, typingResult).isValid
 
     def valueOpt: Option[Any]
 
@@ -463,7 +470,7 @@ object typing {
 
     def unapply(typingResult: TypingResult): Option[TypingResultTypedValue[T]] = {
       Option(typingResult)
-        .filter(_.canBeImplicitlyConvertedTo(Typed.fromDetailedType[T]))
+        .filter(_.canBeConvertedTo(Typed.fromDetailedType[T]))
         .map(new TypingResultTypedValue(_))
     }
 

diff --git a/...ents-api/src/test/scala/pl/touk/nussknacker/engine/api/typed/SubclassDeterminerSpec.scala b/...ents-api/src/test/scala/pl/touk/nussknacker/engine/api/typed/SubclassDeterminerSpec.scala
@@ -7,21 +7,21 @@ import org.scalatest.matchers.should.Matchers
 class SubclassDeterminerSpec extends AnyFunSuite with Matchers {
 
   test("Should validate assignability for decimal types") {
-    SubclassDeterminer.isAssignable(classOf[java.lang.Long], classOf[java.lang.Integer]) shouldBe false
-    SubclassDeterminer.isAssignable(classOf[Number], classOf[Integer]) shouldBe false
-    SubclassDeterminer.isAssignable(classOf[Integer], classOf[java.lang.Short]) shouldBe false
+    StrictConversionDeterminer.isAssignable(classOf[java.lang.Long], classOf[java.lang.Integer]) shouldBe false
+    StrictConversionDeterminer.isAssignable(classOf[Number], classOf[Integer]) shouldBe false
+    StrictConversionDeterminer.isAssignable(classOf[Integer], classOf[java.lang.Short]) shouldBe false
 
-    SubclassDeterminer.isAssignable(classOf[Integer], classOf[java.lang.Long]) shouldBe true
-    SubclassDeterminer.isAssignable(classOf[Integer], classOf[Number]) shouldBe true
-    SubclassDeterminer.isAssignable(classOf[java.lang.Short], classOf[Integer]) shouldBe true
+    StrictConversionDeterminer.isAssignable(classOf[Integer], classOf[java.lang.Long]) shouldBe true
+    StrictConversionDeterminer.isAssignable(classOf[Integer], classOf[Number]) shouldBe true
+    StrictConversionDeterminer.isAssignable(classOf[java.lang.Short], classOf[Integer]) shouldBe true
   }
 
   test("Should validate assignability for numerical types") {
-    SubclassDeterminer.isAssignable(classOf[java.lang.Long], classOf[java.lang.Double]) shouldBe true
-    SubclassDeterminer.isAssignable(classOf[java.lang.Float], classOf[Double]) shouldBe true
+    StrictConversionDeterminer.isAssignable(classOf[java.lang.Long], classOf[java.lang.Double]) shouldBe true
+    StrictConversionDeterminer.isAssignable(classOf[java.lang.Float], classOf[Double]) shouldBe true
 
-    SubclassDeterminer.isAssignable(classOf[Integer], classOf[java.lang.Float]) shouldBe true
-    SubclassDeterminer.isAssignable(classOf[java.lang.Long], classOf[java.lang.Double]) shouldBe true
+    StrictConversionDeterminer.isAssignable(classOf[Integer], classOf[java.lang.Float]) shouldBe true
+    StrictConversionDeterminer.isAssignable(classOf[java.lang.Long], classOf[java.lang.Double]) shouldBe true
   }
 
   // to check if autoboxing lang3 is failing - we can remove our fallback from SubclassDeterminer.isAssignable if the lib works properly

diff --git a/...nents-api/src/test/scala/pl/touk/nussknacker/engine/api/typed/TypedFromInstanceTest.scala b/...nents-api/src/test/scala/pl/touk/nussknacker/engine/api/typed/TypedFromInstanceTest.scala
@@ -60,20 +60,20 @@ class TypedFromInstanceTest extends AnyFunSuite with Matchers with LoneElement w
   }
 
   test("should type empty list") {
-    Typed.fromInstance(Nil).canBeImplicitlyConvertedTo(Typed(classOf[List[_]])) shouldBe true
-    Typed.fromInstance(Nil.asJava).canBeImplicitlyConvertedTo(Typed(classOf[java.util.List[_]])) shouldBe true
+    Typed.fromInstance(Nil).canBeConvertedTo(Typed(classOf[List[_]])) shouldBe true
+    Typed.fromInstance(Nil.asJava).canBeConvertedTo(Typed(classOf[java.util.List[_]])) shouldBe true
   }
 
   test("should type lists and return union of types coming from all elements") {
     def checkTypingResult(obj: Any, klass: Class[_], paramTypingResult: TypingResult): Unit = {
       val typingResult = Typed.fromInstance(obj)
 
-      typingResult.canBeImplicitlyConvertedTo(Typed(klass)) shouldBe true
+      typingResult.canBeConvertedTo(Typed(klass)) shouldBe true
       typingResult.withoutValue
         .asInstanceOf[TypedClass]
         .params
         .loneElement
-        .canBeImplicitlyConvertedTo(paramTypingResult) shouldBe true
+        .canBeConvertedTo(paramTypingResult) shouldBe true
     }
 
     def checkNotASubclassOfOtherParamTypingResult(obj: Any, otherParamTypingResult: TypingResult): Unit = {
@@ -82,7 +82,7 @@ class TypedFromInstanceTest extends AnyFunSuite with Matchers with LoneElement w
         .asInstanceOf[TypedClass]
         .params
         .loneElement
-        .canBeImplicitlyConvertedTo(otherParamTypingResult) shouldBe false
+        .canBeConvertedTo(otherParamTypingResult) shouldBe false
     }
 
     val listOfSimpleObjects = List[Any](1.1, 2)