From fa6285d32a2650aee6a9ad43a7121157a44d413d Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Thu, 14 Dec 2017 14:29:47 +0300 Subject: [PATCH] Fix NPE in JavaNullabilityChecker when checking equals-calls This NPE was introduced in ce41b5745a7c67dfcaa4a0d341e18f37c0364572 Prior to the latter change there was a synthetic PSI element for each equals-related call, thus callOperationNode was not null here. == are intentionally treated as safe calls, but for nullability checker it's not relevant, it only should report warnings on real safe-calls --- .../jvm/checkers/JavaNullabilityChecker.kt | 3 +- .../typeQualifierDefault/equalsOnNonNull.kt | 49 +++++++++++++++++++ .../typeQualifierDefault/equalsOnNonNull.txt | 21 ++++++++ ...sNoAnnotationInClasspathTestGenerated.java | 6 +++ ...pathWithFastClassReadingTestGenerated.java | 6 +++ .../ForeignAnnotationsTestGenerated.java | 6 +++ .../JavacForeignAnnotationsTestGenerated.java | 6 +++ 7 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/equalsOnNonNull.kt create mode 100644 compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/equalsOnNonNull.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 e4590b99eb2..cc5d09b759d 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 @@ -109,8 +109,9 @@ class JavaNullabilityChecker : AdditionalTypeChecker { } if (safeAccess) { + val safeAccessElement = c.call.callOperationNode?.psi ?: return doIfNotNull(receiverArgument.type, { dataFlowValue }, c) { - c.trace.report(Errors.UNNECESSARY_SAFE_CALL.on(c.call.callOperationNode!!.psi, receiverArgument.type)) + c.trace.report(Errors.UNNECESSARY_SAFE_CALL.on(safeAccessElement, receiverArgument.type)) } return diff --git a/compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/equalsOnNonNull.kt b/compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/equalsOnNonNull.kt new file mode 100644 index 00000000000..7a293e23b98 --- /dev/null +++ b/compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/equalsOnNonNull.kt @@ -0,0 +1,49 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER +// JSR305_GLOBAL_REPORT warn + +// FILE: spr/NonNullApi.java + +package spr; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import javax.annotation.Nonnull; +import javax.annotation.meta.TypeQualifierDefault; + +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Nonnull +@TypeQualifierDefault({ElementType.METHOD, ElementType.PARAMETER}) +public @interface NonNullApi { +} + +// FILE: A.java + +import spr.*; + +@NonNullApi +public class A { + public String getBar() { + return ""; + } +} + +// FILE: main.kt + +fun main(a: A) { + if (a.getBar() == "1" && a.hashCode() != 0) return + if (a.getBar() != "2" && a.hashCode() != 0) return + + if (a.getBar() == null && a.hashCode() != 0) return + if (a.getBar() != null && a.hashCode() != 0) return + + if (a.bar == "1" && a.hashCode() != 0) return + if (a.bar != "2" && a.hashCode() != 0) return + + if (a.bar == null && a.hashCode() != 0) return + if (a.bar != null && a.hashCode() != 0) return +} diff --git a/compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/equalsOnNonNull.txt b/compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/equalsOnNonNull.txt new file mode 100644 index 00000000000..98a8f2a2731 --- /dev/null +++ b/compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/equalsOnNonNull.txt @@ -0,0 +1,21 @@ +package + +public fun main(/*0*/ a: A): kotlin.Unit + +@spr.NonNullApi public open class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open fun getBar(): kotlin.String! + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +package spr { + + @kotlin.annotation.Target(allowedTargets = {AnnotationTarget.CLASS, AnnotationTarget.FILE}) @kotlin.annotation.Retention(value = AnnotationRetention.RUNTIME) @kotlin.annotation.MustBeDocumented @javax.annotation.Nonnull @javax.annotation.meta.TypeQualifierDefault(value = {ElementType.METHOD, ElementType.PARAMETER}) public final annotation class NonNullApi : kotlin.Annotation { + public constructor NonNullApi() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + 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/org/jetbrains/kotlin/checkers/ForeignAnnotationsNoAnnotationInClasspathTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/ForeignAnnotationsNoAnnotationInClasspathTestGenerated.java index e97b4b1e3dc..64ff45a2d22 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/ForeignAnnotationsNoAnnotationInClasspathTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/ForeignAnnotationsNoAnnotationInClasspathTestGenerated.java @@ -294,6 +294,12 @@ public class ForeignAnnotationsNoAnnotationInClasspathTestGenerated extends Abst KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); } + @TestMetadata("equalsOnNonNull.kt") + public void testEqualsOnNonNull() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/equalsOnNonNull.kt"); + doTest(fileName); + } + @TestMetadata("fieldsAreNullable.kt") public void testFieldsAreNullable() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/fieldsAreNullable.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/ForeignAnnotationsNoAnnotationInClasspathWithFastClassReadingTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/ForeignAnnotationsNoAnnotationInClasspathWithFastClassReadingTestGenerated.java index 61f8851a3de..23c90871576 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/ForeignAnnotationsNoAnnotationInClasspathWithFastClassReadingTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/ForeignAnnotationsNoAnnotationInClasspathWithFastClassReadingTestGenerated.java @@ -294,6 +294,12 @@ public class ForeignAnnotationsNoAnnotationInClasspathWithFastClassReadingTestGe KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); } + @TestMetadata("equalsOnNonNull.kt") + public void testEqualsOnNonNull() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/equalsOnNonNull.kt"); + doTest(fileName); + } + @TestMetadata("fieldsAreNullable.kt") public void testFieldsAreNullable() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/fieldsAreNullable.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/ForeignAnnotationsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/ForeignAnnotationsTestGenerated.java index 0389aed9adb..611c700d4ed 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/ForeignAnnotationsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/ForeignAnnotationsTestGenerated.java @@ -294,6 +294,12 @@ public class ForeignAnnotationsTestGenerated extends AbstractForeignAnnotationsT KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); } + @TestMetadata("equalsOnNonNull.kt") + public void testEqualsOnNonNull() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/equalsOnNonNull.kt"); + doTest(fileName); + } + @TestMetadata("fieldsAreNullable.kt") public void testFieldsAreNullable() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/fieldsAreNullable.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/JavacForeignAnnotationsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/JavacForeignAnnotationsTestGenerated.java index 107975b9c13..e4a9dd4b0e8 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/JavacForeignAnnotationsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/JavacForeignAnnotationsTestGenerated.java @@ -294,6 +294,12 @@ public class JavacForeignAnnotationsTestGenerated extends AbstractJavacForeignAn KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); } + @TestMetadata("equalsOnNonNull.kt") + public void testEqualsOnNonNull() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/equalsOnNonNull.kt"); + doTest(fileName); + } + @TestMetadata("fieldsAreNullable.kt") public void testFieldsAreNullable() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/foreignAnnotations/tests/jsr305/nullabilityWarnings/typeQualifierDefault/fieldsAreNullable.kt");