Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NU-1926] Typing of indexing of map #7434

Merged
merged 13 commits into from
Jan 17, 2025
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
Loading