diff --git a/.space/CODEOWNERS b/.space/CODEOWNERS index f4af73bd8eb..1912fe27059 100644 --- a/.space/CODEOWNERS +++ b/.space/CODEOWNERS @@ -1,4 +1,22 @@ # Check-out https://www.jetbrains.com/help/space/code-owners.html#codeowners-file-syntax +# This file is checked by org.jetbrains.kotlin.code.SpaceCodeOwnersTest +# Directive OWNER_LIST specifies possible values for owner field +# Directive UNKNOWN specifies that path is unclaimed +# Ownership must be exhaustive modulo UNKNOWN + +# OWNER_LIST: Kotlin +# OWNER_LIST: "Kotlin Libraries" +# OWNER_LIST: "Kotlin JVM" "Kotlin JS" "Kotlin Native" "Kotlin Wasm" +# OWNER_LIST: "Kotlin Compiler Core" +# OWNER_LIST: "Kotlin Build Infrastructure" "Kotlin Build Tools" +# OWNER_LIST: "Kotlin IDE" +# OWNER_LIST: "Kotlin Release" +# OWNER_LIST: "Kotlin Multiplatform" +# OWNER_LIST: Mikhail.Glukhikh +# OWNER_LIST: Yan.Zhulanow +# OWNER_LIST: Alexander.Udalov +# Kotlin Libraries +# OWNER_LIST: A.Qurbonzoda Vsevolod.Tolstopyato Ilya.Gorbunov Sergey.Shanshin Leonid.Startsev # Fallback rule, must always be top * Kotlin diff --git a/repo/codebase-tests/build.gradle.kts b/repo/codebase-tests/build.gradle.kts index 479e51143b0..fd7e2358a16 100644 --- a/repo/codebase-tests/build.gradle.kts +++ b/repo/codebase-tests/build.gradle.kts @@ -10,6 +10,8 @@ dependencies { testImplementation("com.fasterxml.jackson.dataformat:jackson-dataformat-xml:2.12.7") testImplementation("com.fasterxml.jackson.module:jackson-module-kotlin:2.12.7") testImplementation("com.fasterxml.woodstox:woodstox-core:6.2.4") + + testImplementation("org.eclipse.jgit:org.eclipse.jgit:5.13.0.202109080827-r") } sourceSets { diff --git a/repo/codebase-tests/tests/org/jetbrains/kotlin/code/SpaceCodeOwnersTest.kt b/repo/codebase-tests/tests/org/jetbrains/kotlin/code/SpaceCodeOwnersTest.kt new file mode 100644 index 00000000000..a15fc7e25e2 --- /dev/null +++ b/repo/codebase-tests/tests/org/jetbrains/kotlin/code/SpaceCodeOwnersTest.kt @@ -0,0 +1,297 @@ +/* + * Copyright 2010-2022 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.code + +import junit.framework.TestCase +import org.eclipse.jgit.ignore.FastIgnoreRule +import org.eclipse.jgit.ignore.IgnoreNode +import java.io.File +import kotlin.test.assertIs + +class SpaceCodeOwnersTest : TestCase() { + private val ownersFile = File(".space/CODEOWNERS") + private val owners = parseCodeOwners(ownersFile) + + + fun testOwnerListNoDuplicates() { + val duplicatedOwnerListEntries = owners.permittedOwners.groupBy { it.name } + .filterValues { occurrences -> occurrences.size > 1 } + .values + + if (duplicatedOwnerListEntries.isNotEmpty()) { + fail( + buildString { + appendLine("Duplicated OWNER_LIST entries in $ownersFile:") + for (group in duplicatedOwnerListEntries) { + group.joinTo(this, separator = "\n", postfix = "\n---") + } + } + ) + } + } + + fun testAllOwnersInOwnerList() { + val permittedOwnerNames = owners.permittedOwners.map { it.name }.toSet() + val problems = mutableListOf() + for (pattern in owners.patterns) { + if (pattern !is OwnershipPattern.Pattern) continue + for (owner in pattern.owners) { + if (owner !in permittedOwnerNames) { + problems += "Owner ${owner.quoteIfContainsSpaces()} not listed in OWNER_LIST of $ownersFile, but used in $pattern" + } + } + } + if (problems.isNotEmpty()) { + fail(problems.joinToString("\n")) + } + } + + fun testFallbackRuleMatchEverything() { + val fallbackRule = owners.patterns.first() + assertEquals("Fallback rule must be '*', while it is $fallbackRule", "*", fallbackRule.pattern) + assertIs(fallbackRule, "Fallback rule must not be UNKNOWN, but it is $fallbackRule") + } + + fun testPatterns() { + val checker = FileOwnershipChecker( + owners, + root = File(".") + ) + checker.check() + + val problems = mutableListOf() + + if (checker.unmatchedFilesTop.isNotEmpty()) { + problems.add( + "Found files without owner, please add it to $ownersFile:\n" + + checker.unmatchedFilesTop.joinToString("\n") { " $it" } + ) + } + + val unusedPatterns = checker.unusedMatchers() + if (unusedPatterns.isNotEmpty()) { + problems.add( + "Found unused patterns in $ownersFile:\n" + + unusedPatterns.joinToString("\n") { " ${it.item}" } + ) + } + + if (problems.isNotEmpty()) { + fail(problems.joinToString("\n")) + } + } + + private class FileOwnershipChecker( + owners: CodeOwners, + val root: File + ) { + private val fileWalkDepthLimit = 8 + + val matchers = + owners.patterns + .map { ItemUse(it, FastIgnoreRule(it.pattern)) } + .reversed() + + val fallbackMatcher = matchers.last() + + val fileMatchers = matchers.filterNot { (_, rule) -> rule.dirOnly() } + + val ignoreTracker = GitIgnoreTracker() + + val unmatchedFilesTop = mutableListOf() + + data class ItemUse(val item: OwnershipPattern, val rule: FastIgnoreRule) { + + var used: Boolean = false + + override fun toString(): String { + return "use($item) = $used" + } + } + + fun List.findFirstMatching(path: String, isDirectory: Boolean, parentMatch: ItemUse?): ItemUse? { + val parentMatchLine = parentMatch?.item?.line + // Here, input list should be reversed, so that + // lines are in reverse direction + // We then run matcher till find more specific rule or break when parent matches already + // Ex: + // (line = 10, pattern = /some/file), + // (line = 5, pattern = /some/), + // (line = 1, pattern = *) + // With input of parent = (line = 5, pattern = /some/) and path = /some/other + // we only search till our parent pattern line, as other rules has lower specifity + for (use in this) { + if (parentMatchLine != null && use.item.line < parentMatchLine) break + if (use.rule.isMatch(path, isDirectory)) { + use.used = true + return use + } + } + return parentMatch + } + + fun findMatchLine(path: String, isDirectory: Boolean, parentMatch: ItemUse?): ItemUse? { + return if (isDirectory) { + matchers.findFirstMatching(path, isDirectory = true, parentMatch) + } else { + fileMatchers.findFirstMatching(path, isDirectory = false, parentMatch) + } + } + + fun visitFile(file: File, parentMatch: ItemUse?) { + val path = file.path.replace(File.separatorChar, '/') + if (ignoreTracker.isIgnored(path, isDirectory = false)) return + + val matchedItem = findMatchLine(path, isDirectory = false, parentMatch) + if (matchedItem != fallbackMatcher) return + if (unmatchedFilesTop.size < 10) { + unmatchedFilesTop.add(file) + } + } + + fun visitDirectory(directory: File, parentMatch: ItemUse?, depth: Int) { + if (depth > fileWalkDepthLimit) return + val path = directory.path.replace(File.separatorChar, '/') + + if (ignoreTracker.isIgnored(path, isDirectory = true)) return + val directoryMatch = findMatchLine(path, isDirectory = true, parentMatch) + ignoreTracker.withDirectory(directory) { + for (childName in (directory.list() ?: emptyArray())) { + val child = if (directory == root) { + File(childName) + } else { + File(directory, childName) + } + if (child.isDirectory) { + visitDirectory(child, directoryMatch, depth + 1) + } else { + visitFile(child, directoryMatch) + } + } + } + } + + fun check() { + visitDirectory(root, null, 0) + } + + fun unusedMatchers(): List { + return matchers.filterNot { it.used } + } + } +} + + +private class GitIgnoreTracker { + private val ignoreNodeStack = mutableListOf( + IgnoreNode(listOf(FastIgnoreRule("/.git"))) + ) + private val reversedIgnoreNodeStack = ignoreNodeStack.asReversed() + + fun isIgnored(path: String, isDirectory: Boolean): Boolean { + return reversedIgnoreNodeStack.firstNotNullOfOrNull { ignoreNode -> ignoreNode.checkIgnored(path, isDirectory) } ?: false + } + + inline fun withDirectory(directory: File, action: () -> Unit) { + val ignoreFile = directory.resolve(".gitignore").takeIf { it.exists() } + if (ignoreFile != null) { + val ignoreNode = IgnoreNode().apply { + ignoreFile.inputStream().use { + parse(ignoreFile.path, ignoreFile.inputStream()) + } + } + ignoreNodeStack.add(ignoreNode) + } + action() + if (ignoreFile != null) { + ignoreNodeStack.removeAt(ignoreNodeStack.lastIndex) + } + } +} + +private data class CodeOwners( + val permittedOwners: List, + val patterns: List +) { + data class OwnerListEntry(val name: String, val line: Int) { + override fun toString(): String { + return "line $line |# $OWNER_LIST_DIRECTIVE: $name" + } + } +} + +private sealed class OwnershipPattern { + abstract val line: Int + abstract val pattern: String + + data class Pattern(override val pattern: String, val owners: List, override val line: Int) : OwnershipPattern() { + override fun toString(): String { + return "line $line |$pattern " + owners.joinToString(separator = " ") { it.quoteIfContainsSpaces() } + } + } + + data class UnknownPathPattern(override val pattern: String, override val line: Int) : OwnershipPattern() { + override fun toString(): String { + return "line $line |# $UNKNOWN_DIRECTIVE: $pattern" + } + } +} + +private fun String.quoteIfContainsSpaces() = if (contains(' ')) "\"$this\"" else this + +private fun parseCodeOwners(file: File): CodeOwners { + fun parseDirective(line: String, directive: String): String? { + val value = line.substringAfter("# $directive: ") + if (value != line) return value + return null + } + + val ownersPattern = "(\"[^\"]+\")|(\\S+)".toRegex() + + fun parseOwnerNames(ownerString: String): List { + return ownersPattern.findAll(ownerString).map { it.value.removeSurrounding("\"") }.toList() + } + + val permittedOwners = mutableListOf() + val patterns = mutableListOf() + + file.useLines { lines -> + + for ((index, line) in lines.withIndex()) { + val lineNumber = index + 1 + if (line.startsWith("#")) { + val unknownDirective = parseDirective(line, UNKNOWN_DIRECTIVE) + if (unknownDirective != null) { + patterns += OwnershipPattern.UnknownPathPattern(unknownDirective.trim(), lineNumber) + continue + } + + val ownerListDirective = parseDirective(line, OWNER_LIST_DIRECTIVE) + if (ownerListDirective != null) { + parseOwnerNames(ownerListDirective).mapTo(permittedOwners) { owner -> + CodeOwners.OwnerListEntry(owner, lineNumber) + } + } + } else if (line.isNotBlank()) { + // Note: Space CODEOWNERS grammar is ambiguous, as it is impossible to distinguish between file pattern with spaces + // and team name, so we re-use similar logic + // ex: + // ``` + // /some/path/Read Me.md Owner + // ``` + // In such pattern it is impossible to distinguish between file ".../Read Me.md" or file ".../Read" owned by "Me.md" + // See SPACE-17772 + val (pattern, owners) = line.split(' ', limit = 2) + patterns += OwnershipPattern.Pattern(pattern, parseOwnerNames(owners), lineNumber) + } + } + } + + return CodeOwners(permittedOwners, patterns) +} + +private const val OWNER_LIST_DIRECTIVE = "OWNER_LIST" +private const val UNKNOWN_DIRECTIVE = "UNKNOWN" \ No newline at end of file