Fix corrupt mapping file (#91)

* Fixed tests actuals propagation * Fixed computation of owners per file * Fixed computation of owners per file * Fixed computation of owners per file
gmazzo · Jun 3, 2024 · 42e5fac · 42e5fac
1 parent fb414b0
commit 42e5fac
Show file tree

Hide file tree

Showing 30 changed files with 230 additions and 154 deletions.
diff --git a/demo-project-kotlin/app/src/androidUnitTest/kotlin/org/test/kotlin/app/Rethrow.kt b/demo-project-kotlin/app/src/androidUnitTest/kotlin/org/test/kotlin/app/Rethrow.kt
@@ -2,5 +2,5 @@ package org.test.kotlin.app
 
 actual const val isJVM = true
 
-actual fun rethrow(throwable: () -> Throwable) =
+actual fun rethrow(throwable: Throwable) =
     org.test.kotlin.utils.rethrow(throwable)
diff --git a/demo-project-kotlin/app/src/commonTest/kotlin/org/test/kotlin/app/AppOwnersTest.kt b/demo-project-kotlin/app/src/commonTest/kotlin/org/test/kotlin/app/AppOwnersTest.kt
@@ -3,7 +3,6 @@ package org.test.kotlin.app
 import io.github.gmazzo.codeowners.codeOwners
 import io.github.gmazzo.codeowners.codeOwnersOf
 import org.test.kotlin.utils.AppUtils
-import kotlin.jvm.JvmSerializableLambda
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
@@ -27,7 +26,7 @@ class AppOwnersTest {
 
     @Test
     fun ownerFromExceptionStacktrace() {
-        val exception = assertFailsWith<AppException> { rethrow @JvmSerializableLambda { AppException("myException") } }
+        val exception = assertFailsWith<AppException> { rethrow(AppException()) }
 
         val expectedOwners = setOf(if (isJVM) "utils-devs" else "app-devs") // on JVM we have stackstraces
 

diff --git a/demo-project-kotlin/app/src/commonTest/kotlin/org/test/kotlin/app/Rethrow.kt b/demo-project-kotlin/app/src/commonTest/kotlin/org/test/kotlin/app/Rethrow.kt
@@ -3,8 +3,8 @@ package org.test.kotlin.app
 
 import kotlin.jvm.JvmName
 
-class AppException(message: String) : RuntimeException(message)
+class AppException : RuntimeException("anException")
 
 expect val isJVM: Boolean
 
-expect fun rethrow(throwable: () -> Throwable)
+expect fun rethrow(throwable: Throwable)
diff --git a/demo-project-kotlin/app/src/jsTest/kotlin/org/test/kotlin/app/Rethrow.kt b/demo-project-kotlin/app/src/jsTest/kotlin/org/test/kotlin/app/Rethrow.kt
@@ -2,6 +2,6 @@ package org.test.kotlin.app
 
 actual const val isJVM = false
 
-actual fun rethrow(throwable: () -> Throwable) {
-    throw throwable()
+actual fun rethrow(throwable: Throwable) {
+    throw throwable
 }
diff --git a/demo-project-kotlin/app/src/jvmTest/kotlin/org/test/kotlin/app/Rethrow.kt b/demo-project-kotlin/app/src/jvmTest/kotlin/org/test/kotlin/app/Rethrow.kt
@@ -2,5 +2,5 @@ package org.test.kotlin.app
 
 actual const val isJVM = true
 
-actual fun rethrow(throwable: () -> Throwable) =
+actual fun rethrow(throwable: Throwable) =
     org.test.kotlin.utils.rethrow(throwable)
diff --git a/demo-project-kotlin/app/src/nativeTest/kotlin/org/test/kotlin/app/Rethrow.kt b/demo-project-kotlin/app/src/nativeTest/kotlin/org/test/kotlin/app/Rethrow.kt
@@ -2,6 +2,6 @@ package org.test.kotlin.app
 
 actual const val isJVM = false
 
-actual fun rethrow(throwable: () -> Throwable) {
-    throw throwable()
+actual fun rethrow(throwable: Throwable) {
+    throw throwable
 }
diff --git a/demo-project-kotlin/lib1/src/test/kotlin/org/test/kotlin/lib/LibOwnersTest.kt b/demo-project-kotlin/lib1/src/test/kotlin/org/test/kotlin/lib/LibOwnersTest.kt
@@ -21,7 +21,7 @@ class LibOwnersTest {
     @Test
     fun ownerOfMoreUtils() {
         assertEquals(setOf("utils-devs"), MoreUtils::class.java.codeOwners)
-        assertEquals(setOf("utils-more"), MoreUtils.Companion::class.java.codeOwners)
+        assertEquals(setOf("utils-devs"), MoreUtils.Companion::class.java.codeOwners)
     }
 
 }
diff --git a/demo-project-kotlin/lib2/src/main/kotlin/org/test/kotlin/utils/AndroidLibUtils.kt b/demo-project-kotlin/lib2/src/main/kotlin/org/test/kotlin/utils/AndroidLibUtils.kt
@@ -4,8 +4,6 @@ import io.github.gmazzo.codeowners.CodeOwners
 
 sealed class AndroidLibUtils {
 
-    // manual override owners
-    @CodeOwners("libs-impl")
     data object Impl : AndroidLibUtils()
 
 }
diff --git a/demo-project-kotlin/lib2/src/test/kotlin/org/test/kotlin/lib/AndroidLibOwnersTest.kt b/demo-project-kotlin/lib2/src/test/kotlin/org/test/kotlin/lib/AndroidLibOwnersTest.kt
@@ -10,7 +10,7 @@ class AndroidLibOwnersTest {
     @Test
     fun ownerOfAndroidLib() {
         assertEquals(setOf("libs-devs"), codeOwnersOf<AndroidLibUtils>())
-        assertEquals(setOf("libs-impl"), codeOwnersOf<AndroidLibUtils.Impl>())
+        assertEquals(setOf("libs-devs"), codeOwnersOf<AndroidLibUtils.Impl>())
     }
 
 }
diff --git a/demo-project-kotlin/utils/src/main/kotlin/org/test/kotlin/utils/Rethrow.kt b/demo-project-kotlin/utils/src/main/kotlin/org/test/kotlin/utils/Rethrow.kt
@@ -1,5 +1,5 @@
 package org.test.kotlin.utils
 
-fun rethrow(throwable: () -> Throwable) {
-    throw throwable()
+fun rethrow(throwable: Throwable) {
+    throw throwable.javaClass.newInstance()
 }
diff --git a/demo-project-kotlin/utils/src/main/kotlin/org/test/kotlin/utils/more/MoreUtils.kt b/demo-project-kotlin/utils/src/main/kotlin/org/test/kotlin/utils/more/MoreUtils.kt
@@ -1,11 +1,7 @@
 package org.test.kotlin.utils.more
 
-import io.github.gmazzo.codeowners.CodeOwners
-
 class MoreUtils {
 
-    // manual override owners
-    @CodeOwners("utils-more")
     companion object
 
 }
diff --git a/demo-project-kotlin/utils/src/test/java/org/test/kotlin/utils/UtilsOwnersTest.java b/demo-project-kotlin/utils/src/test/java/org/test/kotlin/utils/UtilsOwnersTest.java
@@ -17,7 +17,7 @@ public void ownerOfUtils() {
     @Test
     public void ownerOfMoreUtils() {
         assertEquals(setOf("utils-devs"), getCodeOwners(MoreUtils.class));
-        assertEquals(setOf("utils-more"), getCodeOwners(MoreUtils.Companion.class));
+        assertEquals(setOf("utils-devs"), getCodeOwners(MoreUtils.Companion.class));
     }
 
 }
diff --git a/demo-project-tests/build.gradle.kts b/demo-project-tests/build.gradle.kts
@@ -17,21 +17,23 @@ testing.suites.withType<JvmTestSuite>().configureEach {
     useJUnitJupiter()
 }
 
-val collectTaskOutputs = copySpec()
 val collectTask = tasks.register<Sync>("collectTaskOutputs") {
-    with(collectTaskOutputs)
     into(temporaryDir)
 }
 
 rootProject.allprojects project@{
     tasks.withType<CodeOwnersResourcesTask>().all task@{
-        collectTaskOutputs.from(files(simplifiedMappedCodeOwnersFile, rawMappedCodeOwnersFile)) {
-            into("actualMappings/${project.path}")
+        collectTask.configure {
+            from(listOf(simplifiedMappedCodeOwnersFile, rawMappedCodeOwnersFile)) {
+                into("actualMappings/${this@task.project.path}")
+            }
         }
     }
     tasks.withType<CodeOwnersReportTask>().all task@{
-        collectTaskOutputs.from(codeOwnersReportFile) {
-            into("actualReports/${project.path}")
+        collectTask.configure {
+            from(codeOwnersReportFile) {
+                into("actualReports/${this@task.project.path}")
+            }
         }
     }
 }

diff --git a/...st/resources/expectedReports/:demo-project-kotlin:app/app-androidDebugUnitTest.codeowners b/...st/resources/expectedReports/:demo-project-kotlin:app/app-androidDebugUnitTest.codeowners
@@ -1,6 +1,6 @@
 # CodeOwners of module ':demo-project-kotlin:app' (source set 'androidDebugUnitTest')
 
-org/test/kotlin/app/AppException                                                    app-devs
-org/test/kotlin/app/AppOwnersTest                                                   app-devs
-org/test/kotlin/app/AppOwnersTest$ownerFromExceptionStacktrace$exception$1$1        app-devs
-org/test/kotlin/app/RethrowKt                                                       app-devs
+org/test/kotlin/app/AppException        app-devs
+org/test/kotlin/app/AppOwnersTest       app-devs
+org/test/kotlin/app/RethrowKt           app-devs
+org/test/kotlin/app/RethrowUtils        app-devs
diff --git a/.../resources/expectedReports/:demo-project-kotlin:app/app-androidReleaseUnitTest.codeowners b/.../resources/expectedReports/:demo-project-kotlin:app/app-androidReleaseUnitTest.codeowners
@@ -1,6 +1,6 @@
 # CodeOwners of module ':demo-project-kotlin:app' (source set 'androidReleaseUnitTest')
 
-org/test/kotlin/app/AppException                                                    app-devs
-org/test/kotlin/app/AppOwnersTest                                                   app-devs
-org/test/kotlin/app/AppOwnersTest$ownerFromExceptionStacktrace$exception$1$1        app-devs
-org/test/kotlin/app/RethrowKt                                                       app-devs
+org/test/kotlin/app/AppException        app-devs
+org/test/kotlin/app/AppOwnersTest       app-devs
+org/test/kotlin/app/RethrowKt           app-devs
+org/test/kotlin/app/RethrowUtils        app-devs
diff --git a/...s/src/test/resources/expectedReports/:demo-project-kotlin:app/app-iosArm64Test.codeowners b/...s/src/test/resources/expectedReports/:demo-project-kotlin:app/app-iosArm64Test.codeowners
@@ -2,3 +2,5 @@
 
 org/test/kotlin/app/AppException        app-devs
 org/test/kotlin/app/AppOwnersTest       app-devs
+org/test/kotlin/app/RethrowKt           app-devs
+org/test/kotlin/app/RethrowUtils        app-devs
diff --git a/...t/resources/expectedReports/:demo-project-kotlin:app/app-iosSimulatorArm64Test.codeowners b/...t/resources/expectedReports/:demo-project-kotlin:app/app-iosSimulatorArm64Test.codeowners
@@ -2,3 +2,5 @@
 
 org/test/kotlin/app/AppException        app-devs
 org/test/kotlin/app/AppOwnersTest       app-devs
+org/test/kotlin/app/RethrowKt           app-devs
+org/test/kotlin/app/RethrowUtils        app-devs
diff --git a/...t-tests/src/test/resources/expectedReports/:demo-project-kotlin:app/app-jsTest.codeowners b/...t-tests/src/test/resources/expectedReports/:demo-project-kotlin:app/app-jsTest.codeowners
@@ -2,3 +2,5 @@
 
 org/test/kotlin/app/AppException        app-devs
 org/test/kotlin/app/AppOwnersTest       app-devs
+org/test/kotlin/app/RethrowKt           app-devs
+org/test/kotlin/app/RethrowUtils        app-devs
diff --git a/...-tests/src/test/resources/expectedReports/:demo-project-kotlin:app/app-jvmTest.codeowners b/...-tests/src/test/resources/expectedReports/:demo-project-kotlin:app/app-jvmTest.codeowners
@@ -1,8 +1,8 @@
 # CodeOwners of module ':demo-project-kotlin:app' (source set 'jvmTest')
 
-org/test/kotlin/app/AppException                                                    app-devs
-org/test/kotlin/app/AppOwnersJVMTest                                                app-devs
-org/test/kotlin/app/AppOwnersJVMTest$ownerOfUtilFunctions$1                         app-devs
-org/test/kotlin/app/AppOwnersTest                                                   app-devs
-org/test/kotlin/app/AppOwnersTest$ownerFromExceptionStacktrace$exception$1$1        app-devs
-org/test/kotlin/app/RethrowKt                                                       app-devs
+org/test/kotlin/app/AppException                                app-devs
+org/test/kotlin/app/AppOwnersJVMTest                            app-devs
+org/test/kotlin/app/AppOwnersJVMTest$ownerOfUtilFunctions$1     app-devs
+org/test/kotlin/app/AppOwnersTest                               app-devs
+org/test/kotlin/app/RethrowKt                                   app-devs
+org/test/kotlin/app/RethrowUtils                                app-devs
diff --git a/...-project-tests/src/test/resources/expectedReports/:demo-project-kotlin:app/app.codeowners b/...-project-tests/src/test/resources/expectedReports/:demo-project-kotlin:app/app.codeowners
@@ -1,12 +1,12 @@
 # CodeOwners of module ':demo-project-kotlin:app'
 
-org/test/kotlin/app/AppClass                                                        app-devs
-org/test/kotlin/app/AppException                                                    app-devs
-org/test/kotlin/app/AppOwnersJVMTest                                                app-devs
-org/test/kotlin/app/AppOwnersJVMTest$ownerOfUtilFunctions$1                         app-devs
-org/test/kotlin/app/AppOwnersTest                                                   app-devs
-org/test/kotlin/app/AppOwnersTest$ownerFromExceptionStacktrace$exception$1$1        app-devs
-org/test/kotlin/app/BuildConfig                                                     app-devs
-org/test/kotlin/app/RethrowKt                                                       app-devs
-org/test/kotlin/app/test/BuildConfig                                                app-devs
-org/test/kotlin/utils/AppUtils                                                      app-devs
+org/test/kotlin/app/AppClass                                    app-devs
+org/test/kotlin/app/AppException                                app-devs
+org/test/kotlin/app/AppOwnersJVMTest                            app-devs
+org/test/kotlin/app/AppOwnersJVMTest$ownerOfUtilFunctions$1     app-devs
+org/test/kotlin/app/AppOwnersTest                               app-devs
+org/test/kotlin/app/BuildConfig                                 app-devs
+org/test/kotlin/app/RethrowKt                                   app-devs
+org/test/kotlin/app/RethrowUtils                                app-devs
+org/test/kotlin/app/test/BuildConfig                            app-devs
+org/test/kotlin/utils/AppUtils                                  app-devs
diff --git a/gradle.properties b/gradle.properties
@@ -2,3 +2,6 @@ org.gradle.jvmargs=-Xmx2g
 org.gradle.caching=true
 org.gradle.configuration-cache=true
 org.gradle.parallel=true
+
+# facilitates debugging
+# kotlin.compiler.execution.strategy=in-process
diff --git a/...iler/src/main/kotlin/io/github/gmazzo/codeowners/compiler/CodeOwnersComponentRegistrar.kt b/...iler/src/main/kotlin/io/github/gmazzo/codeowners/compiler/CodeOwnersComponentRegistrar.kt
@@ -12,6 +12,7 @@ import org.jetbrains.kotlin.backend.common.extensions.IrGenerationExtension
 import org.jetbrains.kotlin.compiler.plugin.CompilerPluginRegistrar
 import org.jetbrains.kotlin.compiler.plugin.ExperimentalCompilerApi
 import org.jetbrains.kotlin.config.CompilerConfiguration
+import org.jetbrains.kotlin.fir.extensions.FirExtensionRegistrarAdapter
 import org.jetbrains.kotlin.ir.util.IrMessageLogger
 import org.jetbrains.kotlin.ir.util.irMessageLogger
 
@@ -31,11 +32,12 @@ internal class CodeOwnersComponentRegistrar : CompilerPluginRegistrar() {
         val codeOwnersRoot = configuration.get(CODEOWNERS_ROOT)!!
         val codeOwnersFile = configuration.get(CODEOWNERS_FILE)!!.useLines { CodeOwnersFile(it) }
         val mappingFile = configuration.get(MAPPINGS_OUTPUT)
+
         val matcher = CodeOwnersMatcher(codeOwnersRoot, codeOwnersFile)
+        val mappings = CodeOwnersMappings(matcher, mappingFile)
 
-        mappingFile?.delete()
-        mappingFile?.parentFile?.mkdirs()
-        IrGenerationExtension.registerExtension(CodeOwnersIrGenerationExtension(matcher, mappingFile))
+        FirExtensionRegistrarAdapter.registerExtension(CodeOwnersFirExtensionRegistrar(mappings))
+        IrGenerationExtension.registerExtension(CodeOwnersIrGenerationExtension(mappings))
     }
 
 }
diff --git a/...r/src/main/kotlin/io/github/gmazzo/codeowners/compiler/CodeOwnersFirExtensionRegistrar.kt b/...r/src/main/kotlin/io/github/gmazzo/codeowners/compiler/CodeOwnersFirExtensionRegistrar.kt
@@ -0,0 +1,14 @@
+package io.github.gmazzo.codeowners.compiler
+
+import org.jetbrains.kotlin.fir.FirSession
+import org.jetbrains.kotlin.fir.extensions.FirExtensionRegistrar
+
+class CodeOwnersFirExtensionRegistrar(
+    private val mappings: CodeOwnersMappings,
+) : FirExtensionRegistrar() {
+
+    override fun ExtensionRegistrarContext.configurePlugin() {
+        +{ session: FirSession -> CodeOwnersFirProcessor(session, mappings) }
+    }
+
+}
diff --git a/...n-compiler/src/main/kotlin/io/github/gmazzo/codeowners/compiler/CodeOwnersFirProcessor.kt b/...n-compiler/src/main/kotlin/io/github/gmazzo/codeowners/compiler/CodeOwnersFirProcessor.kt
@@ -0,0 +1,63 @@
+package io.github.gmazzo.codeowners.compiler
+
+import org.jetbrains.kotlin.backend.common.serialization.toIoFileOrNull
+import org.jetbrains.kotlin.diagnostics.DiagnosticReporter
+import org.jetbrains.kotlin.fir.FirElement
+import org.jetbrains.kotlin.fir.FirSession
+import org.jetbrains.kotlin.fir.analysis.checkers.MppCheckerKind
+import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
+import org.jetbrains.kotlin.fir.analysis.checkers.declaration.DeclarationCheckers
+import org.jetbrains.kotlin.fir.analysis.checkers.declaration.FirFileChecker
+import org.jetbrains.kotlin.fir.analysis.extensions.FirAdditionalCheckersExtension
+import org.jetbrains.kotlin.fir.declarations.FirFile
+import org.jetbrains.kotlin.fir.declarations.FirRegularClass
+import org.jetbrains.kotlin.fir.declarations.FirSimpleFunction
+import org.jetbrains.kotlin.fir.declarations.utils.classId
+import org.jetbrains.kotlin.fir.java.findJvmNameValue
+import org.jetbrains.kotlin.fir.packageFqName
+import org.jetbrains.kotlin.fir.visitors.FirVisitor
+import org.jetbrains.kotlin.name.Name
+import org.jetbrains.kotlin.resolve.jvm.JvmClassName
+
+class CodeOwnersFirProcessor(
+    session: FirSession,
+    private val mappings: CodeOwnersMappings,
+) : FirAdditionalCheckersExtension(session) {
+
+    override val declarationCheckers = object : DeclarationCheckers() {
+        override val fileCheckers = setOf(CodeOwnersMapper())
+    }
+
+    inner class CodeOwnersMapper : FirFileChecker(MppCheckerKind.Platform) {
+        override fun check(declaration: FirFile, context: CheckerContext, reporter: DiagnosticReporter) {
+            val mappings = declaration.sourceFile?.toIoFileOrNull()?.let(mappings::resolve) ?: return
+
+            declaration.accept(fileVisitor, mappings)
+        }
+    }
+
+    private val fileVisitor = object : FirVisitor<Unit, CodeOwnersMappings.Mapping>() {
+
+        override fun visitFile(file: FirFile, data: CodeOwnersMappings.Mapping) {
+            file.acceptChildren(this, data)
+
+            if (file.declarations.any { it is FirSimpleFunction }) {
+                val fileJvmName = file.findJvmNameValue() ?: file.name.replace("\\.kt".toRegex(), "Kt")
+                val fileClass = JvmClassName.byFqNameWithoutInnerClasses(file.packageFqName.child(Name.identifier(fileJvmName))).internalName
+
+                data.classes += fileClass
+            }
+        }
+
+        override fun visitRegularClass(regularClass: FirRegularClass, data: CodeOwnersMappings.Mapping) {
+            data.classes += JvmClassName.byClassId(regularClass.classId).internalName
+
+            regularClass.acceptChildren(this, data)
+        }
+
+        override fun visitElement(element: FirElement, data: CodeOwnersMappings.Mapping) {
+        }
+
+    }
+
+}
diff --git a/...r/src/main/kotlin/io/github/gmazzo/codeowners/compiler/CodeOwnersIrGenerationExtension.kt b/...r/src/main/kotlin/io/github/gmazzo/codeowners/compiler/CodeOwnersIrGenerationExtension.kt
@@ -1,34 +1,23 @@
 package io.github.gmazzo.codeowners.compiler
 
-import io.github.gmazzo.codeowners.matcher.CodeOwnersFile
-import io.github.gmazzo.codeowners.matcher.CodeOwnersMatcher
 import org.jetbrains.kotlin.backend.common.extensions.IrGenerationExtension
 import org.jetbrains.kotlin.backend.common.extensions.IrPluginContext
 import org.jetbrains.kotlin.ir.declarations.IrModuleFragment
-import java.io.File
 
 internal class CodeOwnersIrGenerationExtension(
-    private val matcher: CodeOwnersMatcher,
-    private val mappingsFile: File?,
+    private val mappings: CodeOwnersMappings,
 ) : IrGenerationExtension {
 
     override fun generate(moduleFragment: IrModuleFragment, pluginContext: IrPluginContext) {
-        val mappings = mappingsFile?.let {  mutableMapOf<String, MutableSet<String>>() }
-        val transformer = CodeOwnersIrTransformer(pluginContext, mappings)
-        val owners = moduleFragment.files.asSequence()
-            .flatMap { matcher.ownerOf(File(it.fileEntry.name)).orEmpty() }
-            .toSet()
+        mappings.noteFrontedFinished()
 
-        if (owners.isNotEmpty()) {
-            moduleFragment.accept(transformer, owners)
+        val transformer = CodeOwnersIrTransformer(pluginContext, mappings)
 
-            if (mappings != null) {
-                val entries = mappings.map { (file, owners) -> CodeOwnersFile.Entry(file, owners.toList()) }
-                val codeOwners = CodeOwnersFile(entries)
+        moduleFragment.accept(transformer, InvalidOwners)
+    }
 
-                mappingsFile!!.appendText(codeOwners.content)
-            }
-        }
+    private data object InvalidOwners : Set<String> by emptySet() {
+        override val size: Int get() = error("Invalid owners")
     }
 
 }