Rewrite GradleDaemonAnalyzerTest

Previously, GradleDaemonAnalyzer reused common IJ infrastructure for
highlighting tests (see DaemonAnalyzerTеstCase, ExpectedHighlightingData
and such).

Unfortunately, this infrastructure had several flaws,
mainly around checking expected highlighting against actual one:

- overlapping line markers lead to crash (fixed in 193+)

- no way to sanitize descriptions of line markers (important for cases
where description returns HTML-formatted text, which makes testdata
completely unreadable and also drives parser insane)

- thrown FileNotFoundException doesn't have a physical file with
expected testdata attached, which makes browsing diff a little less
convenient (no way to apply changes to expected file)

- no easy way to plug-in with additional after-highlighting checks

This commit fixes it by overriding doCheckResult and providing custom
checking of highlighting/line markers, based on TagsTestDataUtil.
Because we don't rely on IJ-checking anymore, we also remove weird hoops
with removing/adding testdata markup in checkFiles.
Also this commit adds strings sanitization in TagsTestDataUtil, removing HTML-tags
and line breaks, so that description of tag is always one-liner with
plain text.

compiler/tests-common/tests/org/jetbrains/kotlin/test/TagsTestDataUtil.java

@@ -21,28 +21,23 @@ import com.intellij.codeInsight.daemon.LineMarkerInfo;
 import com.intellij.codeInsight.daemon.impl.HighlightInfo;
 import com.intellij.lang.annotation.HighlightSeverity;
 import com.intellij.openapi.editor.Editor;
