Allow expect/actual annotation constructors to have default values

When a parameter has a default argument value both in the expected annotation and in the actual annotation, they must be equal. This check has been only implemented for the case when actual annotation is Kotlin source code, and NOT a Java class coming from an actual typealias. The latter case would require a bit more work in passing a platform-specific annotation-value-reading component to ExpectedActualDeclarationChecker, and is therefore postponed. For now, Java annotations that are visible through actual type aliases cannot have default argument values for parameters which already have default values in the expected annotation declaration #KT-22703 Fixed #KT-22704 Open
2018-01-31 17:29:52 +01:00
parent 71fe8c02a3
commit 9e500831dd
24 changed files with 790 additions and 48 deletions
@@ -578,6 +578,7 @@ public interface Errors {
    DiagnosticFactory0<KtTypeAlias> ACTUAL_TYPE_ALIAS_WITH_USE_SITE_VARIANCE = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE);
    DiagnosticFactory0<KtTypeAlias> ACTUAL_TYPE_ALIAS_WITH_COMPLEX_SUBSTITUTION = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE);
    DiagnosticFactory0<PsiElement> ACTUAL_FUNCTION_WITH_DEFAULT_ARGUMENTS = DiagnosticFactory0.create(ERROR);
+    DiagnosticFactory1<PsiElement, ValueParameterDescriptor> ACTUAL_ANNOTATION_CONFLICTING_DEFAULT_ARGUMENT_VALUE = DiagnosticFactory1.create(ERROR);

    DiagnosticFactory0<PsiElement> EXPECTED_FUNCTION_SOURCE_WITH_DEFAULT_ARGUMENTS_NOT_FOUND = DiagnosticFactory0.create(ERROR);

