From aa63ad4a456b8e01ad61d44752680e49b7aa16f0 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Thu, 15 Nov 2018 18:07:49 +0300 Subject: [PATCH] Report a warning when comparing incompatible enums I've put `isIncompatibleEnums` to TypeIntersector because I placed all of its usages after all of the TypeIntersector::isIntersectionEmpty ones ^KT-28225 Fixed --- .../jetbrains/kotlin/diagnostics/Errors.java | 2 + .../rendering/DefaultErrorMessages.java | 2 + .../kotlin/types/TypeIntersector.java | 6 ++ .../BasicExpressionTypingVisitor.java | 4 + .../PatternMatchingTypingVisitor.kt | 4 + .../tests/enum/compareTwoDifferentEnums.kt | 6 +- .../enum/incompatibleEnumEntryClasses.kt | 90 +++++++++++++++++++ .../enum/incompatibleEnumEntryClasses.txt | 56 ++++++++++++ .../tests/enum/incompatibleEnums.kt | 58 ++++++++++++ .../tests/enum/incompatibleEnums.txt | 47 ++++++++++ .../checkers/DiagnosticsTestGenerated.java | 10 +++ .../DiagnosticsUsingJavacTestGenerated.java | 10 +++ 12 files changed, 292 insertions(+), 3 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/enum/incompatibleEnumEntryClasses.kt create mode 100644 compiler/testData/diagnostics/tests/enum/incompatibleEnumEntryClasses.txt create mode 100644 compiler/testData/diagnostics/tests/enum/incompatibleEnums.kt create mode 100644 compiler/testData/diagnostics/tests/enum/incompatibleEnums.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index 3e0b0c286be..1fd06a5416c 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -790,6 +790,8 @@ public interface Errors { DiagnosticFactory3 EQUALITY_NOT_APPLICABLE = DiagnosticFactory3.create(ERROR); + DiagnosticFactory2 INCOMPATIBLE_ENUM_COMPARISON = + DiagnosticFactory2.create(WARNING); DiagnosticFactory1 HAS_NEXT_MISSING = DiagnosticFactory1.create(ERROR); DiagnosticFactory1 HAS_NEXT_FUNCTION_AMBIGUITY = DiagnosticFactory1.create(ERROR); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java index 9ce2413946a..86527510608 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -731,6 +731,8 @@ public class DefaultErrorMessages { return nameExpression.getReferencedName(); }, RENDER_TYPE, RENDER_TYPE); + MAP.put(INCOMPATIBLE_ENUM_COMPARISON, "Comparison of incompatible enums ''{1}'' and ''{2}'' is always unsuccessful", RENDER_TYPE, RENDER_TYPE); + MAP.put(SENSELESS_COMPARISON, "Condition ''{0}'' is always ''{1}''", ELEMENT_TEXT, TO_STRING); MAP.put(SENSELESS_NULL_IN_WHEN, "Expression under 'when' is never equal to null"); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/types/TypeIntersector.java b/compiler/frontend/src/org/jetbrains/kotlin/types/TypeIntersector.java index 18616c7901d..af0743daa1b 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/types/TypeIntersector.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/types/TypeIntersector.java @@ -28,6 +28,7 @@ import org.jetbrains.kotlin.resolve.calls.inference.CallHandle; import org.jetbrains.kotlin.resolve.calls.inference.ConstraintSystem; import org.jetbrains.kotlin.resolve.calls.inference.ConstraintSystemBuilderImpl; import org.jetbrains.kotlin.types.checker.KotlinTypeChecker; +import org.jetbrains.kotlin.types.typeUtil.TypeUtilsKt; import java.util.*; @@ -40,6 +41,11 @@ public class TypeIntersector { return intersectTypes(new LinkedHashSet<>(Arrays.asList(typeA, typeB))) == null; } + public static boolean isIncompatibleEnums(@NotNull KotlinType typeA, @NotNull KotlinType typeB) { + if (!TypeUtilsKt.isEnum(typeA) || !TypeUtilsKt.isEnum(typeB)) return false; + return !typeA.getConstructor().equals(typeB.getConstructor()); + } + @Nullable public static KotlinType intersectTypes(@NotNull Collection types) { assert !types.isEmpty() : "Attempting to intersect empty collection of types, this case should be dealt with on the call site."; diff --git a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java index a6b261552a4..1aa940ff059 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java @@ -1431,6 +1431,10 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { if (TypeIntersector.isIntersectionEmpty(leftType, rightType)) { context.trace.report(EQUALITY_NOT_APPLICABLE.on(expression, expression.getOperationReference(), leftType, rightType)); } + else if (TypeIntersector.isIncompatibleEnums(leftType, rightType)) { + context.trace.report(INCOMPATIBLE_ENUM_COMPARISON.on(expression, leftType, rightType)); + } + SenselessComparisonChecker.checkSenselessComparisonWithNull( expression, left, right, context, expr -> facade.getTypeInfo(expr, context).getType(), diff --git a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/PatternMatchingTypingVisitor.kt b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/PatternMatchingTypingVisitor.kt index 0203999e4a1..ed3a5c12fe6 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/PatternMatchingTypingVisitor.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/PatternMatchingTypingVisitor.kt @@ -684,6 +684,10 @@ class PatternMatchingTypingVisitor internal constructor(facade: ExpressionTyping return } + if (TypeIntersector.isIncompatibleEnums(type, subjectType)) { + context.trace.report(INCOMPATIBLE_ENUM_COMPARISON.on(reportErrorOn, type, subjectType)) + } + // check if the pattern is essentially a 'null' expression if (KotlinBuiltIns.isNullableNothing(type) && !TypeUtils.isNullableType(subjectType)) { context.trace.report(SENSELESS_NULL_IN_WHEN.on(reportErrorOn)) diff --git a/compiler/testData/diagnostics/tests/enum/compareTwoDifferentEnums.kt b/compiler/testData/diagnostics/tests/enum/compareTwoDifferentEnums.kt index b07eaeb661f..d356d91620e 100644 --- a/compiler/testData/diagnostics/tests/enum/compareTwoDifferentEnums.kt +++ b/compiler/testData/diagnostics/tests/enum/compareTwoDifferentEnums.kt @@ -11,6 +11,6 @@ public enum JavaEnumB {} enum class KotlinEnumA enum class KotlinEnumB -fun jj(a: JavaEnumA, b: JavaEnumB) = a == b -fun jk(a: JavaEnumA, b: KotlinEnumB) = a == b -fun kk(a: KotlinEnumA, b: KotlinEnumB) = a == b +fun jj(a: JavaEnumA, b: JavaEnumB) = a == b +fun jk(a: JavaEnumA, b: KotlinEnumB) = a == b +fun kk(a: KotlinEnumA, b: KotlinEnumB) = a == b diff --git a/compiler/testData/diagnostics/tests/enum/incompatibleEnumEntryClasses.kt b/compiler/testData/diagnostics/tests/enum/incompatibleEnumEntryClasses.kt new file mode 100644 index 00000000000..b5ccf69c342 --- /dev/null +++ b/compiler/testData/diagnostics/tests/enum/incompatibleEnumEntryClasses.kt @@ -0,0 +1,90 @@ +interface I { + fun foo() +} + +enum class E1 : I { + A { + override fun foo() { + this == E2.A + + val q = this + when (q) { + this -> {} + E1.A -> {} + E1.B -> {} + E2.A -> {} + 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) { + e1 == e2 + e1 != e2 + + e1 == E2.A + E1.B == e2 + + 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 -> {} + E2.A -> {} + E2.B -> {} + e1 -> {} + e2 -> {} + else -> {} + } +} + +fun foo3(e1: Enum, e2: Enum, 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 -> {} + } +} diff --git a/compiler/testData/diagnostics/tests/enum/incompatibleEnumEntryClasses.txt b/compiler/testData/diagnostics/tests/enum/incompatibleEnumEntryClasses.txt new file mode 100644 index 00000000000..632eb8c3149 --- /dev/null +++ b/compiler/testData/diagnostics/tests/enum/incompatibleEnumEntryClasses.txt @@ -0,0 +1,56 @@ +package + +public fun foo1(/*0*/ e1: E1, /*1*/ e2: E2): kotlin.Unit +public fun foo2(/*0*/ e1: E1, /*1*/ e2: E2): kotlin.Unit +public fun foo3(/*0*/ e1: kotlin.Enum, /*1*/ e2: kotlin.Enum, /*2*/ e: kotlin.Enum<*>): kotlin.Unit + +public final enum class E1 : kotlin.Enum, I { + enum entry A + + enum entry B + + private constructor E1() + public final override /*1*/ /*fake_override*/ val name: kotlin.String + public final override /*1*/ /*fake_override*/ val ordinal: kotlin.Int + protected final override /*1*/ /*fake_override*/ fun clone(): kotlin.Any + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: E1): kotlin.Int + public final override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + protected/*protected and package*/ final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun finalize(): kotlin.Unit + public abstract override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit + public final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun getDeclaringClass(): java.lang.Class! + public final override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + public final /*synthesized*/ fun valueOf(/*0*/ value: kotlin.String): E1 + public final /*synthesized*/ fun values(): kotlin.Array +} + +public final enum class E2 : kotlin.Enum, I { + enum entry A + + enum entry B + + private constructor E2() + public final override /*1*/ /*fake_override*/ val name: kotlin.String + public final override /*1*/ /*fake_override*/ val ordinal: kotlin.Int + protected final override /*1*/ /*fake_override*/ fun clone(): kotlin.Any + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: E2): kotlin.Int + public final override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + protected/*protected and package*/ final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun finalize(): kotlin.Unit + public abstract override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit + public final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun getDeclaringClass(): java.lang.Class! + public final override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + public final /*synthesized*/ fun valueOf(/*0*/ value: kotlin.String): E2 + public final /*synthesized*/ fun values(): kotlin.Array +} + +public interface I { + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract fun foo(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/tests/enum/incompatibleEnums.kt b/compiler/testData/diagnostics/tests/enum/incompatibleEnums.kt new file mode 100644 index 00000000000..7be3faad7b1 --- /dev/null +++ b/compiler/testData/diagnostics/tests/enum/incompatibleEnums.kt @@ -0,0 +1,58 @@ +enum class E1 { + A, B +} + +enum class E2 { + A, B +} + +fun foo1(e1: E1, e2: E2) { + e1 == e2 + e1 != e2 + + e1 == E2.A + E1.B == e2 + + 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 -> {} + E2.A -> {} + E2.B -> {} + e1 -> {} + e2 -> {} + else -> {} + } +} + +fun foo3(e1: Enum, e2: Enum, 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 -> {} + } +} diff --git a/compiler/testData/diagnostics/tests/enum/incompatibleEnums.txt b/compiler/testData/diagnostics/tests/enum/incompatibleEnums.txt new file mode 100644 index 00000000000..33d652b54a3 --- /dev/null +++ b/compiler/testData/diagnostics/tests/enum/incompatibleEnums.txt @@ -0,0 +1,47 @@ +package + +public fun foo1(/*0*/ e1: E1, /*1*/ e2: E2): kotlin.Unit +public fun foo2(/*0*/ e1: E1, /*1*/ e2: E2): kotlin.Unit +public fun foo3(/*0*/ e1: kotlin.Enum, /*1*/ e2: kotlin.Enum, /*2*/ e: kotlin.Enum<*>): kotlin.Unit + +public final enum class E1 : kotlin.Enum { + enum entry A + + enum entry B + + private constructor E1() + public final override /*1*/ /*fake_override*/ val name: kotlin.String + public final override /*1*/ /*fake_override*/ val ordinal: kotlin.Int + protected final override /*1*/ /*fake_override*/ fun clone(): kotlin.Any + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: E1): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + protected/*protected and package*/ final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun finalize(): kotlin.Unit + public final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun getDeclaringClass(): java.lang.Class! + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + public final /*synthesized*/ fun valueOf(/*0*/ value: kotlin.String): E1 + public final /*synthesized*/ fun values(): kotlin.Array +} + +public final enum class E2 : kotlin.Enum { + enum entry A + + enum entry B + + private constructor E2() + public final override /*1*/ /*fake_override*/ val name: kotlin.String + public final override /*1*/ /*fake_override*/ val ordinal: kotlin.Int + protected final override /*1*/ /*fake_override*/ fun clone(): kotlin.Any + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: E2): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + protected/*protected and package*/ final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun finalize(): kotlin.Unit + public final override /*1*/ /*fake_override*/ /*isHiddenForResolutionEverywhereBesideSupercalls*/ fun getDeclaringClass(): java.lang.Class! + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + public final /*synthesized*/ fun valueOf(/*0*/ value: kotlin.String): E2 + public final /*synthesized*/ fun values(): kotlin.Array +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 007ba2de518..8e591403c78 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -6863,6 +6863,16 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { runTest("compiler/testData/diagnostics/tests/enum/importEnumFromJava.kt"); } + @TestMetadata("incompatibleEnumEntryClasses.kt") + public void testIncompatibleEnumEntryClasses() throws Exception { + runTest("compiler/testData/diagnostics/tests/enum/incompatibleEnumEntryClasses.kt"); + } + + @TestMetadata("incompatibleEnums.kt") + public void testIncompatibleEnums() throws Exception { + runTest("compiler/testData/diagnostics/tests/enum/incompatibleEnums.kt"); + } + @TestMetadata("inheritFromEnumEntry.kt") public void testInheritFromEnumEntry() throws Exception { runTest("compiler/testData/diagnostics/tests/enum/inheritFromEnumEntry.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java index 58420a36f16..48fb9a64219 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java @@ -6863,6 +6863,16 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing runTest("compiler/testData/diagnostics/tests/enum/importEnumFromJava.kt"); } + @TestMetadata("incompatibleEnumEntryClasses.kt") + public void testIncompatibleEnumEntryClasses() throws Exception { + runTest("compiler/testData/diagnostics/tests/enum/incompatibleEnumEntryClasses.kt"); + } + + @TestMetadata("incompatibleEnums.kt") + public void testIncompatibleEnums() throws Exception { + runTest("compiler/testData/diagnostics/tests/enum/incompatibleEnums.kt"); + } + @TestMetadata("inheritFromEnumEntry.kt") public void testInheritFromEnumEntry() throws Exception { runTest("compiler/testData/diagnostics/tests/enum/inheritFromEnumEntry.kt");