Support preference of TYPE_USE annotations to enhance over others like METHOD, FIELD and VALUE_PARAMETER to avoid double applying them in case of arrays: @NotNull Integer [] (both to the array element and to the entire array)

^KT-24392 Fixed
2020-12-08 18:32:33 +03:00
parent 69f31afecc
commit 857cc92326
8 changed files with 83 additions and 47 deletions
@@ -37,6 +37,7 @@ import org.jetbrains.kotlin.ir.util.defaultType
 import org.jetbrains.kotlin.ir.util.getAnnotation
 import org.jetbrains.kotlin.ir.util.hasAnnotation
 import org.jetbrains.kotlin.ir.util.isAnnotationClass
+import org.jetbrains.kotlin.load.java.JvmAnnotationNames
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.utils.addToStdlib.safeAs
@@ -191,7 +192,7 @@ private class AdditionalClassAnnotationLowering(private val context: JvmBackendC
        }

    private fun generateTargetAnnotation(irClass: IrClass) {
-        if (irClass.hasAnnotation(FqName("java.lang.annotation.Target"))) return
+        if (irClass.hasAnnotation(JvmAnnotationNames.TARGET_ANNOTATION)) return
        val annotationTargetMap = annotationTargetMaps[jvmTarget]
            ?: throw AssertionError("No annotation target map for JVM target $jvmTarget")

@@ -5,6 +5,11 @@

 package org.jetbrains.kotlin.load.java

-enum class AnnotationQualifierApplicabilityType {
-    METHOD_RETURN_TYPE, VALUE_PARAMETER, FIELD, TYPE_USE, TYPE_PARAMETER_BOUNDS
+enum class AnnotationQualifierApplicabilityType(val javaTarget: String) {
+    METHOD_RETURN_TYPE("METHOD"),
+    VALUE_PARAMETER("PARAMETER"),
+    FIELD("FIELD"),
+    TYPE_USE("TYPE_USE"),
+    TYPE_PARAMETER_BOUNDS("TYPE_USE"),
+    TYPE_PARAMETER("TYPE_PARAMETER")
 }
@@ -16,10 +16,15 @@

 package org.jetbrains.kotlin.load.java;

+import kotlin.annotation.Repeatable;
 import org.jetbrains.kotlin.name.FqName;
 import org.jetbrains.kotlin.name.Name;
 import org.jetbrains.kotlin.resolve.jvm.JvmClassName;

+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
@SuppressWarnings("PointlessBitwiseExpression")
 public final class JvmAnnotationNames {
    public static final FqName METADATA_FQ_NAME = new FqName("kotlin.Metadata");
@@ -44,6 +49,12 @@ public final class JvmAnnotationNames {

    public static final Name DEFAULT_ANNOTATION_MEMBER_NAME = Name.identifier("value");

+    public static final FqName TARGET_ANNOTATION = new FqName(Target.class.getCanonicalName());
+    public static final FqName RETENTION_ANNOTATION = new FqName(Retention.class.getCanonicalName());
+    public static final FqName DEPRECATED_ANNOTATION = new FqName(Deprecated.class.getCanonicalName());
+    public static final FqName DOCUMENTED_ANNOTATION = new FqName(Documented.class.getCanonicalName());
+    public static final FqName REPEATABLE_ANNOTATION = new FqName("java.lang.annotation.Repeatable");
+
    public static final FqName JETBRAINS_NOT_NULL_ANNOTATION = new FqName("org.jetbrains.annotations.NotNull");
    public static final FqName JETBRAINS_NULLABLE_ANNOTATION = new FqName("org.jetbrains.annotations.Nullable");
    public static final FqName JETBRAINS_MUTABLE_ANNOTATION = new FqName("org.jetbrains.annotations.Mutable");
@@ -19,6 +19,7 @@ package org.jetbrains.kotlin.load.java
 import org.jetbrains.kotlin.descriptors.ClassDescriptor
 import org.jetbrains.kotlin.descriptors.ClassKind
 import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor
+import org.jetbrains.kotlin.load.java.components.JavaAnnotationTargetMapper
 import org.jetbrains.kotlin.resolve.constants.ArrayValue
 import org.jetbrains.kotlin.resolve.constants.ConstantValue
 import org.jetbrains.kotlin.resolve.constants.EnumValue
@@ -114,7 +115,7 @@ class AnnotationTypeQualifierResolver(storageManager: StorageManager, private va
                .allValueArguments
                .flatMap { (parameter, argument) ->
                    if (parameter == JvmAnnotationNames.DEFAULT_ANNOTATION_MEMBER_NAME)
-                        argument.mapConstantToQualifierApplicabilityTypes()
+                        argument.mapJavaConstantToQualifierApplicabilityTypes()
                    else
                        emptyList()
                }
@@ -126,6 +127,16 @@ class AnnotationTypeQualifierResolver(storageManager: StorageManager, private va
        return TypeQualifierWithApplicability(typeQualifier, elementTypesMask)
    }

+    fun resolveAnnotation(annotationDescriptor: AnnotationDescriptor): TypeQualifierWithApplicability? {
+        val annotatedClass = annotationDescriptor.annotationClass ?: return null
+        val target = annotatedClass.annotations.findAnnotation(JvmAnnotationNames.TARGET_ANNOTATION) ?: return null
+        val elementTypesMask = target.allValueArguments
+            .flatMap { (_, argument) -> argument.mapKotlinConstantToQualifierApplicabilityTypes() }
+            .fold(0) { acc: Int, applicabilityType -> acc or (1 shl applicabilityType.ordinal) }
+
+        return TypeQualifierWithApplicability(annotationDescriptor, elementTypesMask)
+    }
+
    fun resolveJsr305AnnotationState(annotationDescriptor: AnnotationDescriptor): ReportLevel {
        resolveJsr305CustomState(annotationDescriptor)?.let { return it }
        return javaTypeEnhancementState.globalJsr305Level
@@ -150,20 +161,22 @@ class AnnotationTypeQualifierResolver(storageManager: StorageManager, private va
        }
    }

-    private fun ConstantValue<*>.mapConstantToQualifierApplicabilityTypes(): List<AnnotationQualifierApplicabilityType> =
+    private fun String.toKotlinTargetNames() = JavaAnnotationTargetMapper.mapJavaTargetArgumentByName(this).map { it.name }
+
+    private fun ConstantValue<*>.mapConstantToQualifierApplicabilityTypes(
+        findPredicate: EnumValue.(AnnotationQualifierApplicabilityType) -> Boolean
+    ): List<AnnotationQualifierApplicabilityType> =
        when (this) {
-            is ArrayValue -> value.flatMap { it.mapConstantToQualifierApplicabilityTypes() }
-            is EnumValue -> listOfNotNull(
-                when (enumEntryName.identifier) {
-                    "METHOD" -> AnnotationQualifierApplicabilityType.METHOD_RETURN_TYPE
-                    "FIELD" -> AnnotationQualifierApplicabilityType.FIELD
-                    "PARAMETER" -> AnnotationQualifierApplicabilityType.VALUE_PARAMETER
-                    "TYPE_USE" -> AnnotationQualifierApplicabilityType.TYPE_USE
-                    else -> null
-                }
-            )
+            is ArrayValue -> value.flatMap { it.mapConstantToQualifierApplicabilityTypes(findPredicate) }
+            is EnumValue -> listOfNotNull(AnnotationQualifierApplicabilityType.values().find { findPredicate(it) })
            else -> emptyList()
        }
+
+    private fun ConstantValue<*>.mapJavaConstantToQualifierApplicabilityTypes(): List<AnnotationQualifierApplicabilityType> =
+        mapConstantToQualifierApplicabilityTypes { enumEntryName.identifier == it.javaTarget }
+
+    private fun ConstantValue<*>.mapKotlinConstantToQualifierApplicabilityTypes(): List<AnnotationQualifierApplicabilityType> =
+        mapConstantToQualifierApplicabilityTypes { enumEntryName.identifier in it.javaTarget.toKotlinTargetNames() }
 }

 private val ClassDescriptor.isAnnotatedWithTypeQualifier: Boolean
@@ -21,6 +21,8 @@ import org.jetbrains.kotlin.descriptors.SourceElement
 import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor
 import org.jetbrains.kotlin.descriptors.annotations.KotlinRetention
 import org.jetbrains.kotlin.descriptors.annotations.KotlinTarget
+import org.jetbrains.kotlin.load.java.JvmAnnotationNames
+import org.jetbrains.kotlin.load.java.JvmAnnotationNames.*
 import org.jetbrains.kotlin.load.java.descriptors.PossiblyExternalAnnotationDescriptor
 import org.jetbrains.kotlin.load.java.lazy.LazyJavaResolverContext
 import org.jetbrains.kotlin.load.java.lazy.descriptors.LazyJavaAnnotationDescriptor
@@ -35,31 +37,21 @@ import org.jetbrains.kotlin.resolve.constants.StringValue
 import org.jetbrains.kotlin.storage.getValue
 import org.jetbrains.kotlin.types.ErrorUtils
 import org.jetbrains.kotlin.types.SimpleType
-import java.lang.annotation.Documented
-import java.lang.annotation.Retention
-import java.lang.annotation.Target
 import java.util.*

 object JavaAnnotationMapper {
-
-    private val JAVA_TARGET_FQ_NAME = FqName(Target::class.java.canonicalName)
-    private val JAVA_RETENTION_FQ_NAME = FqName(Retention::class.java.canonicalName)
-    private val JAVA_DEPRECATED_FQ_NAME = FqName(java.lang.Deprecated::class.java.canonicalName)
-    private val JAVA_DOCUMENTED_FQ_NAME = FqName(Documented::class.java.canonicalName)
    // Java8-specific thing
-    private val JAVA_REPEATABLE_FQ_NAME = FqName("java.lang.annotation.Repeatable")
-
    internal val DEPRECATED_ANNOTATION_MESSAGE = Name.identifier("message")
    internal val TARGET_ANNOTATION_ALLOWED_TARGETS = Name.identifier("allowedTargets")
    internal val RETENTION_ANNOTATION_VALUE = Name.identifier("value")

    fun mapOrResolveJavaAnnotation(annotation: JavaAnnotation, c: LazyJavaResolverContext): AnnotationDescriptor? =
        when (annotation.classId) {
-            ClassId.topLevel(JAVA_TARGET_FQ_NAME) -> JavaTargetAnnotationDescriptor(annotation, c)
-            ClassId.topLevel(JAVA_RETENTION_FQ_NAME) -> JavaRetentionAnnotationDescriptor(annotation, c)
-            ClassId.topLevel(JAVA_REPEATABLE_FQ_NAME) -> JavaAnnotationDescriptor(c, annotation, StandardNames.FqNames.repeatable)
-            ClassId.topLevel(JAVA_DOCUMENTED_FQ_NAME) -> JavaAnnotationDescriptor(c, annotation, StandardNames.FqNames.mustBeDocumented)
-            ClassId.topLevel(JAVA_DEPRECATED_FQ_NAME) -> null
+            ClassId.topLevel(TARGET_ANNOTATION) -> JavaTargetAnnotationDescriptor(annotation, c)
+            ClassId.topLevel(RETENTION_ANNOTATION) -> JavaRetentionAnnotationDescriptor(annotation, c)
+            ClassId.topLevel(REPEATABLE_ANNOTATION) -> JavaAnnotationDescriptor(c, annotation, StandardNames.FqNames.repeatable)
+            ClassId.topLevel(DOCUMENTED_ANNOTATION) -> JavaAnnotationDescriptor(c, annotation, StandardNames.FqNames.mustBeDocumented)
+            ClassId.topLevel(DEPRECATED_ANNOTATION) -> null
            else -> LazyJavaAnnotationDescriptor(c, annotation)
        }

@@ -69,7 +61,7 @@ object JavaAnnotationMapper {
        c: LazyJavaResolverContext
    ): AnnotationDescriptor? {
        if (kotlinName == StandardNames.FqNames.deprecated) {
-            val javaAnnotation = annotationOwner.findAnnotation(JAVA_DEPRECATED_FQ_NAME)
+            val javaAnnotation = annotationOwner.findAnnotation(DEPRECATED_ANNOTATION)
            if (javaAnnotation != null || annotationOwner.isDeprecatedInJavaDoc) {
                return JavaDeprecatedAnnotationDescriptor(javaAnnotation, c)
            }
@@ -84,19 +76,19 @@ object JavaAnnotationMapper {
    // kotlin.annotation.annotation is treated separately
    private val kotlinToJavaNameMap: Map<FqName, FqName> =
        mapOf(
-            StandardNames.FqNames.target to JAVA_TARGET_FQ_NAME,
-            StandardNames.FqNames.retention to JAVA_RETENTION_FQ_NAME,
-            StandardNames.FqNames.repeatable to JAVA_REPEATABLE_FQ_NAME,
-            StandardNames.FqNames.mustBeDocumented to JAVA_DOCUMENTED_FQ_NAME
+            StandardNames.FqNames.target to TARGET_ANNOTATION,
+            StandardNames.FqNames.retention to RETENTION_ANNOTATION,
+            StandardNames.FqNames.repeatable to REPEATABLE_ANNOTATION,
+            StandardNames.FqNames.mustBeDocumented to DOCUMENTED_ANNOTATION
        )

    val javaToKotlinNameMap: Map<FqName, FqName> =
        mapOf(
-            JAVA_TARGET_FQ_NAME to StandardNames.FqNames.target,
-            JAVA_RETENTION_FQ_NAME to StandardNames.FqNames.retention,
-            JAVA_DEPRECATED_FQ_NAME to StandardNames.FqNames.deprecated,
-            JAVA_REPEATABLE_FQ_NAME to StandardNames.FqNames.repeatable,
-            JAVA_DOCUMENTED_FQ_NAME to StandardNames.FqNames.mustBeDocumented
+            TARGET_ANNOTATION to StandardNames.FqNames.target,
+            RETENTION_ANNOTATION to StandardNames.FqNames.retention,
+            DEPRECATED_ANNOTATION to StandardNames.FqNames.deprecated,
+            REPEATABLE_ANNOTATION to StandardNames.FqNames.repeatable,
+            DOCUMENTED_ANNOTATION to StandardNames.FqNames.mustBeDocumented
        )
 }

@@ -327,10 +327,23 @@ class SignatureEnhancement(
            isFromStarProjection: Boolean
        ): JavaTypeQualifiers {
            val composedAnnotation =
-                if (isHeadTypeConstructor && typeContainer != null)
+                if (isHeadTypeConstructor && typeContainer is TypeParameterDescriptor) {
                    composeAnnotations(typeContainer.annotations, annotations)
-                else
-                    annotations
+                } else if (isHeadTypeConstructor && typeContainer != null) {
+                    val filteredContainerAnnotations = typeContainer.annotations.filter {
+                        val (_, targets) = annotationTypeQualifierResolver.resolveAnnotation(it) ?: return@filter false
+                        /*
+                         * We don't apply container type use annotations to avoid double applying them like with arrays:
+                         *      @NotNull Integer [] f15();
+                         * Otherwise, in the example above we would apply `@NotNull` to `Integer` (i.e. array element; as TYPE_USE annotation)
+                         * and to entire array (as METHOD annotation).
+                         * In other words, we prefer TYPE_USE target of an annotation, and apply the annotation only according to it, if it's present.
+                         * See KT-24392 for more details.
+                         */
+                        AnnotationQualifierApplicabilityType.TYPE_USE !in targets
+                    }
+                    composeAnnotations(Annotations.create(filteredContainerAnnotations), annotations)
+                } else annotations

            fun <T : Any> List<FqName>.ifPresent(qualifier: T) =
                if (any { composedAnnotation.findAnnotation(it) != null }) qualifier else null
@@ -14,8 +14,8 @@ object SpecialJvmAnnotations {
        JvmAnnotationNames.METADATA_FQ_NAME,
        JvmAnnotationNames.JETBRAINS_NOT_NULL_ANNOTATION,
        JvmAnnotationNames.JETBRAINS_NULLABLE_ANNOTATION,
-        FqName("java.lang.annotation.Target"),
-        FqName("java.lang.annotation.Retention"),
-        FqName("java.lang.annotation.Documented")
+        JvmAnnotationNames.TARGET_ANNOTATION,
+        JvmAnnotationNames.RETENTION_ANNOTATION,
+        JvmAnnotationNames.DOCUMENTED_ANNOTATION
    ).mapTo(mutableSetOf(), ClassId::topLevel)
 }
@@ -5,6 +5,7 @@

 package org.jetbrains.kotlin.nj2k.conversions

+import org.jetbrains.kotlin.load.java.JvmAnnotationNames
 import org.jetbrains.kotlin.nj2k.NewJ2kConverterContext
 import org.jetbrains.kotlin.nj2k.tree.*

@@ -30,7 +31,7 @@ class JavaAnnotationsConversion(context: NewJ2kConverterContext) : RecursiveAppl
            annotation.classSymbol = symbolProvider.provideClassSymbol("kotlin.Deprecated")
            annotation.arguments = listOf(JKAnnotationParameterImpl(JKLiteralExpression("\"\"", JKLiteralExpression.LiteralType.STRING)))
        }
-        if (annotation.classSymbol.fqName == "java.lang.annotation.Target") {
+        if (annotation.classSymbol.fqName == JvmAnnotationNames.TARGET_ANNOTATION.asString()) {
            annotation.classSymbol = symbolProvider.provideClassSymbol("kotlin.annotation.Target")

            val arguments = annotation.arguments.singleOrNull()?.let { parameter ->