Cleanup: code review fixes

https://jetbrains.team/p/ij/reviews/131139/timeline The annotation name doesn't contain "Data" - AddAnnotationFix.AddConsistentDataCopyVisibilityAnnotationFactory, + AddAnnotationFix.AddConsistentCopyVisibilityAnnotationFactory, I accidentially dropeed wrapping with <html> in 2e5b5df8c6e2830abc21af3f69ddca0a55435c9b. This patch returns it back. Though most probably it doesn't matter. The rendered text looks the same - val htmlMessage = XmlStringUtil.escapeString(message).replace("\n", "<br>") + val htmlMessage = XmlStringUtil.wrapInHtml(XmlStringUtil.escapeString(message).replace("\n", "<br>")) It would be nice to emphasize the nature of those tests (that they are not just about the private constructors in data classes, but about the annotations/copy method consistency) rename from community/plugins/kotlin/idea/tests/testData/quickfix/dataClassPrivateConstructor rename to community/plugins/kotlin/idea/tests/testData/quickfix/dataClassConstructorVsCopyVisibility GitOrigin-RevId: 11e996b008b288750ccdebb91f42f3c9dc843bf6
2026-09-27 10:03:11 +07:00 · 2024-04-16 11:58:29 +00:00
parent fc2094f1fe
commit d5da414213
14 changed files with 28 additions and 46 deletions
@@ -300,11 +300,11 @@ class KotlinK2QuickFixRegistrar : KotlinQuickFixRegistrar() {
        )
        registerPsiQuickFixes(
            KtFirDiagnostic.DataClassCopyVisibilityWillBeChangedWarning::class,
-            AddAnnotationFix.AddConsistentDataCopyVisibilityAnnotationFactory,
+            AddAnnotationFix.AddConsistentCopyVisibilityAnnotationFactory,
        )
        registerPsiQuickFixes(
            KtFirDiagnostic.DataClassCopyVisibilityWillBeChangedError::class,
-            AddAnnotationFix.AddConsistentDataCopyVisibilityAnnotationFactory,
+            AddAnnotationFix.AddConsistentCopyVisibilityAnnotationFactory,
        )
        registerPsiQuickFixes(KtFirDiagnostic.RedundantAnnotation::class, RemoveAnnotationFix)
        registerPsiQuickFixes(KtFirDiagnostic.DataClassConsistentCopyWrongAnnotationTarget::class, RemoveAnnotationFix)
@@ -4636,30 +4636,8 @@ public abstract class HighLevelQuickFixTestGenerated extends AbstractHighLevelQu

    @RunWith(JUnit3RunnerWithInners.class)
    @TestMetadata("../../../idea/tests/testData/quickfix/dataClassPrivateConstructor")
