K2: ignore conflicting nullability annotations as stated in jspecify …

…spec Related to KT-53834
JetBrains · Apr 18, 2024 · 3c716c1 · 3c716c1
1 parent 8f28880
commit 3c716c1
Show file tree

Hide file tree

Showing 9 changed files with 61 additions and 53 deletions.
diff --git a/.../java/src/org/jetbrains/kotlin/fir/java/enhancement/FirAnnotationTypeQualifierResolver.kt b/.../java/src/org/jetbrains/kotlin/fir/java/enhancement/FirAnnotationTypeQualifierResolver.kt
@@ -29,6 +29,9 @@ class FirAnnotationTypeQualifierResolver(
     private val javaModuleAnnotationsProvider: JavaModuleAnnotationsProvider,
 ) : AbstractAnnotationTypeQualifierResolver<FirAnnotation>(javaTypeEnhancementState), FirSessionComponent {
 
+    override val isK2: Boolean
+        get() = true
+
     override val FirAnnotation.metaAnnotations: Iterable<FirAnnotation>
         get() = unexpandedConeClassLikeType?.lookupTag?.toSymbol(session)?.fir?.annotations.orEmpty()
 

diff --git a/...Tests/java11Tests/jspecify/moduleAnnotations/strict/NullUnmarkedAndNullMarkedConflictB.kt b/...Tests/java11Tests/jspecify/moduleAnnotations/strict/NullUnmarkedAndNullMarkedConflictB.kt
@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 // ALLOW_KOTLIN_PACKAGE
 // JSPECIFY_STATE: strict
 // DIAGNOSTICS: -UNUSED_PARAMETER

diff --git a/...nsTests/java11Tests/jspecify/moduleAnnotations/warn/NullUnmarkedAndNullMarkedConflictB.kt b/...nsTests/java11Tests/jspecify/moduleAnnotations/warn/NullUnmarkedAndNullMarkedConflictB.kt
@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 // ALLOW_KOTLIN_PACKAGE
 // JSPECIFY_STATE: warn
 // DIAGNOSTICS: -UNUSED_PARAMETER

diff --git a/...NullUnmarkedAndNullMarkedConflictB.fir.kt → ...NullUnmarkedAndNullMarkedConflictA.fir.kt b/...NullUnmarkedAndNullMarkedConflictB.fir.kt → ...NullUnmarkedAndNullMarkedConflictA.fir.kt
@@ -1,27 +1,15 @@
-// ALLOW_KOTLIN_PACKAGE
 // JSPECIFY_STATE: strict
 // DIAGNOSTICS: -UNUSED_PARAMETER
 
-// FILE: null_marked_module/module-info.java
+// FILE: conflictinglyannotatedpackage/package-info.java
 
-import org.jspecify.annotations.*;
-
-@NullMarked
-module null_marked_module {
-    requires java9_annotations;
-    exports conflictinglyannotatedpackage;
-    exports unannotatedpackage;
-}
-
-// FILE: null_marked_module/conflictinglyannotatedpackage/package-info.java
-
-@NullMarked
 @NullUnmarked
+@NullMarked
 package conflictinglyannotatedpackage;
 
 import org.jspecify.annotations.*;
 
-// FILE: null_marked_module/conflictinglyannotatedpackage/UnannotatedType.java
+// FILE: conflictinglyannotatedpackage/UnannotatedType.java
 
 package conflictinglyannotatedpackage;
 
@@ -30,43 +18,43 @@ public interface UnannotatedType {
     public void unannotatedConsume(String arg);
 }
 
-// FILE: null_marked_module/unannotatedpackage/ConflictinglyAnnotatedType.java
+// FILE: unannotatedpackage/ConflictinglyAnnotatedType.java
 
 package unannotatedpackage;
 
 import org.jspecify.annotations.*;
 
-@NullMarked
 @NullUnmarked
+@NullMarked
 public interface ConflictinglyAnnotatedType {
     public String unannotatedProduce();
     public void unannotatedConsume(String arg);
 }
 
-// FILE: null_marked_module/unannotatedpackage/UnannotatedType.java
+// FILE: unannotatedpackage/UnannotatedType.java
 
 package unannotatedpackage;
 
 import org.jspecify.annotations.*;
 
 public interface UnannotatedType {
-    @NullMarked
     @NullUnmarked
-    public String conflictinglyAnnotatedProduce();
     @NullMarked
+    public String conflictinglyAnnotatedProduce();
     @NullUnmarked
+    @NullMarked
     public void conflictinglyAnnotatedConsume(String arg);
 }
 
-// FILE: null_marked_module/unannotatedpackage/UnannotatedTypeWithConflictinglyAnnotatedConstructor.java
+// FILE: unannotatedpackage/UnannotatedTypeWithConflictinglyAnnotatedConstructor.java
 
 package unannotatedpackage;
 
 import org.jspecify.annotations.*;
 
 public class UnannotatedTypeWithConflictinglyAnnotatedConstructor {
-    @NullMarked
     @NullUnmarked
+    @NullMarked
     public UnannotatedTypeWithConflictinglyAnnotatedConstructor(String arg) {}
 }
 

diff --git a/...eignAnnotationsTests/java8Tests/jspecify/strictMode/NullUnmarkedAndNullMarkedConflictA.kt b/...eignAnnotationsTests/java8Tests/jspecify/strictMode/NullUnmarkedAndNullMarkedConflictA.kt
@@ -1,4 +1,3 @@
-// FIR_IDENTICAL
 // JSPECIFY_STATE: strict
 // DIAGNOSTICS: -UNUSED_PARAMETER
 

diff --git a/...NullUnmarkedAndNullMarkedConflictB.fir.kt → ...NullUnmarkedAndNullMarkedConflictA.fir.kt b/...NullUnmarkedAndNullMarkedConflictB.fir.kt → ...NullUnmarkedAndNullMarkedConflictA.fir.kt
@@ -1,27 +1,15 @@
-// ALLOW_KOTLIN_PACKAGE
 // JSPECIFY_STATE: warn
 // DIAGNOSTICS: -UNUSED_PARAMETER
 
-// FILE: null_marked_module/module-info.java
+// FILE: conflictinglyannotatedpackage/package-info.java
 
-import org.jspecify.annotations.*;
-
-@NullMarked
-module null_marked_module {
-    requires java9_annotations;
-    exports conflictinglyannotatedpackage;
-    exports unannotatedpackage;
-}
-
-// FILE: null_marked_module/conflictinglyannotatedpackage/package-info.java
-
-@NullMarked
 @NullUnmarked
+@NullMarked
 package conflictinglyannotatedpackage;
 
 import org.jspecify.annotations.*;
 
-// FILE: null_marked_module/conflictinglyannotatedpackage/UnannotatedType.java
+// FILE: conflictinglyannotatedpackage/UnannotatedType.java
 
 package conflictinglyannotatedpackage;
 
@@ -30,43 +18,43 @@ public interface UnannotatedType {
     public void unannotatedConsume(String arg);
 }
 
-// FILE: null_marked_module/unannotatedpackage/ConflictinglyAnnotatedType.java
+// FILE: unannotatedpackage/ConflictinglyAnnotatedType.java
 
 package unannotatedpackage;
 
 import org.jspecify.annotations.*;
 
-@NullMarked
 @NullUnmarked
+@NullMarked
 public interface ConflictinglyAnnotatedType {
     public String unannotatedProduce();
     public void unannotatedConsume(String arg);
 }
 
-// FILE: null_marked_module/unannotatedpackage/UnannotatedType.java
+// FILE: unannotatedpackage/UnannotatedType.java
 
 package unannotatedpackage;
 
 import org.jspecify.annotations.*;
 
 public interface UnannotatedType {
-    @NullMarked
     @NullUnmarked
-    public String conflictinglyAnnotatedProduce();
     @NullMarked
+    public String conflictinglyAnnotatedProduce();
     @NullUnmarked
+    @NullMarked
     public void conflictinglyAnnotatedConsume(String arg);
 }
 
-// FILE: null_marked_module/unannotatedpackage/UnannotatedTypeWithConflictinglyAnnotatedConstructor.java
+// FILE: unannotatedpackage/UnannotatedTypeWithConflictinglyAnnotatedConstructor.java
 
 package unannotatedpackage;
 
 import org.jspecify.annotations.*;
 
 public class UnannotatedTypeWithConflictinglyAnnotatedConstructor {
-    @NullMarked
     @NullUnmarked
+    @NullMarked
     public UnannotatedTypeWithConflictinglyAnnotatedConstructor(String arg) {}
 }
 

diff --git a/...oreignAnnotationsTests/java8Tests/jspecify/warnMode/NullUnmarkedAndNullMarkedConflictA.kt b/...oreignAnnotationsTests/java8Tests/jspecify/warnMode/NullUnmarkedAndNullMarkedConflictA.kt
@@ -1,4 +1,3 @@
-// FIR_IDENTICAL
 // JSPECIFY_STATE: warn
 // DIAGNOSTICS: -UNUSED_PARAMETER
 

diff --git a/....common.jvm/src/org/jetbrains/kotlin/load/java/AbstractAnnotationTypeQualifierResolver.kt b/....common.jvm/src/org/jetbrains/kotlin/load/java/AbstractAnnotationTypeQualifierResolver.kt
@@ -31,6 +31,8 @@ abstract class AbstractAnnotationTypeQualifierResolver<TAnnotation : Any>(
 
     private val resolvedNicknames = ConcurrentHashMap<Any, TAnnotation>()
 
+    abstract val isK2: Boolean
+
     fun resolveTypeQualifierAnnotation(annotation: TAnnotation): TAnnotation? {
         if (javaTypeEnhancementState.jsr305.isDisabled) return null
         if (annotation.fqName in BUILT_IN_TYPE_QUALIFIER_ANNOTATIONS || annotation.hasAnnotation(JAVAX_TYPE_QUALIFIER_ANNOTATION_FQ_NAME))
@@ -159,19 +161,43 @@ abstract class AbstractAnnotationTypeQualifierResolver<TAnnotation : Any>(
     ): JavaTypeQualifiersByElementType? {
         if (javaTypeEnhancementState.disabledDefaultAnnotations) return oldQualifiers
 
-        val defaultQualifiers = annotations.mapNotNull { extractDefaultQualifiers(it) }
-        if (defaultQualifiers.isEmpty()) return oldQualifiers
+        val extractedQualifiers = annotations.mapNotNull { extractDefaultQualifiers(it) }
+        if (extractedQualifiers.isEmpty()) return oldQualifiers
+
+        val newQualifiers = QualifierByApplicabilityType(AnnotationQualifierApplicabilityType::class.java)
+
+        // filter out inconsistencies in extracted qualifiers per applicability type
+        for (extractedQualifier in extractedQualifiers) {
+            for (applicabilityType in extractedQualifier.qualifierApplicabilityTypes) {
+                if (applicabilityType !in newQualifiers || !isK2) {
+                    // no qualifiers for applicabilityType were previously considered
+                    // Or, in K1 mode, we just use the last qualifier
+                    newQualifiers[applicabilityType] = extractedQualifier
+                } else {
+                    // one+ qualifiers for applicabilityType were already considered
+                    val preexistingQualifier = newQualifiers[applicabilityType]
+                        ?: continue // an inconsistency was already found when considering previous qualifiers
+                    val preexistingNullability = preexistingQualifier.nullabilityQualifier
+                    val extractedNullability = extractedQualifier.nullabilityQualifier
+                    newQualifiers[applicabilityType] = when {
+                        extractedNullability == preexistingNullability -> preexistingQualifier
+                        extractedNullability.isForWarningOnly && !preexistingNullability.isForWarningOnly -> preexistingQualifier
+                        !extractedNullability.isForWarningOnly && preexistingNullability.isForWarningOnly -> extractedQualifier
+                        else -> null // qualifiers are inconsistent
+                    }
+                }
+            }
+        }
 
         val defaultQualifiersByType =
             oldQualifiers?.defaultQualifiers?.let(::QualifierByApplicabilityType)
                 ?: QualifierByApplicabilityType(AnnotationQualifierApplicabilityType::class.java)
 
         var wasUpdate = false
-        for (qualifier in defaultQualifiers) {
-            for (applicabilityType in qualifier.qualifierApplicabilityTypes) {
-                defaultQualifiersByType[applicabilityType] = qualifier
-                wasUpdate = true
-            }
+        for ((applicabilityType, newQualifier) in newQualifiers) {
+            if (newQualifier == null) continue // ignore inconsistent qualifiers
+            defaultQualifiersByType[applicabilityType] = newQualifier
+            wasUpdate = true
         }
 
         return if (!wasUpdate) oldQualifiers else JavaTypeQualifiersByElementType(defaultQualifiersByType)

diff --git a/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/AnnotationTypeQualifierResolver.kt b/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/AnnotationTypeQualifierResolver.kt
@@ -27,6 +27,9 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.annotationClass
 class AnnotationTypeQualifierResolver(javaTypeEnhancementState: JavaTypeEnhancementState) :
     AbstractAnnotationTypeQualifierResolver<AnnotationDescriptor>(javaTypeEnhancementState) {
 
+    override val isK2: Boolean
+        get() = false
+
     override val AnnotationDescriptor.metaAnnotations: Iterable<AnnotationDescriptor>
         get() = annotationClass?.annotations ?: emptyList()