From da52716bfdfb55d61abc741c2787ded23df9fe92 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Fri, 20 Oct 2017 09:43:25 +0300 Subject: [PATCH] Fix wrong nullability enhancement for annotated java.lang.Object type Effectively, this commit drops cached value for j.l.Object type This cache was introduced when types were immutable, but they became mutable after starting reading top-level TYPE_USE annotations, that lead to changing shared JAVA_LANG_OBJECT_CLASSIFIER_TYPE instance #KT-20826 Fixed --- .../compiler/KotlinCliJavaFileManagerImpl.kt | 2 +- .../classFiles/BinaryClassSignatureParser.kt | 31 ++++--------------- .../tests/typeUseOnObject.kt | 17 ++++++++++ .../tests/typeUseOnObject.txt | 21 +++++++++++++ ...sNoAnnotationInClasspathTestGenerated.java | 6 ++++ ...pathWithFastClassReadingTestGenerated.java | 6 ++++ .../ForeignJava8AnnotationsTestGenerated.java | 6 ++++ ...cForeignJava8AnnotationsTestGenerated.java | 6 ++++ 8 files changed, 69 insertions(+), 26 deletions(-) create mode 100644 compiler/testData/foreignAnnotationsJava8/tests/typeUseOnObject.kt create mode 100644 compiler/testData/foreignAnnotationsJava8/tests/typeUseOnObject.txt diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinCliJavaFileManagerImpl.kt b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinCliJavaFileManagerImpl.kt index f59b14fe75b..c3f6f7f68d1 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinCliJavaFileManagerImpl.kt +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinCliJavaFileManagerImpl.kt @@ -80,7 +80,7 @@ class KotlinCliJavaFileManagerImpl(private val myPsiManager: PsiManager) : CoreJ private val binaryCache: MutableMap = THashMap() private val signatureParsingComponent = - BinaryClassSignatureParser(ClassifierResolutionContext { findClass(it, allScope) }) + BinaryClassSignatureParser() override fun findClass(classId: ClassId, searchScope: GlobalSearchScope): JavaClass? { val virtualFile = findVirtualFileForTopLevelClass(classId, searchScope) ?: return null diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/load/java/structure/impl/classFiles/BinaryClassSignatureParser.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/load/java/structure/impl/classFiles/BinaryClassSignatureParser.kt index df2eae1b58e..fd5dba6ff93 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/load/java/structure/impl/classFiles/BinaryClassSignatureParser.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/load/java/structure/impl/classFiles/BinaryClassSignatureParser.kt @@ -16,14 +16,11 @@ package org.jetbrains.kotlin.load.java.structure.impl.classFiles -import com.intellij.psi.CommonClassNames import com.intellij.util.containers.ContainerUtil import org.jetbrains.kotlin.builtins.PrimitiveType import org.jetbrains.kotlin.load.java.structure.JavaClassifierType import org.jetbrains.kotlin.load.java.structure.JavaType import org.jetbrains.kotlin.load.java.structure.JavaTypeParameter -import org.jetbrains.kotlin.name.ClassId -import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.utils.addToStdlib.flattenTo import org.jetbrains.kotlin.utils.compact @@ -33,14 +30,11 @@ import java.text.StringCharacterIterator /** * Take a look at com.intellij.psi.impl.compiled.SignatureParsing + * NOTE: currently this class can simply be converted to an object, but there are postponed plans + * to introduce cached instance for java.lang.Object type that would require injected class finder. + * So please, do not convert it to object */ -class BinaryClassSignatureParser(globalContext: ClassifierResolutionContext) { - companion object { - private val JAVA_LANG_OBJECT = ClassId.topLevel(FqName(CommonClassNames.JAVA_LANG_OBJECT)) - } - - private val JAVA_LANG_OBJECT_CLASSIFIER_TYPE: JavaClassifierType = - PlainJavaClassifierType({ globalContext.resolveClass(JAVA_LANG_OBJECT) }, emptyList()) +class BinaryClassSignatureParser { fun parseTypeParametersDeclaration(signature: CharacterIterator, context: ClassifierResolutionContext): List { if (signature.current() != '<') { @@ -68,25 +62,14 @@ class BinaryClassSignatureParser(globalContext: ClassifierResolutionContext) { val parameterName = name.toString() // postpone list allocation till a second bound is seen; ignore sole Object bound - var bounds: MutableList? = null - var jlo = false + val bounds: MutableList = ContainerUtil.newSmartList() while (signature.current() == ':') { signature.next() val bound = parseClassifierRefSignature(signature, context) ?: continue - if (bounds == null) { - if (JAVA_LANG_OBJECT_CLASSIFIER_TYPE === bound) { - jlo = true - continue - } - bounds = ContainerUtil.newSmartList() - if (jlo) { - bounds.add(JAVA_LANG_OBJECT_CLASSIFIER_TYPE) - } - } bounds.add(bound) } - return BinaryJavaTypeParameter(Name.identifier(parameterName), bounds ?: emptyList()) + return BinaryJavaTypeParameter(Name.identifier(parameterName), bounds) } fun parseClassifierRefSignature(signature: CharacterIterator, context: ClassifierResolutionContext): JavaClassifierType? { @@ -148,8 +131,6 @@ class BinaryClassSignatureParser(globalContext: ClassifierResolutionContext) { } signature.next() - if (canonicalName.toString() == "java/lang/Object") return JAVA_LANG_OBJECT_CLASSIFIER_TYPE - return PlainJavaClassifierType( { context.resolveByInternalName(canonicalName.toString()) }, argumentGroups.reversed().flattenTo(arrayListOf()).compact() diff --git a/compiler/testData/foreignAnnotationsJava8/tests/typeUseOnObject.kt b/compiler/testData/foreignAnnotationsJava8/tests/typeUseOnObject.kt new file mode 100644 index 00000000000..5c924aa92a9 --- /dev/null +++ b/compiler/testData/foreignAnnotationsJava8/tests/typeUseOnObject.kt @@ -0,0 +1,17 @@ +// FILE: A.java +import org.checkerframework.checker.nullness.qual.*; +public class A { + @NonNull + public Object foo() { return null; } +} +// FILE: B.java +public class B { + public static void assertNonNull(Object x) {} +} + +// FILE: main.kt + +fun main() { + A() + B.assertNonNull(null) +} diff --git a/compiler/testData/foreignAnnotationsJava8/tests/typeUseOnObject.txt b/compiler/testData/foreignAnnotationsJava8/tests/typeUseOnObject.txt new file mode 100644 index 00000000000..cc792c44726 --- /dev/null +++ b/compiler/testData/foreignAnnotationsJava8/tests/typeUseOnObject.txt @@ -0,0 +1,21 @@ +package + +public fun main(): kotlin.Unit + +public open class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + @org.checkerframework.checker.nullness.qual.NonNull public open fun foo(): @org.checkerframework.checker.nullness.qual.NonNull kotlin.Any + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public open class B { + public constructor B() + 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 + + // Static members + public open fun assertNonNull(/*0*/ x: kotlin.Any!): kotlin.Unit +} 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 4e59e622e7b..77d792fed6f 100644 --- a/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsNoAnnotationInClasspathTestGenerated.java +++ b/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsNoAnnotationInClasspathTestGenerated.java @@ -48,6 +48,12 @@ public class ForeignJava8AnnotationsNoAnnotationInClasspathTestGenerated extends doTest(fileName); } + @TestMetadata("typeUseOnObject.kt") + public void testTypeUseOnObject() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/foreignAnnotationsJava8/tests/typeUseOnObject.kt"); + doTest(fileName); + } + @TestMetadata("compiler/testData/foreignAnnotationsJava8/tests/jsr305") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsNoAnnotationInClasspathWithFastClassReadingTestGenerated.java b/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsNoAnnotationInClasspathWithFastClassReadingTestGenerated.java index 33ac4d93fbf..686b832d928 100644 --- a/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsNoAnnotationInClasspathWithFastClassReadingTestGenerated.java +++ b/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsNoAnnotationInClasspathWithFastClassReadingTestGenerated.java @@ -48,6 +48,12 @@ public class ForeignJava8AnnotationsNoAnnotationInClasspathWithFastClassReadingT doTest(fileName); } + @TestMetadata("typeUseOnObject.kt") + public void testTypeUseOnObject() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/foreignAnnotationsJava8/tests/typeUseOnObject.kt"); + doTest(fileName); + } + @TestMetadata("compiler/testData/foreignAnnotationsJava8/tests/jsr305") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) 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 5822c98f8e3..bfa48c8dd70 100644 --- a/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsTestGenerated.java +++ b/compiler/tests-java8/tests/org/jetbrains/kotlin/checkers/ForeignJava8AnnotationsTestGenerated.java @@ -48,6 +48,12 @@ public class ForeignJava8AnnotationsTestGenerated extends AbstractForeignJava8An doTest(fileName); } + @TestMetadata("typeUseOnObject.kt") + public void testTypeUseOnObject() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/foreignAnnotationsJava8/tests/typeUseOnObject.kt"); + doTest(fileName); + } + @TestMetadata("compiler/testData/foreignAnnotationsJava8/tests/jsr305") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) 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 ab8fa8f1b6d..75399ef9ea4 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 @@ -48,6 +48,12 @@ public class JavacForeignJava8AnnotationsTestGenerated extends AbstractJavacFore doTest(fileName); } + @TestMetadata("typeUseOnObject.kt") + public void testTypeUseOnObject() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/foreignAnnotationsJava8/tests/typeUseOnObject.kt"); + doTest(fileName); + } + @TestMetadata("compiler/testData/foreignAnnotationsJava8/tests/jsr305") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)