[SLC] LazyAnnotationsBox: drop redundant synchronization

We can use less strict rule for produced annotations: * Previously: the same annotations from `findAnnotation` and `annotations` have the same identity * Now: the same annotations from `findAnnotation` and `annotations` are equals by 'equals' ^KT-56046
2023-01-30 13:26:37 +01:00
parent e1c8b3d674
commit 3b9318bd3a
14 changed files with 257 additions and 51 deletions
@@ -18,8 +18,6 @@ internal class LazyAnnotationsBox(
    private val annotationFilter: AnnotationFilter = AlwaysAllowedAnnotationFilter,
 ) : AnnotationsBox {
    private val annotationsArray: AtomicReference<Array<PsiAnnotation>?> = AtomicReference()
-    private var specialAnnotations: SmartList<PsiAnnotation>? = null
-    private val monitor = Any()

    override fun annotations(owner: PsiModifierList): Array<PsiAnnotation> {
        annotationsArray.get()?.let { return it }
@@ -30,25 +28,13 @@ internal class LazyAnnotationsBox(
            }
        }

-        val valueToReturn = synchronized(monitor) {
-            specialAnnotations?.forEach { specialAnnotation ->
-                val index = annotations.indexOfFirst { it.qualifiedName == specialAnnotation.qualifiedName }
-                if (index != -1) {
-                    annotations[index] = specialAnnotation
-                } else {
-                    annotations += specialAnnotation
-                }
-            }
+        val foundQualifiers = annotations.mapNotNullTo(hashSetOf()) { it.qualifiedName }
+        additionalAnnotationsProvider.addAllAnnotations(annotations, foundQualifiers, owner)

-            val foundQualifiers = annotations.mapNotNullTo(hashSetOf()) { it.qualifiedName }
-            additionalAnnotationsProvider.addAllAnnotations(annotations, foundQualifiers, owner)
-
-            val resultAnnotations = annotationFilter.filtered(annotations)
-            specialAnnotations = null
-            setAnnotationsArray(if (resultAnnotations.isNotEmpty()) resultAnnotations.toTypedArray() else PsiAnnotation.EMPTY_ARRAY)
-        }
-
-        return valueToReturn
+        val resultAnnotations = annotationFilter.filtered(annotations)
+        return setAnnotationsArray(
+            if (resultAnnotations.isNotEmpty()) resultAnnotations.toTypedArray<PsiAnnotation>() else PsiAnnotation.EMPTY_ARRAY
+        )
    }

    private fun setAnnotationsArray(array: Array<PsiAnnotation>): Array<PsiAnnotation> =
@@ -80,29 +66,7 @@ internal class LazyAnnotationsBox(
            null
        }

