From 4882ac65999f50e2e24e93f18b3455e0d9e5272e Mon Sep 17 00:00:00 2001 From: Kirill Rakhman Date: Mon, 20 Nov 2023 10:46:21 +0100 Subject: [PATCH] [FIR] Fix duplicate WRONG_NULLABILITY_FOR_JAVA_OVERRIDE Also, the test shows a missing warning in K1 which is present in K2. #KT-63600 Fixed #KT-63599 #KT-63745 #KT-63746 --- ...nAnnotationsCompiledJavaTestGenerated.java | 6 ++ ...dJavaWithPsiClassReadingTestGenerated.java | 6 ++ ...ignAnnotationsSourceJavaTestGenerated.java | 6 ++ ...irOverrideJavaNullabilityWarningChecker.kt | 11 ++-- .../nullabilityWarnings/override.fir.kt | 64 +++++++++++++++++++ .../jsr305/nullabilityWarnings/override.kt | 64 +++++++++++++++++++ ...nAnnotationsCompiledJavaTestGenerated.java | 6 ++ ...dJavaWithPsiClassReadingTestGenerated.java | 6 ++ ...ignAnnotationsSourceJavaTestGenerated.java | 6 ++ 9 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/override.fir.kt create mode 100644 compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/override.kt diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendForeignAnnotationsCompiledJavaTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendForeignAnnotationsCompiledJavaTestGenerated.java index 7836942a62c..47051963b45 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendForeignAnnotationsCompiledJavaTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendForeignAnnotationsCompiledJavaTestGenerated.java @@ -240,6 +240,12 @@ public class FirPsiOldFrontendForeignAnnotationsCompiledJavaTestGenerated extend runTest("compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/nullabilityNicknames.kt"); } + @Test + @TestMetadata("override.kt") + public void testOverride() throws Exception { + runTest("compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/override.kt"); + } + @Test @TestMetadata("safeCalls.kt") public void testSafeCalls() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendForeignAnnotationsCompiledJavaWithPsiClassReadingTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendForeignAnnotationsCompiledJavaWithPsiClassReadingTestGenerated.java index 55fc4a582f5..dca33df2b12 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendForeignAnnotationsCompiledJavaWithPsiClassReadingTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendForeignAnnotationsCompiledJavaWithPsiClassReadingTestGenerated.java @@ -298,6 +298,12 @@ public class FirPsiOldFrontendForeignAnnotationsCompiledJavaWithPsiClassReadingT runTest("compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/nullabilityNicknames.kt"); } + @Test + @TestMetadata("override.kt") + public void testOverride() throws Exception { + runTest("compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/override.kt"); + } + @Test @TestMetadata("safeCalls.kt") public void testSafeCalls() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendForeignAnnotationsSourceJavaTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendForeignAnnotationsSourceJavaTestGenerated.java index 952454228f8..8e5a2b4ab59 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendForeignAnnotationsSourceJavaTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendForeignAnnotationsSourceJavaTestGenerated.java @@ -298,6 +298,12 @@ public class FirPsiOldFrontendForeignAnnotationsSourceJavaTestGenerated extends runTest("compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/nullabilityNicknames.kt"); } + @Test + @TestMetadata("override.kt") + public void testOverride() throws Exception { + runTest("compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/override.kt"); + } + @Test @TestMetadata("safeCalls.kt") public void testSafeCalls() throws Exception { diff --git a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/declaration/FirOverrideJavaNullabilityWarningChecker.kt b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/declaration/FirOverrideJavaNullabilityWarningChecker.kt index 3b1ea1159de..6c343ea75b8 100644 --- a/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/declaration/FirOverrideJavaNullabilityWarningChecker.kt +++ b/compiler/fir/checkers/checkers.jvm/src/org/jetbrains/kotlin/fir/analysis/jvm/checkers/declaration/FirOverrideJavaNullabilityWarningChecker.kt @@ -35,6 +35,7 @@ object FirOverrideJavaNullabilityWarningChecker : FirAbstractOverrideChecker() { for (member in declaration.declarations) { var anyBaseEnhanced = false + var anyReported = false if (member is FirSimpleFunction) { val enhancedOverrides = scope @@ -44,7 +45,8 @@ object FirOverrideJavaNullabilityWarningChecker : FirAbstractOverrideChecker() { val substitutedBase = it.fir.substituteOrNull(substitutor, context) ?: return@map it anyBaseEnhanced = true - if (!context.session.firOverrideChecker.isOverriddenFunction(member, substitutedBase)) { + if (!anyReported && !context.session.firOverrideChecker.isOverriddenFunction(member, substitutedBase)) { + anyReported = true reporter.reportOn( member.source, FirJvmErrors.WRONG_NULLABILITY_FOR_JAVA_OVERRIDE, @@ -57,7 +59,7 @@ object FirOverrideJavaNullabilityWarningChecker : FirAbstractOverrideChecker() { substitutedBase.symbol } - if (anyBaseEnhanced) { + if (anyBaseEnhanced && !anyReported) { member.symbol.checkReturnType(enhancedOverrides, typeCheckerState, context)?.let { reporter.reportOn( member.source, FirJvmErrors.WRONG_NULLABILITY_FOR_JAVA_OVERRIDE, member.symbol, it, context @@ -72,7 +74,8 @@ object FirOverrideJavaNullabilityWarningChecker : FirAbstractOverrideChecker() { val substitutedBase = it.fir.substituteOrNull(substitutor, context) ?: return@map it anyBaseEnhanced = true - if (!context.session.firOverrideChecker.isOverriddenProperty(member, substitutedBase)) { + if (!anyReported && !context.session.firOverrideChecker.isOverriddenProperty(member, substitutedBase)) { + anyReported = true reporter.reportOn( member.source, FirJvmErrors.WRONG_NULLABILITY_FOR_JAVA_OVERRIDE, @@ -85,7 +88,7 @@ object FirOverrideJavaNullabilityWarningChecker : FirAbstractOverrideChecker() { substitutedBase.symbol } - if (anyBaseEnhanced) { + if (anyBaseEnhanced && !anyReported) { member.symbol.checkReturnType(enhancedOverrides, typeCheckerState, context)?.let { reporter.reportOn( member.source, FirJvmErrors.WRONG_NULLABILITY_FOR_JAVA_OVERRIDE, member.symbol, it, context diff --git a/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/override.fir.kt b/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/override.fir.kt new file mode 100644 index 00000000000..b0e97da4f49 --- /dev/null +++ b/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/override.fir.kt @@ -0,0 +1,64 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER +// JSR305_GLOBAL_REPORT: warn + +// FILE: test/NonNullApi.java +package test; + +import java.lang.annotation.*; +import javax.annotation.Nonnull; +import javax.annotation.meta.TypeQualifierDefault; + +@Target(ElementType.PACKAGE) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Nonnull +@TypeQualifierDefault({ElementType.METHOD, ElementType.PARAMETER, ElementType.PACKAGE}) +public @interface NonNullApi { +} + +// FILE: test/package-info.java +@NonNullApi +package test; + +// FILE: test/Provider.java +package test; + +import javax.annotation.*; + +public interface Provider { + T get(); + @Nullable T getNullable(); + void set(T x); + void setNullable(@Nullable T x); + + T getSet(T x); + @Nullable T getSetNullable(@Nullable T x); +} + +// FILE: test/I.java +package test; + +import javax.annotation.*; + +public interface I { + T get(); + void set(T x); +} + +// FILE: main.kt +package test; + +abstract class Multiple : Provider, I { + override fun get(): T = null!! + override fun set(x: T) {} // Missing warning in K1, K2 get's this right +} + +abstract class A : Provider { + override fun get(): T = null!! + override fun getNullable(): T = null!! + override fun set(x: T) {} // Missing warning in K1, K2 get's this right + override fun setNullable(x: T) {} + + override fun getSet(x: T): T = x + override fun getSetNullable(x: T): T = x +} diff --git a/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/override.kt b/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/override.kt new file mode 100644 index 00000000000..13544ad0b55 --- /dev/null +++ b/compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/override.kt @@ -0,0 +1,64 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER +// JSR305_GLOBAL_REPORT: warn + +// FILE: test/NonNullApi.java +package test; + +import java.lang.annotation.*; +import javax.annotation.Nonnull; +import javax.annotation.meta.TypeQualifierDefault; + +@Target(ElementType.PACKAGE) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Nonnull +@TypeQualifierDefault({ElementType.METHOD, ElementType.PARAMETER, ElementType.PACKAGE}) +public @interface NonNullApi { +} + +// FILE: test/package-info.java +@NonNullApi +package test; + +// FILE: test/Provider.java +package test; + +import javax.annotation.*; + +public interface Provider { + T get(); + @Nullable T getNullable(); + void set(T x); + void setNullable(@Nullable T x); + + T getSet(T x); + @Nullable T getSetNullable(@Nullable T x); +} + +// FILE: test/I.java +package test; + +import javax.annotation.*; + +public interface I { + T get(); + void set(T x); +} + +// FILE: main.kt +package test; + +abstract class Multiple : Provider, I { + override fun get(): T = null!! + override fun set(x: T) {} // Missing warning in K1, K2 get's this right +} + +abstract class A : Provider { + override fun get(): T = null!! + override fun getNullable(): T = null!! + override fun set(x: T) {} // Missing warning in K1, K2 get's this right + override fun setNullable(x: T) {} + + override fun getSet(x: T): T = x + override fun getSetNullable(x: T): T = x +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ForeignAnnotationsCompiledJavaTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ForeignAnnotationsCompiledJavaTestGenerated.java index aac52b9a68c..99894fe4c70 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ForeignAnnotationsCompiledJavaTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ForeignAnnotationsCompiledJavaTestGenerated.java @@ -240,6 +240,12 @@ public class ForeignAnnotationsCompiledJavaTestGenerated extends AbstractForeign runTest("compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/nullabilityNicknames.kt"); } + @Test + @TestMetadata("override.kt") + public void testOverride() throws Exception { + runTest("compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/override.kt"); + } + @Test @TestMetadata("safeCalls.kt") public void testSafeCalls() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ForeignAnnotationsCompiledJavaWithPsiClassReadingTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ForeignAnnotationsCompiledJavaWithPsiClassReadingTestGenerated.java index ea8602228c2..69f10f8a7f3 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ForeignAnnotationsCompiledJavaWithPsiClassReadingTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ForeignAnnotationsCompiledJavaWithPsiClassReadingTestGenerated.java @@ -298,6 +298,12 @@ public class ForeignAnnotationsCompiledJavaWithPsiClassReadingTestGenerated exte runTest("compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/nullabilityNicknames.kt"); } + @Test + @TestMetadata("override.kt") + public void testOverride() throws Exception { + runTest("compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/override.kt"); + } + @Test @TestMetadata("safeCalls.kt") public void testSafeCalls() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ForeignAnnotationsSourceJavaTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ForeignAnnotationsSourceJavaTestGenerated.java index 17bd7f6f469..ad3375e9889 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ForeignAnnotationsSourceJavaTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/ForeignAnnotationsSourceJavaTestGenerated.java @@ -298,6 +298,12 @@ public class ForeignAnnotationsSourceJavaTestGenerated extends AbstractForeignAn runTest("compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/nullabilityNicknames.kt"); } + @Test + @TestMetadata("override.kt") + public void testOverride() throws Exception { + runTest("compiler/testData/diagnostics/foreignAnnotationsTests/tests/jsr305/nullabilityWarnings/override.kt"); + } + @Test @TestMetadata("safeCalls.kt") public void testSafeCalls() throws Exception {