From 8ecbbe7bc709228a5ffb8704cd0fc1db7de1f8c5 Mon Sep 17 00:00:00 2001 From: Pawel Czajka Date: Fri, 10 Jan 2025 13:06:15 +0100 Subject: [PATCH 01/10] squash indexing of map --- notes.txt | 17 ++++++++ .../touk/nussknacker/engine/spel/Typer.scala | 7 +++- .../engine/spel/SpelExpressionSpec.scala | 40 +++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 notes.txt diff --git a/notes.txt b/notes.txt new file mode 100644 index 00000000000..a9789f0b312 --- /dev/null +++ b/notes.txt @@ -0,0 +1,17 @@ +notes + +Indexer linia 154 tam jest ta konwersja typow zdaje sie + +teraz ten problem z tym, ze indeksowanie mapy w spelu rzuca wyjatkiem. +w Indekser przy indeksowaniu mapy wyrazenie "targetDescriptor.getMapKeyTypeDescriptor()" +rzuca wyjatkiem, bo chyba tam jest typ "object" a nie mapa. +(co ciekawe jak wsadzilem mape z kontekstu zbudowanego w scali +to ten "targetDescriptor.getMapKeyTypeDescriptor()" zwracal null, co oznacza, ze chyba zwykle nie bedzie konwertowac wcale +tych kluczy. + +ExtensionMethodResolver +tam gdzies jest returnType dostepna metoda "returnType", i ona debuggerem widze, ze zwrocila wlasnie object. Wiec cos jest nie teges + +kurcze, jest problem, nie jest znany typ klucza wiec nie robi konwersji ten indekser +w tym Indexer jest sprawdzenie czy da sie wybrac ten typ klucza, by sie dalo to trzeba zbudowac dobry ten TypeDetail +a do tego trzeba TypeDetail.map() metody uzyc diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala index 94398a81cd6..9218e728978 100644 --- a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala +++ b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala @@ -217,8 +217,13 @@ private[spel] class Typer( if clazz.isAssignableFrom(classOf[java.util.List[_]]) || clazz.isAssignableFrom(classOf[Array[Object]]) => // TODO: validate indexer key - the only valid key is an integer - but its more complicated with references withTypedChildren(_ => valid(param)) + // TODO_PAWEL tutaj ten isAsignableFrom to chyba na wyrost, albo jak juz odwrotnie, albo rownosc niech bedzie case TypedClass(clazz, keyParam :: valueParam :: Nil) if clazz.isAssignableFrom(classOf[java.util.Map[_, _]]) => - withTypedChildren(_ => valid(valueParam)) + withTypedChildren { + // TODO_PAWEL czy na pewno tak? trzeba zajrzec jakie konwersje sie robi w runtime + // mam wrazenie, ze tych konwersji w runtime nie robi. pytanie czy to z powodu braku typu? + case indexKey :: Nil if indexKey.canBeConvertedTo(keyParam) => valid(valueParam) + case _ => invalid(IllegalIndexingOperation)} case d: TypedDict => dictTyper.typeDictValue(d, e).map(toNodeResult) case union: TypedUnion => typeUnion(e, union) case TypedTaggedValue(underlying, _) => typeIndexer(e, underlying) diff --git a/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala b/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala index 71d0b6ab4c9..a23d712e45b 100644 --- a/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala +++ b/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala @@ -897,6 +897,46 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD parse[java.math.BigDecimal]("-1.1", ctx) shouldBe Symbol("valid") } + // TODO_PAWEL usun + test("asdf2") { + // to dziala + val sth = evaluate[Any]("""#someMap["1"]""", Context("abc").withVariable("someMap", Map("1" -> 2).asJava)) + val a = 5 + } + + // TODO_PAWEL usun + test("asdf3") { + // to nie dziala, integer cannot be cast to string, powiedzmy ze rozumiem, bo nie zna generycznego typu i probuje go zgadnac, i zgaduje ze ma byc stringiem, i tutaj zle zgadl + val sth = evaluate[Any]("""#someMap[1]""", Context("abc").withVariable("someMap", Map(1 -> 2).asJava)) + val a = 5 + } + + test("asdf") { + evaluate[Int]("""{{key: 1.toLong, value: 5}}.toMap[1]""") shouldBe 5 + } + + test("asdffsadf2") { + evaluate[Int]("""{{key: 1, value: 5}}.toMap[1.toLong]""") shouldBe 5 + } + + test("asdf222") { + evaluate[Int]("""{{key: 1.toLong, value: 5}}.toMap.get(1)""") shouldBe 5 + } + + test("asdffsadf2222") { + evaluate[Int]("""{{key: 1, value: 5}}.toMap.get(1.toLong)""") shouldBe 5 + } + + // TODO_PAWEL lepszy test w ktorym jeszcz esprawdzam co sie zaselektuje + test("should not validate map indexing if index cannot be converted to map key type") { + parse[Any]("""{{key: "a", value: 5}}.toMap[0]""") shouldBe Symbol("invalid") + parse[Any]("""{{key: 1, value: 5}}.toMap[0]""") shouldBe Symbol("valid") + parse[Any]("""{{key: 1, value: 5}}.toMap["0"]""") shouldBe Symbol("invalid") + // TODO_PAWEL no dobra, ale czy to spel zrobi dobrze? chyba ma za malo type info + parse[Any]("""{{key: 1.toLong, value: 5}}.toMap[0]""") shouldBe Symbol("valid") + parse[Any]("""{{key: 1, value: 5}}.toMap[0.toLong]""") shouldBe Symbol("valid") + } + test("validate ternary operator") { parse[Long]("'d'? 3 : 4", ctx) should not be Symbol("valid") parse[String]("1 > 2 ? 12 : 23", ctx) should not be Symbol("valid") From c4c059d5ad8725d37833a165ffacd6df4682cd0c Mon Sep 17 00:00:00 2001 From: Pawel Czajka Date: Mon, 13 Jan 2025 14:29:47 +0100 Subject: [PATCH 02/10] implemented no conversion conversion --- .../api/typed/AssignabilityDeterminer.scala | 16 +++++++ .../nussknacker/engine/api/typed/typing.scala | 6 +++ .../touk/nussknacker/engine/spel/Typer.scala | 16 +++++-- .../engine/spel/SpelExpressionSpec.scala | 48 ++++++------------- 4 files changed, 49 insertions(+), 37 deletions(-) diff --git a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala index 8e4cec7ee42..024116a81c9 100644 --- a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala +++ b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala @@ -32,6 +32,9 @@ object AssignabilityDeterminer { def isAssignableStrict(from: TypingResult, to: TypingResult): ValidatedNel[String, Unit] = isAssignable(from, to, StrictConversionChecker) + def isAssignableNoConversion(from: TypingResult, to: TypingResult): ValidatedNel[String, Unit] = + isAssignable(from, to, NoConversionConversionChecker) + private def isAssignable(from: TypingResult, to: TypingResult, conversionChecker: ConversionChecker) = { (from, to) match { case (_, Unknown) => ().validNel @@ -223,6 +226,19 @@ object AssignabilityDeterminer { } + private object NoConversionConversionChecker extends ConversionChecker { + + override def isConvertable( + from: SingleTypingResult, + to: TypedClass + ): ValidatedNel[String, Unit] = { + val errMsgPrefix = + s"${from.runtimeObjType.display} is not the same as ${to.display}" + condNel(from.withoutValue == to.withoutValue, (), errMsgPrefix) + } + + } + private object StrictConversionChecker extends ConversionChecker { override def isConvertable( 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 index 1aff3f9a890..66bd6091437 100644 --- 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 @@ -40,6 +40,12 @@ object typing { final def canBeStrictlyConvertedTo(typingResult: TypingResult): Boolean = AssignabilityDeterminer.isAssignableStrict(this, typingResult).isValid + /** + * Checks if the conversion to a given typingResult can be made without any conversion. + */ + final def canBeConvertedWithNoConversionTo(typingResult: TypingResult): Boolean = + AssignabilityDeterminer.isAssignableNoConversion(this, typingResult).isValid + def valueOpt: Option[Any] def withoutValue: TypingResult diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala index 9218e728978..7ea8c8faff2 100644 --- a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala +++ b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala @@ -220,10 +220,18 @@ private[spel] class Typer( // TODO_PAWEL tutaj ten isAsignableFrom to chyba na wyrost, albo jak juz odwrotnie, albo rownosc niech bedzie case TypedClass(clazz, keyParam :: valueParam :: Nil) if clazz.isAssignableFrom(classOf[java.util.Map[_, _]]) => withTypedChildren { - // TODO_PAWEL czy na pewno tak? trzeba zajrzec jakie konwersje sie robi w runtime - // mam wrazenie, ze tych konwersji w runtime nie robi. pytanie czy to z powodu braku typu? - case indexKey :: Nil if indexKey.canBeConvertedTo(keyParam) => valid(valueParam) - case _ => invalid(IllegalIndexingOperation)} + // Spel implementation of map indexer (in class org.springframework.expression.spel.ast.Indexer, line 154) tries to convert + // indexer to key type of map, but this conversion can be accomplished only if key type of map is known. + // Currently .asMap extension is implemented in such a way, that spel does not know key type of the resulting map + // (that is when spel evaluates this expression it only knows that it got map, but does not know its type parameters). + // It would be hard to change implementation of .asMap extension so we partially turn off this feature of key conversion + // by allowing in typing only situations when map key type and indexer type are the same (though we have to allow + // indexing with unknown type) + case indexKey :: Nil if indexKey.canBeConvertedWithNoConversionTo(keyParam) => { + valid(valueParam) + } + case _ => invalid(IllegalIndexingOperation) + } case d: TypedDict => dictTyper.typeDictValue(d, e).map(toNodeResult) case union: TypedUnion => typeUnion(e, union) case TypedTaggedValue(underlying, _) => typeIndexer(e, underlying) diff --git a/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala b/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala index a23d712e45b..498b7ace77f 100644 --- a/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala +++ b/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala @@ -897,44 +897,26 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD parse[java.math.BigDecimal]("-1.1", ctx) shouldBe Symbol("valid") } - // TODO_PAWEL usun - test("asdf2") { - // to dziala - val sth = evaluate[Any]("""#someMap["1"]""", Context("abc").withVariable("someMap", Map("1" -> 2).asJava)) - val a = 5 - } - - // TODO_PAWEL usun - test("asdf3") { - // to nie dziala, integer cannot be cast to string, powiedzmy ze rozumiem, bo nie zna generycznego typu i probuje go zgadnac, i zgaduje ze ma byc stringiem, i tutaj zle zgadl - val sth = evaluate[Any]("""#someMap[1]""", Context("abc").withVariable("someMap", Map(1 -> 2).asJava)) - val a = 5 - } - - test("asdf") { - evaluate[Int]("""{{key: 1.toLong, value: 5}}.toMap[1]""") shouldBe 5 - } - - test("asdffsadf2") { - evaluate[Int]("""{{key: 1, value: 5}}.toMap[1.toLong]""") shouldBe 5 + test("should not validate map indexing if index type and map key type are different") { + parse[Any]("""{{key: "a", value: 5}}.toMap[0]""") shouldBe Symbol("invalid") + parse[Any]("""{{key: 1, value: 5}}.toMap["0"]""") shouldBe Symbol("invalid") + parse[Any]("""{{key: 1.toLong, value: 5}}.toMap[0]""") shouldBe Symbol("invalid") + parse[Any]("""{{key: 1, value: 5}}.toMap[0.toLong]""") shouldBe Symbol("invalid") } - test("asdf222") { - evaluate[Int]("""{{key: 1.toLong, value: 5}}.toMap.get(1)""") shouldBe 5 + test("should validate map indexing if index type and map key type are the same") { + parse[Any]("""{{key: 1, value: 5}}.toMap[0]""") shouldBe Symbol("valid") } - test("asdffsadf2222") { - evaluate[Int]("""{{key: 1, value: 5}}.toMap.get(1.toLong)""") shouldBe 5 - } + test("should handle handle map indexing with unknown key type") { + val context = Context("sth").withVariables( + Map( + "unknownString" -> ContainerOfUnknown("a"), + ) + ) - // TODO_PAWEL lepszy test w ktorym jeszcz esprawdzam co sie zaselektuje - test("should not validate map indexing if index cannot be converted to map key type") { - parse[Any]("""{{key: "a", value: 5}}.toMap[0]""") shouldBe Symbol("invalid") - parse[Any]("""{{key: 1, value: 5}}.toMap[0]""") shouldBe Symbol("valid") - parse[Any]("""{{key: 1, value: 5}}.toMap["0"]""") shouldBe Symbol("invalid") - // TODO_PAWEL no dobra, ale czy to spel zrobi dobrze? chyba ma za malo type info - parse[Any]("""{{key: 1.toLong, value: 5}}.toMap[0]""") shouldBe Symbol("valid") - parse[Any]("""{{key: 1, value: 5}}.toMap[0.toLong]""") shouldBe Symbol("valid") + evaluate[Int]("""{{key: "a", value: 5}}.toMap[#unknownString.value]""", context) shouldBe 5 + evaluate[Integer]("""{{key: "b", value: 5}}.toMap[#unknownString.value]""", context) shouldBe null } test("validate ternary operator") { From dc63f69b2b0fbd2260c346f0916ea5cea41cb530 Mon Sep 17 00:00:00 2001 From: Pawel Czajka Date: Mon, 13 Jan 2025 14:33:42 +0100 Subject: [PATCH 03/10] fixed the isassignableto --- .../main/scala/pl/touk/nussknacker/engine/spel/Typer.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala index 7ea8c8faff2..6af371f3ceb 100644 --- a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala +++ b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala @@ -214,11 +214,10 @@ private[spel] class Typer( def typeIndexer(e: Indexer, typingResult: TypingResult): NodeTypingResult = { typingResult match { case TypedClass(clazz, param :: Nil) - if clazz.isAssignableFrom(classOf[java.util.List[_]]) || clazz.isAssignableFrom(classOf[Array[Object]]) => + if classOf[java.util.List[_]].isAssignableFrom(clazz) || classOf[Array[Object]].isAssignableFrom(clazz) => // TODO: validate indexer key - the only valid key is an integer - but its more complicated with references withTypedChildren(_ => valid(param)) - // TODO_PAWEL tutaj ten isAsignableFrom to chyba na wyrost, albo jak juz odwrotnie, albo rownosc niech bedzie - case TypedClass(clazz, keyParam :: valueParam :: Nil) if clazz.isAssignableFrom(classOf[java.util.Map[_, _]]) => + case TypedClass(clazz, keyParam :: valueParam :: Nil) if classOf[java.util.Map[_, _]].isAssignableFrom(clazz) => withTypedChildren { // Spel implementation of map indexer (in class org.springframework.expression.spel.ast.Indexer, line 154) tries to convert // indexer to key type of map, but this conversion can be accomplished only if key type of map is known. From 47199245c8775a732d4df99ae1be9bcd215cd7a7 Mon Sep 17 00:00:00 2001 From: Pawel Czajka Date: Mon, 13 Jan 2025 14:50:13 +0100 Subject: [PATCH 04/10] fixes --- .../api/typed/AssignabilityDeterminer.scala | 2 +- .../nussknacker/engine/api/typed/typing.scala | 2 +- notes.txt | 17 ----------------- .../pl/touk/nussknacker/engine/spel/Typer.scala | 8 +++----- 4 files changed, 5 insertions(+), 24 deletions(-) delete mode 100644 notes.txt diff --git a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala index 024116a81c9..bce9c04d1ca 100644 --- a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala +++ b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala @@ -32,7 +32,7 @@ object AssignabilityDeterminer { def isAssignableStrict(from: TypingResult, to: TypingResult): ValidatedNel[String, Unit] = isAssignable(from, to, StrictConversionChecker) - def isAssignableNoConversion(from: TypingResult, to: TypingResult): ValidatedNel[String, Unit] = + def isAssignableWithNoConversion(from: TypingResult, to: TypingResult): ValidatedNel[String, Unit] = isAssignable(from, to, NoConversionConversionChecker) private def isAssignable(from: TypingResult, to: TypingResult, conversionChecker: ConversionChecker) = { 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 index 66bd6091437..664d0279e5d 100644 --- 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 @@ -44,7 +44,7 @@ object typing { * Checks if the conversion to a given typingResult can be made without any conversion. */ final def canBeConvertedWithNoConversionTo(typingResult: TypingResult): Boolean = - AssignabilityDeterminer.isAssignableNoConversion(this, typingResult).isValid + AssignabilityDeterminer.isAssignableWithNoConversion(this, typingResult).isValid def valueOpt: Option[Any] diff --git a/notes.txt b/notes.txt deleted file mode 100644 index a9789f0b312..00000000000 --- a/notes.txt +++ /dev/null @@ -1,17 +0,0 @@ -notes - -Indexer linia 154 tam jest ta konwersja typow zdaje sie - -teraz ten problem z tym, ze indeksowanie mapy w spelu rzuca wyjatkiem. -w Indekser przy indeksowaniu mapy wyrazenie "targetDescriptor.getMapKeyTypeDescriptor()" -rzuca wyjatkiem, bo chyba tam jest typ "object" a nie mapa. -(co ciekawe jak wsadzilem mape z kontekstu zbudowanego w scali -to ten "targetDescriptor.getMapKeyTypeDescriptor()" zwracal null, co oznacza, ze chyba zwykle nie bedzie konwertowac wcale -tych kluczy. - -ExtensionMethodResolver -tam gdzies jest returnType dostepna metoda "returnType", i ona debuggerem widze, ze zwrocila wlasnie object. Wiec cos jest nie teges - -kurcze, jest problem, nie jest znany typ klucza wiec nie robi konwersji ten indekser -w tym Indexer jest sprawdzenie czy da sie wybrac ten typ klucza, by sie dalo to trzeba zbudowac dobry ten TypeDetail -a do tego trzeba TypeDetail.map() metody uzyc diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala index 6af371f3ceb..983a330eaf7 100644 --- a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala +++ b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala @@ -220,15 +220,13 @@ private[spel] class Typer( case TypedClass(clazz, keyParam :: valueParam :: Nil) if classOf[java.util.Map[_, _]].isAssignableFrom(clazz) => withTypedChildren { // Spel implementation of map indexer (in class org.springframework.expression.spel.ast.Indexer, line 154) tries to convert - // indexer to key type of map, but this conversion can be accomplished only if key type of map is known. + // indexer to key type of map, but this conversion can be accomplished only if key type of map is known to spel. // Currently .asMap extension is implemented in such a way, that spel does not know key type of the resulting map // (that is when spel evaluates this expression it only knows that it got map, but does not know its type parameters). - // It would be hard to change implementation of .asMap extension so we partially turn off this feature of key conversion + // It would be hard to change implementation of .asMap extension so we partially turn off this feature of indexer conversion // by allowing in typing only situations when map key type and indexer type are the same (though we have to allow // indexing with unknown type) - case indexKey :: Nil if indexKey.canBeConvertedWithNoConversionTo(keyParam) => { - valid(valueParam) - } + case indexKey :: Nil if indexKey.canBeConvertedWithNoConversionTo(keyParam) => valid(valueParam) case _ => invalid(IllegalIndexingOperation) } case d: TypedDict => dictTyper.typeDictValue(d, e).map(toNodeResult) From 93dcb08d42685c19449882b141b4d36c82f86528 Mon Sep 17 00:00:00 2001 From: Pawel Czajka Date: Tue, 14 Jan 2025 11:41:54 +0100 Subject: [PATCH 05/10] rename to isAssignableWithNoConversion --- .../nussknacker/engine/api/typed/AssignabilityDeterminer.scala | 2 +- .../scala/pl/touk/nussknacker/engine/api/typed/typing.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala index bce9c04d1ca..0a3f963cdd8 100644 --- a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala +++ b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala @@ -32,7 +32,7 @@ object AssignabilityDeterminer { def isAssignableStrict(from: TypingResult, to: TypingResult): ValidatedNel[String, Unit] = isAssignable(from, to, StrictConversionChecker) - def isAssignableWithNoConversion(from: TypingResult, to: TypingResult): ValidatedNel[String, Unit] = + def isAssignableWithoutConversion(from: TypingResult, to: TypingResult): ValidatedNel[String, Unit] = isAssignable(from, to, NoConversionConversionChecker) private def isAssignable(from: TypingResult, to: TypingResult, conversionChecker: ConversionChecker) = { 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 index 664d0279e5d..e3011f93482 100644 --- 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 @@ -44,7 +44,7 @@ object typing { * Checks if the conversion to a given typingResult can be made without any conversion. */ final def canBeConvertedWithNoConversionTo(typingResult: TypingResult): Boolean = - AssignabilityDeterminer.isAssignableWithNoConversion(this, typingResult).isValid + AssignabilityDeterminer.isAssignableWithoutConversion(this, typingResult).isValid def valueOpt: Option[Any] From d9099877292415b3fa173bf43235000b0534852a Mon Sep 17 00:00:00 2001 From: Pawel Czajka Date: Tue, 14 Jan 2025 11:45:18 +0100 Subject: [PATCH 06/10] more renames with word without --- .../engine/api/typed/AssignabilityDeterminer.scala | 4 ++-- .../scala/pl/touk/nussknacker/engine/api/typed/typing.scala | 2 +- .../main/scala/pl/touk/nussknacker/engine/spel/Typer.scala | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala index 0a3f963cdd8..0c016839665 100644 --- a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala +++ b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala @@ -33,7 +33,7 @@ object AssignabilityDeterminer { isAssignable(from, to, StrictConversionChecker) def isAssignableWithoutConversion(from: TypingResult, to: TypingResult): ValidatedNel[String, Unit] = - isAssignable(from, to, NoConversionConversionChecker) + isAssignable(from, to, WithoutConversionChecker) private def isAssignable(from: TypingResult, to: TypingResult, conversionChecker: ConversionChecker) = { (from, to) match { @@ -226,7 +226,7 @@ object AssignabilityDeterminer { } - private object NoConversionConversionChecker extends ConversionChecker { + private object WithoutConversionChecker extends ConversionChecker { override def isConvertable( from: SingleTypingResult, 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 index e3011f93482..343af1e201d 100644 --- 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 @@ -43,7 +43,7 @@ object typing { /** * Checks if the conversion to a given typingResult can be made without any conversion. */ - final def canBeConvertedWithNoConversionTo(typingResult: TypingResult): Boolean = + final def canBeConvertedWithoutConversionTo(typingResult: TypingResult): Boolean = AssignabilityDeterminer.isAssignableWithoutConversion(this, typingResult).isValid def valueOpt: Option[Any] diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala index 983a330eaf7..2c138db9d41 100644 --- a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala +++ b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala @@ -226,7 +226,7 @@ private[spel] class Typer( // It would be hard to change implementation of .asMap extension so we partially turn off this feature of indexer conversion // by allowing in typing only situations when map key type and indexer type are the same (though we have to allow // indexing with unknown type) - case indexKey :: Nil if indexKey.canBeConvertedWithNoConversionTo(keyParam) => valid(valueParam) + case indexKey :: Nil if indexKey.canBeConvertedWithoutConversionTo(keyParam) => valid(valueParam) case _ => invalid(IllegalIndexingOperation) } case d: TypedDict => dictTyper.typeDictValue(d, e).map(toNodeResult) From a7504595b8748d863c5e4819aadf9d905e39f718 Mon Sep 17 00:00:00 2001 From: Pawel Czajka Date: Tue, 14 Jan 2025 11:47:35 +0100 Subject: [PATCH 07/10] typo fix --- .../pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala b/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala index 498b7ace77f..552e16e4968 100644 --- a/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala +++ b/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala @@ -908,7 +908,7 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD parse[Any]("""{{key: 1, value: 5}}.toMap[0]""") shouldBe Symbol("valid") } - test("should handle handle map indexing with unknown key type") { + test("should handle map indexing with unknown key type") { val context = Context("sth").withVariables( Map( "unknownString" -> ContainerOfUnknown("a"), From a5f825f349f5976e03614dcf749dec92e4f45cbf Mon Sep 17 00:00:00 2001 From: Pawel Czajka Date: Wed, 15 Jan 2025 13:15:58 +0100 Subject: [PATCH 08/10] change back the order in isAssignableFrom --- .../main/scala/pl/touk/nussknacker/engine/spel/Typer.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala index 2c138db9d41..4b31d54d8c7 100644 --- a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala +++ b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala @@ -214,10 +214,10 @@ private[spel] class Typer( def typeIndexer(e: Indexer, typingResult: TypingResult): NodeTypingResult = { typingResult match { case TypedClass(clazz, param :: Nil) - if classOf[java.util.List[_]].isAssignableFrom(clazz) || classOf[Array[Object]].isAssignableFrom(clazz) => + if clazz.isAssignableFrom(classOf[java.util.List[_]]) || clazz.isAssignableFrom(classOf[Array[Object]]) => // TODO: validate indexer key - the only valid key is an integer - but its more complicated with references withTypedChildren(_ => valid(param)) - case TypedClass(clazz, keyParam :: valueParam :: Nil) if classOf[java.util.Map[_, _]].isAssignableFrom(clazz) => + case TypedClass(clazz, keyParam :: valueParam :: Nil) if clazz.isAssignableFrom(classOf[java.util.Map[_, _]]) => withTypedChildren { // Spel implementation of map indexer (in class org.springframework.expression.spel.ast.Indexer, line 154) tries to convert // indexer to key type of map, but this conversion can be accomplished only if key type of map is known to spel. From 13268d87dcc2ccfecb9f32e77014382ed8685cb9 Mon Sep 17 00:00:00 2001 From: Pawel Czajka Date: Fri, 17 Jan 2025 10:53:14 +0100 Subject: [PATCH 09/10] empty to trigger tests From 4de569d3b6d164bd18830b61e71b388d0af17b21 Mon Sep 17 00:00:00 2001 From: Pawel Czajka Date: Fri, 17 Jan 2025 12:58:31 +0100 Subject: [PATCH 10/10] empty to trigger tests