From 060af85a933175f61d9844694e56c7b117706d53 Mon Sep 17 00:00:00 2001 From: Victor Petukhov Date: Tue, 10 Aug 2021 10:52:16 +0300 Subject: [PATCH] Extract checks on equality expressions into a separate checker ^KT-43493 Fixed --- .../resolve/PlatformConfiguratorBase.kt | 1 + .../calls/checkers/EqualityCallChecker.kt | 100 ++++++++++++++++++ .../kotlin/types/enumCompatibilityChecker.kt | 4 +- .../BasicExpressionTypingVisitor.java | 77 +++----------- .../equalityChecksWithIlt.fir.kt | 19 ++++ .../builderInference/equalityChecksWithIlt.kt | 19 ++++ .../equalityChecksWithIlt.txt | 4 + 7 files changed, 160 insertions(+), 64 deletions(-) create mode 100644 compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/EqualityCallChecker.kt create mode 100644 compiler/testData/diagnostics/tests/inference/builderInference/equalityChecksWithIlt.fir.kt create mode 100644 compiler/testData/diagnostics/tests/inference/builderInference/equalityChecksWithIlt.kt create mode 100644 compiler/testData/diagnostics/tests/inference/builderInference/equalityChecksWithIlt.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/PlatformConfiguratorBase.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/PlatformConfiguratorBase.kt index 13578df07b0..3249a9b419e 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/PlatformConfiguratorBase.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/PlatformConfiguratorBase.kt @@ -67,6 +67,7 @@ private val DEFAULT_CALL_CHECKERS = listOf( ReferencingToUnderscoreNamedParameterOfCatchBlockChecker, VarargWrongExecutionOrderChecker, SelfCallInNestedObjectConstructorChecker, NewSchemeOfIntegerOperatorResolutionChecker, EnumEntryVsCompanionPriorityCallChecker, CompanionInParenthesesLHSCallChecker, ResolutionToPrivateConstructorOfSealedClassChecker, + EqualityCallChecker, ) private val DEFAULT_TYPE_CHECKERS = emptyList() private val DEFAULT_CLASSIFIER_USAGE_CHECKERS = listOf( diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/EqualityCallChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/EqualityCallChecker.kt new file mode 100644 index 00000000000..8dc125b1000 --- /dev/null +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/EqualityCallChecker.kt @@ -0,0 +1,100 @@ +/* + * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.resolve.calls.checkers + +import com.intellij.psi.PsiElement +import org.jetbrains.kotlin.builtins.KotlinBuiltIns +import org.jetbrains.kotlin.diagnostics.Errors +import org.jetbrains.kotlin.psi.KtBinaryExpression +import org.jetbrains.kotlin.resolve.calls.context.ResolutionContext +import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall +import org.jetbrains.kotlin.resolve.isInlineClassType +import org.jetbrains.kotlin.types.KotlinType +import org.jetbrains.kotlin.types.TypeIntersector +import org.jetbrains.kotlin.types.checkEnumsForCompatibility +import org.jetbrains.kotlin.types.checker.KotlinTypeChecker +import org.jetbrains.kotlin.types.expressions.OperatorConventions +import org.jetbrains.kotlin.types.expressions.SenselessComparisonChecker.checkSenselessComparisonWithNull +import org.jetbrains.kotlin.types.typeUtil.builtIns +import org.jetbrains.kotlin.util.OperatorNameConventions + +object EqualityCallChecker : CallChecker { + override fun check(resolvedCall: ResolvedCall<*>, reportOn: PsiElement, context: CallCheckerContext) { + val callExpression = resolvedCall.call.callElement as? KtBinaryExpression ?: return + val operationType = callExpression.operationReference.getReferencedNameElementType() + + if (operationType in OperatorConventions.EQUALS_OPERATIONS) { + checkEquality(resolvedCall, callExpression, context.resolutionContext) + } + } + + private fun checkEquality(resolvedCall: ResolvedCall<*>, expression: KtBinaryExpression, context: ResolutionContext<*>) { + val returnType = resolvedCall.resultingDescriptor.returnType ?: return + val builtIns = returnType.builtIns + + if (!builtIns.isBooleanOrSubtype(returnType)) { + context.trace.report( + Errors.RESULT_TYPE_MISMATCH.on( + expression.operationReference, + "'${OperatorNameConventions.EQUALS}'", + builtIns.booleanType, + returnType + ) + ) + return + } + + ensureNonemptyIntersectionOfOperandTypes(expression, context) + } + + // We check identity equality separately because we don't have a call for it + fun checkIdentityEquality(expression: KtBinaryExpression, context: ResolutionContext<*>) { + ensureNonemptyIntersectionOfOperandTypes(expression, context) + checkIdentityOnPrimitiveOrInlineClassTypes(expression, context) + } + + private fun checkIdentityOnPrimitiveOrInlineClassTypes(expression: KtBinaryExpression, context: ResolutionContext<*>) { + val left = expression.left ?: return + val right = expression.right ?: return + + val leftType = context.trace.getType(left) ?: return + val rightType = context.trace.getType(right) ?: return + + if (KotlinTypeChecker.DEFAULT.equalTypes(leftType, rightType)) { + if (KotlinBuiltIns.isPrimitiveType(leftType)) { + context.trace.report(Errors.DEPRECATED_IDENTITY_EQUALS.on(expression, leftType, rightType)) + } + } else if (isIdentityComparedWithImplicitBoxing(leftType, rightType) || isIdentityComparedWithImplicitBoxing(rightType, leftType)) { + context.trace.report(Errors.IMPLICIT_BOXING_IN_IDENTITY_EQUALS.on(expression, leftType, rightType)) + } + if (leftType.isInlineClassType() || rightType.isInlineClassType()) { + context.trace.report(Errors.FORBIDDEN_IDENTITY_EQUALS.on(expression, leftType, rightType)) + } + } + + private fun isIdentityComparedWithImplicitBoxing(leftType: KotlinType, rightType: KotlinType) = + KotlinBuiltIns.isPrimitiveType(leftType) + && !KotlinBuiltIns.isPrimitiveType(rightType) + && KotlinTypeChecker.DEFAULT.isSubtypeOf(leftType, rightType) + + private fun ensureNonemptyIntersectionOfOperandTypes(expression: KtBinaryExpression, context: ResolutionContext<*>) { + val left = expression.left ?: return + val right = expression.right ?: return + + val leftType = context.trace.getType(left) ?: return + val rightType = context.trace.getType(right) ?: return + + if (TypeIntersector.isIntersectionEmpty(leftType, rightType)) { + context.trace.report(Errors.EQUALITY_NOT_APPLICABLE.on(expression, expression.operationReference, leftType, rightType)) + } else { + checkEnumsForCompatibility(context, expression, leftType, rightType) + } + + checkSenselessComparisonWithNull( + expression, left, right, context, context.trace::getType, context.dataFlowInfo::getStableNullability + ) + } +} \ No newline at end of file diff --git a/compiler/frontend/src/org/jetbrains/kotlin/types/enumCompatibilityChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/types/enumCompatibilityChecker.kt index 384a95c5ae9..d04751e529f 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/types/enumCompatibilityChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/types/enumCompatibilityChecker.kt @@ -11,10 +11,10 @@ import org.jetbrains.kotlin.descriptors.TypeParameterDescriptor import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.psi.KtElement import org.jetbrains.kotlin.resolve.DescriptorUtils -import org.jetbrains.kotlin.types.expressions.ExpressionTypingContext +import org.jetbrains.kotlin.resolve.calls.context.ResolutionContext import org.jetbrains.kotlin.types.typeUtil.* -fun checkEnumsForCompatibility(context: ExpressionTypingContext, reportOn: KtElement, typeA: KotlinType, typeB: KotlinType) { +fun checkEnumsForCompatibility(context: ResolutionContext<*>, reportOn: KtElement, typeA: KotlinType, typeB: KotlinType) { if (isIncompatibleEnums(typeA, typeB)) { val diagnostic = if (context.languageVersionSettings.supportsFeature(LanguageFeature.ProhibitComparisonOfIncompatibleEnums)) { Errors.INCOMPATIBLE_ENUM_COMPARISON_ERROR 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 526f2965155..1f7d53b54de 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java @@ -1096,10 +1096,7 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { result = visitEquality(expression, context, operationSign, left, right); } else if (OperatorConventions.IDENTITY_EQUALS_OPERATIONS.contains(operationType)) { - ensureNonemptyIntersectionOfOperandTypes(expression, context); - // TODO : Check comparison pointlessness - result = TypeInfoFactoryKt.createTypeInfo(components.builtIns.getBooleanType(), context); - checkIdentityOnPrimitiveOrInlineClassTypes(expression, context); + result = visitIdentityEquality(expression, context); } else if (OperatorConventions.IN_OPERATIONS.contains(operationType)) { ValueArgument leftArgument = CallMaker.makeValueArgument(left, left != null ? left : operationSign); @@ -1121,33 +1118,24 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { return components.dataFlowAnalyzer.checkType(result, expression, contextWithExpectedType); } - private static void checkIdentityOnPrimitiveOrInlineClassTypes( + private KotlinTypeInfo visitIdentityEquality( @NotNull KtBinaryExpression expression, @NotNull ExpressionTypingContext context ) { - if (expression.getLeft() == null || expression.getRight() == null) return; + KotlinTypeInfo typeInfo = TypeInfoFactoryKt.createTypeInfo(components.builtIns.getBooleanType(), context); - KotlinType leftType = context.trace.getType(expression.getLeft()); - KotlinType rightType = context.trace.getType(expression.getRight()); - if (leftType == null || rightType == null) return; + KtExpression left = expression.getLeft(); + KtExpression right = expression.getRight(); - if (KotlinTypeChecker.DEFAULT.equalTypes(leftType, rightType)) { - if (KotlinBuiltIns.isPrimitiveType(leftType)) { - context.trace.report(DEPRECATED_IDENTITY_EQUALS.on(expression, leftType, rightType)); - } - } - else if (isIdentityComparedWithImplicitBoxing(leftType, rightType) || isIdentityComparedWithImplicitBoxing(rightType, leftType)) { - context.trace.report(IMPLICIT_BOXING_IN_IDENTITY_EQUALS.on(expression, leftType, rightType)); - } - if (InlineClassesUtilsKt.isInlineClassType(leftType) || InlineClassesUtilsKt.isInlineClassType(rightType)) { - context.trace.report(FORBIDDEN_IDENTITY_EQUALS.on(expression, leftType, rightType)); - } - } + if (left == null || right == null) return typeInfo; - private static boolean isIdentityComparedWithImplicitBoxing(KotlinType leftType, KotlinType rightType) { - return KotlinBuiltIns.isPrimitiveType(leftType) && - !KotlinBuiltIns.isPrimitiveType(rightType) && - KotlinTypeChecker.DEFAULT.isSubtypeOf(leftType, rightType); + // compute type of LHS and RHS + facade.getTypeInfo(left, context); + facade.getTypeInfo(right, context); + + EqualityCallChecker.INSTANCE.checkIdentityEquality(expression, context); + + return typeInfo; } private KotlinTypeInfo visitEquality( @@ -1185,17 +1173,10 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { OverloadResolutionResults resolutionResults = components.callResolver.resolveEqualsCallWithGivenDescriptors(contextWithDataFlow, operationSign, receiver, call, equalsFunctions); - if (resolutionResults.isSuccess()) { - FunctionDescriptor equals = resolutionResults.getResultingCall().getResultingDescriptor(); - if (ensureBooleanResult(operationSign, OperatorNameConventions.EQUALS, equals.getReturnType(), contextWithDataFlow)) { - ensureNonemptyIntersectionOfOperandTypes(expression, contextWithDataFlow); - } - } - else { + if (!resolutionResults.isSuccess()) { if (resolutionResults.isAmbiguity()) { context.trace.report(OVERLOAD_RESOLUTION_AMBIGUITY.on(operationSign, resolutionResults.getResultingCalls())); - } - else { + } else { context.trace.report(EQUALS_MISSING.on(operationSign)); } } @@ -1463,34 +1444,6 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { return true; } - private void ensureNonemptyIntersectionOfOperandTypes(KtBinaryExpression expression, ExpressionTypingContext context) { - KtExpression left = expression.getLeft(); - if (left == null) return; - - KtExpression right = expression.getRight(); - - // TODO : duplicated effort for == and != - KotlinType leftType = facade.getTypeInfo(left, context).getType(); - if (leftType != null && right != null) { - KotlinType rightType = facade.getTypeInfo(right, context).getType(); - - if (rightType != null) { - if (TypeIntersector.isIntersectionEmpty(leftType, rightType)) { - context.trace.report(EQUALITY_NOT_APPLICABLE.on(expression, expression.getOperationReference(), leftType, rightType)); - } else { - EnumCompatibilityCheckerKt.checkEnumsForCompatibility(context, expression, leftType, rightType); - } - - - SenselessComparisonChecker.checkSenselessComparisonWithNull( - expression, left, right, context, - expr -> facade.getTypeInfo(expr, context).getType(), - context.dataFlowInfo::getStableNullability - ); - } - } - } - @NotNull private KotlinTypeInfo visitAssignmentOperation(KtBinaryExpression expression, ExpressionTypingContext context) { return assignmentIsNotAnExpressionError(expression, context); diff --git a/compiler/testData/diagnostics/tests/inference/builderInference/equalityChecksWithIlt.fir.kt b/compiler/testData/diagnostics/tests/inference/builderInference/equalityChecksWithIlt.fir.kt new file mode 100644 index 00000000000..d828d96a7c8 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/builderInference/equalityChecksWithIlt.fir.kt @@ -0,0 +1,19 @@ +// WITH_RUNTIME + +fun main(x: Long, y: Int) { + sequence { + 1L == 3 + x == 3 + 3 == 1L + 3 == x + y == x + + 1L === 3 + x === 3 + 3 === 1L + 3 === x + y === x + + yield("") + } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/inference/builderInference/equalityChecksWithIlt.kt b/compiler/testData/diagnostics/tests/inference/builderInference/equalityChecksWithIlt.kt new file mode 100644 index 00000000000..2ddeb80a888 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/builderInference/equalityChecksWithIlt.kt @@ -0,0 +1,19 @@ +// WITH_RUNTIME + +fun main(x: Long, y: Int) { + sequence { + 1L == 3 + x == 3 + 3 == 1L + 3 == x + y == x + + 1L === 3 + x === 3 + 3 === 1L + 3 === x + y === x + + yield("") + } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/inference/builderInference/equalityChecksWithIlt.txt b/compiler/testData/diagnostics/tests/inference/builderInference/equalityChecksWithIlt.txt new file mode 100644 index 00000000000..b003dff7eed --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/builderInference/equalityChecksWithIlt.txt @@ -0,0 +1,4 @@ +package + +public fun main(/*0*/ x: kotlin.Long, /*1*/ y: kotlin.Int): kotlin.Unit +