-        if (specialAnnotation == null) {
-            return annotations(owner).find { it.qualifiedName == qualifiedName }
-        }
-
-        return synchronized(monitor) {
-            annotationsArray.get()?.let { array ->
-                return array.find { it.qualifiedName == qualifiedName }
-            }
-
-            if (specialAnnotations != null) {
-                val specialAnnotations = specialAnnotations!!
-                val oldAnnotation = specialAnnotations.find { it.qualifiedName == specialAnnotation.qualifiedName }
-                if (oldAnnotation != null) {
-                    oldAnnotation
-                } else {
-                    specialAnnotations += specialAnnotation
-                    specialAnnotation
-                }
-            } else {
-                specialAnnotations = SmartList(specialAnnotation)
-                specialAnnotation
-            }
-        }
+        return specialAnnotation ?: annotations(owner).find { it.qualifiedName == qualifiedName }
    }

    override fun hasAnnotation(owner: PsiModifierList, qualifiedName: String): Boolean {
@@ -51,7 +51,10 @@ internal class SymbolLightLazyAnnotation(
    override fun equals(other: Any?): Boolean = this === other ||
            other is SymbolLightLazyAnnotation &&
            other.fqName == fqName &&
-            other.annotationApplication == annotationApplication &&
+            other.annotationApplication.classId == annotationApplication.classId &&
+            other.annotationApplication.index == annotationApplication.index &&
+            other.annotationApplication.useSiteTarget == annotationApplication.useSiteTarget &&
+            other.annotationApplication.isCallWithArguments == annotationApplication.isCallWithArguments &&
            annotationsProvider.isTheSameAs(other.annotationsProvider) &&
            other.parent == parent

@@ -0,0 +1,6 @@
+public final class OverrideMethod /* one.OverrideMethod*/ extends one.AbstractClass {
+  @java.lang.Override()
+  public void foo();//  foo()
+
+  public  OverrideMethod();//  .ctor()
+}
@@ -0,0 +1,13 @@
+// PSI: org.jetbrains.kotlin.light.classes.symbol.methods.SymbolLightSimpleMethod
+// EXPECTED: java.lang.Override
+
+package one
+
+abstract class AbstractClass {
+    abstract fun foo()
+}
+
+class OverrideMethod : AbstractClass() {
+    override fun f<caret>oo() {
+    }
+}
@@ -0,0 +1,5 @@
+public final class TopLevelFunctionWithDeprecatedAnnotationKt /* one.TopLevelFunctionWithDeprecatedAnnotationKt*/ {
+  @kotlin.Deprecated(message = "")
+  @one.Anno()
+  public static final void regularFunction();//  regularFunction()
+}
@@ -0,0 +1,12 @@
+// PSI: org.jetbrains.kotlin.light.classes.symbol.methods.SymbolLightSimpleMethod
+// EXPECTED: kotlin.Deprecated
+// UNEXPECTED: kotlin.jvm.JvmRecord
+// EXPECTED: one.Anno
+// UNEXPECTED: one.Anno2
+
+package one
+
+annotation class Anno
+
+@Deprecated("") @Anno
+fun regul<caret>arFunction() {}
@@ -0,0 +1,4 @@
+public final class TopLevelFunctionWithRegularAnnotationKt /* one.TopLevelFunctionWithRegularAnnotationKt*/ {
+  @one.Anno()
+  public static final void regularFunction();//  regularFunction()
+}
@@ -0,0 +1,10 @@
+// PSI: org.jetbrains.kotlin.light.classes.symbol.methods.SymbolLightSimpleMethod
+// EXPECTED: one.Anno
+// UNEXPECTED: one.Anno2
+
+package one
+
+annotation class Anno
+
+@Anno
+fun regul<caret>arFunction() {}
@@ -0,0 +1,110 @@
+/*
+ * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.light.classes.symbol.base
+
+import com.intellij.openapi.project.Project
+import com.intellij.psi.PsiAnnotation
+import com.intellij.psi.PsiClass
+import com.intellij.psi.PsiMember
+import org.jetbrains.kotlin.analysis.test.framework.services.expressionMarkerProvider
+import org.jetbrains.kotlin.analysis.test.framework.test.configurators.AnalysisApiTestConfigurator
+import org.jetbrains.kotlin.analysis.utils.printer.parentOfType
+import org.jetbrains.kotlin.asJava.renderClass
+import org.jetbrains.kotlin.asJava.toLightElements
+import org.jetbrains.kotlin.psi.KtDeclaration
+import org.jetbrains.kotlin.psi.KtFile
+import org.jetbrains.kotlin.test.builders.TestConfigurationBuilder
+import org.jetbrains.kotlin.test.directives.model.SimpleDirectivesContainer
+import org.jetbrains.kotlin.test.directives.model.singleValue
+import org.jetbrains.kotlin.test.model.TestModule
+import org.jetbrains.kotlin.test.services.TestServices
+import org.jetbrains.kotlin.test.services.assertions
+import java.nio.file.Path
+
+abstract class AbstractSymbolLightClassesAnnotationEqualityTest(
+    configurator: AnalysisApiTestConfigurator,
+    override val currentExtension: String,
+    override val stopIfCompilationErrorDirectivePresent: Boolean,
+) : AbstractSymbolLightClassesTestBase(configurator) {
+    override fun doTestByFileStructure(ktFiles: List<KtFile>, module: TestModule, testServices: TestServices) {
+        val directives = module.directives
+        val expectedAnnotations = directives[Directives.EXPECTED]
+        val unexpectedAnnotations = directives[Directives.UNEXPECTED]
+        val qualifiersToCheck = expectedAnnotations + unexpectedAnnotations
+        testServices.assertions.assertTrue(qualifiersToCheck.isNotEmpty()) { error("Nothing to check") }
+
+        val actualLightDeclaration = findLightDeclaration(ktFiles, module, testServices)
+
+        val annotationsFromFindAnnotation = mutableSetOf<PsiAnnotation>()
+        val modifierList = actualLightDeclaration.modifierList!!
+        for ((qualifier, isExpected) in qualifiersToCheck) {
+            val actual = modifierList.hasAnnotation(qualifier)
+            testServices.assertions.assertEquals(expected = isExpected, actual = actual) {
+                "$qualifier isExpected: $isExpected, but $actual is found"
+            }
+
+            val psiAnnotation = modifierList.findAnnotation(qualifier)
+            if (isExpected) {
+                testServices.assertions.assertNotNull(psiAnnotation)
+            }
+
+            psiAnnotation?.let(annotationsFromFindAnnotation::add)
+        }
+
+        testServices.assertions.assertEquals(expected = expectedAnnotations.size, actual = annotationsFromFindAnnotation.size)
+        val annotations = modifierList.annotations.toList()
+        for (annotation in annotationsFromFindAnnotation) {
+            testServices.assertions.assertContainsElements(collection = annotations, annotation)
+        }
+
+        val unexpectedQualifiers = unexpectedAnnotations.mapTo(hashSetOf(), AnnotationData::qualifierName)
+        for (annotation in annotations) {
+            val qualifiedName = annotation.qualifiedName
+            testServices.assertions.assertTrue(qualifiedName !in unexpectedQualifiers) {
+                "$qualifiedName is unexpected annotation"
+            }
+        }
+
+        compareResults(module, testServices) {
+            val psiClass = actualLightDeclaration.parentOfType<PsiClass>(withSelf = true) ?: error("PsiClass is not found")
+            psiClass.renderClass()
+        }
+    }
+
+    override fun getRenderResult(ktFile: KtFile, ktFiles: List<KtFile>, testDataFile: Path, module: TestModule, project: Project): String {
+        throw UnsupportedOperationException()
+    }
+
+    private fun findLightDeclaration(ktFiles: List<KtFile>, module: TestModule, testServices: TestServices): PsiMember {
+        val directives = module.directives
+        val lightElementClassQualifier = directives.singleValue(Directives.PSI)
+        val declaration = testServices.expressionMarkerProvider.getElementOfTypeAtCaret<KtDeclaration>(ktFiles.first())
+        val lightElements = declaration.toLightElements()
+        val actualLightDeclaration = lightElements.find { it::class.qualifiedName == lightElementClassQualifier }
+            ?: error("$lightElementClassQualifier is not found in ${lightElements.map { it::class.qualifiedName }}")
+
+        return actualLightDeclaration as PsiMember
+    }
+
+    override fun configureTest(builder: TestConfigurationBuilder) {
+        super.configureTest(builder)
+        builder.useDirectives(Directives)
+    }
+
+    private object Directives : SimpleDirectivesContainer() {
+        val EXPECTED by valueDirective(description = "Expected annotation qualifier to check equality") {
+            AnnotationData(qualifierName = it, isExpected = true)
+        }
+
+        val UNEXPECTED by valueDirective(description = "Unexpected annotation qualifier to check equality") {
+            AnnotationData(qualifierName = it, isExpected = false)
+        }
+
+        val PSI by stringDirective(description = "Qualified name of expected light declaration")
+    }
+}
+
+private data class AnnotationData(val qualifierName: String, val isExpected: Boolean)
@@ -65,13 +65,19 @@ abstract class AbstractSymbolLightClassesTestBase(
        val project = ktFile.project

        ignoreExceptionIfIgnoreFirPresent(module) {
-            val actual = getRenderResult(ktFile, ktFiles, testDataPath, module, project).cleanup()
-            compareResults(testServices, actual)
-            removeIgnoreFir(module)
-            removeDuplicatedFirJava(testServices)
+            compareResults(module, testServices) {
+                getRenderResult(ktFile, ktFiles, testDataPath, module, project)
+            }
        }
    }

+    protected fun compareResults(module: TestModule, testServices: TestServices, computeActual: () -> String) {
+        val actual = computeActual().cleanup()
+        compareResults(testServices, actual)
+        removeIgnoreFir(module)
+        removeDuplicatedFirJava(testServices)
+    }
+
    private fun String.cleanup(): String {
        val lines = this.lines().mapTo(mutableListOf()) { it.ifBlank { "" } }
        if (lines.last().isNotBlank()) {
@@ -0,0 +1,15 @@
+/*
+ * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.light.classes.symbol.source
+
+import org.jetbrains.kotlin.analysis.low.level.api.fir.test.configurators.AnalysisApiFirSourceTestConfigurator
+import org.jetbrains.kotlin.light.classes.symbol.base.AbstractSymbolLightClassesAnnotationEqualityTest
+
+abstract class AbstractSymbolLightClassesAnnotationEqualityForSourceTest : AbstractSymbolLightClassesAnnotationEqualityTest(
+    AnalysisApiFirSourceTestConfigurator(analyseInDependentSession = false),
+    EXTENSIONS.FIR_JAVA,
+    stopIfCompilationErrorDirectivePresent = false
+)
@@ -0,0 +1,44 @@
+/*
+ * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.light.classes.symbol.source;
+
+import com.intellij.testFramework.TestDataPath;
+import org.jetbrains.kotlin.test.util.KtTestUtil;
+import org.jetbrains.kotlin.test.TestMetadata;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+
+import java.io.File;
+import java.util.regex.Pattern;
+
+/** This class is generated by {@link org.jetbrains.kotlin.generators.tests.analysis.api.GenerateAnalysisApiTestsKt}. DO NOT MODIFY MANUALLY */
+@SuppressWarnings("all")
+@TestMetadata("analysis/symbol-light-classes/testData/annotationsEquality")
+@TestDataPath("$PROJECT_ROOT")
+public class SymbolLightClassesAnnotationEqualityForSourceTestGenerated extends AbstractSymbolLightClassesAnnotationEqualityForSourceTest {
+    @Test
+    public void testAllFilesPresentInAnnotationsEquality() throws Exception {
+        KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("analysis/symbol-light-classes/testData/annotationsEquality"), Pattern.compile("^(.+)\\.(kt)$"), null, true);
+    }
+
+    @Test
+    @TestMetadata("OverrideMethod.kt")
+    public void testOverrideMethod() throws Exception {
+        runTest("analysis/symbol-light-classes/testData/annotationsEquality/OverrideMethod.kt");
+    }
+
+    @Test
+    @TestMetadata("TopLevelFunctionWithDeprecatedAnnotation.kt")
+    public void testTopLevelFunctionWithDeprecatedAnnotation() throws Exception {
+        runTest("analysis/symbol-light-classes/testData/annotationsEquality/TopLevelFunctionWithDeprecatedAnnotation.kt");
+    }
+
+    @Test
+    @TestMetadata("TopLevelFunctionWithRegularAnnotation.kt")
+    public void testTopLevelFunctionWithRegularAnnotation() throws Exception {
+        runTest("analysis/symbol-light-classes/testData/annotationsEquality/TopLevelFunctionWithRegularAnnotation.kt");
+    }
+}
@@ -1,5 +1,5 @@
 /*
- * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors.
 * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
 */

@@ -90,7 +90,7 @@ class RegisteredDirectivesImpl(
        return buildString {
            simpleDirectives.forEach { appendLine("  $it") }
            stringDirectives.forEach { (d, v) -> appendLine("  $d: ${v.joinToArrayString()}") }
-            valueDirectives.forEach { (d, v) -> appendLine("  $d: ${v.joinToArrayString()}")}
+            valueDirectives.forEach { (d, v) -> appendLine("  $d: ${v.joinToArrayString()}") }
        }
    }

@@ -154,6 +154,10 @@ fun RegisteredDirectives.singleOrZeroValue(directive: StringDirective): String?
    }
 }

