From 94402ba4885ccd67d85c0643dfb564b7365e8a71 Mon Sep 17 00:00:00 2001 From: pyos Date: Tue, 7 Sep 2021 12:24:00 +0200 Subject: [PATCH] FE: prefer enhancements for errors over enhancements for warnings --- .../java8Tests/jspecify/warnMode/SelfType.txt | 2 +- .../migration/specialCollision.fir.kt | 35 ------------------- .../migration/specialCollision.kt | 1 + .../migration/specialCollision.txt | 3 +- ...AbstractAnnotationTypeQualifierResolver.kt | 18 +++++++--- .../typeEnhancement/AbstractSignatureParts.kt | 17 ++++----- .../java/typeEnhancement/typeQualifiers.kt | 2 +- 7 files changed, 24 insertions(+), 54 deletions(-) delete mode 100644 compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305NullabilityWarnings/migration/specialCollision.fir.kt diff --git a/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/warnMode/SelfType.txt b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/warnMode/SelfType.txt index 2436f6496f6..be8558b2e1f 100644 --- a/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/warnMode/SelfType.txt +++ b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/warnMode/SelfType.txt @@ -34,7 +34,7 @@ public open class BK : B { public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } -@org.jspecify.nullness.NullMarked public open class C!> : SelfType { +@org.jspecify.nullness.NullMarked public open class C!> : SelfType { public constructor C!>() public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean public open override /*1*/ /*fake_override*/ fun foo(/*0*/ t: E!): kotlin.Unit diff --git a/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305NullabilityWarnings/migration/specialCollision.fir.kt b/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305NullabilityWarnings/migration/specialCollision.fir.kt deleted file mode 100644 index 8f48e137f6f..00000000000 --- a/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305NullabilityWarnings/migration/specialCollision.fir.kt +++ /dev/null @@ -1,35 +0,0 @@ -// !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER -// JSR305_GLOBAL_REPORT: ignore -// JSR305_MIGRATION_REPORT: ignore -// JSR305_SPECIAL_REPORT: MyNonnull:warn, MyMigrationNonnull:strict - -// FILE: A.java -import javax.annotation.*; - -public class A { - @MyMigrationNullable public String field = null; - - @MyMigrationNullable - public String foo(@MyMigrationNonnull String x, CharSequence y) { - return ""; - } - - @MyNonnull - @MyMigrationNonnull - public String bar() { - return ""; - } -} - -// FILE: main.kt -fun main(a: A) { - a.foo("", null)?.length - a.foo("", null).length - a.foo(null, "").length - - a.bar().length - a.bar()!!.length - - a.field?.length - a.field.length -} diff --git a/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305NullabilityWarnings/migration/specialCollision.kt b/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305NullabilityWarnings/migration/specialCollision.kt index 6d8aae90a20..0def2c9bbbd 100644 --- a/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305NullabilityWarnings/migration/specialCollision.kt +++ b/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305NullabilityWarnings/migration/specialCollision.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER // JSR305_GLOBAL_REPORT: ignore // JSR305_MIGRATION_REPORT: ignore diff --git a/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305NullabilityWarnings/migration/specialCollision.txt b/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305NullabilityWarnings/migration/specialCollision.txt index 9835fbefeee..a2db05717b8 100644 --- a/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305NullabilityWarnings/migration/specialCollision.txt +++ b/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305NullabilityWarnings/migration/specialCollision.txt @@ -5,9 +5,10 @@ public fun main(/*0*/ a: A): kotlin.Unit public open class A { public constructor A() @MyMigrationNullable public final var field: kotlin.String! - @MyNonnull @MyMigrationNonnull public open fun bar(): kotlin.String! + @MyNonnull @MyMigrationNonnull public open fun bar(): kotlin.String public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean @MyMigrationNullable public open fun foo(/*0*/ @MyMigrationNonnull x: kotlin.String, /*1*/ y: kotlin.CharSequence!): kotlin.String! public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } + diff --git a/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/AbstractAnnotationTypeQualifierResolver.kt b/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/AbstractAnnotationTypeQualifierResolver.kt index e17aa4910c3..b1a3f84ad2c 100644 --- a/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/AbstractAnnotationTypeQualifierResolver.kt +++ b/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/AbstractAnnotationTypeQualifierResolver.kt @@ -121,17 +121,25 @@ abstract class AbstractAnnotationTypeQualifierResolver( fun extractNullability( annotations: Iterable, forceWarning: TAnnotation.() -> Boolean = { false } ): NullabilityQualifierWithMigrationStatus? = - annotations.firstNotNullOfOrNull { extractNullability(it, forceWarning) } + annotations.fold(null as NullabilityQualifierWithMigrationStatus?) { found, annotation -> + val extracted = extractNullability(annotation, forceWarning) + when { + found == null -> extracted + extracted == null || extracted == found -> found + extracted.isForWarningOnly && !found.isForWarningOnly -> found + !extracted.isForWarningOnly && found.isForWarningOnly -> extracted + else -> return null // inconsistent + } + } - fun extractMutability(annotations: Iterable): MutabilityQualifier? { - return annotations.fold(null as MutabilityQualifier?) { found, annotation -> + fun extractMutability(annotations: Iterable): MutabilityQualifier? = + annotations.fold(null as MutabilityQualifier?) { found, annotation -> when (annotation.fqName) { in READ_ONLY_ANNOTATIONS -> MutabilityQualifier.READ_ONLY in MUTABLE_ANNOTATIONS -> MutabilityQualifier.MUTABLE - else -> found + else -> return@fold found }.also { if (found != null && found != it) return null /* inconsistent */ } } - } private fun extractDefaultQualifiers(annotation: TAnnotation): JavaDefaultQualifiers? { resolveQualifierBuiltInDefaultAnnotation(annotation)?.let { return it } diff --git a/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/typeEnhancement/AbstractSignatureParts.kt b/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/typeEnhancement/AbstractSignatureParts.kt index 90bac36b65c..2119c9ba981 100644 --- a/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/typeEnhancement/AbstractSignatureParts.kt +++ b/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/typeEnhancement/AbstractSignatureParts.kt @@ -135,18 +135,13 @@ abstract class AbstractSignatureParts { a: NullabilityQualifierWithMigrationStatus?, b: NullabilityQualifierWithMigrationStatus? ): NullabilityQualifierWithMigrationStatus? { - if (a == null) return b + if (a == null) return b // null < not null if (b == null) return a - // TODO: this probably behaves really weirdly when some of those are warnings. - if (a.qualifier == NullabilityQualifier.FORCE_FLEXIBILITY) return b - if (b.qualifier == NullabilityQualifier.FORCE_FLEXIBILITY) return a - if (a.qualifier == NullabilityQualifier.NULLABLE) return b - if (b.qualifier == NullabilityQualifier.NULLABLE) return a - assert(a.qualifier == b.qualifier && a.qualifier == NullabilityQualifier.NOT_NULL) { - "Expected everything is NOT_NULL, but $a and $b are found" - } - - return NullabilityQualifierWithMigrationStatus(NullabilityQualifier.NOT_NULL) + if (a.isForWarningOnly && !b.isForWarningOnly) return b // warnings < errors + if (!a.isForWarningOnly && b.isForWarningOnly) return a + if (a.qualifier < b.qualifier) return b // T! < T? < T + if (a.qualifier > b.qualifier) return a + return b // they are equal } private val TypeParameterMarker.boundsNullability: NullabilityQualifierWithMigrationStatus? diff --git a/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/typeEnhancement/typeQualifiers.kt b/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/typeEnhancement/typeQualifiers.kt index 0e1d5bf16db..4b2d889f200 100644 --- a/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/typeEnhancement/typeQualifiers.kt +++ b/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/typeEnhancement/typeQualifiers.kt @@ -6,9 +6,9 @@ package org.jetbrains.kotlin.load.java.typeEnhancement enum class NullabilityQualifier { + FORCE_FLEXIBILITY, NULLABLE, NOT_NULL, - FORCE_FLEXIBILITY } enum class MutabilityQualifier {