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 d5a0f816a30..3c1d650aa80 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java @@ -837,33 +837,43 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { return !context.dataFlowInfo.getNullability(dataFlowValue).canBeNull(); } - public static void checkLValue(@NotNull BindingTrace trace, @NotNull JetExpression expression) { - checkLValue(trace, expression, false); + /** + * @return {@code true} iff expression can be assigned to + */ + public static boolean checkLValue(@NotNull BindingTrace trace, @NotNull JetExpression expression) { + return checkLValue(trace, expression, false); } - private static void checkLValue(@NotNull BindingTrace trace, @NotNull JetExpression expressionWithParenthesis, boolean canBeThis) { + private static boolean checkLValue(@NotNull BindingTrace trace, @NotNull JetExpression expressionWithParenthesis, boolean canBeThis) { JetExpression expression = JetPsiUtil.deparenthesize(expressionWithParenthesis); if (expression instanceof JetArrayAccessExpression) { JetExpression arrayExpression = ((JetArrayAccessExpression) expression).getArrayExpression(); - if (arrayExpression != null) { - checkLValue(trace, arrayExpression, true); - } - return; + if (arrayExpression == null) return false; + + return checkLValue(trace, arrayExpression, true); } - if (canBeThis && expression instanceof JetThisExpression) return; + if (canBeThis && expression instanceof JetThisExpression) return true; VariableDescriptor variable = BindingContextUtils.extractVariableDescriptorIfAny(trace.getBindingContext(), expression, true); + boolean result = true; JetExpression reportOn = expression != null ? expression : expressionWithParenthesis; if (variable instanceof PropertyDescriptor) { PropertyDescriptor propertyDescriptor = (PropertyDescriptor) variable; if (propertyDescriptor.isSetterProjectedOut()) { trace.report(SETTER_PROJECTED_OUT.on(reportOn, propertyDescriptor)); + result = false; } } if (variable == null) { trace.report(VARIABLE_EXPECTED.on(reportOn)); + result = false; } + else if (!variable.isVar()) { + result = false; + } + + return result; } @Override diff --git a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ExpressionTypingVisitorForStatements.java b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ExpressionTypingVisitorForStatements.java index 4308d3ef8ba..59dd3345420 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ExpressionTypingVisitorForStatements.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ExpressionTypingVisitorForStatements.java @@ -36,6 +36,7 @@ import org.jetbrains.kotlin.resolve.TemporaryBindingTrace; import org.jetbrains.kotlin.resolve.calls.context.TemporaryTraceAndCache; import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall; import org.jetbrains.kotlin.resolve.calls.results.OverloadResolutionResults; +import org.jetbrains.kotlin.resolve.calls.results.OverloadResolutionResultsImpl; import org.jetbrains.kotlin.resolve.calls.results.OverloadResolutionResultsUtil; import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowInfo; import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValue; @@ -276,15 +277,24 @@ public class ExpressionTypingVisitorForStatements extends ExpressionTypingVisito JetType assignmentOperationType = OverloadResolutionResultsUtil.getResultingType(assignmentOperationDescriptors, context.contextDependency); - // Check for '+' - Name counterpartName = OperatorConventions.BINARY_OPERATION_NAMES.get(OperatorConventions.ASSIGNMENT_OPERATION_COUNTERPARTS.get(operationType)); + OverloadResolutionResults binaryOperationDescriptors; + JetType binaryOperationType; TemporaryTraceAndCache temporaryForBinaryOperation = TemporaryTraceAndCache.create( context, "trace to check binary operation like '+' for", expression); - OverloadResolutionResults binaryOperationDescriptors = components.callResolver.resolveBinaryCall( - context.replaceTraceAndCache(temporaryForBinaryOperation).replaceScope(scope), - receiver, expression, counterpartName - ); - JetType binaryOperationType = OverloadResolutionResultsUtil.getResultingType(binaryOperationDescriptors, context.contextDependency); + boolean lhsAssignable = BasicExpressionTypingVisitor.checkLValue(TemporaryBindingTrace.create(context.trace, "Trace for checking assignability"), left); + if (assignmentOperationType == null || lhsAssignable) { + // Check for '+' + Name counterpartName = OperatorConventions.BINARY_OPERATION_NAMES.get(OperatorConventions.ASSIGNMENT_OPERATION_COUNTERPARTS.get(operationType)); + binaryOperationDescriptors = components.callResolver.resolveBinaryCall( + context.replaceTraceAndCache(temporaryForBinaryOperation).replaceScope(scope), + receiver, expression, counterpartName + ); + binaryOperationType = OverloadResolutionResultsUtil.getResultingType(binaryOperationDescriptors, context.contextDependency); + } + else { + binaryOperationDescriptors = OverloadResolutionResultsImpl.nameNotFound(); + binaryOperationType = null; + } JetType type = assignmentOperationType != null ? assignmentOperationType : binaryOperationType; if (assignmentOperationDescriptors.isSuccess() && binaryOperationDescriptors.isSuccess()) { diff --git a/compiler/testData/diagnostics/tests/operatorsOverloading/AssignOperatorAmbiguity.kt b/compiler/testData/diagnostics/tests/operatorsOverloading/AssignOperatorAmbiguity.kt index e08c71ea005..e2c66b75fc3 100644 --- a/compiler/testData/diagnostics/tests/operatorsOverloading/AssignOperatorAmbiguity.kt +++ b/compiler/testData/diagnostics/tests/operatorsOverloading/AssignOperatorAmbiguity.kt @@ -8,7 +8,7 @@ class MyInt(val i: Int) { fun Any.plusAssign(a: Any) {} fun test(m: MyInt) { - m += m + m += m var i = 1 i += 34 diff --git a/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnArray.kt b/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnArray.kt new file mode 100644 index 00000000000..79434adae30 --- /dev/null +++ b/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnArray.kt @@ -0,0 +1,20 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER + +class C { + fun get(i: Int): C = this +} + +fun C.plus(a: Any): C = this +fun C.plusAssign(a: Any) {} + +class C1 { + fun get(i: Int): C = C() + fun set(i: Int, v: C) {} +} + +fun test() { + val c = C() + c[0] += "" + var c1 = C1() + c1[0] += "" +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnArray.txt b/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnArray.txt new file mode 100644 index 00000000000..91b3ecc28ac --- /dev/null +++ b/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnArray.txt @@ -0,0 +1,22 @@ +package + +internal fun test(): kotlin.Unit +internal fun C.plus(/*0*/ a: kotlin.Any): C +internal fun C.plusAssign(/*0*/ a: kotlin.Any): kotlin.Unit + +internal final class C { + public constructor C() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + internal final fun get(/*0*/ i: kotlin.Int): C + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +internal final class C1 { + public constructor C1() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + internal final fun get(/*0*/ i: kotlin.Int): C + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + internal final fun set(/*0*/ i: kotlin.Int, /*1*/ v: C): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnLocal.kt b/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnLocal.kt new file mode 100644 index 00000000000..0dd93d57495 --- /dev/null +++ b/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnLocal.kt @@ -0,0 +1,13 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER + +class C + +fun C.plus(a: Any): C = this +fun C.plusAssign(a: Any) {} + +fun test() { + val c = C() + c += "" + var c1 = C() + c1 += "" +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnLocal.txt b/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnLocal.txt new file mode 100644 index 00000000000..02927607dbe --- /dev/null +++ b/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnLocal.txt @@ -0,0 +1,12 @@ +package + +internal fun test(): kotlin.Unit +internal fun C.plus(/*0*/ a: kotlin.Any): C +internal fun C.plusAssign(/*0*/ a: kotlin.Any): kotlin.Unit + +internal final class C { + public constructor C() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + 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/operatorsOverloading/plusAssignOnProperty.kt b/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnProperty.kt new file mode 100644 index 00000000000..3e0a7ff42bb --- /dev/null +++ b/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnProperty.kt @@ -0,0 +1,19 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER + +class C { + val c: C = C() +} + +fun C.plus(a: Any): C = this +fun C.plusAssign(a: Any) {} + +class C1 { + var c: C = C() +} + +fun test() { + val c = C() + c.c += "" + var c1 = C1() + c1.c += "" +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnProperty.txt b/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnProperty.txt new file mode 100644 index 00000000000..421fc109e25 --- /dev/null +++ b/compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnProperty.txt @@ -0,0 +1,21 @@ +package + +internal fun test(): kotlin.Unit +internal fun C.plus(/*0*/ a: kotlin.Any): C +internal fun C.plusAssign(/*0*/ a: kotlin.Any): kotlin.Unit + +internal final class C { + public constructor C() + internal final val c: C + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +internal final class C1 { + public constructor C1() + internal final var c: C + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java index af471419a1f..3d0b438281a 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java @@ -8110,6 +8110,24 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/operatorsOverloading/kt3450.kt"); doTest(fileName); } + + @TestMetadata("plusAssignOnArray.kt") + public void testPlusAssignOnArray() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnArray.kt"); + doTest(fileName); + } + + @TestMetadata("plusAssignOnLocal.kt") + public void testPlusAssignOnLocal() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnLocal.kt"); + doTest(fileName); + } + + @TestMetadata("plusAssignOnProperty.kt") + public void testPlusAssignOnProperty() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/operatorsOverloading/plusAssignOnProperty.kt"); + doTest(fileName); + } } @TestMetadata("compiler/testData/diagnostics/tests/overload") diff --git a/spec-docs/operator-conventions.md b/spec-docs/operator-conventions.md index 8d9730b0468..28aefbc5769 100644 --- a/spec-docs/operator-conventions.md +++ b/spec-docs/operator-conventions.md @@ -135,9 +135,16 @@ Parentheses are translated to calls to invoke with appropriate number of argumen For the assignment operations, e.g. `a += b`, the compiler performs the following steps: * If the function from the right column is available - * If the corresponding binary function (i.e. `plus()` for `plusAssign()`) is available too, report error (ambiguity). + * If the left-hand side can be assigned to and the corresponding binary function (i.e. `plus()` for `plusAssign()`) is available, report error (ambiguity). * Make sure its return type is `Unit`, and report an error otherwise. * Generate code for `a.plusAssign(b)` * Otherwise, try to generate code for `a = a + b` (this includes a type check: the type of `a + b` must be a subtype of `a`). -*Note*: assignments are *NOT* expressions in Kotlin. \ No newline at end of file +*Note*: assignments are *NOT* expressions in Kotlin. + +**Discussion of the ambiguity rule**: +We raise an error when both `plus()` and `plusAssign()` are available only if the lhs is assignable. Otherwise, the availability of `plus()` +is irrelevant, because we know that `a = a + b` can not compile. An important concern here is what happens when the lhs *becomes assignable* +after the fact (e.g. the user changes *val* to *var* or provides a `set()` function for indexing convention): in this case, the previously +correct call site may become incorrect, but not the other way around, which is safe, because former calls to `plusAssign()` can not be silently +turned into calls to `plus()`. \ No newline at end of file