+import com.intellij.openapi.util.text.StringUtil;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.stream.Collectors;
 
 public class TagsTestDataUtil {
     public static String insertInfoTags(List<LineMarkerInfo> lineMarkers, boolean withDescription, String text) {
-        List<LineMarkerTagPoint> lineMarkerPoints = Lists.newArrayList();
+        List<LineMarkerTagPoint> lineMarkerPoints = toLineMarkerTagPoints(lineMarkers, withDescription);
-        for (LineMarkerInfo markerInfo : lineMarkers) {
-            lineMarkerPoints.add(new LineMarkerTagPoint(markerInfo.startOffset, true, markerInfo, withDescription));
-            lineMarkerPoints.add(new LineMarkerTagPoint(markerInfo.endOffset, false, markerInfo, withDescription));
-        }
 
         return insertTagsInText(lineMarkerPoints, text);
     }
 
     public static String insertInfoTags(List<HighlightInfo> highlights, String text) {
-        List<HighlightTagPoint> highlightPoints = Lists.newArrayList();
+        List<HighlightTagPoint> highlightPoints = toHighlightTagPoints(highlights);
-        for (HighlightInfo highlight : highlights) {
-            highlightPoints.add(new HighlightTagPoint(highlight.startOffset, true, highlight));
-            highlightPoints.add(new HighlightTagPoint(highlight.endOffset, false, highlight));
-        }
 
         return insertTagsInText(highlightPoints, text);
     }
@@ -88,6 +83,29 @@ public class TagsTestDataUtil {
         return builder.toString();
     }
 
+    @NotNull
+    public static List<LineMarkerTagPoint> toLineMarkerTagPoints(
+            Collection<LineMarkerInfo> lineMarkers,
+            boolean withDescription
+    ) {
+        List<LineMarkerTagPoint> lineMarkerPoints = Lists.newArrayList();
+        for (LineMarkerInfo markerInfo : lineMarkers) {
+            lineMarkerPoints.add(new LineMarkerTagPoint(markerInfo.startOffset, true, markerInfo, withDescription));
+            lineMarkerPoints.add(new LineMarkerTagPoint(markerInfo.endOffset, false, markerInfo, withDescription));
+        }
+        return lineMarkerPoints;
+    }
+
+    @NotNull
+    public static List<HighlightTagPoint> toHighlightTagPoints(Collection<HighlightInfo> highlights) {
+        List<HighlightTagPoint> highlightPoints = Lists.newArrayList();
+        for (HighlightInfo highlight : highlights) {
+            highlightPoints.add(new HighlightTagPoint(highlight.startOffset, true, highlight));
+            highlightPoints.add(new HighlightTagPoint(highlight.endOffset, false, highlight));
+        }
+        return highlightPoints;
+    }
+
     public static class TagInfo<Data> implements Comparable<TagInfo<?>> {
         protected final int offset;
         protected final boolean isStart;
@@ -144,7 +162,7 @@ public class TagsTestDataUtil {
         }
     }
 
-    private static class HighlightTagPoint extends TagInfo<HighlightInfo> {
+    public static class HighlightTagPoint extends TagInfo<HighlightInfo> {
         private final HighlightInfo highlightInfo;
 
         private HighlightTagPoint(int offset, boolean start, HighlightInfo info) {
@@ -165,9 +183,10 @@ public class TagsTestDataUtil {
         public String getAttributesString() {
             if (isStart) {
                 if (highlightInfo.getDescription() != null) {
-                    return String.format("textAttributesKey=\"%s\" descr=%s",
+                    return String.format("descr=\"%s\" textAttributesKey=\"%s\"",
-                                         highlightInfo.forcedTextAttributesKey,
+                                         sanitizeLineBreaks(highlightInfo.getDescription()),
-                                         highlightInfo.getDescription());
+                                         highlightInfo.forcedTextAttributesKey
+                    );
                 }
                 else {
                     return String.format("textAttributesKey=\"%s\"", highlightInfo.forcedTextAttributesKey);
@@ -179,7 +198,7 @@ public class TagsTestDataUtil {
         }
     }
 
-    private static class LineMarkerTagPoint extends TagInfo<LineMarkerInfo> {
+    public static class LineMarkerTagPoint extends TagInfo<LineMarkerInfo> {
         private final boolean withDescription;
 
         public LineMarkerTagPoint(int offset, boolean start, LineMarkerInfo info, boolean withDescription) {
@@ -196,7 +215,17 @@ public class TagsTestDataUtil {
         @NotNull
         @Override
         public String getAttributesString() {
-            return withDescription ? String.format("descr=\"%s\"", data.getLineMarkerTooltip()) : "descr=\"*\"";
+            return withDescription ? String.format("descr=\"%s\"", sanitizeLineMarkerTooltip(data.getLineMarkerTooltip())) : "descr=\"*\"";
         }
     }
+
+    private static @NotNull String sanitizeLineMarkerTooltip(@Nullable String originalText) {
+        if (originalText == null) return "null";
+        String noHtmlTags = StringUtil.removeHtmlTags(originalText);
+        return sanitizeLineBreaks(noHtmlTags);
+    }
+
+    private static String sanitizeLineBreaks(String originalText) {
+        return StringUtil.replace(originalText, "\n", " ");
+    }
 }

idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/GradleMultiplatformHighlightingTest.kt

@@ -8,19 +8,31 @@ package org.jetbrains.kotlin.gradle
 import com.intellij.codeInsight.EditorInfo
 import com.intellij.codeInsight.daemon.DaemonAnalyzerTestCase
 import com.intellij.codeInsight.daemon.DaemonCodeAnalyzer
+import com.intellij.codeInsight.daemon.impl.DaemonCodeAnalyzerImpl
+import com.intellij.codeInsight.daemon.impl.HighlightInfo
+import com.intellij.lang.annotation.HighlightSeverity
 import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.editor.Editor
 import com.intellij.openapi.editor.impl.EditorImpl
 import com.intellij.openapi.fileEditor.FileEditorManager
 import com.intellij.openapi.fileEditor.OpenFileDescriptor
 import com.intellij.openapi.project.Project
+import com.intellij.openapi.vfs.VfsUtil
 import com.intellij.openapi.vfs.VirtualFile
 import com.intellij.psi.PsiDocumentManager
+import com.intellij.psi.PsiFile
+import com.intellij.testFramework.ExpectedHighlightingData
+import com.intellij.testFramework.VfsTestUtil
 import com.intellij.testFramework.runInEdtAndWait
+import junit.framework.TestCase
 import org.jetbrains.kotlin.idea.codeInsight.gradle.GradleImportingTestCase
+import org.jetbrains.kotlin.test.KotlinTestUtils
+import org.jetbrains.kotlin.test.TagsTestDataUtil
+import org.jetbrains.kotlin.test.TagsTestDataUtil.TagInfo
 import org.jetbrains.plugins.gradle.tooling.annotation.TargetVersions
 import org.junit.Assert
 import org.junit.Test
+import java.io.File
 
 class GradleMultiplatformHighlightingTest : GradleImportingTestCase() {
 
@@ -62,9 +74,47 @@ abstract class GradleDaemonAnalyzerTestCase(
     fun checkHighlighting(project: Project, editor: Editor) {
         myProject = project
         runInEdtAndWait {
+            // This will prepare ExpectedHighlightingData, clear current editor from testing tags, and then
+            // call doCheckResult, which we override
             checkHighlighting(editor, checkWarnings, checkInfos)
         }
     }
+
+    // We have to override doCheckResult because one from DaemonAnalyzerTestCase has few flaws:
+    // - overlapping line markers lead to crash in 193 (fixed in later versions)
+    // - it checks line markers and rest of highlighting *separately*, which means that we can't put both in
+    //   expected test data
+    // - no API to sanitize error descriptions (in particular, we have to remove endlines in diagnostic messages,
+    //   otherwise parser in testdata goes completely insane)
+    override fun doCheckResult(data: ExpectedHighlightingData, infos: MutableCollection<HighlightInfo>, text: String) {
+        performGenericHighlightingAndLineMarkersChecks(infos, text)
+        performAdditionalChecksAfterHighlighting(editor)
+    }
+
+    private fun performGenericHighlightingAndLineMarkersChecks(infos: Collection<HighlightInfo>, text: String) {
+        val lineMarkersTags: List<TagInfo<*>> = if (testLineMarkers) {
+            TagsTestDataUtil.toLineMarkerTagPoints(
+                DaemonCodeAnalyzerImpl.getLineMarkers(getDocument(file), project),
+                /* withDescription = */ true
+            )
+        } else {
+            emptyList()
+        }
+
+        val filteredHighlights = infos.filterNot {
+            it.severity == HighlightSeverity.INFORMATION && !checkInfos ||
+                    it.severity == HighlightSeverity.WARNING && !checkWarnings
+        }
+        val highlightsTags: List<TagInfo<*>> = TagsTestDataUtil.toHighlightTagPoints(filteredHighlights)
+
+        val allTags = lineMarkersTags + highlightsTags
+        val actualTextWithTags = TagsTestDataUtil.insertTagsInText(allTags, text)
+
+        val physicalFileWithExpectedTestData = file.testDataFileByUserData
+        KotlinTestUtils.assertEqualsToFile(physicalFileWithExpectedTestData, actualTextWithTags)
+    }
+
+    protected open fun performAdditionalChecksAfterHighlighting(editor: Editor) { }
 }
 
 internal fun checkFiles(
@@ -73,17 +123,21 @@ internal fun checkFiles(
     analyzer: GradleDaemonAnalyzerTestCase
 ) {
     var atLeastOneFile = false
-    val content = mutableMapOf<VirtualFile, String>()
+
-    files.forEach { file ->
+    // We have to first load all files into the project and only then start
-        val (_, textWithTags) = configureEditorByExistingFile(file, project)
+    // highlighting, otherwise cross-file references might not work
+    val editors = files.map { file ->
         atLeastOneFile = true
-        content[file] = textWithTags
+
+        val originalText= VfsUtil.loadText(file)
+        val textWithoutTags = textWithoutTags(originalText)
+
+        configureEditorByExistingFile(file, project, textWithoutTags)
     }
+
+    editors.forEach { analyzer.checkHighlighting(project, it) }
+
     Assert.assertTrue(atLeastOneFile)
-    files.forEach { file ->
-        val (editor, _) = configureEditorByExistingFile(file, project, content[file])
-        analyzer.checkHighlighting(project, editor)
-    }
 }
 
 internal fun textWithoutTags(text: String): String {
@@ -104,24 +158,43 @@ private fun createEditor(file: VirtualFile, project: Project): Editor {
 internal fun configureEditorByExistingFile(
     virtualFile: VirtualFile,
     project: Project,
-    contentToSet: String? = null
+    contentToSet: String
-): Pair<Editor, String> {
+): Editor {
-    var result: Pair<Editor, String>? = null
+    var result: Editor? = null
     runInEdtAndWait {
         val editor = createEditor(virtualFile, project)
         val document = editor.document
         val editorInfo = EditorInfo(document.text)
-        val textWithTags = editorInfo.newFileText
         ApplicationManager.getApplication().runWriteAction {
-            val newText = contentToSet ?: textWithoutTags(textWithTags)
+            if (document.text != contentToSet) {
-            if (document.text != newText) {
+                document.setText(contentToSet)
-                document.setText(newText)
             }
 
             editorInfo.applyToEditor(editor)
         }
         PsiDocumentManager.getInstance(project).commitAllDocuments()
-        result = editor to textWithTags
+        result = editor
     }
     return result!!
-}
+}
+
+/**
+ * Returns original physical file with expected test data, i.e. something like 'kotlin/idea/testData/gradle/runConfigurations/myTest/Foo.kt'
+ *
+ * Note that it's different from physical file used in project during test run (which is usually a copy in temporary folder)
+ *
+ * All inheritors of GradleImportingTestCase should normally have PsiFiles with properly attached UserData to them
+ * (see [GradleImportingTestCase.configureByFiles])
+ */
+internal val VirtualFile.testDataFileByUserData: File
+    get() {
+        val physicalFileWithExpectedTestData = File(this.getUserData(VfsTestUtil.TEST_DATA_FILE_PATH)!!)
+        TestCase.assertTrue(
+            "Can't find file with expected test data by absolute path ${physicalFileWithExpectedTestData.absolutePath}",
+            physicalFileWithExpectedTestData.exists()
+        )
+        return physicalFileWithExpectedTestData
+    }
+
+internal val PsiFile.testDataFileByUserData: File
+    get() = virtualFile.testDataFileByUserData

idea/idea-gradle/tests/org/jetbrains/kotlin/idea/codeInsight/gradle/GradleImportingTestCase.kt

@@ -35,6 +35,7 @@ import com.intellij.openapi.util.io.FileUtil
 import com.intellij.openapi.vfs.LocalFileSystem
 import com.intellij.openapi.vfs.VirtualFile
 import com.intellij.testFramework.IdeaTestUtil
+import com.intellij.testFramework.VfsTestUtil
 import com.intellij.util.PathUtil
 import com.intellij.util.containers.ContainerUtil
 import junit.framework.TestCase
@@ -243,13 +244,21 @@ abstract class GradleImportingTestCase : ExternalSystemImportingTestCase() {
         return rootDir.walk().mapNotNull {
             when {
                 it.isDirectory -> null
+
                 !it.name.endsWith(SUFFIX) -> {
                     var text = it.readText()
                     (properties ?: mapOf("kotlin_plugin_version" to LATEST_STABLE_GRADLE_PLUGIN_VERSION)).forEach { key, value ->
                         text = text.replace("{{${key}}}", value)
                     }
-                    createProjectSubFile(it.path.substringAfter(rootDir.path + File.separator), text)
+                    val virtualFile = createProjectSubFile(it.path.substringAfter(rootDir.path + File.separator), text)
+
+                    // Real file with expected testdata allows to throw nicer exceptions in
+                    // case of mismatch, as well as open interactive diff window in IDEA
+                    virtualFile.putUserData(VfsTestUtil.TEST_DATA_FILE_PATH, it.absolutePath)
+
+                    virtualFile
                 }
+
                 else -> null
             }
         }.toList()

idea/testData/gradle/importAndCheckHighlighting/multiplatformLibrary/src/commonMain/kotlin/sample/Sample.kt

@@ -1,7 +1,7 @@
 package sample
 
 expect class <lineMarker descr="Has actuals in JS, JVM">Sample</lineMarker>() {
-    fun <lineMarker>checkMe</lineMarker>(): Int
+    fun <lineMarker descr="Has actuals in JS, JVM">checkMe</lineMarker>(): Int
 }
 
 expect object <lineMarker descr="Has actuals in JS, JVM">Platform</lineMarker> {

idea/testData/gradle/importAndCheckHighlighting/unresolvedInMultiplatformLibrary/src/commonMain/kotlin/sample/Sample.kt

@@ -1,4 +1,4 @@
 package sample
 fun common(): Boolean {
-    return <error descr="[TYPE_MISMATCH] Type mismatch: inferred type is String but Boolean was expected">""</error>
+    return <error descr="[TYPE_MISMATCH] Type mismatch: inferred type is String but Boolean was expected" textAttributesKey="ERRORS_ATTRIBUTES">""</error>
 }

idea/testData/gradle/importAndCheckHighlighting/unresolvedInMultiplatformLibrary/src/jvmMain/kotlin/sample/SampleJvm.kt

@@ -2,5 +2,5 @@ package sample
 
 fun jvm() {
     println(common())
-    println(<error descr="[UNRESOLVED_REFERENCE] Unresolved reference: js">js</error>())
+    println(<error descr="[UNRESOLVED_REFERENCE] Unresolved reference: js" textAttributesKey="WRONG_REFERENCES_ATTRIBUTES">js</error>())
 }

idea/testData/gradle/newMultiplatformHighlighting/first/src/commonMain/kotlin/My.kt

@@ -1,3 +1,3 @@
-expect class <error descr="[NO_ACTUAL_FOR_EXPECT] Expected class 'My' has no actual declaration in module first_jsMain for JS"><lineMarker>My</lineMarker></error> {
+expect class <lineMarker descr="Has actuals in JVM"><error descr="[NO_ACTUAL_FOR_EXPECT] Expected class 'My' has no actual declaration in module first_jsMain for JS" textAttributesKey="ERRORS_ATTRIBUTES">My</error></lineMarker> {
-    fun <lineMarker>foo</lineMarker>()
+    fun <lineMarker descr="Has actuals in JVM">foo</lineMarker>()
 }

idea/testData/gradle/newMultiplatformHighlighting/first/src/jvmMain/kotlin/My.kt

@@ -1,5 +1,5 @@
-actual class <lineMarker>My</lineMarker> {
+actual class <lineMarker descr="Has declaration in common module">My</lineMarker> {
-    actual fun <lineMarker>foo</lineMarker>() {}
+    actual fun <lineMarker descr="Has declaration in common module">foo</lineMarker>() {}
 
-    actual fun <error descr="[ACTUAL_WITHOUT_EXPECT] Actual function 'bar' has no corresponding expected declaration">bar</error>() {}
+    actual fun <error descr="[ACTUAL_WITHOUT_EXPECT] Actual function 'bar' has no corresponding expected declaration" textAttributesKey="ERRORS_ATTRIBUTES">bar</error>() {}
 }

idea/testData/gradle/newMultiplatformHighlighting/noErrors/src/commonMain/kotlin/My.kt

@@ -1,3 +1,3 @@
-expect class <lineMarker><lineMarker>My</lineMarker></lineMarker> {
+expect class <lineMarker descr="Has actuals in JS, JVM">My</lineMarker> {
-    fun <lineMarker><lineMarker>foo</lineMarker></lineMarker>()
+    fun <lineMarker descr="Has actuals in JS, JVM">foo</lineMarker>()
 }

idea/testData/gradle/newMultiplatformHighlighting/noErrors/src/jsMain/kotlin/My.kt

@@ -1,3 +1,3 @@
-actual class <lineMarker>My</lineMarker> {
+actual class <lineMarker descr="Has declaration in common module">My</lineMarker> {
-    actual fun <lineMarker>foo</lineMarker>() {}
+    actual fun <lineMarker descr="Has declaration in common module">foo</lineMarker>() {}
 }

idea/testData/gradle/newMultiplatformHighlighting/noErrors/src/jvmMain/kotlin/My.kt

@@ -1,3 +1,3 @@
-actual class <lineMarker>My</lineMarker> {
+actual class <lineMarker descr="Has declaration in common module">My</lineMarker> {
-    actual fun <lineMarker>foo</lineMarker>() {}
+    actual fun <lineMarker descr="Has declaration in common module">foo</lineMarker>() {}
 }

idea/testData/multiModuleLineMarker/kotlinTestAnnotations/a_js_test_dep(kotlin-test)/test.kt

@@ -1,7 +1,7 @@
 import kotlin.test.*
 
-class SimpleTest {
+class <lineMarker descr="Run Test">SimpleTest</lineMarker> {
-    @Test fun testFoo() {
+    @Test fun <lineMarker descr="Run Test">testFoo</lineMarker>() {
         // Will run
     }