From 49c95e54d266ea9297ebff629abf8b6b0987f015 Mon Sep 17 00:00:00 2001 From: Yahor Berdnikau Date: Tue, 27 Aug 2019 00:03:44 +0200 Subject: [PATCH] Replace editorconfig parser to ec4j. (#561) * Change editorconfig parser. Replaced custom parser, that is hard to maintain and not working well, with https://github.com/ec4j/ec4j parser. Signed-off-by: Yahor Berdnikau * Trim whitespaces in parsed disabled rules values. Signed-off-by: Yahor Berdnikau * Add ec4j dependency to maven. Signed-off-by: Yahor Berdnikau --- build.gradle | 1 + ktlint-core/build.gradle | 1 + ktlint-core/pom.xml | 5 + .../com/pinterest/ktlint/core/KtLint.kt | 5 +- .../core/internal/EditorConfigInternal.kt | 62 +++-- .../core/internal/EditorConfigInternalTest.kt | 214 ++++++++++++------ pom.xml | 1 + 7 files changed, 200 insertions(+), 89 deletions(-) diff --git a/build.gradle b/build.gradle index cb63563458..7f6c4eaf4e 100644 --- a/build.gradle +++ b/build.gradle @@ -14,6 +14,7 @@ ext.deps = [ 'compiler': "org.jetbrains.kotlin:kotlin-compiler-embeddable:${versions.kotlin}" ], 'klob' : 'com.github.shyiko.klob:klob:0.2.1', + ec4j : 'org.ec4j.core:ec4j-core:0.2.0', 'aether' : [ 'api' : "org.eclipse.aether:aether-api:${versions.aether}", 'spi' : "org.eclipse.aether:aether-spi:${versions.aether}", diff --git a/ktlint-core/build.gradle b/ktlint-core/build.gradle index d3fa3f12f6..bb6a23eb1f 100644 --- a/ktlint-core/build.gradle +++ b/ktlint-core/build.gradle @@ -6,6 +6,7 @@ plugins { dependencies { implementation deps.kotlin.stdlib implementation deps.kotlin.compiler + implementation deps.ec4j testImplementation deps.junit testImplementation deps.assertj diff --git a/ktlint-core/pom.xml b/ktlint-core/pom.xml index e4f8a7a507..94a722e949 100644 --- a/ktlint-core/pom.xml +++ b/ktlint-core/pom.xml @@ -23,6 +23,11 @@ kotlin-compiler-embeddable ${kotlin.version} + + org.ec4j.core + ec4j-core + ${ec4j.version} + junit junit diff --git a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt index dc652b5fc8..d711633bbe 100644 --- a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt +++ b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt @@ -230,7 +230,10 @@ object KtLint { node.putUserData(FILE_PATH_USER_DATA_KEY, userData["file_path"]) node.putUserData(EDITOR_CONFIG_USER_DATA_KEY, EditorConfig.fromMap(editorConfigMap - "android" - "file_path")) node.putUserData(ANDROID_USER_DATA_KEY, android) - node.putUserData(DISABLED_RULES, userData["disabled_rules"]?.split(",")?.toSet() ?: emptySet()) + node.putUserData( + DISABLED_RULES, + userData["disabled_rules"]?.split(",")?.map { it.trim() }?.toSet() ?: emptySet() + ) } private fun visitor( diff --git a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternal.kt b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternal.kt index 2bf29a9697..564cb79f62 100644 --- a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternal.kt +++ b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternal.kt @@ -1,12 +1,18 @@ package com.pinterest.ktlint.core.internal -import java.io.ByteArrayInputStream +import java.nio.charset.StandardCharsets import java.nio.file.Files import java.nio.file.Path import java.nio.file.Paths -import java.util.Properties import java.util.concurrent.CompletableFuture import java.util.concurrent.ConcurrentHashMap +import org.ec4j.core.PropertyTypeRegistry +import org.ec4j.core.Resource +import org.ec4j.core.model.EditorConfig +import org.ec4j.core.model.Version +import org.ec4j.core.parser.EditorConfigModelHandler +import org.ec4j.core.parser.EditorConfigParser +import org.ec4j.core.parser.ErrorHandler /** * This class handles traversing the filetree and parsing and merging the contents of any discovered .editorconfig files @@ -18,11 +24,10 @@ class EditorConfigInternal private constructor ( ) : Map by data { companion object : EditorConfigLookup { - override fun of(dir: String) = of(Paths.get(dir)) override fun of(dir: Path) = generateSequence(locate(dir)) { seed -> locate(seed.parent.parent) } // seed.parent == .editorconfig dir - .map { it to lazy { load(it) } } + .map { it to lazy { loadEditorconfigFile(it) } } .let { seq -> // stop when .editorconfig with "root = true" is found, go deeper otherwise var prev: Pair>>>? = null @@ -64,7 +69,7 @@ class EditorConfigInternal private constructor ( ( parent?.data ?: emptyMap() - ) + flatten(load(editorConfigPath)) + ) + flatten(loadEditorconfigFile(editorConfigPath)) ) } else { parent @@ -91,7 +96,7 @@ class EditorConfigInternal private constructor ( } } - private fun flatten(data: LinkedHashMap>): Map { + private fun flatten(data: Map>): Map { val map = mutableMapOf() val patternsToSearchFor = arrayOf("*", "*.kt", "*.kts") for ((sectionName, section) in data) { @@ -99,7 +104,7 @@ class EditorConfigInternal private constructor ( continue } val patterns = try { - parseSection(sectionName.substring(1, sectionName.length - 1)) + parseSection(sectionName) } catch (e: Exception) { throw RuntimeException( "ktlint failed to parse .editorconfig section \"$sectionName\"" + @@ -114,26 +119,37 @@ class EditorConfigInternal private constructor ( return map.toSortedMap() } - private fun load(path: Path) = - linkedMapOf>().also { map -> - object : Properties() { - private var section: MutableMap? = null + private fun parseEditorconfigFile(path: Path): EditorConfig { + val parser = EditorConfigParser.builder().build() + val handler = EditorConfigModelHandler(PropertyTypeRegistry.default_(), Version.CURRENT) - override fun put(key: Any, value: Any): Any? { - val sectionName = (key as String).trim() - if (sectionName.startsWith('[') && sectionName.endsWith(']') && value == "") { - section = mutableMapOf().also { map.put(sectionName, it) } - } else { - val section = - section - ?: mutableMapOf().also { section = it; map.put("", it) } - section[key] = value.toString() + parser.parse( + Resource.Resources.ofPath(path, StandardCharsets.UTF_8), + handler, + ErrorHandler.THROW_SYNTAX_ERRORS_IGNORE_OTHERS + ) + return handler.editorConfig + } + + private fun loadEditorconfigFile(editorConfigFile: Path): Map> { + val editorConfig = parseEditorconfigFile(editorConfigFile) + + var mapRepresentation = editorConfig.sections + .associate { section -> + section.glob.toString() to section + .properties + .mapValues { entry -> + entry.value.sourceValue } - return null - } - }.load(ByteArrayInputStream(Files.readAllBytes(path))) + } + + if (editorConfig.isRoot) { + mapRepresentation = mapRepresentation + ("" to mapOf("root" to true.toString())) } + return mapRepresentation + } + internal fun parseSection(sectionName: String): List { val result = mutableListOf() fun List>.collect0(i: Int = 0, str: Array, acc: MutableList) { diff --git a/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternalTest.kt b/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternalTest.kt index 6705ecc646..2eec608a54 100644 --- a/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternalTest.kt +++ b/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternalTest.kt @@ -2,105 +2,143 @@ package com.pinterest.ktlint.core.internal import com.google.common.jimfs.Configuration import com.google.common.jimfs.Jimfs +import java.nio.file.FileSystem import java.nio.file.Files import org.assertj.core.api.Assertions.assertThat +import org.junit.After import org.junit.Test class EditorConfigInternalTest { + private val tempFileSystem = Jimfs.newFileSystem(Configuration.forCurrentPlatform()) + + private fun FileSystem.writeEditorConfigFile( + filePath: String, + content: String + ) { + Files.createDirectories(getPath(filePath)) + Files.write(getPath("$filePath/.editorconfig"), content.toByteArray()) + } + + @After + fun tearDown() { + tempFileSystem.close() + } @Test fun testParentDirectoryFallback() { - val fs = Jimfs.newFileSystem(Configuration.unix()) - Files.createDirectories(fs.getPath("/projects/project-1/project-1-subdirectory")) - for ( - cfg in arrayOf( - """ - [*] - indent_size = 2 - """, - """ - root = true - [*] - indent_size = 2 - """, - """ - [*] - indent_size = 4 - [*.{kt,kts}] - indent_size = 2 - """, - """ - [*.{kt,kts}] - indent_size = 4 - [*] - indent_size = 2 - """ + val projectDir = "/projects/project-1" + val projectSubDirectory = "$projectDir/project-1-subdirectory" + Files.createDirectories(tempFileSystem.getPath(projectSubDirectory)) + val editorConfigFiles = arrayOf( + """ + [*] + indent_size = 2 + """.trimIndent(), + """ + root = true + [*] + indent_size = 2 + """.trimIndent(), + """ + [*] + indent_size = 4 + [*.{kt,kts}] + indent_size = 2 + """.trimIndent(), + """ + [*.{kt,kts}] + indent_size = 4 + [*] + indent_size = 2 + """.trimIndent() + ) + + editorConfigFiles.forEach { editorConfigFileContent -> + tempFileSystem.writeEditorConfigFile(projectDir, editorConfigFileContent) + + val editorConfig = EditorConfigInternal.of( + tempFileSystem.getPath(projectSubDirectory) ) - ) { - Files.write(fs.getPath("/projects/project-1/.editorconfig"), cfg.trimIndent().toByteArray()) - val editorConfig = EditorConfigInternal.of(fs.getPath("/projects/project-1/project-1-subdirectory")) + assertThat(editorConfig?.parent).isNull() assertThat(editorConfig?.toMap()) - .overridingErrorMessage("Expected \n%s\nto yield indent_size = 2", cfg.trimIndent()) - .isEqualTo(mapOf("indent_size" to "2")) + .overridingErrorMessage( + "Expected \n%s\nto yield indent_size = 2", + editorConfigFileContent + ) + .isEqualTo( + mapOf( + "indent_size" to "2", + "tab_width" to "2" + ) + ) } } @Test fun testRootTermination() { - val fs = Jimfs.newFileSystem(Configuration.unix()) - Files.createDirectories(fs.getPath("/projects/project-1/project-1-subdirectory")) - Files.write( - fs.getPath("/projects/.editorconfig"), + val rootDir = "/projects" + val project1Dir = "$rootDir/project-1" + val project1Subdirectory = "$project1Dir/project-1-subdirectory" + tempFileSystem.writeEditorConfigFile( + rootDir, """ root = true [*] end_of_line = lf - """.trimIndent().toByteArray() + """.trimIndent() ) - Files.write( - fs.getPath("/projects/project-1/.editorconfig"), + tempFileSystem.writeEditorConfigFile( + project1Dir, """ root = true [*.{kt,kts}] indent_size = 4 indent_style = space - """.trimIndent().toByteArray() + """.trimIndent() ) - Files.write( - fs.getPath("/projects/project-1/project-1-subdirectory/.editorconfig"), + tempFileSystem.writeEditorConfigFile( + project1Subdirectory, """ [*] indent_size = 2 - """.trimIndent().toByteArray() + """.trimIndent() ) - EditorConfigInternal.of(fs.getPath("/projects/project-1/project-1-subdirectory")).let { editorConfig -> - assertThat(editorConfig?.parent).isNotNull() - assertThat(editorConfig?.parent?.parent).isNull() - assertThat(editorConfig?.toMap()).isEqualTo( - mapOf( - "indent_size" to "2", - "indent_style" to "space" - ) + + var parsedEditorConfig = EditorConfigInternal.of( + tempFileSystem.getPath(project1Subdirectory) + ) + assertThat(parsedEditorConfig?.parent).isNotNull + assertThat(parsedEditorConfig?.parent?.parent).isNull() + assertThat(parsedEditorConfig?.toMap()).isEqualTo( + mapOf( + "indent_size" to "2", + "tab_width" to "2", + "indent_style" to "space" ) - } - EditorConfigInternal.of(fs.getPath("/projects/project-1")).let { editorConfig -> - assertThat(editorConfig?.parent).isNull() - assertThat(editorConfig?.toMap()).isEqualTo( - mapOf( - "indent_size" to "4", - "indent_style" to "space" - ) + ) + + parsedEditorConfig = EditorConfigInternal.of( + tempFileSystem.getPath(project1Dir) + ) + assertThat(parsedEditorConfig?.parent).isNull() + assertThat(parsedEditorConfig?.toMap()).isEqualTo( + mapOf( + "indent_size" to "4", + "tab_width" to "4", + "indent_style" to "space" ) - } - EditorConfigInternal.of(fs.getPath("/projects")).let { editorConfig -> - assertThat(editorConfig?.parent).isNull() - assertThat(editorConfig?.toMap()).isEqualTo( - mapOf( - "end_of_line" to "lf" - ) + ) + + parsedEditorConfig = EditorConfigInternal.of( + tempFileSystem.getPath(rootDir) + ) + assertThat(parsedEditorConfig?.parent).isNull() + assertThat(parsedEditorConfig?.toMap()).isEqualTo( + mapOf( + "end_of_line" to "lf" ) - } + ) } @Test @@ -123,4 +161,50 @@ class EditorConfigInternalTest { assertThat(EditorConfigInternal.parseSection("*.{js,{py")).isEqualTo(listOf("*.js", "*.{py")) assertThat(EditorConfigInternal.parseSection("*.py}")).isEqualTo(listOf("*.py}")) } + + @Test + fun `Should parse assignment with spaces`() { + val projectDir = "/project" + val editorconfigFile = + """ + [*.{kt, kts}] + insert_final_newline = true + disabled_rules = import-ordering + """.trimIndent() + tempFileSystem.writeEditorConfigFile(projectDir, editorconfigFile) + + val parsedEditorConfig = EditorConfigInternal.of( + tempFileSystem.getPath(projectDir) + ) + + assertThat(parsedEditorConfig).isNotNull + assertThat(parsedEditorConfig?.toMap()).isEqualTo( + mapOf( + "insert_final_newline" to "true", + "disabled_rules" to "import-ordering" + ) + ) + } + + @Test + fun `Should parse list with spaces after comma`() { + val projectDir = "/project" + val editorconfigFile = + """ + [*.{kt, kts}] + disabled_rules = import-ordering, no-wildcard-imports + """.trimIndent() + tempFileSystem.writeEditorConfigFile(projectDir, editorconfigFile) + + val parsedEditorConfig = EditorConfigInternal.of( + tempFileSystem.getPath(projectDir) + ) + + assertThat(parsedEditorConfig).isNotNull + assertThat(parsedEditorConfig?.toMap()).isEqualTo( + mapOf( + "disabled_rules" to "import-ordering, no-wildcard-imports" + ) + ) + } } diff --git a/pom.xml b/pom.xml index bd53c9f9a0..ff9a268661 100644 --- a/pom.xml +++ b/pom.xml @@ -56,6 +56,7 @@ 1.1.0 3.3.9 3.9.6 + 0.2.0 4.12 3.12.2 1.1