@@ -253,6 +253,7 @@ public class DefaultErrorMessages {
        MAP.put(ACTUAL_TYPE_ALIAS_WITH_USE_SITE_VARIANCE, "Right-hand side of actual type alias cannot contain use-site variance or star projections");
        MAP.put(ACTUAL_TYPE_ALIAS_WITH_COMPLEX_SUBSTITUTION, "Type arguments in the right-hand side of actual type alias should be its type parameters in the same order, e.g. 'actual typealias Foo<A, B> = Bar<A, B>'");
        MAP.put(ACTUAL_FUNCTION_WITH_DEFAULT_ARGUMENTS, "Actual function cannot have default argument values, they should be declared in the expected function");
+        MAP.put(ACTUAL_ANNOTATION_CONFLICTING_DEFAULT_ARGUMENT_VALUE, "Parameter ''{0}'' has conflicting values in the expected and actual annotation", NAME);

        MAP.put(EXPECTED_FUNCTION_SOURCE_WITH_DEFAULT_ARGUMENTS_NOT_FOUND,
                "Expected function source is not found, therefore it's impossible to generate default argument values declared there. " +
@@ -38,6 +38,7 @@ import org.jetbrains.kotlin.resolve.calls.results.TypeSpecificityComparator
 import org.jetbrains.kotlin.resolve.checkers.PlatformDiagnosticSuppressor
 import org.jetbrains.kotlin.resolve.descriptorUtil.builtIns
 import org.jetbrains.kotlin.resolve.descriptorUtil.declaresOrInheritsDefaultValue
+import org.jetbrains.kotlin.resolve.descriptorUtil.isAnnotationConstructor
 import org.jetbrains.kotlin.resolve.descriptorUtil.isEffectivelyExternal
 import org.jetbrains.kotlin.resolve.source.KotlinSourceElement
 import org.jetbrains.kotlin.types.*
@@ -786,11 +787,17 @@ class DeclarationsChecker(
    }

    private fun checkActualFunction(element: KtDeclaration, functionDescriptor: FunctionDescriptor) {
-        for (valueParameter in functionDescriptor.valueParameters) {
-            if (valueParameter.declaresDefaultValue()) {
-                trace.report(
-                    ACTUAL_FUNCTION_WITH_DEFAULT_ARGUMENTS.on(DescriptorToSourceUtils.descriptorToDeclaration(valueParameter) ?: element)
-                )
+        // Actual annotation constructors can have default argument values; their consistency with arguments in the expected annotation
+        // is checked in ExpectedActualDeclarationChecker.checkAnnotationConstructors
+        if (!functionDescriptor.isAnnotationConstructor()) {
+            for (valueParameter in functionDescriptor.valueParameters) {
+                if (valueParameter.declaresDefaultValue()) {
+                    trace.report(
+                        ACTUAL_FUNCTION_WITH_DEFAULT_ARGUMENTS.on(
+                            DescriptorToSourceUtils.descriptorToDeclaration(valueParameter) ?: element
+                        )
+                    )
+                }
            }
        }
    }
@@ -348,7 +348,10 @@ class FunctionDescriptorResolver(
            declarationToTrace.toSourceElement()
        )
        constructorDescriptor.isExpect = classDescriptor.isExpect
-        constructorDescriptor.isActual = modifierList?.hasActualModifier() == true
+        constructorDescriptor.isActual =
+                modifierList?.hasActualModifier() == true ||
+                // We don't require 'actual' for constructors of actual annotations
+                classDescriptor.kind == ClassKind.ANNOTATION_CLASS && classDescriptor.isActual
        if (declarationToTrace is PsiElement)
            trace.record(BindingContext.CONSTRUCTOR, declarationToTrace, constructorDescriptor)
        val parameterScope = LexicalWritableScope(
@@ -17,16 +17,16 @@
 package org.jetbrains.kotlin.resolve.checkers

 import com.intellij.openapi.vfs.VfsUtilCore
+import com.intellij.psi.PsiElement
 import org.jetbrains.kotlin.config.AnalysisFlag
 import org.jetbrains.kotlin.config.LanguageFeature
 import org.jetbrains.kotlin.descriptors.*
-import org.jetbrains.kotlin.diagnostics.DiagnosticSink
 import org.jetbrains.kotlin.diagnostics.Errors
 import org.jetbrains.kotlin.incremental.components.ExpectActualTracker
-import org.jetbrains.kotlin.psi.KtConstructor
-import org.jetbrains.kotlin.psi.KtDeclaration
-import org.jetbrains.kotlin.psi.KtNamedDeclaration
+import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.hasActualModifier
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.BindingTrace
 import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils
 import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.resolve.descriptorUtil.isAnnotationConstructor
@@ -57,7 +57,7 @@ object ExpectedActualDeclarationChecker : DeclarationChecker {
    fun checkExpectedDeclarationHasActual(
        reportOn: KtNamedDeclaration,
        descriptor: MemberDescriptor,
-        diagnosticHolder: DiagnosticSink,
+        trace: BindingTrace,
        platformModule: ModuleDescriptor,
        expectActualTracker: ExpectActualTracker
    ) {
@@ -77,7 +77,7 @@ object ExpectedActualDeclarationChecker : DeclarationChecker {
            assert(compatibility.keys.all { it is Incompatible })
            @Suppress("UNCHECKED_CAST")
            val incompatibility = compatibility as Map<Incompatible, Collection<MemberDescriptor>>
-            diagnosticHolder.report(Errors.NO_ACTUAL_FOR_EXPECT.on(reportOn, descriptor, platformModule, incompatibility))
+            trace.report(Errors.NO_ACTUAL_FOR_EXPECT.on(reportOn, descriptor, platformModule, incompatibility))
        } else {
            val actualMembers = compatibility.asSequence()
                .filter { (compatibility, _) ->
@@ -108,7 +108,7 @@ object ExpectedActualDeclarationChecker : DeclarationChecker {
        this.keys.all { it is Incompatible && it.kind == Compatibility.IncompatibilityKind.STRONG }

    private fun checkActualDeclarationHasExpected(
-        reportOn: KtNamedDeclaration, descriptor: MemberDescriptor, diagnosticHolder: DiagnosticSink, checkActual: Boolean
+        reportOn: KtNamedDeclaration, descriptor: MemberDescriptor, trace: BindingTrace, checkActual: Boolean
    ) {
        // Using the platform module instead of the common module is sort of fine here because the former always depends on the latter.
        // However, it would be clearer to find the common module this platform module implements and look for expected there instead.
@@ -123,7 +123,7 @@ object ExpectedActualDeclarationChecker : DeclarationChecker {
                // we suppress error, because annotation classes can only have one constructor and it's a 100% boilerplate
                // to require every annotation constructor with additional parameters with default values be marked with the `actual` modifier
                if (checkActual && !descriptor.isAnnotationConstructor()) {
-                    diagnosticHolder.report(Errors.ACTUAL_MISSING.on(reportOn))
+                    trace.report(Errors.ACTUAL_MISSING.on(reportOn))
                }

                return
@@ -162,7 +162,7 @@ object ExpectedActualDeclarationChecker : DeclarationChecker {
                val classDescriptor =
                    (descriptor as? TypeAliasDescriptor)?.expandedType?.constructor?.declarationDescriptor as? ClassDescriptor
                            ?: (descriptor as ClassDescriptor)
-                diagnosticHolder.report(
+                trace.report(
                    Errors.NO_ACTUAL_CLASS_MEMBER_FOR_EXPECTED_CLASS.on(
                        reportOn, classDescriptor, nonTrivialUnfulfilled
                    )
@@ -172,7 +172,18 @@ object ExpectedActualDeclarationChecker : DeclarationChecker {
            assert(compatibility.keys.all { it is Incompatible })
            @Suppress("UNCHECKED_CAST")
            val incompatibility = compatibility as Map<Incompatible, Collection<MemberDescriptor>>
-            diagnosticHolder.report(Errors.ACTUAL_WITHOUT_EXPECT.on(reportOn, descriptor, incompatibility))
+            trace.report(Errors.ACTUAL_WITHOUT_EXPECT.on(reportOn, descriptor, incompatibility))
+        } else {
+            val expected = compatibility[Compatible]!!.first()
+            if (expected is ClassDescriptor && expected.kind == ClassKind.ANNOTATION_CLASS) {
+                val actualConstructor =
+                    (descriptor as? ClassDescriptor)?.constructors?.singleOrNull() ?:
+                    (descriptor as? TypeAliasDescriptor)?.constructors?.singleOrNull()?.underlyingConstructorDescriptor
+                val expectedConstructor = expected.constructors.singleOrNull()
+                if (expectedConstructor != null && actualConstructor != null) {
+                    checkAnnotationConstructors(expectedConstructor, actualConstructor, trace, reportOn)
+                }
+            }
        }
    }

@@ -184,4 +195,29 @@ object ExpectedActualDeclarationChecker : DeclarationChecker {
            is CallableMemberDescriptor -> kind == CallableMemberDescriptor.Kind.DECLARATION
            else -> true
        }
+
+    private fun checkAnnotationConstructors(
+        expected: ConstructorDescriptor, actual: ConstructorDescriptor, trace: BindingTrace, reportOn: PsiElement
+    ) {
+        for (expectedParameterDescriptor in expected.valueParameters) {
+            // Actual parameter with the same name is guaranteed to exist because this method is only called for compatible annotations
+            val actualParameterDescriptor = actual.valueParameters.first { it.name == expectedParameterDescriptor.name }
+
+            if (expectedParameterDescriptor.declaresDefaultValue() && actualParameterDescriptor.declaresDefaultValue()) {
+                val expectedParameter =
+                    DescriptorToSourceUtils.descriptorToDeclaration(expectedParameterDescriptor) as? KtParameter ?: continue
+                val actualParameter = DescriptorToSourceUtils.descriptorToDeclaration(actualParameterDescriptor)
+
+                val expectedValue = trace.bindingContext.get(BindingContext.COMPILE_TIME_VALUE, expectedParameter.defaultValue)
+                // TODO: support arguments coming from Java via typealias, see PsiAnnotationMethod.getDefaultValue()
+                val actualValue = (actualParameter as? KtParameter)?.let { parameter ->
+                    trace.bindingContext.get(BindingContext.COMPILE_TIME_VALUE, parameter.defaultValue)
+                }
+                if (expectedValue != actualValue) {
+                    val target = (actualParameter as? KtParameter)?.defaultValue ?: (reportOn as? KtTypeAlias)?.nameIdentifier ?: reportOn
+                    trace.report(Errors.ACTUAL_ANNOTATION_CONFLICTING_DEFAULT_ARGUMENT_VALUE.on(target, actualParameterDescriptor))
+                }
+            }
+        }
+    }
 }