Store lookup info inside testData code instead of separate file

compiler/frontend/src/org/jetbrains/kotlin/incremental/utils.kt

@@ -29,7 +29,6 @@ import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils
 import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameUnsafe
 import org.jetbrains.kotlin.resolve.scopes.JetScope
 
-
 public fun LookupTracker.record(from: LookupLocation, inScope: JetScope, name: Name) {
     if (this == LookupTracker.DO_NOTHING || from is NoLookupLocation) return
 
@@ -41,15 +40,7 @@ public fun LookupTracker.record(from: LookupLocation, inScope: JetScope, name: N
     if (containingJetFile.doNotAnalyze != null) return
 
     val containingFilePath = containingJetFile.virtualFile.path
-    val lookupLocationSuffix =
-            when {
-                verbose ->
-                    getLineAndColumnInPsiFile(containingJetFile, from.element.textRange).let {
-                        ":" + it.line + ":" + it.column
-                    }
-                else -> ""
-            }
-    val lookupLocation = "$containingFilePath$lookupLocationSuffix"
+    val lineAndColumn = if (requiresLookupLineAndColumn) getLineAndColumnInPsiFile(containingJetFile, from.element.textRange) else null
 
     val scopeContainingDeclaration = inScope.getContainingDeclaration()
     val scopeFilePath = DescriptorToSourceUtils.getContainingFile(scopeContainingDeclaration)?.virtualFile?.path
@@ -61,5 +52,7 @@ public fun LookupTracker.record(from: LookupLocation, inScope: JetScope, name: N
                 else -> throw AssertionError("Unexpected containing declaration type: ${scopeContainingDeclaration.javaClass}")
             }
 
-    record(lookupLocation, scopeContainingDeclaration.fqNameUnsafe.asString(), scopeFilePath, scopeKind, name.asString())
+    record(containingFilePath, lineAndColumn?.line, lineAndColumn?.column,
+           scopeContainingDeclaration.fqNameUnsafe.asString(), scopeFilePath, scopeKind,
+           name.asString())
 }

core/descriptors/src/org/jetbrains/kotlin/incremental/components/LookupTracker.kt

@@ -17,14 +17,32 @@
 package org.jetbrains.kotlin.incremental.components
 
 public interface LookupTracker {
-    val verbose: Boolean
+    // used in tests for more accurate checks
+    val requiresLookupLineAndColumn: Boolean
         get() = false
 
-    fun record(lookupLocation: String, scopeFqName: String, scopeContainingFile: String?, scopeKind: ScopeKind, name: String)
+    fun record(
+            lookupContainingFile: String,
+            lookupLine: Int?,
+            lookupColumn: Int?,
+            scopeFqName: String,
+            scopeContainingFile: String?,
+            scopeKind: ScopeKind,
+            name: String
+    )
 
     companion object {
         val DO_NOTHING = object : LookupTracker {
-            override fun record(lookupLocation: String, scopeFqName: String, scopeContainingFile: String?, scopeKind: ScopeKind, name: String) {}
+            override fun record(
+                    lookupContainingFile: String,
+                    lookupLine: Int?,
+                    lookupColumn: Int?,
+                    scopeFqName: String,
+                    scopeContainingFile: String?,
+                    scopeKind: ScopeKind,
+                    name: String
+            ) {
+            }
         }
     }
 }

jps-plugin/test/org/jetbrains/kotlin/jps/build/AbstractIncrementalJpsTest.kt

@@ -43,7 +43,6 @@ import org.jetbrains.jps.model.JpsModuleRootModificationUtil
 import org.jetbrains.jps.model.java.JpsJavaExtensionService
 import org.jetbrains.jps.util.JpsPathUtil
 import org.jetbrains.kotlin.incremental.components.LookupTracker
-import org.jetbrains.kotlin.incremental.components.ScopeKind
 import org.jetbrains.kotlin.jps.build.classFilesComparison.assertEqualDirectories
 import org.jetbrains.kotlin.jps.incremental.getKotlinCache
 import org.jetbrains.kotlin.test.JetTestUtils
@@ -62,8 +61,7 @@ import kotlin.test.fail
 public abstract class AbstractIncrementalJpsTest(
         private val allowNoFilesWithSuffixInTestData: Boolean = false,
         private val checkDumpsCaseInsensitively: Boolean = false,
-        private val allowNoBuildLogFileInTestData: Boolean = false,
-        private val allowNoLookupsFileInTestData: Boolean = true
+        private val allowNoBuildLogFileInTestData: Boolean = false
 ) : JpsBuildTestCase() {
     companion object {
         val COMPILATION_FAILED = "COMPILATION FAILED"
@@ -74,9 +72,9 @@ public abstract class AbstractIncrementalJpsTest(
         val DEBUG_LOGGING_ENABLED = System.getProperty("debug.logging.enabled") == "true"
     }
 
-    private var testDataDir: File by Delegates.notNull()
+    protected var testDataDir: File by Delegates.notNull()
 
-    var workDir: File by Delegates.notNull()
+    protected var workDir: File by Delegates.notNull()
 
     private fun enableDebugLogging() {
         diagnostic.Logger.setFactory(javaClass<TestLoggerFactory>())
@@ -107,12 +105,16 @@ public abstract class AbstractIncrementalJpsTest(
     protected open val mockConstantSearch: Callbacks.ConstantAffectionResolver?
         get() = null
 
+    protected open fun createLookupTracker(): LookupTracker = LookupTracker.DO_NOTHING
+
+    protected open fun checkLookups(@suppress("UNUSED_PARAMETER") lookupTracker: LookupTracker) {}
+
     fun build(scope: CompileScopeTestBuilder = CompileScopeTestBuilder.make().all()): MakeResult {
         val workDirPath = FileUtil.toSystemIndependentName(workDir.getAbsolutePath())
         val logger = MyLogger(workDirPath)
         val descriptor = createProjectDescriptor(BuildLoggingManager(logger))
 
-        val lookupTracker = TestLookupTracker(workDirPath)
+        val lookupTracker = createLookupTracker()
         val dataContainer = JpsElementFactory.getInstance().createSimpleElement(lookupTracker)
         descriptor.project.container.setChild(KotlinBuilder.LOOKUP_TRACKER, dataContainer)
 
@@ -122,6 +124,8 @@ public abstract class AbstractIncrementalJpsTest(
             builder.addMessageHandler(buildResult)
             builder.build(scope.build(), false)
 
+            checkLookups(lookupTracker)
+
             if (!buildResult.isSuccessful()) {
                 val errorMessages =
                         buildResult
@@ -129,10 +133,10 @@ public abstract class AbstractIncrementalJpsTest(
                                 .map { it.getMessageText() }
                                 .map { it.replace("^.+:\\d+:\\s+".toRegex(), "").trim() }
                                 .joinToString("\n")
-                return MakeResult(logger.log + "$COMPILATION_FAILED\n" + errorMessages + "\n", true, null, lookupTracker.lookups)
+                return MakeResult(logger.log + "$COMPILATION_FAILED\n" + errorMessages + "\n", true, null)
             }
             else {
-                return MakeResult(logger.log, false, createMappingsDump(descriptor), lookupTracker.lookups)
+                return MakeResult(logger.log, false, createMappingsDump(descriptor))
             }
         } finally {
             descriptor.release()
@@ -263,7 +267,7 @@ public abstract class AbstractIncrementalJpsTest(
         workDir = FileUtilRt.createTempDirectory(TEMP_DIRECTORY_TO_USE, "jps-build", null)
 
         val moduleNames = configureModules()
-        val initialMakeResult = initialMake()
+        initialMake()
 
         val otherMakeResults = performModificationsAndMake(moduleNames)
 
@@ -273,13 +277,6 @@ public abstract class AbstractIncrementalJpsTest(
             UsefulTestCase.assertSameLinesWithFile(buildLogFile.absolutePath, logs)
         }
 
-        val lookupsFile = File(testDataDir, "lookups.txt")
-        if (lookupsFile.exists() || !allowNoLookupsFileInTestData) {
-            val allActualLookups = otherMakeResults.mapTo(arrayListOf(initialMakeResult.lookups), MakeResult::lookups.getter)
-            val actualLookups = allActualLookups.joinToString("\n\n") { it.toSortedList().join("\n") } + "\n"
-            UsefulTestCase.assertSameLinesWithFile(lookupsFile.absolutePath, actualLookups)
-        }
-
         val lastMakeResult = otherMakeResults.last()
         rebuildAndCheckOutput(lastMakeResult)
         clearCachesRebuildAndCheckOutput(lastMakeResult)
@@ -336,7 +333,7 @@ public abstract class AbstractIncrementalJpsTest(
         return byteArrayOutputStream.toString()
     }
 
-    private data class MakeResult(val log: String, val makeFailed: Boolean, val mappingsDump: String?, val lookups: List<String>)
+    private data class MakeResult(val log: String, val makeFailed: Boolean, val mappingsDump: String?)
 
     private fun performModificationsAndMake(moduleNames: Set<String>?): List<MakeResult> {
         val results = arrayListOf<MakeResult>()
@@ -364,7 +361,10 @@ public abstract class AbstractIncrementalJpsTest(
         if (moduleDependencies == null) {
             addModule("module", arrayOf(getAbsolutePath("src")), null, null, jdk)
 
-            FileUtil.copyDir(testDataDir, File(workDir, "src"), { it.getName().endsWith(".kt") || it.getName().endsWith(".java") })
+            val srcDir = File(workDir, "src")
+            FileUtil.copyDir(testDataDir, srcDir, { it.getName().endsWith(".kt") || it.getName().endsWith(".java") })
+
+            preProcessSources(srcDir)
 
             moduleNames = null
         }
@@ -382,8 +382,11 @@ public abstract class AbstractIncrementalJpsTest(
             for (module in nameToModule.values()) {
                 val moduleName = module.getName()
 
-                FileUtil.copyDir(testDataDir, File(workDir, moduleName + "/src"),
+                val srcDir = File(workDir, moduleName + "/src")
+                FileUtil.copyDir(testDataDir, srcDir,
                                  { it.getName().startsWith(moduleName + "_") && (it.getName().endsWith(".kt") || it.getName().endsWith(".java")) })
+
+                preProcessSources(srcDir)
             }
 
             moduleNames = nameToModule.keySet()
@@ -392,20 +395,11 @@ public abstract class AbstractIncrementalJpsTest(
         return moduleNames
     }
 
-    override fun doGetProjectDir(): File? = workDir
-
-    private class TestLookupTracker(val rootPath: String) : LookupTracker {
-        val lookups = arrayListOf<String>()
-
-        override val verbose: Boolean
-            get() = true
-
-        override fun record(lookupLocation: String, scopeFqName: String, scopeContainingFile: String?, scopeKind: ScopeKind, name: String) {
-            val s = """from "$lookupLocation" by "$name" in $scopeKind scope "$scopeFqName" in file ${scopeContainingFile?.let { "$it" } ?: null}"""
-            lookups.add(s.replace(rootPath, "\$TEST_DIR$"))
-        }
+    protected open fun preProcessSources(srcDir: File) {
     }
 
+    override fun doGetProjectDir(): File? = workDir
+
     private class MyLogger(val rootPath: String) : ProjectBuilderLoggerBase() {
         private val logBuf = StringBuilder()
         public val log: String

jps-plugin/test/org/jetbrains/kotlin/jps/build/AbstractLookupTrackerTest.kt

@@ -16,8 +16,108 @@
 
 package org.jetbrains.kotlin.jps.build
 
+import org.jetbrains.kotlin.incremental.components.LookupTracker
+import org.jetbrains.kotlin.incremental.components.ScopeKind
+import org.jetbrains.kotlin.test.JetTestUtils
+import org.jetbrains.kotlin.utils.join
+import java.io.File
+import java.util.ArrayList
+import kotlin.test.fail
+
 abstract class AbstractLookupTrackerTest : AbstractIncrementalJpsTest(
         allowNoFilesWithSuffixInTestData = true,
-        allowNoBuildLogFileInTestData = true,
-        allowNoLookupsFileInTestData = false
+        allowNoBuildLogFileInTestData = true
+) {
+    val MULTILINE_COMMENT = "/\\*[^\\*]*\\*/".toRegex()
+
+    override fun createLookupTracker() = TestLookupTracker()
+
+    override fun checkLookups(lookupTracker: LookupTracker) {
+        if (lookupTracker !is TestLookupTracker) throw AssertionError("Expected TestLookupTracker, but: ${lookupTracker.javaClass}")
+
+        val fileToLookups = lookupTracker.lookups.groupBy { it.lookupContainingFile }
+        val workSrcDir = File(workDir, "src")
+
+        for (file in workSrcDir.walkTopDown()) {
+            if (!file.isFile) continue
+
+            val lookupsFromFile = fileToLookups[file.path] ?: continue
+
+            val text = file.readText()
+
+            val matchResult = MULTILINE_COMMENT.match(text)
+            if (matchResult != null) {
+                matchResult.groups
+                fail("File $file unexpectedly contains multiline comments. In range ${matchResult.range} found: ${matchResult.value} in $text")
+            }
+
+            val lines = text.lines().toArrayList()
+
+            for ((line, lookupsFromLine) in lookupsFromFile.groupBy { it.lookupLine!! }) {
+                val columnToLookups = lookupsFromLine.groupBy { it.lookupColumn!! }.toList().sortBy { it.first }
+
+                val lineContent = lines[line - 1]
+                val parts = ArrayList<CharSequence>(columnToLookups.size() * 2)
+
+                var start = 0
+
+                for ((column, lookupsFromColumn) in columnToLookups) {
+                    val end = column - 1
+                    parts.add(lineContent.subSequence(start, end))
+
+                    val stringifyLookupInfo: LookupInfo.() -> String = {
+                        val scopeKindChar = scopeKind.toString()[0].toLowerCase()
+                        val scopeFileOrEmpty = scopeContainingFile?.let { "($it)" } ?: ""
+                        "$scopeKindChar:$scopeFqName$scopeFileOrEmpty"
+                    }
+
+                    parts.add(lookupsFromColumn.distinct().joinToString(separator = " ", prefix = "/*", postfix = "*/", transform = stringifyLookupInfo))
+
+                    start = end
+                }
+
+                lines[line - 1] = parts.join("") + lineContent.subSequence(start, lineContent.length())
+            }
+
+            val actual = lines.joinToString("\n")
+
+            JetTestUtils.assertEqualsToFile(File(testDataDir, file.path.replace(".*/src/".toRegex(), "")), actual)
+        }
+    }
+
+    override fun preProcessSources(srcDir: File) = dropBlockComments(srcDir)
+
+    private fun dropBlockComments(workSrcDir: File) {
+        for (file in workSrcDir.walkTopDown()) {
+            if (!file.isFile) continue
+            file.writeText(file.readText().replace(MULTILINE_COMMENT, ""))
+        }
+    }
+}
+
+private data class LookupInfo(
+        val lookupContainingFile: String,
+        val lookupLine: Int?,
+        val lookupColumn: Int?,
+        val scopeFqName: String,
+        val scopeContainingFile: String?,
+        val scopeKind: ScopeKind,
+        val name: String
 )
+
+private class TestLookupTracker : LookupTracker {
+    val lookups = arrayListOf<LookupInfo>()
+
+    override val requiresLookupLineAndColumn: Boolean
+        get() = true
+
+    override fun record(
+            lookupContainingFile: String, lookupLine: Int?, lookupColumn: Int?,
+            scopeFqName: String, scopeContainingFile: String?, scopeKind: ScopeKind,
+            name: String
+    ) {
+        lookups.add(LookupInfo(lookupContainingFile, lookupLine, lookupColumn, scopeFqName, scopeContainingFile, scopeKind, name))
+    }
+}
+
+

jps-plugin/testData/incremental/lookupTracker/simple/lookups.txt

@@ -1,5 +0,0 @@
-from "$TEST_DIR$/src/main.kt:3:16" by "Array" in PACKAGE scope "foo.bar" in file null
-from "$TEST_DIR$/src/main.kt:3:22" by "String" in PACKAGE scope "foo.bar" in file null
-from "$TEST_DIR$/src/main.kt:4:12" by "Array" in PACKAGE scope "foo.bar" in file null
-from "$TEST_DIR$/src/main.kt:4:18" by "Int" in PACKAGE scope "foo.bar" in file null
-from "$TEST_DIR$/src/main.kt:5:12" by "String" in PACKAGE scope "foo.bar" in file null

jps-plugin/testData/incremental/lookupTracker/simple/main.kt

@@ -1,6 +1,6 @@
 package foo.bar
 
-fun main(args: Array<String>) {
-    val f: Array<Int>
-    val s: String
+fun main(args: /*p:foo.bar*/Array</*p:foo.bar*/String>) {
+    val f: /*p:foo.bar*/Array</*p:foo.bar*/Int>
+    val s: /*p:foo.bar*/String
 }