Skip to content

Commit

Permalink
[NU-1926] Typing of indexing of map (#7434)
Browse files Browse the repository at this point in the history
Co-authored-by: Pawel Czajka <[email protected]>
  • Loading branch information
paw787878 and Pawel Czajka authored Jan 17, 2025
1 parent 033a652 commit 0214a5b
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ object AssignabilityDeterminer {
def isAssignableStrict(from: TypingResult, to: TypingResult): ValidatedNel[String, Unit] =
isAssignable(from, to, StrictConversionChecker)

def isAssignableWithoutConversion(from: TypingResult, to: TypingResult): ValidatedNel[String, Unit] =
isAssignable(from, to, WithoutConversionChecker)

private def isAssignable(from: TypingResult, to: TypingResult, conversionChecker: ConversionChecker) = {
(from, to) match {
case (_, Unknown) => ().validNel
Expand Down Expand Up @@ -223,6 +226,19 @@ object AssignabilityDeterminer {

}

private object WithoutConversionChecker 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 canBeConvertedWithoutConversionTo(typingResult: TypingResult): Boolean =
AssignabilityDeterminer.isAssignableWithoutConversion(this, typingResult).isValid

def valueOpt: Option[Any]

def withoutValue: TypingResult
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,17 @@ private[spel] class Typer(
// 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 clazz.isAssignableFrom(classOf[java.util.Map[_, _]]) =>
withTypedChildren(_ => valid(valueParam))
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.
// 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 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.canBeConvertedWithoutConversionTo(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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -906,6 +906,28 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD
parse[java.math.BigDecimal]("-1.1", ctx) shouldBe Symbol("valid")
}

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("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("should handle map indexing with unknown key type") {
val context = Context("sth").withVariables(
Map(
"unknownString" -> ContainerOfUnknown("a"),
)
)

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") {
parse[Long]("'d'? 3 : 4", ctx) should not be Symbol("valid")
parse[String]("1 > 2 ? 12 : 23", ctx) should not be Symbol("valid")
Expand Down

0 comments on commit 0214a5b

Please sign in to comment.