From 8fe964f269f73d42886a35feea046ede2512824a Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 14 Jan 2016 02:36:57 +0300 Subject: [PATCH] Resolve array access RHS always as the last argument of the call Also do not attempt to match any of the arguments in the brackets with the last parameter of the 'set' method #KT-10633 Fixed --- .../ValueArgumentsToParametersMapper.java | 63 +++++++++++++------ .../tests/checkArguments/arrayAccessSet.kt | 35 +++++++++++ .../tests/checkArguments/arrayAccessSet.txt | 35 +++++++++++ .../arrayAccessSetTooManyArgs.kt | 10 +++ .../arrayAccessSetTooManyArgs.txt | 11 ++++ .../diagnostics/tests/regressions/kt10633.kt | 19 ++++++ .../diagnostics/tests/regressions/kt10633.txt | 6 ++ .../checkers/DiagnosticsTestGenerated.java | 18 ++++++ .../KotlinFunctionParameterInfoHandler.kt | 34 ++++++---- 9 files changed, 199 insertions(+), 32 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/checkArguments/arrayAccessSet.kt create mode 100644 compiler/testData/diagnostics/tests/checkArguments/arrayAccessSet.txt create mode 100644 compiler/testData/diagnostics/tests/checkArguments/arrayAccessSetTooManyArgs.kt create mode 100644 compiler/testData/diagnostics/tests/checkArguments/arrayAccessSetTooManyArgs.txt create mode 100644 compiler/testData/diagnostics/tests/regressions/kt10633.kt create mode 100644 compiler/testData/diagnostics/tests/regressions/kt10633.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/ValueArgumentsToParametersMapper.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/ValueArgumentsToParametersMapper.java index 47226c9f101..24b7cbfdaa0 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/ValueArgumentsToParametersMapper.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/ValueArgumentsToParametersMapper.java @@ -19,10 +19,13 @@ package org.jetbrains.kotlin.resolve.calls; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.intellij.psi.impl.source.tree.LeafPsiElement; +import kotlin.collections.CollectionsKt; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.builtins.functions.FunctionInvokeDescriptor; -import org.jetbrains.kotlin.descriptors.*; +import org.jetbrains.kotlin.descriptors.CallableDescriptor; +import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor; +import org.jetbrains.kotlin.descriptors.ValueParameterDescriptor; import org.jetbrains.kotlin.diagnostics.Diagnostic; import org.jetbrains.kotlin.name.Name; import org.jetbrains.kotlin.psi.*; @@ -32,6 +35,7 @@ import org.jetbrains.kotlin.resolve.calls.model.*; import org.jetbrains.kotlin.resolve.calls.tasks.TracingStrategy; import org.jetbrains.kotlin.resolve.descriptorUtil.DescriptorUtilsKt; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; @@ -118,15 +122,18 @@ public class ValueArgumentsToParametersMapper { // We saw only positioned arguments so far private final ProcessorState positionedOnly = new ProcessorState() { - private int currentParameter = 0; + private int numberOfParametersForPositionedArguments() { + int size = candidateCall.getCandidateDescriptor().getValueParameters().size(); + return call.getCallType() == Call.CallType.ARRAY_SET_METHOD ? size - 1 : size; + } + @Nullable public ValueParameterDescriptor nextValueParameter() { - List parameters = candidateCall.getCandidateDescriptor().getValueParameters(); - if (currentParameter >= parameters.size()) return null; + if (currentParameter >= numberOfParametersForPositionedArguments()) return null; - ValueParameterDescriptor head = parameters.get(currentParameter); + ValueParameterDescriptor head = candidateCall.getCandidateDescriptor().getValueParameters().get(currentParameter); // If we found a vararg parameter, we are stuck with it forever if (head.getVarargElementType() == null) { @@ -142,20 +149,27 @@ public class ValueArgumentsToParametersMapper { } @Override - public ProcessorState processPositionedArgument(@NotNull ValueArgument argument, int index) { - ValueParameterDescriptor valueParameterDescriptor = nextValueParameter(); + public ProcessorState processPositionedArgument(@NotNull ValueArgument argument) { + processArgument(argument, nextValueParameter()); + return positionedOnly; + } - if (valueParameterDescriptor != null) { - usedParameters.add(valueParameterDescriptor); - putVararg(valueParameterDescriptor, argument); + @Override + public ProcessorState processArraySetRHS(@NotNull ValueArgument argument) { + processArgument(argument, CollectionsKt.lastOrNull(candidateCall.getCandidateDescriptor().getValueParameters())); + return positionedOnly; + } + + private void processArgument(@NotNull ValueArgument argument, @Nullable ValueParameterDescriptor parameter) { + if (parameter != null) { + usedParameters.add(parameter); + putVararg(parameter, argument); } else { report(TOO_MANY_ARGUMENTS.on(argument.asElement(), candidateCall.getCandidateDescriptor())); unmappedArguments.add(argument); setStatus(WEAK_ERROR); } - - return positionedOnly; } }; @@ -220,27 +234,34 @@ public class ValueArgumentsToParametersMapper { } @Override - public ProcessorState processPositionedArgument( - @NotNull ValueArgument argument, int index - ) { + public ProcessorState processPositionedArgument(@NotNull ValueArgument argument) { report(MIXING_NAMED_AND_POSITIONED_ARGUMENTS.on(argument.asElement())); setStatus(WEAK_ERROR); unmappedArguments.add(argument); return positionedThenNamed; } + + @Override + public ProcessorState processArraySetRHS(@NotNull ValueArgument argument) { + throw new IllegalStateException("Array set RHS cannot appear after a named argument syntactically: " + argument); + } }; public void process() { ProcessorState state = positionedOnly; + boolean isArraySetMethod = call.getCallType() == Call.CallType.ARRAY_SET_METHOD; List argumentsInParentheses = CallUtilKt.getValueArgumentsInParentheses(call); - for (int i = 0; i < argumentsInParentheses.size(); i++) { - ValueArgument valueArgument = argumentsInParentheses.get(i); + for (Iterator iterator = argumentsInParentheses.iterator(); iterator.hasNext(); ) { + ValueArgument valueArgument = iterator.next(); if (valueArgument.isNamed()) { state = state.processNamedArgument(valueArgument); } + else if (isArraySetMethod && !iterator.hasNext()) { + state = state.processArraySetRHS(valueArgument); + } else { - state = state.processPositionedArgument(valueArgument, i); + state = state.processPositionedArgument(valueArgument); } } @@ -341,9 +362,11 @@ public class ValueArgumentsToParametersMapper { private interface ProcessorState { ProcessorState processNamedArgument(@NotNull ValueArgument argument); - ProcessorState processPositionedArgument(@NotNull ValueArgument argument, int index); - } + ProcessorState processPositionedArgument(@NotNull ValueArgument argument); + + ProcessorState processArraySetRHS(@NotNull ValueArgument argument); + } } private ValueArgumentsToParametersMapper() {} diff --git a/compiler/testData/diagnostics/tests/checkArguments/arrayAccessSet.kt b/compiler/testData/diagnostics/tests/checkArguments/arrayAccessSet.kt new file mode 100644 index 00000000000..65a40b9056e --- /dev/null +++ b/compiler/testData/diagnostics/tests/checkArguments/arrayAccessSet.kt @@ -0,0 +1,35 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER + +object A { + operator fun set(x: Int, y: String = "y", z: Double) { + } +} + +object B { + operator fun set(x: Int, y: String = "y", z: Double = 3.14, w: Char = 'w', v: Boolean) { + } +} + +object D { + operator fun set(x: Int, vararg y: String, z: Double) { + } +} + +object Z { + operator fun set() { + } +} + +fun test() { + A[0] = "" + A[0] = 2.72 + + B[0] = "" + B[0] = 2.72 + B[0] = true + + D[0] = "" + D[0] = 2.72 + + Z[0] = "" +} diff --git a/compiler/testData/diagnostics/tests/checkArguments/arrayAccessSet.txt b/compiler/testData/diagnostics/tests/checkArguments/arrayAccessSet.txt new file mode 100644 index 00000000000..8d28e058ec6 --- /dev/null +++ b/compiler/testData/diagnostics/tests/checkArguments/arrayAccessSet.txt @@ -0,0 +1,35 @@ +package + +public fun test(): kotlin.Unit + +public object A { + private constructor A() + 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 final operator fun set(/*0*/ x: kotlin.Int, /*1*/ y: kotlin.String = ..., /*2*/ z: kotlin.Double): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public object B { + private constructor B() + 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 final operator fun set(/*0*/ x: kotlin.Int, /*1*/ y: kotlin.String = ..., /*2*/ z: kotlin.Double = ..., /*3*/ w: kotlin.Char = ..., /*4*/ v: kotlin.Boolean): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public object D { + private constructor D() + 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 final operator fun set(/*0*/ x: kotlin.Int, /*1*/ vararg y: kotlin.String /*kotlin.Array*/, /*2*/ z: kotlin.Double): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public object Z { + private constructor Z() + 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 final operator fun set(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/tests/checkArguments/arrayAccessSetTooManyArgs.kt b/compiler/testData/diagnostics/tests/checkArguments/arrayAccessSetTooManyArgs.kt new file mode 100644 index 00000000000..7158ffebbcb --- /dev/null +++ b/compiler/testData/diagnostics/tests/checkArguments/arrayAccessSetTooManyArgs.kt @@ -0,0 +1,10 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER + +class A { + operator fun get(x: Int) {} + operator fun set(x: String, value: Int) {} + + fun d(x: Int) { + this["", 1] = 1 + } +} diff --git a/compiler/testData/diagnostics/tests/checkArguments/arrayAccessSetTooManyArgs.txt b/compiler/testData/diagnostics/tests/checkArguments/arrayAccessSetTooManyArgs.txt new file mode 100644 index 00000000000..4a4f29296bd --- /dev/null +++ b/compiler/testData/diagnostics/tests/checkArguments/arrayAccessSetTooManyArgs.txt @@ -0,0 +1,11 @@ +package + +public final class A { + public constructor A() + public final fun d(/*0*/ x: kotlin.Int): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final operator fun get(/*0*/ x: kotlin.Int): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final operator fun set(/*0*/ x: kotlin.String, /*1*/ value: kotlin.Int): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/tests/regressions/kt10633.kt b/compiler/testData/diagnostics/tests/regressions/kt10633.kt new file mode 100644 index 00000000000..195e78990a0 --- /dev/null +++ b/compiler/testData/diagnostics/tests/regressions/kt10633.kt @@ -0,0 +1,19 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER + +var count = 0 + +operator fun Int.get(s: Int): Int { + count++ + return this + s +} + +operator fun Int.set(s: Int, x: String = "", z: Int) { +} + +fun main(args: Array) { + 1[2] = 1 + 1.set(2, z = 1) + 1[2] += 1 + + 1.set(2, 1) +} diff --git a/compiler/testData/diagnostics/tests/regressions/kt10633.txt b/compiler/testData/diagnostics/tests/regressions/kt10633.txt new file mode 100644 index 00000000000..90ff23c94f1 --- /dev/null +++ b/compiler/testData/diagnostics/tests/regressions/kt10633.txt @@ -0,0 +1,6 @@ +package + +public var count: kotlin.Int +public fun main(/*0*/ args: kotlin.Array): kotlin.Unit +public operator fun kotlin.Int.get(/*0*/ s: kotlin.Int): kotlin.Int +public operator fun kotlin.Int.set(/*0*/ s: kotlin.Int, /*1*/ x: kotlin.String = ..., /*2*/ z: kotlin.Int): kotlin.Unit diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 933c5b595a5..3af04896214 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -2604,6 +2604,18 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/tests/checkArguments"), Pattern.compile("^(.+)\\.kt$"), true); } + @TestMetadata("arrayAccessSet.kt") + public void testArrayAccessSet() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/checkArguments/arrayAccessSet.kt"); + doTest(fileName); + } + + @TestMetadata("arrayAccessSetTooManyArgs.kt") + public void testArrayAccessSetTooManyArgs() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/checkArguments/arrayAccessSetTooManyArgs.kt"); + doTest(fileName); + } + @TestMetadata("booleanExpressions.kt") public void testBooleanExpressions() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/checkArguments/booleanExpressions.kt"); @@ -13077,6 +13089,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("kt10633.kt") + public void testKt10633() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/regressions/kt10633.kt"); + doTest(fileName); + } + @TestMetadata("kt127.kt") public void testKt127() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/regressions/kt127.kt"); diff --git a/idea/src/org/jetbrains/kotlin/idea/parameterInfo/KotlinFunctionParameterInfoHandler.kt b/idea/src/org/jetbrains/kotlin/idea/parameterInfo/KotlinFunctionParameterInfoHandler.kt index 9886e8a73ae..a22b1a4a09d 100644 --- a/idea/src/org/jetbrains/kotlin/idea/parameterInfo/KotlinFunctionParameterInfoHandler.kt +++ b/idea/src/org/jetbrains/kotlin/idea/parameterInfo/KotlinFunctionParameterInfoHandler.kt @@ -313,14 +313,19 @@ abstract class KotlinParameterInfoWithCallHandlerBase= currentArgumentIndex) + val isArraySetMethod = call.callType == Call.CallType.ARRAY_SET_METHOD - val argumentsBeforeCurrent = call.valueArguments.subList(0, currentArgumentIndex) + val arguments = call.valueArguments.let { args -> + // For array set method call, we're only interested in the arguments in brackets which are all except the last one + if (isArraySetMethod) args.dropLast(1) else args + } + + assert(arguments.size >= currentArgumentIndex) val callToUse: Call val currentArgument: ValueArgument - if (call.valueArguments.size > currentArgumentIndex) { - currentArgument = call.valueArguments[currentArgumentIndex] + if (arguments.size > currentArgumentIndex) { + currentArgument = arguments[currentArgumentIndex] callToUse = call } else { @@ -334,9 +339,12 @@ abstract class KotlinParameterInfoWithCallHandlerBase() override fun getValueArgumentList() = null } @@ -347,20 +355,22 @@ abstract class KotlinParameterInfoWithCallHandlerBase resolvedCall.getArgumentMapping(argument).isError() && !argument.hasError(bindingContext) /* ignore arguments that has error type */ } + val isGrey = argumentsBeforeCurrent.any { argument -> + resolvedCall.getArgumentMapping(argument).isError() && + !argument.hasError(bindingContext) /* ignore arguments that have error type */ + } return SignatureInfo(resultingDescriptor, ::argumentToParameter, highlightParameterIndex, isGrey) }