+fun RegisteredDirectives.notEmptyValues(directive: StringDirective): List<String> = this[directive].ifEmpty {
+    error("No values passed to $directive")
+}
+
 fun <T : Any> RegisteredDirectives.singleValue(directive: ValueDirective<T>): T {
    return singleOrZeroValue(directive) ?: error("No values passed to $directive")
 }
@@ -166,3 +170,7 @@ fun <T : Any> RegisteredDirectives.singleOrZeroValue(directive: ValueDirective<T
        else -> error("Too many values passed to $directive: ${values.joinToArrayString()}")
    }
 }
+
+fun <T : Any> RegisteredDirectives.notEmptyValues(directive: ValueDirective<T>): List<T> = this[directive].ifEmpty {
+    error("No values passed to $directive")
+}
@@ -1,5 +1,5 @@
 /*
- * Copyright 2010-2022 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors.
 * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
 */

@@ -88,5 +88,11 @@ internal fun TestGroupSuite.generateSymbolLightClassesTests() {
                model("equivalentTo", pattern = TestGeneratorUtil.KT)
            }
        }
+
+        run {
+            testClass<AbstractSymbolLightClassesAnnotationEqualityForSourceTest> {
+                model("annotationsEquality", pattern = TestGeneratorUtil.KT)
+            }
+        }
    }
 }