From 5eac949b43a76dcb68a8bf091a435c37adc7f61b Mon Sep 17 00:00:00 2001 From: Vladimir Dolzhenko Date: Fri, 2 Oct 2020 12:31:25 +0200 Subject: [PATCH] Report EXPLICIT_DELEGATION_CALL_REQUIRED on relevant element ^KT-38959 Fixed --- .../jetbrains/kotlin/diagnostics/Errors.java | 8 ++--- .../diagnostics/PositioningStrategies.kt | 32 ++++++++++++------- .../kotlin/resolve/DescriptorResolver.java | 5 ++- .../kotlin/resolve/calls/CallResolver.java | 18 +++++++---- .../kotlin/resolve/calls/CallResolverUtil.kt | 5 +++ ...egyForImplicitConstructorDelegationCall.kt | 8 ++--- .../kotlin/checkers/BaseDiagnosticsTest.kt | 12 +++++++ .../FirHighlightingTestGenerated.java | 5 +++ .../PerformanceHighlightingTestGenerated.java | 5 +++ idea/testData/highlighter/DelegatingCtor.kt | 16 ++++++++++ .../HighlightingTestGenerated.java | 5 +++ 11 files changed, 92 insertions(+), 27 deletions(-) create mode 100644 idea/testData/highlighter/DelegatingCtor.kt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index c0880cc2c7d..d584082260a 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -385,17 +385,17 @@ public interface Errors { DiagnosticFactory0 CONSTRUCTOR_IN_OBJECT = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE); DiagnosticFactory0 SUPERTYPE_INITIALIZED_WITHOUT_PRIMARY_CONSTRUCTOR = DiagnosticFactory0.create(ERROR); - DiagnosticFactory0 PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED = + DiagnosticFactory0 PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED = DiagnosticFactory0.create(ERROR, PositioningStrategies.SECONDARY_CONSTRUCTOR_DELEGATION_CALL); - DiagnosticFactory0 PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED_IN_ENUM = + DiagnosticFactory0 PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED_IN_ENUM = DiagnosticFactory0.create(WARNING, PositioningStrategies.SECONDARY_CONSTRUCTOR_DELEGATION_CALL); DiagnosticFactory0 DELEGATION_SUPER_CALL_IN_ENUM_CONSTRUCTOR = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 PRIMARY_CONSTRUCTOR_REQUIRED_FOR_DATA_CLASS = DiagnosticFactory0.create(ERROR); - DiagnosticFactory0 EXPLICIT_DELEGATION_CALL_REQUIRED = + DiagnosticFactory0 EXPLICIT_DELEGATION_CALL_REQUIRED = DiagnosticFactory0.create(ERROR, PositioningStrategies.SECONDARY_CONSTRUCTOR_DELEGATION_CALL); DiagnosticFactory1 INSTANCE_ACCESS_BEFORE_SUPER_CALL = DiagnosticFactory1.create(ERROR); @@ -1108,7 +1108,7 @@ public interface Errors { DiagnosticFactory1 NESTED_CLASS_ACCESSED_VIA_INSTANCE_REFERENCE = DiagnosticFactory1.create(ERROR); DiagnosticFactory2 NESTED_CLASS_SHOULD_BE_QUALIFIED = DiagnosticFactory2.create(ERROR); - DiagnosticFactory1 INACCESSIBLE_OUTER_CLASS_EXPRESSION = DiagnosticFactory1.create(ERROR); + DiagnosticFactory1 INACCESSIBLE_OUTER_CLASS_EXPRESSION = DiagnosticFactory1.create(ERROR, SECONDARY_CONSTRUCTOR_DELEGATION_CALL); DiagnosticFactory1 NESTED_CLASS_NOT_ALLOWED = DiagnosticFactory1.create(ERROR, DECLARATION_NAME); DiagnosticFactory1 NESTED_CLASS_DEPRECATED = DiagnosticFactory1.create(WARNING, DECLARATION_NAME); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt index bc8a5f08f3e..0e3917bb3fa 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt @@ -46,10 +46,7 @@ object PositioningStrategies { is KtObjectLiteralExpression -> { val objectDeclaration = element.objectDeclaration val objectKeyword = objectDeclaration.getObjectKeyword()!! - val delegationSpecifierList = objectDeclaration.getSuperTypeList() - if (delegationSpecifierList == null) { - return markElement(objectKeyword) - } + val delegationSpecifierList = objectDeclaration.getSuperTypeList() ?: return markElement(objectKeyword) return markRange(objectKeyword, delegationSpecifierList) } is KtObjectDeclaration -> { @@ -612,15 +609,26 @@ object PositioningStrategies { } @JvmField - val SECONDARY_CONSTRUCTOR_DELEGATION_CALL: PositioningStrategy = - object : PositioningStrategy() { - override fun mark(element: KtConstructorDelegationCall): List { - if (element.isImplicit) { - val constructor = element.getStrictParentOfType()!! - val valueParameterList = constructor.valueParameterList ?: return markElement(constructor) - return markRange(constructor.getConstructorKeyword(), valueParameterList.lastChild) + val SECONDARY_CONSTRUCTOR_DELEGATION_CALL: PositioningStrategy = + object : PositioningStrategy() { + override fun mark(element: PsiElement): List { + when (element) { + is KtSecondaryConstructor -> { + val valueParameterList = element.valueParameterList ?: return markElement(element) + return markRange(element.getConstructorKeyword(), valueParameterList.lastChild) + } + is KtConstructorDelegationCall -> { + if (element.isImplicit) { + // TODO: [VD] FIR collects for some reason implicit KtConstructorDelegationCall + // check(!element.isImplicit) { "Implicit KtConstructorDelegationCall should not be collected directly" } + val constructor = element.getStrictParentOfType()!! + val valueParameterList = constructor.valueParameterList ?: return markElement(constructor) + return markRange(constructor.getConstructorKeyword(), valueParameterList.lastChild) + } + return markElement(element.calleeExpression ?: element) + } + else -> error("unexpected element $element") } - return markElement(element.calleeExpression ?: element) } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java index 79292081a6a..c684d9fb48e 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java @@ -42,6 +42,7 @@ import org.jetbrains.kotlin.lexer.KtTokens; import org.jetbrains.kotlin.name.Name; import org.jetbrains.kotlin.psi.*; import org.jetbrains.kotlin.psi.psiUtil.PsiUtilsKt; +import org.jetbrains.kotlin.resolve.calls.callResolverUtil.CallResolverUtilKt; import org.jetbrains.kotlin.resolve.calls.components.InferenceSession; import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowInfo; import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowInfoFactory; @@ -1371,7 +1372,9 @@ public class DescriptorResolver { } if (isStaticNestedClass(classDescriptor)) { - trace.report(INACCESSIBLE_OUTER_CLASS_EXPRESSION.on(reportErrorsOn, classDescriptor)); + PsiElement onReport = (reportErrorsOn instanceof KtConstructorDelegationCall) + ? CallResolverUtilKt.reportOnElement((KtConstructorDelegationCall) reportErrorsOn) : reportErrorsOn; + trace.report(INACCESSIBLE_OUTER_CLASS_EXPRESSION.on(onReport, classDescriptor)); return false; } classDescriptor = getParentOfType(classDescriptor, ClassDescriptor.class, true); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolver.java index de3a627400d..4288246a604 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolver.java @@ -446,15 +446,15 @@ public class CallResolver { if (constructorDescriptor.getConstructedClass().getKind() == ClassKind.ENUM_CLASS && call.isImplicit()) { if (currentClassDescriptor.getUnsubstitutedPrimaryConstructor() != null) { - DiagnosticFactory0 warningOrError; + DiagnosticFactory0 warningOrError; if (languageVersionSettings.supportsFeature(LanguageFeature.RequiredPrimaryConstructorDelegationCallInEnums)) { warningOrError = PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED; // error } else { warningOrError = PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED_IN_ENUM; // warning } - - context.trace.report(warningOrError.on((KtConstructorDelegationCall) calleeExpression.getParent())); + PsiElement reportOn = calcReportOn(calleeExpression); + context.trace.report(warningOrError.on(reportOn)); } return null; } @@ -484,9 +484,8 @@ public class CallResolver { if (!isThisCall && currentClassDescriptor.getUnsubstitutedPrimaryConstructor() != null) { if (DescriptorUtils.canHaveDeclaredConstructors(currentClassDescriptor)) { // Diagnostic is meaningless when reporting on interfaces and object - context.trace.report(PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED.on( - (KtConstructorDelegationCall) calleeExpression.getParent() - )); + PsiElement reportOn = calcReportOn(calleeExpression); + context.trace.report(PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED.on(reportOn)); } if (call.isImplicit()) return OverloadResolutionResultsImpl.nameNotFound(); } @@ -520,6 +519,13 @@ public class CallResolver { return computeTasksFromCandidatesAndResolvedCall(context, candidates, tracing); } + @Nullable + private PsiElement calcReportOn(@NotNull KtConstructorDelegationReferenceExpression calleeExpression) { + PsiElement delegationCall = calleeExpression.getParent(); + return delegationCall instanceof KtConstructorDelegationCall + ? CallResolverUtilKt.reportOnElement((KtConstructorDelegationCall) delegationCall) : delegationCall; + } + @NotNull private static Pair>, BasicCallResolutionContext> prepareCandidatesAndContextForConstructorCall( @NotNull KotlinType superType, diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolverUtil.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolverUtil.kt index af424bed0f6..780f799141a 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolverUtil.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CallResolverUtil.kt @@ -15,6 +15,7 @@ import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.descriptors.impl.TypeAliasConstructorDescriptor import org.jetbrains.kotlin.lexer.KtToken import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType import org.jetbrains.kotlin.resolve.BindingTrace import org.jetbrains.kotlin.resolve.calls.CallTransformer import org.jetbrains.kotlin.resolve.calls.components.isVararg @@ -320,3 +321,7 @@ fun createResolutionCandidatesForConstructors( ResolutionCandidate.create(call, it, dispatchReceiver, receiverKind, knownSubstitutor) } } + +internal fun KtConstructorDelegationCall.reportOnElement() = if (this.isImplicit) { + this.getStrictParentOfType()!! +} else this diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/TracingStrategyForImplicitConstructorDelegationCall.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/TracingStrategyForImplicitConstructorDelegationCall.kt index 223b2c5b69c..2a28b74a773 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/TracingStrategyForImplicitConstructorDelegationCall.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tasks/TracingStrategyForImplicitConstructorDelegationCall.kt @@ -22,12 +22,11 @@ import org.jetbrains.kotlin.diagnostics.Errors.UNRESOLVED_REFERENCE import org.jetbrains.kotlin.diagnostics.Errors.UNRESOLVED_REFERENCE_WRONG_RECEIVER import org.jetbrains.kotlin.psi.Call import org.jetbrains.kotlin.psi.KtConstructorDelegationCall -import org.jetbrains.kotlin.psi.KtLambdaArgument -import org.jetbrains.kotlin.psi.KtSecondaryConstructor import org.jetbrains.kotlin.resolve.BindingContext.CALL import org.jetbrains.kotlin.resolve.BindingContext.REFERENCE_TARGET import org.jetbrains.kotlin.resolve.BindingContext.RESOLVED_CALL import org.jetbrains.kotlin.resolve.BindingTrace +import org.jetbrains.kotlin.resolve.calls.callResolverUtil.reportOnElement import org.jetbrains.kotlin.resolve.calls.context.ResolutionContext import org.jetbrains.kotlin.resolve.calls.inference.InferenceErrorData import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall @@ -83,8 +82,9 @@ class TracingStrategyForImplicitConstructorDelegationCall( } private fun reportError(trace: BindingTrace) { - if (!trace.bindingContext.diagnostics.forElement(delegationCall).any { it.factory == Errors.EXPLICIT_DELEGATION_CALL_REQUIRED }) { - trace.report(Errors.EXPLICIT_DELEGATION_CALL_REQUIRED.on(delegationCall)) + val reportOn = delegationCall.reportOnElement() + if (!trace.bindingContext.diagnostics.forElement(reportOn).any { it.factory == Errors.EXPLICIT_DELEGATION_CALL_REQUIRED }) { + trace.report(Errors.EXPLICIT_DELEGATION_CALL_REQUIRED.on(reportOn)) } } diff --git a/compiler/tests-common/tests/org/jetbrains/kotlin/checkers/BaseDiagnosticsTest.kt b/compiler/tests-common/tests/org/jetbrains/kotlin/checkers/BaseDiagnosticsTest.kt index 47d2b7e80a8..94c031c2e8f 100644 --- a/compiler/tests-common/tests/org/jetbrains/kotlin/checkers/BaseDiagnosticsTest.kt +++ b/compiler/tests-common/tests/org/jetbrains/kotlin/checkers/BaseDiagnosticsTest.kt @@ -274,6 +274,18 @@ abstract class BaseDiagnosticsTest : KotlinMultiFileTestWithJava + val diagnosticElementTextRange = diagnostic.psiElement.textRange + diagnostic.textRanges.forEach { + check(diagnosticElementTextRange.contains(it)) { + "Annotation API violation:" + + " diagnostic text range $it has to be in range of" + + " diagnostic element ${diagnostic.psiElement} '${diagnostic.psiElement.text}'" + + " (factory ${diagnostic.factory.name}): $diagnosticElementTextRange" + } + } + } + actualDiagnostics.addAll(filteredDiagnostics) val uncheckedDiagnostics = mutableListOf() diff --git a/idea/idea-fir/tests/org/jetbrains/kotlin/idea/highlighter/FirHighlightingTestGenerated.java b/idea/idea-fir/tests/org/jetbrains/kotlin/idea/highlighter/FirHighlightingTestGenerated.java index 73fe393be56..b298b2f7174 100644 --- a/idea/idea-fir/tests/org/jetbrains/kotlin/idea/highlighter/FirHighlightingTestGenerated.java +++ b/idea/idea-fir/tests/org/jetbrains/kotlin/idea/highlighter/FirHighlightingTestGenerated.java @@ -40,6 +40,11 @@ public class FirHighlightingTestGenerated extends AbstractFirHighlightingTest { runTest("idea/testData/highlighter/AutoCreatedItParameter.kt"); } + @TestMetadata("DelegatingCtor.kt") + public void testDelegatingCtor() throws Exception { + runTest("idea/testData/highlighter/DelegatingCtor.kt"); + } + @TestMetadata("Destructuring.kt") public void testDestructuring() throws Exception { runTest("idea/testData/highlighter/Destructuring.kt"); diff --git a/idea/performanceTests/test/org/jetbrains/kotlin/idea/perf/PerformanceHighlightingTestGenerated.java b/idea/performanceTests/test/org/jetbrains/kotlin/idea/perf/PerformanceHighlightingTestGenerated.java index ef90c9d8096..13a1bdccd07 100644 --- a/idea/performanceTests/test/org/jetbrains/kotlin/idea/perf/PerformanceHighlightingTestGenerated.java +++ b/idea/performanceTests/test/org/jetbrains/kotlin/idea/perf/PerformanceHighlightingTestGenerated.java @@ -38,6 +38,11 @@ public class PerformanceHighlightingTestGenerated extends AbstractPerformanceHig runTest("idea/testData/highlighter/AutoCreatedItParameter.kt"); } + @TestMetadata("DelegatingCtor.kt") + public void testDelegatingCtor() throws Exception { + runTest("idea/testData/highlighter/DelegatingCtor.kt"); + } + @TestMetadata("Destructuring.kt") public void testDestructuring() throws Exception { runTest("idea/testData/highlighter/Destructuring.kt"); diff --git a/idea/testData/highlighter/DelegatingCtor.kt b/idea/testData/highlighter/DelegatingCtor.kt new file mode 100644 index 00000000000..7f5ce9c14a7 --- /dev/null +++ b/idea/testData/highlighter/DelegatingCtor.kt @@ -0,0 +1,16 @@ +open class Foo { + constructor(i: Int) +} + +class Bar : Foo { + constructor(s: String) +} + +class F(foo: String) { + constructor() {} +} + +enum class E(val a: String) { + A; + constructor() +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/highlighter/HighlightingTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/highlighter/HighlightingTestGenerated.java index 57e2fe8be17..d5c50b12aec 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/highlighter/HighlightingTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/highlighter/HighlightingTestGenerated.java @@ -38,6 +38,11 @@ public class HighlightingTestGenerated extends AbstractHighlightingTest { runTest("idea/testData/highlighter/AutoCreatedItParameter.kt"); } + @TestMetadata("DelegatingCtor.kt") + public void testDelegatingCtor() throws Exception { + runTest("idea/testData/highlighter/DelegatingCtor.kt"); + } + @TestMetadata("Destructuring.kt") public void testDestructuring() throws Exception { runTest("idea/testData/highlighter/Destructuring.kt");