Kotlin: Refactor TrapWriters

Now ClassFileTrapWriter is just FileTrapWriter, which no longer takes a
nullable IrFileEntry.

SourceFileTrapWriter still extends FileTrapWriter, and adds the
IrFileEntry, allowing it to override the location functions with more
useful variants.

populateFileTables no longer has a default. I think that for the sake
of a handful of calls, it's simpler to be explicit so we aren't
confused.

java/kotlin-extractor/src/main/kotlin/ExternalClassExtractor.kt

@@ -33,7 +33,7 @@ class ExternalClassExtractor(val logger: FileLogger, val sourceFilePath: String,
                         } else {
                             GZIPOutputStream(manager.getFile().outputStream()).bufferedWriter().use { trapFileBW ->
                                 val tw =
-                                    ClassFileTrapWriter(TrapLabelManager(), trapFileBW, getIrClassBinaryPath(irClass))
+                                    FileTrapWriter(TrapLabelManager(), trapFileBW, getIrClassBinaryPath(irClass), true)
                                 val fileExtractor = KotlinFileExtractor(logger, tw, manager, this, pluginContext)
                                 fileExtractor.extractClassSource(irClass)
                             }
@@ -45,4 +45,4 @@ class ExternalClassExtractor(val logger: FileLogger, val sourceFilePath: String,
         output.writeTrapSet()
     }
 
-}
+}

java/kotlin-extractor/src/main/kotlin/KotlinExtractorExtension.kt