-    public static class DataClassPrivateConstructor extends AbstractHighLevelQuickFixTest {
-        private void runTest(String testDataFilePath) throws Exception {
-            KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
-        }
+    public abstract static class DataClassPrivateConstructor extends AbstractHighLevelQuickFixTest {

-        @TestMetadata("AddConsistentCopyVisibilityAnnotation.kt")
-        public void testAddConsistentCopyVisibilityAnnotation() throws Exception {
-            runTest("../../../idea/tests/testData/quickfix/dataClassPrivateConstructor/AddConsistentCopyVisibilityAnnotation.kt");
-        }
-
-        @TestMetadata("DataClassConsistentCopyAndExposedCopyAreIncompatibleAnnotations.kt")
-        public void testDataClassConsistentCopyAndExposedCopyAreIncompatibleAnnotations() throws Exception {
-            runTest("../../../idea/tests/testData/quickfix/dataClassPrivateConstructor/DataClassConsistentCopyAndExposedCopyAreIncompatibleAnnotations.kt");
-        }
-
-        @TestMetadata("DataClassConsistentCopyWrongAnnotationTarget.kt")
-        public void testDataClassConsistentCopyWrongAnnotationTarget() throws Exception {
-            runTest("../../../idea/tests/testData/quickfix/dataClassPrivateConstructor/DataClassConsistentCopyWrongAnnotationTarget.kt");
-        }
-
-        @TestMetadata("RemoveRedundantConsistentCopyVisibilityAnnotation.kt")
-        public void testRemoveRedundantConsistentCopyVisibilityAnnotation() throws Exception {
-            runTest("../../../idea/tests/testData/quickfix/dataClassPrivateConstructor/RemoveRedundantConsistentCopyVisibilityAnnotation.kt");
-        }
    }


@@ -7,10 +7,12 @@ import com.intellij.psi.PsiElementVisitor
 import org.jetbrains.kotlin.config.LanguageFeature
 import org.jetbrains.kotlin.config.doesDataClassCopyRespectConstructorVisibility
 import org.jetbrains.kotlin.idea.base.projectStructure.languageVersionSettings
+import org.jetbrains.kotlin.idea.base.psi.KotlinPsiHeuristics
 import org.jetbrains.kotlin.idea.base.resources.KotlinBundle
 import org.jetbrains.kotlin.idea.codeinsight.api.classic.inspections.AbstractKotlinInspection
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.name.StandardClassIds
+import org.jetbrains.kotlin.psi.KtClass
 import org.jetbrains.kotlin.psi.primaryConstructorVisitor
 import org.jetbrains.kotlin.psi.psiUtil.containingClass
 import org.jetbrains.kotlin.psi.psiUtil.isPrivate
@@ -28,15 +30,8 @@ internal class DataClassPrivateConstructorInspection : AbstractKotlinInspection(
        return primaryConstructorVisitor { constructor ->
            val containingClass = constructor.containingClass()
            if (containingClass?.isData() == true && constructor.isPrivate()) {
-                val isAnnotated = containingClass.annotationEntries.any {
-                    // A more correct solution is to resolve the annotation to see if it's FQN matches FQNs of annotations from stdlib.
-                    // But it's too much hassle. This inspection will be eventually dropped anyway.
-                    // "ConsistentCopyVisibility" and "ExposedCopyVisibility" are unique enough names.
-                    // And false negatives are not a big deal in case of annotation name collision
-                    it.shortName == StandardClassIds.Annotations.ConsistentCopyVisibility.shortClassName ||
-                            it.shortName == StandardClassIds.Annotations.ExposedCopyVisibility.shortClassName
-                }
-                if (isAnnotated) {
+
+                if (isAnnotatedWithMigrationAnnotations(containingClass)) {
                    return@primaryConstructorVisitor
                }
                val keyword = constructor.modifierList?.getModifier(KtTokens.PRIVATE_KEYWORD) ?: return@primaryConstructorVisitor
@@ -52,4 +47,13 @@ internal class DataClassPrivateConstructorInspection : AbstractKotlinInspection(
            }
        }
    }
+}
+
+private fun isAnnotatedWithMigrationAnnotations(containingClass: KtClass): Boolean {
+    // A more correct solution is to resolve the annotation to see if it's FQN matches FQNs of annotations from stdlib.
+    // But it's too much hassle. This inspection will be eventually dropped anyway.
+    // "ConsistentCopyVisibility" and "ExposedCopyVisibility" are unique enough names.
+    // And false negatives are not a big deal in case of annotation name collision
+    return KotlinPsiHeuristics.hasAnnotation(containingClass, StandardClassIds.Annotations.ConsistentCopyVisibility.shortClassName) ||
+            KotlinPsiHeuristics.hasAnnotation(containingClass, StandardClassIds.Annotations.ExposedCopyVisibility.shortClassName)
 }
@@ -7,6 +7,7 @@ import com.intellij.openapi.editor.Editor
 import com.intellij.openapi.project.Project
 import com.intellij.psi.PsiElement
 import com.intellij.psi.SmartPsiElementPointer
+import com.intellij.psi.util.parents
 import org.jetbrains.kotlin.builtins.StandardNames
 import org.jetbrains.kotlin.idea.base.codeInsight.ShortenReferencesFacility
 import org.jetbrains.kotlin.idea.base.resources.KotlinBundle
@@ -18,9 +19,7 @@ import org.jetbrains.kotlin.name.ClassId
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.name.StandardClassIds
 import org.jetbrains.kotlin.psi.*
-import org.jetbrains.kotlin.psi.psiUtil.parentsWithSelf
 import org.jetbrains.kotlin.renderer.render
-import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstance
 import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull

 open class AddAnnotationFix(
@@ -73,10 +72,10 @@ open class AddAnnotationFix(
        }
    }

-    object AddConsistentDataCopyVisibilityAnnotationFactory : QuickFixesPsiBasedFactory<PsiElement>(PsiElement::class, PsiElementSuitabilityCheckers.ALWAYS_SUITABLE) {
+    object AddConsistentCopyVisibilityAnnotationFactory : QuickFixesPsiBasedFactory<PsiElement>(PsiElement::class, PsiElementSuitabilityCheckers.ALWAYS_SUITABLE) {
        override fun doCreateQuickFix(psiElement: PsiElement): List<IntentionAction> {
-            val clazz = psiElement.parentsWithSelf.firstIsInstanceOrNull<KtClass>() ?: return emptyList()
-            return listOf(AddAnnotationFix(clazz, StandardClassIds.Annotations.ConsistentCopyVisibility))
+            val containingClass = psiElement.parents(withSelf = true).firstIsInstanceOrNull<KtClass>() ?: return emptyList()
+            return listOf(AddAnnotationFix(containingClass, StandardClassIds.Annotations.ConsistentCopyVisibility))
        }
    }
 }
@@ -15,6 +15,7 @@ import com.intellij.openapi.util.NlsSafe
 import com.intellij.openapi.util.TextRange
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiFile
+import com.intellij.util.containers.toArray
 import com.intellij.xml.util.XmlStringUtil
 import org.jetbrains.kotlin.analysis.api.KtAnalysisSession
 import org.jetbrains.kotlin.analysis.api.analyze
@@ -87,7 +88,7 @@ class KotlinDiagnosticHighlightVisitor : HighlightVisitor {
        } else null

        val message = diagnostic.getMessageToRender()
-        val htmlMessage = XmlStringUtil.escapeString(message).replace("\n", "<br>")
+        val htmlMessage = XmlStringUtil.wrapInHtml(XmlStringUtil.escapeString(message).replace("\n", "<br>"))
        val infoBuilder = HighlightInfo.newHighlightInfo(diagnostic.getHighlightInfoType())
            .escapedToolTip(htmlMessage)
            .description(message)
@@ -6799,30 +6799,30 @@ public abstract class K1QuickFixTestGenerated extends AbstractK1QuickFixTest {
    }

    @RunWith(JUnit3RunnerWithInners.class)
-    @TestMetadata("testData/quickfix/dataClassPrivateConstructor")
-    public static class DataClassPrivateConstructor extends AbstractK1QuickFixTest {
+    @TestMetadata("testData/quickfix/dataClassConstructorVsCopyVisibility")
+    public static class DataClassConstructorVsCopyVisibility extends AbstractK1QuickFixTest {
        private void runTest(String testDataFilePath) throws Exception {
            KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
        }

        @TestMetadata("AddConsistentCopyVisibilityAnnotation.kt")
        public void testAddConsistentCopyVisibilityAnnotation() throws Exception {
-            runTest("testData/quickfix/dataClassPrivateConstructor/AddConsistentCopyVisibilityAnnotation.kt");
+            runTest("testData/quickfix/dataClassConstructorVsCopyVisibility/AddConsistentCopyVisibilityAnnotation.kt");
        }

        @TestMetadata("DataClassConsistentCopyAndExposedCopyAreIncompatibleAnnotations.kt")
        public void testDataClassConsistentCopyAndExposedCopyAreIncompatibleAnnotations() throws Exception {
-            runTest("testData/quickfix/dataClassPrivateConstructor/DataClassConsistentCopyAndExposedCopyAreIncompatibleAnnotations.kt");
+            runTest("testData/quickfix/dataClassConstructorVsCopyVisibility/DataClassConsistentCopyAndExposedCopyAreIncompatibleAnnotations.kt");
        }

        @TestMetadata("DataClassConsistentCopyWrongAnnotationTarget.kt")
        public void testDataClassConsistentCopyWrongAnnotationTarget() throws Exception {
-            runTest("testData/quickfix/dataClassPrivateConstructor/DataClassConsistentCopyWrongAnnotationTarget.kt");
+            runTest("testData/quickfix/dataClassConstructorVsCopyVisibility/DataClassConsistentCopyWrongAnnotationTarget.kt");
        }

        @TestMetadata("RemoveRedundantConsistentCopyVisibilityAnnotation.kt")
        public void testRemoveRedundantConsistentCopyVisibilityAnnotation() throws Exception {
-            runTest("testData/quickfix/dataClassPrivateConstructor/RemoveRedundantConsistentCopyVisibilityAnnotation.kt");
+            runTest("testData/quickfix/dataClassConstructorVsCopyVisibility/RemoveRedundantConsistentCopyVisibilityAnnotation.kt");
        }
    }