[FIR] Forbid erroneous ===-checks

It was decided to forbid such comparisons,
as we know how `===` works. Also, added some more
test cases, just for comparison.

Reusing the proper `canHaveSubtypes()`
from `TypeUtils` prevents a breaking change
in:

- `comparingTripleWithPair.kt`
- `comparisonOfGenericInterfaceWithGenericClass.kt`

But it does lead to warnings
(instead of errors) in
`incompatibleEnumEntryClasses.kt`, which is an
unrelated mistake that will be fixed in the next
commit.

The refactoring in `canHaveSubtypes()` is purely
cosmetic - otherwise reading these conditions is hard
(and they don't fit my screen vertically).

^KT-62646
^KT-65541
^KT-57779
This commit is contained in:
Nikolay Lunyak
2024-02-26 10:46:32 +02:00
committed by Space Team
parent 6bf987e772
commit 226d4df277
23 changed files with 219 additions and 82 deletions
@@ -25,7 +25,6 @@ import org.jetbrains.kotlin.fir.expressions.*
import org.jetbrains.kotlin.fir.isPrimitiveType
import org.jetbrains.kotlin.fir.resolve.fullyExpandedType
import org.jetbrains.kotlin.fir.symbols.impl.FirClassSymbol
import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol
import org.jetbrains.kotlin.fir.types.*
import org.jetbrains.kotlin.fir.types.impl.ConeClassLikeTypeImpl
import org.jetbrains.kotlin.name.StandardClassIds
@@ -101,15 +100,11 @@ object FirEqualityCompatibilityChecker : FirEqualityOperatorCallChecker(MppCheck
}
private fun checkIdentityApplicability(l: TypeInfo, r: TypeInfo, context: CheckerContext): Applicability {
// The compiler should only check comparisons
// when identity-less types or builtins are involved.
val oneIsBuiltin = l.isBuiltin || r.isBuiltin
val oneIsNotNull = !l.type.isNullable || !r.type.isNullable
return when {
l.isIdentityLess || r.isIdentityLess -> Applicability.INAPPLICABLE_AS_IDENTITY_LESS
oneIsBuiltin && oneIsNotNull && shouldReportAsPerRules1(l, r, context) -> getInapplicabilityFor(l, r)
oneIsNotNull && shouldReportAsPerRules1(l, r, context) -> getInapplicabilityFor(l, r)
else -> Applicability.APPLICABLE
}
}
@@ -126,13 +121,12 @@ object FirEqualityCompatibilityChecker : FirEqualityOperatorCallChecker(MppCheck
}
private fun shouldReportAsPerRules1(l: TypeInfo, r: TypeInfo, context: CheckerContext): Boolean {
// Builtins are always final classes, so
// we only need to check if one is related
// to the other
val oneIsFinal = l.isFinal || r.isFinal
return when {
l.type.isNothingOrNullableNothing || r.type.isNothingOrNullableNothing -> false
else -> !l.isSubtypeOf(r, context) && !r.isSubtypeOf(l, context)
oneIsFinal -> !l.isSubtypeOf(r, context) && !r.isSubtypeOf(l, context)
else -> false
}
}
@@ -302,6 +296,7 @@ private class TypeInfo(
val isPrimitive: Boolean,
val isBuiltin: Boolean,
val isValueClass: Boolean,
val isFinal: Boolean,
val canHaveSubtypesAccordingToK1: Boolean,
) {
override fun toString() = "$type"
@@ -309,20 +304,6 @@ private class TypeInfo(
private val FirClassSymbol<*>.isBuiltin get() = isPrimitiveType() || classId == StandardClassIds.String || isEnumClass
// This property is used to replicate K1 behavior, and it
// tries to match the `TypeUtils.canHaveSubtypes(typeChecker, type)`
// check in the K1 intersector.
// In K2 enum classes are final, though enum entries are their subclasses.
private fun ConeKotlinType.canHaveSubtypesAccordingToK1(session: FirSession): Boolean {
val symbol = toSymbol(session)
return when {
symbol is FirRegularClassSymbol && symbol.isEnumClass -> true
symbol is FirClassSymbol<*> && symbol.isFinalClass -> false
else -> true
}
}
private val TypeInfo.isNullableEnum get() = isEnumClass && type.isNullable
private val TypeInfo.isIdentityLess get() = isPrimitive || isValueClass
@@ -350,7 +331,9 @@ private fun ConeKotlinType.toTypeInfo(session: FirSession): TypeInfo {
isPrimitive = bounds.any { it.isPrimitiveOrNullablePrimitive },
isBuiltin = bounds.any { it.toClassSymbol(session)?.isBuiltin == true },
isValueClass = bounds.any { it.toClassSymbol(session)?.isInline == true },
canHaveSubtypesAccordingToK1(session),
isFinal = bounds.any { it.toClassSymbol(session)?.isFinalClass == true },
// In K1's intersector, `canHaveSubtypes()` is called for `nullabilityStripped`.
withNullability(ConeNullability.NOT_NULL, session.typeContext).canHaveSubtypes(session),
)
}
@@ -38,7 +38,8 @@ object FirPrivateToThisAccessChecker : FirQualifiedAccessExpressionChecker(MppCh
// If there was a visibility diagnostic, no need to report another one about visibility
when (reference.diagnostic) {
is ConeVisibilityError,
is ConeSetterVisibilityError -> return
is ConeSetterVisibilityError
-> return
}
}
val dispatchReceiver = expression.dispatchReceiver ?: return
@@ -147,12 +148,4 @@ object FirPrivateToThisAccessChecker : FirQualifiedAccessExpressionChecker(MppCh
}
return false
}
private val ConeTypeProjection.variance: Variance
get() = when (this.kind) {
ProjectionKind.STAR -> Variance.OUT_VARIANCE
ProjectionKind.IN -> Variance.IN_VARIANCE
ProjectionKind.OUT -> Variance.OUT_VARIANCE
ProjectionKind.INVARIANT -> Variance.INVARIANT
}
}
@@ -5,6 +5,7 @@
package org.jetbrains.kotlin.fir.types
import org.jetbrains.kotlin.types.Variance
import org.jetbrains.kotlin.types.model.TypeArgumentMarker
enum class ProjectionKind {
@@ -84,3 +85,11 @@ fun ConeKotlinTypeProjection.replaceType(newType: ConeKotlinType): ConeKotlinTyp
is ConeKotlinTypeConflictingProjection -> ConeKotlinTypeConflictingProjection(newType)
}
}
val ConeTypeProjection.variance: Variance
get() = when (this.kind) {
ProjectionKind.STAR -> Variance.OUT_VARIANCE
ProjectionKind.IN -> Variance.IN_VARIANCE
ProjectionKind.OUT -> Variance.OUT_VARIANCE
ProjectionKind.INVARIANT -> Variance.INVARIANT
}
@@ -584,12 +584,16 @@ fun FirCallableDeclaration.isSubtypeOf(
)
}
/**
* The original K1 function: [org.jetbrains.kotlin.types.TypeUtils.canHaveSubtypes].
*/
fun ConeKotlinType.canHaveSubtypes(session: FirSession): Boolean {
if (this.isMarkedNullable) {
return true
}
val expandedType = fullyExpandedType(session)
val classSymbol = expandedType.toSymbol(session) as? FirRegularClassSymbol ?: return true
// In K2 enum classes are final, though enum entries are their subclasses (which is a compiler implementation detail).
if (classSymbol.isEnumClass || classSymbol.isExpect || classSymbol.modality != Modality.FINAL) {
return true
}
@@ -603,49 +607,18 @@ fun ConeKotlinType.canHaveSubtypes(session: FirSession): Boolean {
val argument = typeProjection.type!! //safe because it is not a star
when (typeParameterSymbol.variance) {
Variance.INVARIANT ->
when (typeProjection.kind) {
ProjectionKind.INVARIANT ->
if (lowerThanBound(session.typeContext, argument, typeParameterSymbol) || argument.canHaveSubtypes(session)) {
return true
}
val canHaveSubtypes = when (typeProjection.variance) {
Variance.OUT_VARIANCE -> argument.canHaveSubtypes(session)
Variance.IN_VARIANCE -> argument.lowerThanBound(typeParameterSymbol, session)
Variance.INVARIANT -> when (typeParameterSymbol.variance) {
Variance.OUT_VARIANCE -> argument.canHaveSubtypes(session)
Variance.IN_VARIANCE -> argument.lowerThanBound(typeParameterSymbol, session)
Variance.INVARIANT -> argument.canHaveSubtypes(session) || argument.lowerThanBound(typeParameterSymbol, session)
}
}
ProjectionKind.IN ->
if (lowerThanBound(session.typeContext, argument, typeParameterSymbol)) {
return true
}
ProjectionKind.OUT ->
if (argument.canHaveSubtypes(session)) {
return true
}
ProjectionKind.STAR ->
return true
}
Variance.IN_VARIANCE ->
if (typeProjection.kind != ProjectionKind.OUT) {
if (lowerThanBound(session.typeContext, argument, typeParameterSymbol)) {
return true
}
} else {
if (argument.canHaveSubtypes(session)) {
return true
}
}
Variance.OUT_VARIANCE ->
if (typeProjection.kind != ProjectionKind.IN) {
if (argument.canHaveSubtypes(session)) {
return true
}
} else {
if (lowerThanBound(session.typeContext, argument, typeParameterSymbol)) {
return true
}
}
if (canHaveSubtypes) {
return true
}
}
@@ -668,9 +641,14 @@ fun ConeClassLikeType.toClassSymbol(session: FirSession): FirClassSymbol<*>? {
return fullyExpandedType(session).toSymbol(session) as? FirClassSymbol<*>
}
private fun lowerThanBound(context: ConeInferenceContext, argument: ConeKotlinType, typeParameterSymbol: FirTypeParameterSymbol): Boolean {
/**
* The original K1 function: [org.jetbrains.kotlin.types.TypeUtils.lowerThanBound].
* This function returns `true` if `argument` suits any bound rather than the
* intersection of them all, and it expects there to be at least a single bound.
*/
private fun ConeKotlinType.lowerThanBound(typeParameterSymbol: FirTypeParameterSymbol, session: FirSession): Boolean {
typeParameterSymbol.resolvedBounds.forEach { boundTypeRef ->
if (argument != boundTypeRef.coneType && argument.isSubtypeOf(context, boundTypeRef.coneType)) {
if (this != boundTypeRef.coneType && isSubtypeOf(session.typeContext, boundTypeRef.coneType)) {
return true
}
}
@@ -5,5 +5,5 @@ class A
class B
fun main() {
A() == B()
A() === B()
<!EQUALITY_NOT_APPLICABLE!>A() === B()<!>
}
@@ -16,6 +16,9 @@ fun test (j: J, k: K) {
j == K::f
j == k::f
j === K::f
j === k::f
when (j) {
k::f -> ""
K::f -> ""
@@ -0,0 +1,8 @@
// ISSUE: KT-47884
interface A<X>
class B<T>
fun foo(a: A<*>, b: B<*>): Boolean = a == b
fun bar(a: A<*>, b: B<*>): Boolean = <!EQUALITY_NOT_APPLICABLE_WARNING!>a === b<!>
@@ -1,7 +1,8 @@
// FIR_IDENTICAL
// ISSUE: KT-47884
interface A<X>
class B<T>
fun foo(a: A<*>, b: B<*>): Boolean = a == b
fun bar(a: A<*>, b: B<*>): Boolean = a === b
@@ -5,3 +5,9 @@ fun test(x: Any, y: Any) =
fun test(x: Float, y: Double) =
<!EQUALITY_NOT_APPLICABLE!>x == y<!>
fun fest(x: Any, y: Any) =
x is Float && y is Double && <!FORBIDDEN_IDENTITY_EQUALS_WARNING!>x === y<!>
fun fest(x: Float, y: Double) =
<!FORBIDDEN_IDENTITY_EQUALS!>x === y<!>
@@ -5,3 +5,9 @@ fun test(x: Any, y: Any) =
fun test(x: Float, y: Double) =
<!EQUALITY_NOT_APPLICABLE!>x == y<!>
fun fest(x: Any, y: Any) =
x is Float && y is Double && x === y
fun fest(x: Float, y: Double) =
<!EQUALITY_NOT_APPLICABLE!>x === y<!>
@@ -16,3 +16,7 @@ enum class KotlinEnumB
fun jj(a: JavaEnumA, b: JavaEnumB) = <!INCOMPATIBLE_ENUM_COMPARISON!>a == b<!>
fun jk(a: JavaEnumA, b: KotlinEnumB) = <!INCOMPATIBLE_ENUM_COMPARISON!>a == b<!>
fun kk(a: KotlinEnumA, b: KotlinEnumB) = <!INCOMPATIBLE_ENUM_COMPARISON!>a == b<!>
fun jj2(a: JavaEnumA, b: JavaEnumB) = <!INCOMPATIBLE_ENUM_COMPARISON!>a === b<!>
fun jk2(a: JavaEnumA, b: KotlinEnumB) = <!INCOMPATIBLE_ENUM_COMPARISON!>a === b<!>
fun kk2(a: KotlinEnumA, b: KotlinEnumB) = <!INCOMPATIBLE_ENUM_COMPARISON!>a === b<!>
@@ -1,8 +1,11 @@
package
public fun jj(/*0*/ a: JavaEnumA, /*1*/ b: JavaEnumB): kotlin.Boolean
public fun jj2(/*0*/ a: JavaEnumA, /*1*/ b: JavaEnumB): kotlin.Boolean
public fun jk(/*0*/ a: JavaEnumA, /*1*/ b: KotlinEnumB): kotlin.Boolean
public fun jk2(/*0*/ a: JavaEnumA, /*1*/ b: KotlinEnumB): kotlin.Boolean
public fun kk(/*0*/ a: KotlinEnumA, /*1*/ b: KotlinEnumB): kotlin.Boolean
public fun kk2(/*0*/ a: KotlinEnumA, /*1*/ b: KotlinEnumB): kotlin.Boolean
public final enum class JavaEnumA : kotlin.Enum<JavaEnumA!> {
public constructor JavaEnumA()
@@ -26,6 +26,11 @@ fun <P> processInfo1(info: String, printer: P) where P: Buffered, P: AIPowered {
<!INCOMPATIBLE_ENUM_COMPARISON!>printer == BufferedEnum.A<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>printer == UsualEnum.C<!>
printer == CleverEnum.E
<!FORBIDDEN_IDENTITY_EQUALS!>printer === 20<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>printer === BufferedEnum.A<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>printer === UsualEnum.C<!>
printer === CleverEnum.E
}
fun <P> processInfo2(info: String, printer: P) where P: AIPowered, P: Buffered {
@@ -33,6 +38,11 @@ fun <P> processInfo2(info: String, printer: P) where P: AIPowered, P: Buffered {
<!INCOMPATIBLE_ENUM_COMPARISON!>printer == BufferedEnum.A<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>printer == UsualEnum.C<!>
printer == CleverEnum.E
<!FORBIDDEN_IDENTITY_EQUALS!>printer === 20<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>printer === BufferedEnum.A<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>printer === UsualEnum.C<!>
printer === CleverEnum.E
}
abstract class Printer {
@@ -44,6 +54,11 @@ fun <P> processInfo3(info: String, printer: P) where P: Buffered, P: Printer {
<!INCOMPATIBLE_ENUM_COMPARISON!>printer == BufferedEnum.A<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>printer == UsualEnum.C<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>printer == CleverEnum.E<!>
<!FORBIDDEN_IDENTITY_EQUALS!>printer === 20<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>printer === BufferedEnum.A<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>printer === UsualEnum.C<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>printer === CleverEnum.E<!>
}
fun test(a: Int, b: Any?) {
@@ -26,6 +26,11 @@ fun <P> processInfo1(info: String, printer: P) where P: Buffered, P: AIPowered {
printer == BufferedEnum.A
<!INCOMPATIBLE_ENUM_COMPARISON_ERROR!>printer == UsualEnum.C<!>
printer == CleverEnum.E
<!EQUALITY_NOT_APPLICABLE!>printer === 20<!>
printer === BufferedEnum.A
<!INCOMPATIBLE_ENUM_COMPARISON_ERROR!>printer === UsualEnum.C<!>
printer === CleverEnum.E
}
fun <P> processInfo2(info: String, printer: P) where P: AIPowered, P: Buffered {
@@ -33,6 +38,11 @@ fun <P> processInfo2(info: String, printer: P) where P: AIPowered, P: Buffered {
<!INCOMPATIBLE_ENUM_COMPARISON_ERROR!>printer == BufferedEnum.A<!>
<!INCOMPATIBLE_ENUM_COMPARISON_ERROR!>printer == UsualEnum.C<!>
printer == CleverEnum.E
<!EQUALITY_NOT_APPLICABLE!>printer === 20<!>
<!INCOMPATIBLE_ENUM_COMPARISON_ERROR!>printer === BufferedEnum.A<!>
<!INCOMPATIBLE_ENUM_COMPARISON_ERROR!>printer === UsualEnum.C<!>
printer === CleverEnum.E
}
abstract class Printer {
@@ -44,6 +54,11 @@ fun <P> processInfo3(info: String, printer: P) where P: Buffered, P: Printer {
<!INCOMPATIBLE_ENUM_COMPARISON_ERROR!>printer == BufferedEnum.A<!>
<!INCOMPATIBLE_ENUM_COMPARISON_ERROR!>printer == UsualEnum.C<!>
<!INCOMPATIBLE_ENUM_COMPARISON_ERROR!>printer == CleverEnum.E<!>
<!EQUALITY_NOT_APPLICABLE!>printer === 20<!>
<!INCOMPATIBLE_ENUM_COMPARISON_ERROR!>printer === BufferedEnum.A<!>
<!INCOMPATIBLE_ENUM_COMPARISON_ERROR!>printer === UsualEnum.C<!>
<!INCOMPATIBLE_ENUM_COMPARISON_ERROR!>printer === CleverEnum.E<!>
}
fun test(a: Int, b: Any?) {
@@ -0,0 +1,92 @@
// !LANGUAGE: -ProhibitComparisonOfIncompatibleEnums
interface I {
fun foo()
}
enum class E1 : I {
A {
override fun foo() {
<!INCOMPATIBLE_ENUM_COMPARISON!>this == E2.A<!>
val q = this
when (q) {
this -> {}
E1.A -> {}
E1.B -> {}
<!INCOMPATIBLE_ENUM_COMPARISON!>E2.A<!> -> {}
<!INCOMPATIBLE_ENUM_COMPARISON!>E2.B<!> -> {}
else -> {}
}
}
},
B {
override fun foo() {
}
}
}
enum class E2 : I {
A {
override fun foo() {
}
},
B {
override fun foo() {
}
}
}
fun foo1(e1: E1, e2: E2) {
<!INCOMPATIBLE_ENUM_COMPARISON!>e1 == e2<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>e1 != e2<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>e1 == E2.A<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>E1.B == e2<!>
<!INCOMPATIBLE_ENUM_COMPARISON!>E1.A == E2.B<!>
e1 == E1.A
E1.A == e1
e2 == E2.B
E2.B == e2
}
fun foo2(e1: E1, e2: E2) {
when (e1) {
E1.A -> {}
<!INCOMPATIBLE_ENUM_COMPARISON!>E2.A<!> -> {}
<!INCOMPATIBLE_ENUM_COMPARISON!>E2.B<!> -> {}
e1 -> {}
<!INCOMPATIBLE_ENUM_COMPARISON!>e2<!> -> {}
else -> {}
}
}
fun foo3(e1: Enum<E1>, e2: Enum<E2>, e: Enum<*>) {
e1 == e
e1 == e2
e1 == E1.A
e1 == E2.A
when (e1) {
e1 -> {}
e2 -> {}
e -> {}
E1.A -> {}
E2.A -> {}
else -> {}
}
when (e) {
e -> {}
e2 -> {}
E1.A -> {}
E2.A -> {}
else -> {}
}
}
@@ -1,4 +1,3 @@
// FIR_IDENTICAL
// !LANGUAGE: -ProhibitComparisonOfIncompatibleEnums
interface I {
@@ -7,4 +7,7 @@ class B : I
fun test(a: A, b: B) {
a == b
a == b as I
<!EQUALITY_NOT_APPLICABLE!>a === b<!>
a === b as I
}
@@ -7,4 +7,7 @@ class B : I
fun test(a: A, b: B) {
<!EQUALITY_NOT_APPLICABLE!>a == b<!>
a == b as I
<!EQUALITY_NOT_APPLICABLE!>a === b<!>
a === b as I
}
@@ -11,4 +11,7 @@ class A
fun main(args: Array<String>) {
(1 to A()) == A()
(1 to B()) == B()
<!EQUALITY_NOT_APPLICABLE!>(1 to A()) === A()<!>
<!EQUALITY_NOT_APPLICABLE!>(1 to B()) === B()<!>
}
@@ -11,4 +11,7 @@ class A
fun main(args: Array<String>) {
<!EQUALITY_NOT_APPLICABLE!>(1 to A()) == A()<!>
<!EQUALITY_NOT_APPLICABLE!>(1 to B()) == B()<!>
<!EQUALITY_NOT_APPLICABLE!>(1 to A()) === A()<!>
<!EQUALITY_NOT_APPLICABLE!>(1 to B()) === B()<!>
}
@@ -7,4 +7,9 @@ fun test() {
if (Triple(0, 1, 2) == Pair(Foo.A, "a")) println("Doesn't compile")
if (Triple(0, 1, 2) == Pair("a", "b")) println("Doesn't compile")
if (Triple(Foo.A, 1, 2) == Pair(Foo.A, "a")) println("Compiles, but why?")
<!EQUALITY_NOT_APPLICABLE!>Triple(Foo.A, 1, 2) === Pair("a", "b")<!>
<!EQUALITY_NOT_APPLICABLE!>Triple(0, 1, 2) === Pair(Foo.A, "a")<!>
<!EQUALITY_NOT_APPLICABLE!>Triple(0, 1, 2) === Pair("a", "b")<!>
<!EQUALITY_NOT_APPLICABLE_WARNING!>Triple(Foo.A, 1, 2) === Pair(Foo.A, "a")<!>
}
@@ -7,4 +7,9 @@ fun test() {
if (<!EQUALITY_NOT_APPLICABLE!>Triple(0, 1, 2) == Pair(Foo.A, "a")<!>) println("Doesn't compile")
if (<!EQUALITY_NOT_APPLICABLE!>Triple(0, 1, 2) == Pair("a", "b")<!>) println("Doesn't compile")
if (Triple(Foo.A, 1, 2) == Pair(Foo.A, "a")) println("Compiles, but why?")
<!EQUALITY_NOT_APPLICABLE!>Triple(Foo.A, 1, 2) === Pair("a", "b")<!>
<!EQUALITY_NOT_APPLICABLE!>Triple(0, 1, 2) === Pair(Foo.A, "a")<!>
<!EQUALITY_NOT_APPLICABLE!>Triple(0, 1, 2) === Pair("a", "b")<!>
Triple(Foo.A, 1, 2) === Pair(Foo.A, "a")
}
@@ -585,7 +585,7 @@ fun case_29(x: Boolean) {
if (false || false || false || false || y !== v) {
val t = <!DEBUG_INFO_EXPRESSION_TYPE("<anonymous>?")!><!UNSAFE_IMPLICIT_INVOKE_CALL!>y<!>()<!>
if (z !== t || false) {
if (<!EQUALITY_NOT_APPLICABLE_WARNING!>z !== t<!> || false) {
<!DEBUG_INFO_EXPRESSION_TYPE("<anonymous>?")!>t<!><!UNSAFE_CALL!>.<!>a
<!DEBUG_INFO_EXPRESSION_TYPE("<anonymous>?")!>t<!><!UNSAFE_CALL!>.<!>equals(null)
<!DEBUG_INFO_EXPRESSION_TYPE("<anonymous>?")!>t<!>.propT