From c1a1986344f7ae149311a5045369e150526a0439 Mon Sep 17 00:00:00 2001 From: Nikolay Krasko Date: Mon, 6 Mar 2023 13:55:20 +0100 Subject: [PATCH] Discourage future trove usages KTI-1135 --- .../kotlin/code/CodeConformanceTest.kt | 114 +++++++++++++++--- 1 file changed, 95 insertions(+), 19 deletions(-) diff --git a/repo/codebase-tests/tests/org/jetbrains/kotlin/code/CodeConformanceTest.kt b/repo/codebase-tests/tests/org/jetbrains/kotlin/code/CodeConformanceTest.kt index 289c8129dfb..2e10fdd04c0 100644 --- a/repo/codebase-tests/tests/org/jetbrains/kotlin/code/CodeConformanceTest.kt +++ b/repo/codebase-tests/tests/org/jetbrains/kotlin/code/CodeConformanceTest.kt @@ -156,14 +156,14 @@ class CodeConformanceTest : TestCase() { } fun testNoBadSubstringsInProjectCode() { - class TestData(val message: String, val filter: (File, String) -> Boolean) { - val result: MutableList = ArrayList() + class FileTestCase(val message: String, allowedFiles: List = emptyList(), val filter: (File, String) -> Boolean) { + val allowedMatcher = FileMatcher(File("."), allowedFiles) } val atAuthorPattern = Pattern.compile("/\\*.+@author.+\\*/", Pattern.DOTALL) val tests = listOf( - TestData( + FileTestCase( "%d source files contain @author javadoc tag.\nPlease remove them or exclude in this test:\n%s" ) { _, source -> // substring check is an optimization @@ -171,7 +171,7 @@ class CodeConformanceTest : TestCase() { "ASM: a very small and fast Java bytecode manipulation framework" !in source && "package org.jetbrains.kotlin.tools.projectWizard.settings.version.maven" !in source }, - TestData( + FileTestCase( "%d source files use something from com.beust.jcommander.internal package.\n" + "This code won't work when there's no TestNG in the classpath of our IDEA plugin, " + "because there's only an optional dependency on testng.jar.\n" + @@ -180,14 +180,14 @@ class CodeConformanceTest : TestCase() { ) { _, source -> "com.beust.jcommander.internal" in source }, - TestData( + FileTestCase( "%d source files contain references to package org.jetbrains.jet.\n" + "Package org.jetbrains.jet is deprecated now in favor of org.jetbrains.kotlin. " + "Please consider changing the package in these files:\n%s" ) { _, source -> "org.jetbrains.jet" in source }, - TestData( + FileTestCase( "%d source files contain references to package kotlin.reflect.jvm.internal.impl.\n" + "This package contains internal reflection implementation and is a result of a " + "post-processing of kotlin-reflect.jar by jarjar.\n" + @@ -196,32 +196,102 @@ class CodeConformanceTest : TestCase() { ) { _, source -> "kotlin.reflect.jvm.internal.impl" in source }, - TestData( + FileTestCase( "%d source files contain references to package org.objectweb.asm.\n" + "Package org.jetbrains.org.objectweb.asm should be used instead to avoid troubles with different asm versions in classpath. " + "Please consider changing the package in these files:\n%s" ) { _, source -> " org.objectweb.asm" in source + }, + FileTestCase( + message = "%d source files contain references to package gnu.trove.\n" + + "Please avoid using trove library in new use cases. " + + "These files are affected:\n%s", + allowedFiles = listOf( + "analysis/light-classes-base/src/org/jetbrains/kotlin/asJava/classes/KotlinClassInnerStuffCache.kt", + "build-common/src/org/jetbrains/kotlin/incremental/IncrementalJvmCache.kt", + "compiler/backend/src/org/jetbrains/kotlin/codegen/FrameMap.kt", + "compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SMAP.kt", + "compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/ControlFlowGraph.kt", + "compiler/cli/cli-base/src/org/jetbrains/kotlin/cli/jvm/compiler/CliVirtualFileFinder.kt", + "compiler/cli/cli-base/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinCliJavaFileManagerImpl.kt", + "compiler/cli/cli-base/src/org/jetbrains/kotlin/cli/jvm/index/JvmDependenciesIndexImpl.kt", + "compiler/daemon/src/org/jetbrains/kotlin/daemon/RemoteLookupTrackerClient.kt", + "compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/FileScopeFactory.kt", + "compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyImportScope.kt", + "compiler/frontend/src/org/jetbrains/kotlin/types/expressions/PreliminaryLoopVisitor.kt", + "compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/EnumClassLowering.kt", + "compiler/psi/src/org/jetbrains/kotlin/psi/KotlinStringLiteralTextEscaper.kt", + "compiler/resolution.common.jvm/src/org/jetbrains/kotlin/load/java/structure/impl/classFiles/BinaryJavaClass.kt", + "compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/results/OverloadingConflictResolver.kt", + "compiler/tests-common/tests/org/jetbrains/kotlin/test/testFramework/KtUsefulTestCase.java", + "js/js.ast/src/org/jetbrains/kotlin/js/backend/JsReservedIdentifiers.java", + "js/js.ast/src/org/jetbrains/kotlin/js/backend/JsToStringGenerationVisitor.java", + "js/js.sourcemap/src/org/jetbrains/kotlin/js/sourceMap/SourceMap3Builder.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/TargetDependent.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/konan/NativeLibrary.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/AssociatedClassifierIdsResolver.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirClassNode.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirClassifierIndex.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirFictitiousFunctionClassifiers.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirKnownClassifiers.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirModuleNode.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirPackageNode.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirProvidedClassifiersByModules.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirRootNode.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/mergedtree/CirTypeSignature.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/metadata/CirDeserializers.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/metadata/CirTypeResolver.kt", + "native/commonizer/src/org/jetbrains/kotlin/commonizer/utils/misc.kt", + "native/native.tests/tests/org/jetbrains/kotlin/konan/blackboxtest/support/settings/SettingsContainers.kt" + ) + ) { _, source -> + "gnu.trove" in source } ) + val testCaseToMatchedFiles: Map> = mutableMapOf>() + .apply { + tests.forEach { testCase -> this[testCase] = mutableListOf() } + } + nonSourcesMatcher.excludeWalkTopDown(SOURCES_FILE_PATTERN).forEach { sourceFile -> val source = sourceFile.readText() for (test in tests) { - if (test.filter(sourceFile, source)) test.result.add(sourceFile) + if (test.filter(sourceFile, source)) { + (testCaseToMatchedFiles[test] ?: error("Should be added during initialization")).add(sourceFile) + } } } - if (tests.flatMap { it.result }.isNotEmpty()) { - fail(buildString { - for (test in tests) { - if (test.result.isNotEmpty()) { - append(test.message.format(test.result.size, test.result.joinToString("\n"))) - appendLine() - appendLine() - } + val failureStr = buildString { + for (test in tests) { + val (allowed, notAllowed) = (testCaseToMatchedFiles[test] ?: error("Should be added during initialization")).partition { + test.allowedMatcher.matchExact(it) } - }) + + if (notAllowed.isNotEmpty()) { + append(test.message.format(notAllowed.size, notAllowed.joinToString("\n"))) + appendLine() + appendLine() + } + + val unmatched = test.allowedMatcher.unmatchedExact(allowed) + if (unmatched.isNotEmpty()) { + val testMessage = test.message.format(unmatched.size, "NONE") + append( + "Unused \"allowed files\" for test:\n" + + "`$testMessage`\n" + + "Remove exceptions for the test list:${unmatched.joinToString("\n", prefix = "\n")}" + ) + appendLine() + appendLine() + } + } + } + + if (failureStr.isNotEmpty()) { + fail(failureStr) } } @@ -260,15 +330,21 @@ class CodeConformanceTest : TestCase() { private val paths = files.mapTo(HashSet()) { it.invariantSeparatorsPath } private val relativePaths = files.filterTo(ArrayList()) { it.isDirectory }.mapTo(HashSet()) { it.invariantSeparatorsPath + "/" } + private fun File.invariantRelativePath() = relativeTo(root).invariantSeparatorsPath + fun matchExact(file: File): Boolean { - return file.relativeTo(root).invariantSeparatorsPath in paths + return file.invariantRelativePath() in paths } fun matchWithContains(file: File): Boolean { if (matchExact(file)) return true - val relativePath = file.relativeTo(root).invariantSeparatorsPath + val relativePath = file.invariantRelativePath() return relativePaths.any { relativePath.startsWith(it) } } + + fun unmatchedExact(files: List): Set { + return paths - files.map { it.invariantRelativePath() }.toSet() + } } private fun FileMatcher.excludeWalkTopDown(filePattern: Pattern): Sequence {