From d87eddaf8b0d51f8028922e9efba091e8eed7f73 Mon Sep 17 00:00:00 2001 From: "Denis.Zharkov" Date: Mon, 10 Jan 2022 13:18:29 +0300 Subject: [PATCH] Improve warnings related to type parameter bounds nullability Explicitly supply the type parameter that needs to be non-nullified ^KT-36770 Relates --- .../jvm/checkers/JavaNullabilityChecker.kt | 24 +++++++------- ...rideWithWrongNullabilityOverrideChecker.kt | 33 +++++++++++++++---- .../diagnostics/DefaultErrorMessagesJvm.java | 8 +++-- .../resolve/jvm/diagnostics/ErrorsJvm.java | 6 +++- ...rameterWithKotlinNullableWarnings.diag.txt | 15 +++++++++ ...ParameterWithKotlinNullableWarnings.fir.kt | 1 + ...TypeParameterWithKotlinNullableWarnings.kt | 1 + ...linOverridesDefinitelyNonNullable.diag.txt | 12 +++++++ ...otlinOverridesDefinitelyNonNullable.fir.kt | 1 + ...ithKotlinOverridesDefinitelyNonNullable.kt | 9 ++--- ...TypeParameterWithKotlinOverridesWarning.kt | 8 ++--- 11 files changed, 88 insertions(+), 30 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinNullableWarnings.diag.txt create mode 100644 compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesDefinitelyNonNullable.diag.txt diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JavaNullabilityChecker.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JavaNullabilityChecker.kt index 0047400f59a..2958af1c94e 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JavaNullabilityChecker.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JavaNullabilityChecker.kt @@ -19,6 +19,7 @@ package org.jetbrains.kotlin.resolve.jvm.checkers import org.jetbrains.kotlin.cfg.WhenChecker import org.jetbrains.kotlin.config.LanguageFeature import org.jetbrains.kotlin.descriptors.ReceiverParameterDescriptor +import org.jetbrains.kotlin.descriptors.TypeParameterDescriptor import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* @@ -54,9 +55,10 @@ class JavaNullabilityChecker(val upperBoundChecker: UpperBoundChecker) : Additio c.dataFlowValueFactory.createDataFlowValue(expression, expressionType, c) } - if (isWrongTypeParameterNullabilityForSubtyping(expressionType, c) { dataFlowValue }) { - c.trace.report(ErrorsJvm.NULLABLE_TYPE_PARAMETER_AGAINST_NOT_NULL_TYPE_PARAMETER.on(expression, expressionType)) + findTypeParameterWithWrongBoundsNullability(expressionType, c) { dataFlowValue }?.let { typeParameterDescriptor -> + c.trace.report(ErrorsJvm.NULLABLE_TYPE_PARAMETER_AGAINST_NOT_NULL_TYPE_PARAMETER.on(expression, typeParameterDescriptor)) } + doCheckType( expressionType, c.expectedType, @@ -154,15 +156,15 @@ class JavaNullabilityChecker(val upperBoundChecker: UpperBoundChecker) : Additio } } - private fun isWrongTypeParameterNullabilityForSubtyping( + private fun findTypeParameterWithWrongBoundsNullability( expressionType: KotlinType, c: ResolutionContext<*>, dataFlowValueForWholeExpression: () -> DataFlowValue - ): Boolean { - if (c.languageVersionSettings.supportsFeature(LanguageFeature.ProhibitUsingNullableTypeParameterAgainstNotNullAnnotated)) return false - if (TypeUtils.noExpectedType(c.expectedType)) return false + ): TypeParameterDescriptor? { + if (c.languageVersionSettings.supportsFeature(LanguageFeature.ProhibitUsingNullableTypeParameterAgainstNotNullAnnotated)) return null + if (TypeUtils.noExpectedType(c.expectedType)) return null - var metWrongNullabilityInsideArguments = false + var foundSubtypeTypeParameter: TypeParameterDescriptor? = null @OptIn(ClassicTypeCheckerStateInternals::class) val typeState: TypeCheckerState = object : ClassicTypeCheckerState(isErrorTypeEqualsToAnything = true) { @@ -172,7 +174,7 @@ class JavaNullabilityChecker(val upperBoundChecker: UpperBoundChecker) : Additio if (isNullableTypeAgainstNotNullTypeParameter(subType as KotlinType, superType as KotlinType)) { // data flow value is only checked for top-level types if (expectsTypeArgument || c.dataFlowInfo.getStableNullability(dataFlowValueForWholeExpression()) != Nullability.NOT_NULL) { - metWrongNullabilityInsideArguments = true + foundSubtypeTypeParameter = subType.constructor.declarationDescriptor as? TypeParameterDescriptor return false } } @@ -186,7 +188,7 @@ class JavaNullabilityChecker(val upperBoundChecker: UpperBoundChecker) : Additio AbstractTypeChecker.isSubtypeOf(typeState, expressionType, c.expectedType) - return metWrongNullabilityInsideArguments + return foundSubtypeTypeParameter } override fun checkReceiver( @@ -268,10 +270,6 @@ class JavaNullabilityChecker(val upperBoundChecker: UpperBoundChecker) : Additio } companion object { - val typePreparatorUnwrappingEnhancement: KotlinTypePreparator = object : KotlinTypePreparator() { - override fun prepareType(type: KotlinTypeMarker): UnwrappedType = - super.prepareType(type).let { it.getEnhancementDeeply() ?: it }.unwrap() - } val typeCheckerForEnhancedTypes = NewKotlinTypeCheckerImpl( kotlinTypeRefiner = KotlinTypeRefiner.Default, kotlinTypePreparator = object : KotlinTypePreparator() { diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JavaOverrideWithWrongNullabilityOverrideChecker.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JavaOverrideWithWrongNullabilityOverrideChecker.kt index 8b29f9025fa..951a6d75f90 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JavaOverrideWithWrongNullabilityOverrideChecker.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JavaOverrideWithWrongNullabilityOverrideChecker.kt @@ -8,6 +8,7 @@ package org.jetbrains.kotlin.resolve.jvm.checkers import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptor +import org.jetbrains.kotlin.descriptors.TypeParameterDescriptor import org.jetbrains.kotlin.load.java.descriptors.JavaMethodDescriptor import org.jetbrains.kotlin.psi.KtDeclaration import org.jetbrains.kotlin.resolve.OverridingUtil @@ -15,13 +16,15 @@ import org.jetbrains.kotlin.resolve.checkers.DeclarationChecker import org.jetbrains.kotlin.resolve.checkers.DeclarationCheckerContext import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm import org.jetbrains.kotlin.types.* +import org.jetbrains.kotlin.types.checker.KotlinTypePreparator +import org.jetbrains.kotlin.types.model.KotlinTypeMarker +import org.jetbrains.kotlin.utils.addIfNotNull object JavaOverrideWithWrongNullabilityOverrideChecker : DeclarationChecker { - private val overridingUtilWithEnhancementUnwrapped = - OverridingUtil - .createWithTypePreparatorAndCustomSubtype(JavaNullabilityChecker.typePreparatorUnwrappingEnhancement) { subtype, supertype -> - !JavaNullabilityChecker.isNullableTypeAgainstNotNullTypeParameter(subtype, supertype) - } + private val typePreparatorUnwrappingEnhancement: KotlinTypePreparator = object : KotlinTypePreparator() { + override fun prepareType(type: KotlinTypeMarker): UnwrappedType = + super.prepareType(type).let { it.getEnhancementDeeply() ?: it }.unwrap() + } override fun check(declaration: KtDeclaration, descriptor: DeclarationDescriptor, context: DeclarationCheckerContext) { if (descriptor !is CallableMemberDescriptor) return @@ -31,6 +34,18 @@ object JavaOverrideWithWrongNullabilityOverrideChecker : DeclarationChecker { for (overriddenDescriptor in descriptor.overriddenDescriptors) { if (overriddenDescriptor !is JavaMethodDescriptor) continue + + val relatedTypeParameters = mutableSetOf() + val overridingUtilWithEnhancementUnwrapped = + OverridingUtil + .createWithTypePreparatorAndCustomSubtype(typePreparatorUnwrappingEnhancement) { subtype, supertype -> + !JavaNullabilityChecker.isNullableTypeAgainstNotNullTypeParameter(subtype, supertype).also { + if (it) { + relatedTypeParameters.addIfNotNull(subtype.constructor.declarationDescriptor as? TypeParameterDescriptor) + } + } + } + // Skip, if even with enhancement unwrapped, it's still a valid override if (overridingUtilWithEnhancementUnwrapped .isOverridableBy( @@ -50,7 +65,13 @@ object JavaOverrideWithWrongNullabilityOverrideChecker : DeclarationChecker { topLevelType.getEnhancementDeeply() ?: topLevelType })) ?: overriddenDescriptor - context.trace.report(ErrorsJvm.WRONG_NULLABILITY_FOR_JAVA_OVERRIDE.on(declaration, descriptor, unwrappedOverridden)) + if (relatedTypeParameters.isNotEmpty()) { + context.trace.report( + ErrorsJvm.WRONG_TYPE_PARAMETER_NULLABILITY_FOR_JAVA_OVERRIDE.on(declaration, relatedTypeParameters.first()) + ) + } else { + context.trace.report(ErrorsJvm.WRONG_NULLABILITY_FOR_JAVA_OVERRIDE.on(declaration, descriptor, unwrappedOverridden)) + } break } diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java index c0dc0f78d65..3f35cd8a242 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java @@ -102,10 +102,10 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension { "should be subtype of ''{0}'', substituted type is ''{1}''", RENDER_TYPE, RENDER_TYPE, NAME); MAP.put(NULLABLE_TYPE_PARAMETER_AGAINST_NOT_NULL_TYPE_PARAMETER, - "Type mismatch: value of a nullable type {0} is used where non-nullable type is expected. " + + "Type parameter ''{0}'' has nullable upper bounds while non-nullable version is expected. " + "This warning will become an error soon. " + "See https://youtrack.jetbrains.com/issue/KT-36770 for details", - RENDER_TYPE + NAME ); MAP.put(RECEIVER_NULLABILITY_MISMATCH_BASED_ON_JAVA_ANNOTATIONS, "Unsafe use of a nullable receiver of type {0}", RENDER_TYPE); @@ -113,6 +113,10 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension { MAP.put(WRONG_NULLABILITY_FOR_JAVA_OVERRIDE, "Override ''{0}'' has incorrect nullability in its signature comparing with overridden ''{1}''", COMPACT, COMPACT); + MAP.put(WRONG_TYPE_PARAMETER_NULLABILITY_FOR_JAVA_OVERRIDE, + "Type parameter ''{0}'' has nullable upper bound, so override has incorrect signature comparing with a base member with NotNull annotation. " + + "Please add a non-nullable upper bound (e.g. Any) to the type parameter. See https://kotlinlang.org/docs/generics.html#upper-bounds for more details", + NAME); MAP.put(ANNOTATION_TARGETS_NON_EXISTENT_ACCESSOR, "An accessor will not be generated for ''{0}'', so the annotation will not be written to the class file", STRING); diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java index a66d00d7a4e..67029b7f10a 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java @@ -165,7 +165,7 @@ public interface ErrorsJvm { UPPER_BOUND_VIOLATED_IN_TYPEALIAS_EXPANSION_BASED_ON_JAVA_ANNOTATIONS = DiagnosticFactory3.create(WARNING); - DiagnosticFactory1 NULLABLE_TYPE_PARAMETER_AGAINST_NOT_NULL_TYPE_PARAMETER + DiagnosticFactory1 NULLABLE_TYPE_PARAMETER_AGAINST_NOT_NULL_TYPE_PARAMETER = DiagnosticFactory1.create(WARNING); DiagnosticFactory1 RECEIVER_NULLABILITY_MISMATCH_BASED_ON_JAVA_ANNOTATIONS @@ -173,6 +173,10 @@ public interface ErrorsJvm { DiagnosticFactory2 WRONG_NULLABILITY_FOR_JAVA_OVERRIDE = DiagnosticFactory2.create(WARNING, OVERRIDE_MODIFIER); + + DiagnosticFactory1 WRONG_TYPE_PARAMETER_NULLABILITY_FOR_JAVA_OVERRIDE = + DiagnosticFactory1.create(WARNING, OVERRIDE_MODIFIER); + DiagnosticFactory1 ANNOTATION_TARGETS_NON_EXISTENT_ACCESSOR = DiagnosticFactory1.create(WARNING); DiagnosticFactory1 SUSPENSION_POINT_INSIDE_MONITOR = DiagnosticFactory1.create(ERROR); diff --git a/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinNullableWarnings.diag.txt b/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinNullableWarnings.diag.txt new file mode 100644 index 00000000000..bbf835e066e --- /dev/null +++ b/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinNullableWarnings.diag.txt @@ -0,0 +1,15 @@ +/main.kt:22:11: warning: type parameter 'V' has nullable upper bounds while non-nullable version is expected. This warning will become an error soon. See https://youtrack.jetbrains.com/issue/KT-36770 for details + takeV(value) + ^ +/main.kt:23:15: warning: type parameter 'V' has nullable upper bounds while non-nullable version is expected. This warning will become an error soon. See https://youtrack.jetbrains.com/issue/KT-36770 for details + takeVList(l) + ^ +/main.kt:25:11: warning: type parameter 'V' has nullable upper bounds while non-nullable version is expected. This warning will become an error soon. See https://youtrack.jetbrains.com/issue/KT-36770 for details + takeE(value) + ^ +/main.kt:26:15: warning: type parameter 'V' has nullable upper bounds while non-nullable version is expected. This warning will become an error soon. See https://youtrack.jetbrains.com/issue/KT-36770 for details + takeEList(l) + ^ +/main.kt:27:11: warning: type parameter 'V' has nullable upper bounds while non-nullable version is expected. This warning will become an error soon. See https://youtrack.jetbrains.com/issue/KT-36770 for details + takeE(id(value)) + ^ diff --git a/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinNullableWarnings.fir.kt b/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinNullableWarnings.fir.kt index 4f437ea8c63..c9da3698aa1 100644 --- a/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinNullableWarnings.fir.kt +++ b/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinNullableWarnings.fir.kt @@ -1,5 +1,6 @@ // !SKIP_JAVAC // !LANGUAGE: -ProhibitUsingNullableTypeParameterAgainstNotNullAnnotated +// !RENDER_DIAGNOSTICS_FULL_TEXT // FILE: SLRUMap.java import org.jetbrains.annotations.NotNull; diff --git a/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinNullableWarnings.kt b/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinNullableWarnings.kt index f0f37293165..ef82745556f 100644 --- a/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinNullableWarnings.kt +++ b/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinNullableWarnings.kt @@ -1,5 +1,6 @@ // !SKIP_JAVAC // !LANGUAGE: -ProhibitUsingNullableTypeParameterAgainstNotNullAnnotated +// !RENDER_DIAGNOSTICS_FULL_TEXT // FILE: SLRUMap.java import org.jetbrains.annotations.NotNull; diff --git a/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesDefinitelyNonNullable.diag.txt b/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesDefinitelyNonNullable.diag.txt new file mode 100644 index 00000000000..b87f997af6d --- /dev/null +++ b/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesDefinitelyNonNullable.diag.txt @@ -0,0 +1,12 @@ +/main.kt:23:5: warning: type parameter 'X' has nullable upper bound, so override has incorrect signature comparing with a base member with NotNull annotation. Please add a non-nullable upper bound (e.g. Any) to the type parameter. See https://kotlinlang.org/docs/generics.html#upper-bounds for more details + override fun takeV(x: X) + ^ +/main.kt:24:5: warning: type parameter 'E1' has nullable upper bound, so override has incorrect signature comparing with a base member with NotNull annotation. Please add a non-nullable upper bound (e.g. Any) to the type parameter. See https://kotlinlang.org/docs/generics.html#upper-bounds for more details + override fun takeE(e: E1) + ^ +/main.kt:26:5: warning: type parameter 'X' has nullable upper bound, so override has incorrect signature comparing with a base member with NotNull annotation. Please add a non-nullable upper bound (e.g. Any) to the type parameter. See https://kotlinlang.org/docs/generics.html#upper-bounds for more details + override fun takeVList(l: List) + ^ +/main.kt:27:5: warning: type parameter 'E2' has nullable upper bound, so override has incorrect signature comparing with a base member with NotNull annotation. Please add a non-nullable upper bound (e.g. Any) to the type parameter. See https://kotlinlang.org/docs/generics.html#upper-bounds for more details + override fun takeEList(l2: List) + ^ diff --git a/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesDefinitelyNonNullable.fir.kt b/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesDefinitelyNonNullable.fir.kt index 80884bf6695..79cb67340c5 100644 --- a/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesDefinitelyNonNullable.fir.kt +++ b/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesDefinitelyNonNullable.fir.kt @@ -1,6 +1,7 @@ // !SKIP_JAVAC // !LANGUAGE: -ProhibitUsingNullableTypeParameterAgainstNotNullAnnotated // !LANGUAGE: +DefinitelyNonNullableTypes +// !RENDER_DIAGNOSTICS_FULL_TEXT // FILE: SLRUMap.java import org.jetbrains.annotations.NotNull; diff --git a/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesDefinitelyNonNullable.kt b/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesDefinitelyNonNullable.kt index c0ee8734c64..d7f30f66062 100644 --- a/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesDefinitelyNonNullable.kt +++ b/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesDefinitelyNonNullable.kt @@ -1,6 +1,7 @@ // !SKIP_JAVAC // !LANGUAGE: -ProhibitUsingNullableTypeParameterAgainstNotNullAnnotated // !LANGUAGE: +DefinitelyNonNullableTypes +// !RENDER_DIAGNOSTICS_FULL_TEXT // FILE: SLRUMap.java import org.jetbrains.annotations.NotNull; @@ -19,11 +20,11 @@ public interface SLRUMap { // FILE: main.kt interface Q1 : SLRUMap { - override fun takeV(x: X) - override fun takeE(e: E1) + override fun takeV(x: X) + override fun takeE(e: E1) - override fun takeVList(l: List) - override fun takeEList(l2: List) + override fun takeVList(l: List) + override fun takeEList(l2: List) override fun id(k2: K2): K2 } diff --git a/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesWarning.kt b/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesWarning.kt index 92cde3c0d8d..25d8484a3dc 100644 --- a/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesWarning.kt +++ b/compiler/testData/diagnostics/tests/j+k/types/notNullTypeParameterWithKotlinOverridesWarning.kt @@ -18,11 +18,11 @@ public interface SLRUMap { // FILE: main.kt interface Q1 : SLRUMap { - override fun takeV(x: X) - override fun takeE(e: E1) + override fun takeV(x: X) + override fun takeE(e: E1) - override fun takeVList(l: List) - override fun takeEList(l2: List) + override fun takeVList(l: List) + override fun takeEList(l2: List) override fun id(k2: K2): K2 }