@@ -41,7 +41,7 @@ class KotlinExtractorExtension(
             val srcDir = File(System.getenv("CODEQL_EXTRACTOR_JAVA_SOURCE_ARCHIVE_DIR").takeUnless { it.isNullOrEmpty() } ?: "kotlin-extractor/src")
             srcDir.mkdirs()
             moduleFragment.files.mapIndexed { index: Int, file: IrFile ->
-                val fileTrapWriter = SourceFileTrapWriter(lm, invocationTrapFileBW, file)
+                val fileTrapWriter = tw.makeSourceFileTrapWriter(file, true)
                 fileTrapWriter.writeCompilation_compiling_files(compilation, index, fileTrapWriter.fileId)
                 doFile(invocationTrapFile, fileTrapWriter, checkTrapIdentical, logCounter, trapDir, srcDir, file, pluginContext)
             }
@@ -109,7 +109,7 @@ fun doFile(invocationTrapFile: String,
     if (checkTrapIdentical || !trapFile.exists()) {
         val trapTmpFile = File.createTempFile("$filePath.", ".trap.tmp", trapFileDir)
         trapTmpFile.bufferedWriter().use { trapFileBW ->
-            val tw = SourceFileTrapWriter(TrapLabelManager(), trapFileBW, file)
+            val tw = SourceFileTrapWriter(TrapLabelManager(), trapFileBW, file, true)
             tw.writeComment("Generated by invocation $invocationTrapFile")
             val externalClassExtractor = ExternalClassExtractor(logger, file.path, pluginContext)
             val fileExtractor = KotlinSourceFileExtractor(logger, tw, file, externalClassExtractor, pluginContext)

java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt

@@ -66,14 +66,14 @@ open class KotlinUsesExtractor(
     /**
      * Gets a KotlinFileExtractor based on this one, except it attributes locations to the file that declares the given class.
      */
-    fun withSourceFileOfClass(cls: IrClass, populateFileTables: Boolean): KotlinFileExtractor {
+    fun withSourceFileOfClass(cls: IrClass): KotlinFileExtractor {
         val clsFile = cls.fileOrNull
 
         val newTrapWriter =
             if (isExternalDeclaration(cls) || clsFile == null)
-                tw.withTargetFile(getIrClassBinaryPath(cls), null, populateFileTables)
+                tw.makeFileTrapWriter(getIrClassBinaryPath(cls))
             else
-                tw.withTargetFile(clsFile.path, clsFile.fileEntry)
+                tw.makeSourceFileTrapWriter(clsFile, false)
 
         val newLogger = FileLogger(logger.logCounter, newTrapWriter)
 
@@ -98,7 +98,7 @@ open class KotlinUsesExtractor(
             // If this is a generic type instantiation then it has no
             // source entity, so we need to extract it here
             if (typeArgs.isNotEmpty()) {
-                this.withSourceFileOfClass(extractClass, false).extractClassInstance(extractClass, typeArgs)
+                this.withSourceFileOfClass(extractClass).extractClassInstance(extractClass, typeArgs)
             }
 
             // Extract both the Kotlin and equivalent Java classes, so that we have database entries
@@ -678,4 +678,4 @@ class X {
 
     fun withQuestionMark(t: IrType, hasQuestionMark: Boolean) = if(hasQuestionMark) t.makeNullable() else t.makeNotNull()
 
-}
+}

java/kotlin-extractor/src/main/kotlin/TrapWriter.kt

@@ -7,6 +7,8 @@ import org.jetbrains.kotlin.ir.IrElement
 import org.jetbrains.kotlin.ir.declarations.path
 import org.jetbrains.kotlin.ir.declarations.IrFile
 import org.jetbrains.kotlin.ir.declarations.IrVariable
+import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET
+import org.jetbrains.kotlin.ir.util.SYNTHETIC_OFFSET
 
 import com.semmle.extractor.java.PopulateFile
 
@@ -147,19 +149,25 @@ open class TrapWriter (val lm: TrapLabelManager, val bw: BufferedWriter) {
     }
 
     /**
-     * Gets a FileTrapWriter like this one (using the same label manager, writer etc), but with the given
-     * default file used in getLocation etc.
+     * Gets a FileTrapWriter like this one (using the same label manager,
+     * writer etc), but using the given `filePath` for locations.
      */
-    fun withTargetFile(filePath: String, fileEntry: FileEntry?, populateFileTables: Boolean = true) =
-        FileTrapWriter(lm, bw, filePath, fileEntry, populateFileTables)
+    fun makeFileTrapWriter(filePath: String) =
+        FileTrapWriter(lm, bw, filePath, false)
+
+    /**
+     * Gets a FileTrapWriter like this one (using the same label manager,
+     * writer etc), but using the given `IrFile` for locations.
+     */
+    fun makeSourceFileTrapWriter(file: IrFile, populateFileTables: Boolean) =
+        SourceFileTrapWriter(lm, bw, file, populateFileTables)
 }
 
 open class FileTrapWriter (
     lm: TrapLabelManager,
     bw: BufferedWriter,
     val filePath: String,
-    val sourceFileEntry: FileEntry?,
-    populateFileTables: Boolean = true
+    populateFileTables: Boolean
 ): TrapWriter (lm, bw) {
     val populateFile = PopulateFile(this)
     val splitFilePath = filePath.split("!/")
@@ -175,37 +183,15 @@ open class FileTrapWriter (
     fun getWholeFileLocation(): Label<DbLocation> {
         return getLocation(fileId, 0, 0, 0, 0)
     }
-    fun getLocation(startOffset: Int, endOffset: Int): Label<DbLocation> {
-        // If the compiler doesn't have a location, then start and end are both -1
-        // If this isn't a source file (sourceFileEntry is null), then nothing has
-        // a source location: we report the source .class file regardless.
-        if((startOffset == -1 && endOffset == -1) || sourceFileEntry == null) {
-            val reportFileId = if (sourceFileEntry == null) fileId else unknownFileId
-            return getLocation(reportFileId, 0, 0, 0, 0)
-        } else {
-            // If this is the location for a compiler-generated element, then it will
-            // be a zero-width location. QL doesn't support these, so we translate it
-            // into a one-width location.
-            val endColumnOffset = if (startOffset == endOffset) 1 else 0
-            return getLocation(
-                fileId,
-                sourceFileEntry.getLineNumber(startOffset) + 1,
-                sourceFileEntry.getColumnNumber(startOffset) + 1,
-                sourceFileEntry.getLineNumber(endOffset) + 1,
-                sourceFileEntry.getColumnNumber(endOffset) + endColumnOffset
-            )
-        }
+    open fun getLocation(startOffset: Int, endOffset: Int): Label<DbLocation> {
+        // We don't have a FileEntry to look up the offsets in, so all
+        // we can do is return a whole-file location.
+        return getWholeFileLocation()
     }
-    fun getLocationString(e: IrElement): String {
-        if ((e.startOffset == -1 && e.endOffset == -1) || sourceFileEntry == null) {
-            return "unknown location, while processing $filePath"
-        } else {
-            val startLine =   sourceFileEntry.getLineNumber(e.startOffset) + 1
-            val startColumn = sourceFileEntry.getColumnNumber(e.startOffset) + 1
-            val endLine =     sourceFileEntry.getLineNumber(e.endOffset) + 1
-            val endColumn =   sourceFileEntry.getColumnNumber(e.endOffset)
-            return "file://$filePath:$startLine:$startColumn:$endLine:$endColumn"
-        }
+    open fun getLocationString(e: IrElement): String {
+        // We don't have a FileEntry to look up the offsets in, so all
+        // we can do is return a whole-file location.
+        return "file://filePath"
     }
     fun <T> getFreshIdLabel(): Label<T> {
         val label: Label<T> = lm.getFreshLabel()
@@ -217,13 +203,58 @@ open class FileTrapWriter (
 class SourceFileTrapWriter (
     lm: TrapLabelManager,
     bw: BufferedWriter,
-    irFile: IrFile) :
-    FileTrapWriter(lm, bw, irFile.path, irFile.fileEntry) {
-}
+    irFile: IrFile,
+    populateFileTables: Boolean) :
+    FileTrapWriter(lm, bw, irFile.path, populateFileTables) {
 
-class ClassFileTrapWriter (
-    lm: TrapLabelManager,
-    bw: BufferedWriter,
-    filePath: String) :
-    FileTrapWriter(lm, bw, filePath, null) {
+    private val fileEntry = irFile.fileEntry
+
+    override fun getLocation(startOffset: Int, endOffset: Int): Label<DbLocation> {
+        if (startOffset == UNDEFINED_OFFSET || endOffset == UNDEFINED_OFFSET) {
+            if (startOffset != endOffset) {
+                // TODO: Warn
+            }
+            return unknownLocation
+        }
+
+        if (startOffset == SYNTHETIC_OFFSET || endOffset == SYNTHETIC_OFFSET) {
+            if (startOffset != endOffset) {
+                // TODO: Warn
+            }
+            return unknownLocation
+        }
+
+        // If this is the location for a compiler-generated element, then it will
+        // be a zero-width location. QL doesn't support these, so we translate it
+        // into a one-width location.
+        val endColumnOffset = if (startOffset == endOffset) 1 else 0
+        return getLocation(
+            fileId,
+            fileEntry.getLineNumber(startOffset) + 1,
+            fileEntry.getColumnNumber(startOffset) + 1,
+            fileEntry.getLineNumber(endOffset) + 1,
+            fileEntry.getColumnNumber(endOffset) + endColumnOffset)
+    }
+
+    override fun getLocationString(e: IrElement): String {
+        if (e.startOffset == UNDEFINED_OFFSET || e.endOffset == UNDEFINED_OFFSET) {
+            if (e.startOffset != e.endOffset) {
+                // TODO: Warn
+            }
+            return "<unknown location while processing $filePath>"
+        }
+
+        if (e.startOffset == SYNTHETIC_OFFSET || e.endOffset == SYNTHETIC_OFFSET) {
+            if (e.startOffset != e.endOffset) {
+                // TODO: Warn
+            }
+            return "<synthetic location while processing $filePath>"
+        }
+
+        val startLine =   fileEntry.getLineNumber(e.startOffset) + 1
+        val startColumn = fileEntry.getColumnNumber(e.startOffset) + 1
+        val endLine =     fileEntry.getLineNumber(e.endOffset) + 1
+        val endColumn =   fileEntry.getColumnNumber(e.endOffset)
+        return "file://$filePath:$startLine:$startColumn:$endLine:$endColumn"
+    }
 }