From e9e05c53e152b93df4322215e4dc07bf0593a317 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Fri, 9 Aug 2019 15:22:31 +0300 Subject: [PATCH] Support enhancement for type parameter based types Load them as non-flexible when a relevant type parameter has non-flexible upper bound (Currently, it only works for case of codeanalysis annotations) --- .../jspecify/nonPlatformTypeParameter.kt | 43 ++++++++++++++++ .../jspecify/nonPlatformTypeParameter.txt | 21 ++++++++ .../tests/jspecify/typeParameterBounds.kt | 8 +-- .../tests/jspecify/typeParameterBounds.txt | 4 +- .../jspecify/unknownNullnessTypeParameter.kt | 29 +++++++++++ .../jspecify/unknownNullnessTypeParameter.txt | 12 +++++ ...sNoAnnotationInClasspathTestGenerated.java | 10 ++++ ...spathWithPsiClassReadingTestGenerated.java | 10 ++++ .../ForeignJava8AnnotationsTestGenerated.java | 10 ++++ ...cForeignJava8AnnotationsTestGenerated.java | 10 ++++ .../load/java/AnnotationQualifiersFqNames.kt | 10 ++-- .../java/JavaTypeQualifiersByElementType.kt | 2 +- .../typeEnhancement/signatureEnhancement.kt | 50 +++++++++++++++++-- 13 files changed, 204 insertions(+), 15 deletions(-) create mode 100644 compiler/testData/foreignAnnotationsJava8/tests/jspecify/nonPlatformTypeParameter.kt create mode 100644 compiler/testData/foreignAnnotationsJava8/tests/jspecify/nonPlatformTypeParameter.txt create mode 100644 compiler/testData/foreignAnnotationsJava8/tests/jspecify/unknownNullnessTypeParameter.kt create mode 100644 compiler/testData/foreignAnnotationsJava8/tests/jspecify/unknownNullnessTypeParameter.txt diff --git a/compiler/testData/foreignAnnotationsJava8/tests/jspecify/nonPlatformTypeParameter.kt b/compiler/testData/foreignAnnotationsJava8/tests/jspecify/nonPlatformTypeParameter.kt new file mode 100644 index 00000000000..bf43680b83a --- /dev/null +++ b/compiler/testData/foreignAnnotationsJava8/tests/jspecify/nonPlatformTypeParameter.kt @@ -0,0 +1,43 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER +// FILE: A.java + +import jspecify.annotations.*; + +public class A { + public void foo(T t) {} + public void bar(E e) {} +} + +// FILE: B.java + +import jspecify.annotations.*; + +@DefaultNullable +public class B { + public void foo(T t) {} + public void bar(E e) {} +} + +// FILE: main.kt + +fun main(a1: A, a2: A, b1: B, b2: B) { + a1.foo(null) + a1.bar(null) + a1.bar(null) + a1.bar("") + + a2.foo(null) + a2.bar(null) + a2.bar(null) + a2.bar("") + + b1.foo(null) + b1.bar(null) + b1.bar(null) + b1.bar("") + + b2.foo(null) + b2.bar(null) + b2.bar(null) + b2.bar("") +} diff --git a/compiler/testData/foreignAnnotationsJava8/tests/jspecify/nonPlatformTypeParameter.txt b/compiler/testData/foreignAnnotationsJava8/tests/jspecify/nonPlatformTypeParameter.txt new file mode 100644 index 00000000000..f5509fb1e25 --- /dev/null +++ b/compiler/testData/foreignAnnotationsJava8/tests/jspecify/nonPlatformTypeParameter.txt @@ -0,0 +1,21 @@ +package + +public fun main(/*0*/ a1: A, /*1*/ a2: A, /*2*/ b1: B, /*3*/ b2: B): kotlin.Unit + +public open class A { + public constructor A() + public open fun bar(/*0*/ e: E): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open fun foo(/*0*/ t: T): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +@jspecify.annotations.DefaultNullable public open class B { + public constructor B() + public open fun bar(/*0*/ e: E): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open fun foo(/*0*/ t: T): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/foreignAnnotationsJava8/tests/jspecify/typeParameterBounds.kt b/compiler/testData/foreignAnnotationsJava8/tests/jspecify/typeParameterBounds.kt index 995ca10e22e..01c5195791c 100644 --- a/compiler/testData/foreignAnnotationsJava8/tests/jspecify/typeParameterBounds.kt +++ b/compiler/testData/foreignAnnotationsJava8/tests/jspecify/typeParameterBounds.kt @@ -21,12 +21,12 @@ public class B { // FILE: main.kt fun main(a1: A<Any?>, a2: A, b1: B<Any?>, b2: B) { - a1.foo(null) - a1.bar<String?>(null) + a1.foo(null) + a1.bar<String?>(null) a1.bar("") - a2.foo(null) - a2.bar<String?>(null) + a2.foo(null) + a2.bar<String?>(null) a2.bar("") b1.foo(null) diff --git a/compiler/testData/foreignAnnotationsJava8/tests/jspecify/typeParameterBounds.txt b/compiler/testData/foreignAnnotationsJava8/tests/jspecify/typeParameterBounds.txt index dd3929be1cb..175ddb83d32 100644 --- a/compiler/testData/foreignAnnotationsJava8/tests/jspecify/typeParameterBounds.txt +++ b/compiler/testData/foreignAnnotationsJava8/tests/jspecify/typeParameterBounds.txt @@ -4,9 +4,9 @@ public fun main(/*0*/ a1: A, /*1*/ a2: A, /*2*/ b1: public open class A { public constructor A() - public open fun bar(/*0*/ e: E!): kotlin.Unit + public open fun bar(/*0*/ e: E): kotlin.Unit public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public open fun foo(/*0*/ t: T!): kotlin.Unit + public open fun foo(/*0*/ t: T): kotlin.Unit public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } diff --git a/compiler/testData/foreignAnnotationsJava8/tests/jspecify/unknownNullnessTypeParameter.kt b/compiler/testData/foreignAnnotationsJava8/tests/jspecify/unknownNullnessTypeParameter.kt new file mode 100644 index 00000000000..0091cae1e1a --- /dev/null +++ b/compiler/testData/foreignAnnotationsJava8/tests/jspecify/unknownNullnessTypeParameter.kt @@ -0,0 +1,29 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER +// FILE: A.java + +import jspecify.annotations.*; + +public class A { + public void foo(T t) {} + + @DefaultNotNull + public void bar(String s, T t) {} // t should not become not nullable +} + +// FILE: main.kt + +fun main(a1: A, a2: A) { + a1.foo(null) + a1.foo(1) + + a2.foo(null) + a2.foo(1) + + a1.bar(null, null) + a1.bar("", null) + a1.bar("", 1) + + a2.bar(null, null) + a2.bar("", null) + a2.bar("", 1) +} diff --git a/compiler/testData/foreignAnnotationsJava8/tests/jspecify/unknownNullnessTypeParameter.txt b/compiler/testData/foreignAnnotationsJava8/tests/jspecify/unknownNullnessTypeParameter.txt new file mode 100644 index 00000000000..d3749530b1f --- /dev/null +++ b/compiler/testData/foreignAnnotationsJava8/tests/jspecify/unknownNullnessTypeParameter.txt @@ -0,0 +1,12 @@ +package + +public fun main(/*0*/ a1: A, /*1*/ a2: A): kotlin.Unit + +public open class A { + public constructor A() + @jspecify.annotations.DefaultNotNull public open fun bar(/*0*/ s: kotlin.String, /*1*/ t: T!): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open fun foo(/*0*/ t: T!): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsNoAnnotationInClasspathTestGenerated.java b/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsNoAnnotationInClasspathTestGenerated.java index 71d41932970..83f24d49ea0 100644 --- a/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsNoAnnotationInClasspathTestGenerated.java +++ b/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsNoAnnotationInClasspathTestGenerated.java @@ -60,6 +60,11 @@ public class ForeignJava8AnnotationsNoAnnotationInClasspathTestGenerated extends runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/defaults.kt"); } + @TestMetadata("nonPlatformTypeParameter.kt") + public void testNonPlatformTypeParameter() throws Exception { + runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/nonPlatformTypeParameter.kt"); + } + @TestMetadata("simple.kt") public void testSimple() throws Exception { runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/simple.kt"); @@ -69,6 +74,11 @@ public class ForeignJava8AnnotationsNoAnnotationInClasspathTestGenerated extends public void testTypeParameterBounds() throws Exception { runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/typeParameterBounds.kt"); } + + @TestMetadata("unknownNullnessTypeParameter.kt") + public void testUnknownNullnessTypeParameter() throws Exception { + runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/unknownNullnessTypeParameter.kt"); + } } @TestMetadata("compiler/testData/foreignAnnotationsJava8/tests/jsr305") diff --git a/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsNoAnnotationInClasspathWithPsiClassReadingTestGenerated.java b/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsNoAnnotationInClasspathWithPsiClassReadingTestGenerated.java index 73ff013bfdd..ce6652f4a9b 100644 --- a/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsNoAnnotationInClasspathWithPsiClassReadingTestGenerated.java +++ b/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsNoAnnotationInClasspathWithPsiClassReadingTestGenerated.java @@ -60,6 +60,11 @@ public class ForeignJava8AnnotationsNoAnnotationInClasspathWithPsiClassReadingTe runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/defaults.kt"); } + @TestMetadata("nonPlatformTypeParameter.kt") + public void testNonPlatformTypeParameter() throws Exception { + runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/nonPlatformTypeParameter.kt"); + } + @TestMetadata("simple.kt") public void testSimple() throws Exception { runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/simple.kt"); @@ -69,6 +74,11 @@ public class ForeignJava8AnnotationsNoAnnotationInClasspathWithPsiClassReadingTe public void testTypeParameterBounds() throws Exception { runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/typeParameterBounds.kt"); } + + @TestMetadata("unknownNullnessTypeParameter.kt") + public void testUnknownNullnessTypeParameter() throws Exception { + runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/unknownNullnessTypeParameter.kt"); + } } @TestMetadata("compiler/testData/foreignAnnotationsJava8/tests/jsr305") diff --git a/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsTestGenerated.java b/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsTestGenerated.java index 5336fd61b7e..1a494f92359 100644 --- a/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsTestGenerated.java +++ b/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsTestGenerated.java @@ -60,6 +60,11 @@ public class ForeignJava8AnnotationsTestGenerated extends AbstractForeignJava8An runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/defaults.kt"); } + @TestMetadata("nonPlatformTypeParameter.kt") + public void testNonPlatformTypeParameter() throws Exception { + runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/nonPlatformTypeParameter.kt"); + } + @TestMetadata("simple.kt") public void testSimple() throws Exception { runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/simple.kt"); @@ -69,6 +74,11 @@ public class ForeignJava8AnnotationsTestGenerated extends AbstractForeignJava8An public void testTypeParameterBounds() throws Exception { runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/typeParameterBounds.kt"); } + + @TestMetadata("unknownNullnessTypeParameter.kt") + public void testUnknownNullnessTypeParameter() throws Exception { + runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/unknownNullnessTypeParameter.kt"); + } } @TestMetadata("compiler/testData/foreignAnnotationsJava8/tests/jsr305") diff --git a/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/javac/JavacForeignJava8AnnotationsTestGenerated.java b/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/javac/JavacForeignJava8AnnotationsTestGenerated.java index c4fb8dccc69..010f7b303f4 100644 --- a/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/javac/JavacForeignJava8AnnotationsTestGenerated.java +++ b/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/javac/JavacForeignJava8AnnotationsTestGenerated.java @@ -60,6 +60,11 @@ public class JavacForeignJava8AnnotationsTestGenerated extends AbstractJavacFore runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/defaults.kt"); } + @TestMetadata("nonPlatformTypeParameter.kt") + public void testNonPlatformTypeParameter() throws Exception { + runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/nonPlatformTypeParameter.kt"); + } + @TestMetadata("simple.kt") public void testSimple() throws Exception { runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/simple.kt"); @@ -69,6 +74,11 @@ public class JavacForeignJava8AnnotationsTestGenerated extends AbstractJavacFore public void testTypeParameterBounds() throws Exception { runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/typeParameterBounds.kt"); } + + @TestMetadata("unknownNullnessTypeParameter.kt") + public void testUnknownNullnessTypeParameter() throws Exception { + runTest("compiler/testData/foreignAnnotationsJava8/tests/jspecify/unknownNullnessTypeParameter.kt"); + } } @TestMetadata("compiler/testData/foreignAnnotationsJava8/tests/jsr305") diff --git a/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/AnnotationQualifiersFqNames.kt b/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/AnnotationQualifiersFqNames.kt index 0c7d1994744..2b67336b40c 100644 --- a/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/AnnotationQualifiersFqNames.kt +++ b/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/AnnotationQualifiersFqNames.kt @@ -11,8 +11,11 @@ import org.jetbrains.kotlin.name.FqName data class JavaDefaultQualifiers( val nullabilityQualifier: NullabilityQualifierWithMigrationStatus, - val qualifierApplicabilityTypes: Collection -) + val qualifierApplicabilityTypes: Collection, + val affectsTypeParameterBasedTypes: Boolean = nullabilityQualifier.qualifier == NullabilityQualifier.NOT_NULL +) { + val makesTypeParameterNotNull get() = nullabilityQualifier.qualifier == NullabilityQualifier.NOT_NULL && affectsTypeParameterBasedTypes +} val TYPE_QUALIFIER_NICKNAME_FQNAME = FqName("javax.annotation.meta.TypeQualifierNickname") val TYPE_QUALIFIER_FQNAME = FqName("javax.annotation.meta.TypeQualifier") @@ -45,7 +48,8 @@ val BUILT_IN_TYPE_QUALIFIER_DEFAULT_ANNOTATIONS = mapOf( ), JSPECIFY_DEFAULT_NOT_NULL to JavaDefaultQualifiers( NullabilityQualifierWithMigrationStatus(NullabilityQualifier.NOT_NULL), - DEFAULT_JSPECIFY_APPLICABILITY + DEFAULT_JSPECIFY_APPLICABILITY, + affectsTypeParameterBasedTypes = false ), JSPECIFY_DEFAULT_NULLNESS_UNKNOWN to JavaDefaultQualifiers( NullabilityQualifierWithMigrationStatus(NullabilityQualifier.FORCE_FLEXIBILITY), diff --git a/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/JavaTypeQualifiersByElementType.kt b/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/JavaTypeQualifiersByElementType.kt index 91e60078f03..158281b1ff4 100644 --- a/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/JavaTypeQualifiersByElementType.kt +++ b/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/JavaTypeQualifiersByElementType.kt @@ -16,4 +16,4 @@ class JavaTypeQualifiersByElementType(val defaultQualifiers: QualifierByApplicab ): JavaDefaultQualifiers? { return defaultQualifiers[applicabilityType] } -} \ No newline at end of file +} diff --git a/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/typeEnhancement/signatureEnhancement.kt b/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/typeEnhancement/signatureEnhancement.kt index 9b75711a1ed..ba5efaa32dd 100644 --- a/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/typeEnhancement/signatureEnhancement.kt +++ b/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/typeEnhancement/signatureEnhancement.kt @@ -325,13 +325,20 @@ class SignatureEnhancement( fun uniqueNotNull(x: T?, y: T?) = if (x == null || y == null || x == y) x ?: y else null val defaultTypeQualifier = - if (isHeadTypeConstructor) + (if (isHeadTypeConstructor) containerContext.defaultTypeQualifiers?.get(containerApplicabilityType) else - defaultQualifiersForType + defaultQualifiersForType)?.takeIf { + it.affectsTypeParameterBasedTypes || !isTypeParameter() + } + val (nullabilityFromBoundsForTypeBasedOnTypeParameter, isTypeParameterWithNotNullableBounds) = + nullabilityInfoBoundsForTypeParameterUsage() + + val annotationsNullability = composedAnnotation.extractNullability() val nullabilityInfo = - composedAnnotation.extractNullability() + annotationsNullability + ?: nullabilityFromBoundsForTypeBasedOnTypeParameter ?: defaultTypeQualifier?.nullabilityQualifier?.let { nullabilityQualifierWithMigrationStatus -> NullabilityQualifierWithMigrationStatus( nullabilityQualifierWithMigrationStatus.qualifier, @@ -339,6 +346,12 @@ class SignatureEnhancement( ) } + val isNotNullTypeParameter = + if (annotationsNullability != null) + annotationsNullability.qualifier == NullabilityQualifier.NOT_NULL + else + isTypeParameterWithNotNullableBounds || defaultTypeQualifier?.makesTypeParameterNotNull == true + return JavaTypeQualifiers( nullabilityInfo?.qualifier, uniqueNotNull( @@ -349,11 +362,34 @@ class SignatureEnhancement( MutabilityQualifier.MUTABLE ) ), - isNotNullTypeParameter = nullabilityInfo?.qualifier == NullabilityQualifier.NOT_NULL && isTypeParameter(), + isNotNullTypeParameter = isNotNullTypeParameter && isTypeParameter(), isNullabilityQualifierForWarning = nullabilityInfo?.isForWarningOnly == true ) } + private fun KotlinType.nullabilityInfoBoundsForTypeParameterUsage(): Pair { + val typeParameterBoundsNullability = + (constructor.declarationDescriptor as? TypeParameterDescriptor)?.boundsNullability() ?: return Pair(null, false) + + if (typeParameterBoundsNullability == NullabilityQualifier.FORCE_FLEXIBILITY) return Pair(null, false) + + // If type parameter has a nullable (non-flexible) upper bound + // We shouldn't mark its type usages as nullable: + // interface A { + // void foo(T t); // should be loaded as "fun foo(t: T)" but not as "fun foo(t: T?)" + // } + return Pair( + NullabilityQualifierWithMigrationStatus(NullabilityQualifier.NOT_NULL), + typeParameterBoundsNullability == NullabilityQualifier.NOT_NULL + ) + } + + private fun TypeParameterDescriptor.boundsNullability(): NullabilityQualifier = when { + upperBounds.all(KotlinType::isNullabilityFlexible) -> NullabilityQualifier.FORCE_FLEXIBILITY + upperBounds.any { !it.isNullable() } -> NullabilityQualifier.NOT_NULL + else -> NullabilityQualifier.NULLABLE + } + private fun Annotations.extractNullability(): NullabilityQualifierWithMigrationStatus? = this.firstNotNullResult { extractNullability(it, onlyForJspecify = typeParameterBounds) } @@ -463,7 +499,6 @@ class SignatureEnhancement( isAnyNonNullTypeParameter = isAnyNonNullTypeParameter ) } - } private open class PartEnhancementResult( @@ -516,3 +551,8 @@ private data class TypeAndDefaultQualifiers( val type: KotlinType, val defaultQualifiers: JavaDefaultQualifiers? ) + +private fun KotlinType.isNullabilityFlexible(): Boolean { + val flexibility = unwrap() as? FlexibleType ?: return false + return flexibility.lowerBound.isMarkedNullable != flexibility.upperBound.isMarkedNullable +}