From ba513d7b164e9e98deafec68b2888101d8c34165 Mon Sep 17 00:00:00 2001 From: Sha Sha Chu Date: Wed, 3 Jul 2019 12:00:24 -0700 Subject: [PATCH] Add support for disabled_rules property in .editorconfig for globally disabling rules (#503) * Add support for disabled_rules property to .editorconfig for globally disabling rules Fixes #208 * Takes in a comma-separated list of rule ids, with namespaces (e.g. `experimental:indent`) * Re-enabled `NoWildcardImports`, `PackageNameRule` * Un-commented `AnnotationRule`, `MultiLineIfElseRule`, and `NoItParamInMultilineLambdaRule`, and moved disabling into default `.editorconfig` * Also cleaned up params passed to lint and format --- .editorconfig | 5 + ktlint-core/build.gradle | 1 + ktlint-core/pom.xml | 6 + .../com/pinterest/ktlint/core/EditorConfig.kt | 10 +- .../com/pinterest/ktlint/core/KtLint.kt | 210 +++++++++--------- .../core/internal/EditorConfigInternal.kt | 25 ++- .../ktlint/core/DisabledRulesTest.kt | 67 ++++++ .../ktlint/core/ErrorSuppressionTest.kt | 8 +- .../com/pinterest/ktlint/core/KtLintTest.kt | 23 +- .../core/internal/EditorConfigInternalTest.kt | 36 +-- .../ruleset/experimental/IndentationRule.kt | 2 +- .../standard/StandardRuleSetProvider.kt | 20 +- .../pinterest/ktlint/test/RuleExtension.kt | 57 +++-- ktlint/build.gradle | 1 - ktlint/pom.xml | 6 - .../main/kotlin/com/pinterest/ktlint/Main.kt | 86 ++----- .../pinterest/ktlint/internal/FileUtils.kt | 53 ++++- .../internal/IntellijIDEAIntegration.kt | 3 +- 18 files changed, 341 insertions(+), 278 deletions(-) rename ktlint/src/main/kotlin/com/pinterest/ktlint/internal/EditorConfig.kt => ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternal.kt (92%) create mode 100644 ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/DisabledRulesTest.kt rename ktlint/src/test/kotlin/com/pinterest/ktlint/internal/EditorConfigTest.kt => ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternalTest.kt (67%) diff --git a/.editorconfig b/.editorconfig index 4a7f15e243..a8c3709c19 100644 --- a/.editorconfig +++ b/.editorconfig @@ -12,6 +12,11 @@ insert_final_newline = true [*.{java,kt,kts,scala,rs,xml,kt.spec,kts.spec}] indent_size = 4 +# annotation - "./mvnw clean verify" fails with "Internal Error") +# multiline-if-else - disabled until auto-correct is working properly +# (e.g. try formatting "if (true)\n return { _ ->\n _\n}") +# no-it-in-multiline-lambda - disabled until it's clear what to do in case of `import _.it` +disabled_rules=annotation,multiline-if-else,no-it-in-multiline-lambda [{Makefile,*.go}] indent_style = tab diff --git a/ktlint-core/build.gradle b/ktlint-core/build.gradle index 3598f818d1..d3fa3f12f6 100644 --- a/ktlint-core/build.gradle +++ b/ktlint-core/build.gradle @@ -9,4 +9,5 @@ dependencies { testImplementation deps.junit testImplementation deps.assertj + testImplementation deps.jimfs } diff --git a/ktlint-core/pom.xml b/ktlint-core/pom.xml index c81f087544..e4f8a7a507 100644 --- a/ktlint-core/pom.xml +++ b/ktlint-core/pom.xml @@ -35,6 +35,12 @@ ${assertj.version} test + + com.google.jimfs + jimfs + ${jimfs.version} + test + diff --git a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/EditorConfig.kt b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/EditorConfig.kt index 9b9b83eea0..bca3560a4b 100644 --- a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/EditorConfig.kt +++ b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/EditorConfig.kt @@ -2,12 +2,14 @@ package com.pinterest.ktlint.core /** * @see [EditorConfig](http://editorconfig.org/) + * + * This class is injected into the user data, so it is available to rules via [KtLint.EDITOR_CONFIG_USER_DATA_KEY] */ interface EditorConfig { - enum class IntentStyle { SPACE, TAB } + enum class IndentStyle { SPACE, TAB } - val indentStyle: IntentStyle + val indentStyle: IndentStyle val indentSize: Int val tabWidth: Int val maxLineLength: Int @@ -17,8 +19,8 @@ interface EditorConfig { companion object { fun fromMap(map: Map): EditorConfig { val indentStyle = when { - map["indent_style"]?.toLowerCase() == "tab" -> IntentStyle.TAB - else -> IntentStyle.SPACE + map["indent_style"]?.toLowerCase() == "tab" -> IndentStyle.TAB + else -> IndentStyle.SPACE } val indentSize = map["indent_size"].let { v -> if (v?.toLowerCase() == "unset") -1 else v?.toIntOrNull() ?: 4 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 94c8a3ee17..a6c97049c6 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 @@ -1,8 +1,14 @@ package com.pinterest.ktlint.core import com.pinterest.ktlint.core.ast.prevLeaf +import com.pinterest.ktlint.core.internal.EditorConfigInternal +import java.io.File +import java.nio.file.Path +import java.nio.file.Paths import java.util.ArrayList import java.util.HashSet +import java.util.concurrent.ConcurrentHashMap +import org.jetbrains.kotlin.backend.common.onlyIf import org.jetbrains.kotlin.cli.common.CLIConfigurationKeys import org.jetbrains.kotlin.cli.common.messages.MessageCollector import org.jetbrains.kotlin.cli.jvm.compiler.EnvironmentConfigFiles @@ -37,10 +43,32 @@ object KtLint { val EDITOR_CONFIG_USER_DATA_KEY = Key("EDITOR_CONFIG") val ANDROID_USER_DATA_KEY = Key("ANDROID") val FILE_PATH_USER_DATA_KEY = Key("FILE_PATH") + val DISABLED_RULES = Key>("DISABLED_RULES") private val psiFileFactory: PsiFileFactory private val nullSuppression = { _: Int, _: String, _: Boolean -> false } + /** + * @param fileName path of file to lint/format + * @param text Contents of file to lint/format + * @param ruleSets a collection of "RuleSet"s used to validate source + * @param userData Map of user options + * @param cb callback invoked for each lint error + * @param script true if this is a Kotlin script file + * @param editorConfigPath optional path of the .editorconfig file (otherwise will use working directory) + * @param debug True if invoked with the --debug flag + */ + data class Params( + val fileName: String? = null, + val text: String, + val ruleSets: Iterable, + val userData: Map = emptyMap(), + val cb: (e: LintError, corrected: Boolean) -> Unit, + val script: Boolean = false, + val editorConfigPath: String? = null, + val debug: Boolean = false + ) + init { // do not print anything to the stderr when lexer is unable to match input class LoggerFactory : DiagnosticLogger.Factory { @@ -93,65 +121,25 @@ object KtLint { /** * Check source for lint errors. * - * @param text source - * @param ruleSets a collection of "RuleSet"s used to validate source - * @param cb callback that is going to be executed for every lint error - * * @throws ParseException if text is not a valid Kotlin code * @throws RuleExecutionException in case of internal failure caused by a bug in rule implementation */ - fun lint(text: String, ruleSets: Iterable, cb: (e: LintError) -> Unit) { - lint(text, ruleSets, emptyMap(), cb, script = false) - } - - fun lint(text: String, ruleSets: Iterable, userData: Map, cb: (e: LintError) -> Unit) { - lint(text, ruleSets, userData, cb, script = false) - } - - /** - * Check source for lint errors. - * - * @param text script source - * @param ruleSets a collection of "RuleSet"s used to validate source - * @param cb callback that is going to be executed for every lint error - * - * @throws ParseException if text is not a valid Kotlin code - * @throws RuleExecutionException in case of internal failure caused by a bug in rule implementation - */ - fun lintScript(text: String, ruleSets: Iterable, cb: (e: LintError) -> Unit) { - lint(text, ruleSets, emptyMap(), cb, script = true) - } - - fun lintScript( - text: String, - ruleSets: Iterable, - userData: Map, - cb: (e: LintError) -> Unit - ) { - lint(text, ruleSets, userData, cb, script = true) - } - - private fun lint( - text: String, - ruleSets: Iterable, - userData: Map, - cb: (e: LintError) -> Unit, - script: Boolean - ) { - val normalizedText = text.replace("\r\n", "\n").replace("\r", "\n") + fun lint(params: Params) { + val normalizedText = params.text.replace("\r\n", "\n").replace("\r", "\n") val positionByOffset = calculateLineColByOffset(normalizedText) - val fileName = if (script) "file.kts" else "file.kt" - val psiFile = psiFileFactory.createFileFromText(fileName, KotlinLanguage.INSTANCE, normalizedText) as KtFile + val psiFileName = if (params.script) "file.kts" else "file.kt" + val psiFile = psiFileFactory.createFileFromText(psiFileName, KotlinLanguage.INSTANCE, normalizedText) as KtFile val errorElement = psiFile.findErrorElement() if (errorElement != null) { val (line, col) = positionByOffset(errorElement.textOffset) throw ParseException(line, col, errorElement.errorDescription) } val rootNode = psiFile.node - injectUserData(rootNode, userData) + val mergedUserData = params.userData + userDataResolver(params.editorConfigPath, params.debug)(params.fileName) + injectUserData(rootNode, mergedUserData) val isSuppressed = calculateSuppressedRegions(rootNode) val errors = mutableListOf() - visitor(rootNode, ruleSets).invoke { node, rule, fqRuleId -> + visitor(rootNode, params.ruleSets).invoke { node, rule, fqRuleId -> // fixme: enforcing suppression based on node.startOffset is wrong // (not just because not all nodes are leaves but because rules are free to emit (and fix!) errors at any position) if (!isSuppressed(node.startOffset, fqRuleId, node === rootNode)) { @@ -172,7 +160,59 @@ object KtLint { } errors .sortedWith(Comparator { l, r -> if (l.line != r.line) l.line - r.line else l.col - r.col }) - .forEach(cb) + .forEach { e -> params.cb(e, false) } + } + + private fun userDataResolver(editorConfigPath: String?, debug: Boolean): (String?) -> Map { + if (editorConfigPath != null) { + val userData = ( + EditorConfigInternal.of(File(editorConfigPath).canonicalPath) + ?.onlyIf({ debug }) { printEditorConfigChain(it) } + ?: emptyMap() + ) + return fun (fileName: String?) = if (fileName != null) { + userData + ("file_path" to fileName) + } else { + emptyMap() + } + } + val workDir = File(".").canonicalPath + val workdirUserData = lazy { + ( + EditorConfigInternal.of(workDir) + ?.onlyIf({ debug }) { printEditorConfigChain(it) } + ?: emptyMap() + ) + } + val editorConfig = EditorConfigInternal.cached() + val editorConfigSet = ConcurrentHashMap() + return fun (fileName: String?): Map { + if (fileName == null) { + return emptyMap() + } + + if (fileName == "") { + return workdirUserData.value + } + return ( + editorConfig.of(Paths.get(fileName).parent) + ?.onlyIf({ debug }) { + printEditorConfigChain(it) { + editorConfigSet.put(it.path, true) != true + } + } + ?: emptyMap() + ) + ("file_path" to fileName) + } + } + + private fun printEditorConfigChain(ec: EditorConfigInternal, predicate: (EditorConfigInternal) -> Boolean = { true }) { + for (lec in generateSequence(ec) { it.parent }.takeWhile(predicate)) { + System.err.println( + "[DEBUG] Discovered .editorconfig (${lec.path.parent.toFile().path})" + + " {${lec.entries.joinToString(", ")}}" + ) + } } private fun injectUserData(node: ASTNode, userData: Map) { @@ -188,13 +228,15 @@ 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()) } private fun visitor( rootNode: ASTNode, ruleSets: Iterable, concurrent: Boolean = true, - filter: (fqRuleId: String) -> Boolean = { true } + filter: (rootNode: ASTNode, fqRuleId: String) -> Boolean = this::filterDisabledRules + ): ((node: ASTNode, rule: Rule, fqRuleId: String) -> Unit) -> Unit { val fqrsRestrictedToRoot = mutableListOf>() val fqrs = mutableListOf>() @@ -204,7 +246,7 @@ object KtLint { val prefix = if (ruleSet.id === "standard") "" else "${ruleSet.id}:" for (rule in ruleSet) { val fqRuleId = "$prefix${rule.id}" - if (!filter(fqRuleId)) { + if (!filter(rootNode, fqRuleId)) { continue } val fqr = fqRuleId to rule @@ -254,6 +296,10 @@ object KtLint { } } + private fun filterDisabledRules(rootNode: ASTNode, fqRuleId: String): Boolean { + return rootNode.getUserData(DISABLED_RULES)?.contains(fqRuleId) == false + } + private fun calculateLineColByOffset(text: String): (offset: Int) -> Pair { var i = -1 val e = text.length @@ -292,69 +338,27 @@ object KtLint { /** * Fix style violations. * - * @param text source - * @param ruleSets a collection of "RuleSet"s used to validate source - * @param cb callback that is going to be executed for every lint error - * * @throws ParseException if text is not a valid Kotlin code * @throws RuleExecutionException in case of internal failure caused by a bug in rule implementation */ - fun format(text: String, ruleSets: Iterable, cb: (e: LintError, corrected: Boolean) -> Unit): String = - format(text, ruleSets, emptyMap(), cb, script = false) - - fun format( - text: String, - ruleSets: Iterable, - userData: Map, - cb: (e: LintError, corrected: Boolean) -> Unit - ): String = format(text, ruleSets, userData, cb, script = false) - - /** - * Fix style violations. - * - * @param text script source - * @param ruleSets a collection of "RuleSet"s used to validate source - * @param cb callback that is going to be executed for every lint error - * - * @throws ParseException if text is not a valid Kotlin code - * @throws RuleExecutionException in case of internal failure caused by a bug in rule implementation - */ - fun formatScript( - text: String, - ruleSets: Iterable, - cb: (e: LintError, corrected: Boolean) -> Unit - ): String = - format(text, ruleSets, emptyMap(), cb, script = true) - - fun formatScript( - text: String, - ruleSets: Iterable, - userData: Map, - cb: (e: LintError, corrected: Boolean) -> Unit - ): String = format(text, ruleSets, userData, cb, script = true) - - private fun format( - text: String, - ruleSets: Iterable, - userData: Map, - cb: (e: LintError, corrected: Boolean) -> Unit, - script: Boolean - ): String { - val normalizedText = text.replace("\r\n", "\n").replace("\r", "\n") + fun format(params: Params): String { + val normalizedText = params.text.replace("\r\n", "\n").replace("\r", "\n") val positionByOffset = calculateLineColByOffset(normalizedText) - val fileName = if (script) "file.kts" else "file.kt" - val psiFile = psiFileFactory.createFileFromText(fileName, KotlinLanguage.INSTANCE, normalizedText) as KtFile + val psiFileName = if (params.script) "file.kts" else "file.kt" + val psiFile = psiFileFactory.createFileFromText(psiFileName, KotlinLanguage.INSTANCE, normalizedText) as KtFile val errorElement = psiFile.findErrorElement() if (errorElement != null) { val (line, col) = positionByOffset(errorElement.textOffset) throw ParseException(line, col, errorElement.errorDescription) } val rootNode = psiFile.node - injectUserData(rootNode, userData) + // Passed-in userData overrides .editorconfig + val mergedUserData = userDataResolver(params.editorConfigPath, params.debug)(params.fileName) + params.userData + injectUserData(rootNode, mergedUserData) var isSuppressed = calculateSuppressedRegions(rootNode) var tripped = false var mutated = false - visitor(rootNode, ruleSets, concurrent = false) + visitor(rootNode, params.ruleSets, concurrent = false) .invoke { node, rule, fqRuleId -> // fixme: enforcing suppression based on node.startOffset is wrong // (not just because not all nodes are leaves but because rules are free to emit (and fix!) errors at any position) @@ -378,7 +382,7 @@ object KtLint { } if (tripped) { val errors = mutableListOf>() - visitor(rootNode, ruleSets).invoke { node, rule, fqRuleId -> + visitor(rootNode, params.ruleSets).invoke { node, rule, fqRuleId -> // fixme: enforcing suppression based on node.startOffset is wrong // (not just because not all nodes are leaves but because rules are free to emit (and fix!) errors at any position) if (!isSuppressed(node.startOffset, fqRuleId, node === rootNode)) { @@ -399,9 +403,9 @@ object KtLint { } errors .sortedWith(Comparator { (l), (r) -> if (l.line != r.line) l.line - r.line else l.col - r.col }) - .forEach { (e, corrected) -> cb(e, corrected) } + .forEach { (e, corrected) -> params.cb(e, corrected) } } - return if (mutated) rootNode.text.replace("\n", determineLineSeparator(text, userData)) else text + return if (mutated) rootNode.text.replace("\n", determineLineSeparator(params.text, params.userData)) else params.text } private fun determineLineSeparator(fileContent: String, userData: Map): String { diff --git a/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/EditorConfig.kt b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternal.kt similarity index 92% rename from ktlint/src/main/kotlin/com/pinterest/ktlint/internal/EditorConfig.kt rename to ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternal.kt index 416f35264b..2bf29a9697 100644 --- a/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/EditorConfig.kt +++ b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternal.kt @@ -1,4 +1,4 @@ -package com.pinterest.ktlint.internal +package com.pinterest.ktlint.core.internal import java.io.ByteArrayInputStream import java.nio.file.Files @@ -8,8 +8,11 @@ import java.util.Properties import java.util.concurrent.CompletableFuture import java.util.concurrent.ConcurrentHashMap -class EditorConfig private constructor ( - val parent: EditorConfig?, +/** + * This class handles traversing the filetree and parsing and merging the contents of any discovered .editorconfig files + */ +class EditorConfigInternal private constructor ( + val parent: EditorConfigInternal?, val path: Path, private val data: Map ) : Map by data { @@ -29,8 +32,8 @@ class EditorConfig private constructor ( } .toList() .asReversed() - .fold(null as EditorConfig?) { parent, (path, data) -> - EditorConfig( + .fold(null as EditorConfigInternal?) { parent, (path, data) -> + EditorConfigInternal( parent, path, ( parent?.data @@ -41,22 +44,22 @@ class EditorConfig private constructor ( fun cached(): EditorConfigLookup = object : EditorConfigLookup { // todo: concurrent radix tree can potentially save a lot of memory here - private val cache = ConcurrentHashMap>() + private val cache = ConcurrentHashMap>() override fun of(dir: String) = of(Paths.get(dir)) - override fun of(dir: Path): EditorConfig? { + override fun of(dir: Path): EditorConfigInternal? { val cachedEditorConfig = cache[dir] return when { cachedEditorConfig != null -> cachedEditorConfig.get() else -> { - val future = CompletableFuture() + val future = CompletableFuture() val cachedFuture = cache.putIfAbsent(dir, future) if (cachedFuture == null) { val editorConfigPath = dir.resolve(".editorconfig") val parent = if (dir.parent != null) of(dir.parent) else null try { val editorConfig = if (Files.exists(editorConfigPath)) { - EditorConfig( + EditorConfigInternal( parent, editorConfigPath, ( parent?.data @@ -181,6 +184,6 @@ class EditorConfig private constructor ( } interface EditorConfigLookup { - fun of(dir: String): EditorConfig? - fun of(dir: Path): EditorConfig? + fun of(dir: String): EditorConfigInternal? + fun of(dir: Path): EditorConfigInternal? } diff --git a/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/DisabledRulesTest.kt b/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/DisabledRulesTest.kt new file mode 100644 index 0000000000..708cb4e82a --- /dev/null +++ b/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/DisabledRulesTest.kt @@ -0,0 +1,67 @@ +package com.pinterest.ktlint.core + +import com.pinterest.ktlint.core.ast.ElementType +import java.util.ArrayList +import org.assertj.core.api.Assertions.assertThat +import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.junit.Test + +class DisabledRulesTest { + + @Test + fun testDisabledRule() { + class NoVarRule : Rule("no-var") { + override fun visit( + node: ASTNode, + autoCorrect: Boolean, + emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit + ) { + if (node.elementType == ElementType.VAR_KEYWORD) { + emit(node.startOffset, "Unexpected var, use val instead", false) + } + } + } + + assertThat( + ArrayList().apply { + KtLint.lint( + KtLint.Params( + text = "var foo", + ruleSets = listOf(RuleSet("standard", NoVarRule())), + cb = { e, _ -> add(e) } + ) + ) + } + ).isEqualTo( + listOf( + LintError(1, 1, "no-var", "Unexpected var, use val instead") + ) + ) + + assertThat( + ArrayList().apply { + KtLint.lint( + KtLint.Params( + text = "var foo", + ruleSets = listOf(RuleSet("standard", NoVarRule())), + cb = { e, _ -> add(e) }, + userData = mapOf(("disabled_rules" to "no-var")) + ) + ) + } + ).isEmpty() + + assertThat( + ArrayList().apply { + KtLint.lint( + KtLint.Params( + text = "var foo", + ruleSets = listOf(RuleSet("experimental", NoVarRule())), + cb = { e, _ -> add(e) }, + userData = mapOf(("disabled_rules" to "experimental:no-var")) + ) + ) + } + ).isEmpty() + } +} diff --git a/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/ErrorSuppressionTest.kt b/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/ErrorSuppressionTest.kt index b7f38609cd..dfbda068b7 100644 --- a/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/ErrorSuppressionTest.kt +++ b/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/ErrorSuppressionTest.kt @@ -25,7 +25,13 @@ class ErrorSuppressionTest { } fun lint(text: String) = ArrayList().apply { - KtLint.lint(text, listOf(RuleSet("standard", NoWildcardImportsRule()))) { e -> add(e) } + KtLint.lint( + KtLint.Params( + text = text, + ruleSets = listOf(RuleSet("standard", NoWildcardImportsRule())), + cb = { e, _ -> add(e) } + ) + ) } assertThat( lint( diff --git a/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/KtLintTest.kt b/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/KtLintTest.kt index c05b28d84b..46bc6f4698 100644 --- a/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/KtLintTest.kt +++ b/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/KtLintTest.kt @@ -26,17 +26,20 @@ class KtLintTest { } val bus = mutableListOf() KtLint.lint( - "fun main() {}", - listOf( - RuleSet( - "standard", - object : R(bus, "d"), Rule.Modifier.RestrictToRootLast {}, - R(bus, "b"), - object : R(bus, "a"), Rule.Modifier.RestrictToRoot {}, - R(bus, "c") - ) + KtLint.Params( + text = "fun main() {}", + ruleSets = listOf( + RuleSet( + "standard", + object : R(bus, "d"), Rule.Modifier.RestrictToRootLast {}, + R(bus, "b"), + object : R(bus, "a"), Rule.Modifier.RestrictToRoot {}, + R(bus, "c") + ) + ), + cb = { _, _ -> } ) - ) {} + ) assertThat(bus).isEqualTo(listOf("file:a", "file:b", "file:c", "b", "c", "file:d")) } } diff --git a/ktlint/src/test/kotlin/com/pinterest/ktlint/internal/EditorConfigTest.kt b/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternalTest.kt similarity index 67% rename from ktlint/src/test/kotlin/com/pinterest/ktlint/internal/EditorConfigTest.kt rename to ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternalTest.kt index 03d2815782..6705ecc646 100644 --- a/ktlint/src/test/kotlin/com/pinterest/ktlint/internal/EditorConfigTest.kt +++ b/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/internal/EditorConfigInternalTest.kt @@ -1,4 +1,4 @@ -package com.pinterest.ktlint.internal +package com.pinterest.ktlint.core.internal import com.google.common.jimfs.Configuration import com.google.common.jimfs.Jimfs @@ -6,7 +6,7 @@ import java.nio.file.Files import org.assertj.core.api.Assertions.assertThat import org.junit.Test -class EditorConfigTest { +class EditorConfigInternalTest { @Test fun testParentDirectoryFallback() { @@ -38,7 +38,7 @@ class EditorConfigTest { ) ) { Files.write(fs.getPath("/projects/project-1/.editorconfig"), cfg.trimIndent().toByteArray()) - val editorConfig = EditorConfig.of(fs.getPath("/projects/project-1/project-1-subdirectory")) + 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()) @@ -74,7 +74,7 @@ class EditorConfigTest { indent_size = 2 """.trimIndent().toByteArray() ) - EditorConfig.of(fs.getPath("/projects/project-1/project-1-subdirectory")).let { editorConfig -> + 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( @@ -84,7 +84,7 @@ class EditorConfigTest { ) ) } - EditorConfig.of(fs.getPath("/projects/project-1")).let { editorConfig -> + EditorConfigInternal.of(fs.getPath("/projects/project-1")).let { editorConfig -> assertThat(editorConfig?.parent).isNull() assertThat(editorConfig?.toMap()).isEqualTo( mapOf( @@ -93,7 +93,7 @@ class EditorConfigTest { ) ) } - EditorConfig.of(fs.getPath("/projects")).let { editorConfig -> + EditorConfigInternal.of(fs.getPath("/projects")).let { editorConfig -> assertThat(editorConfig?.parent).isNull() assertThat(editorConfig?.toMap()).isEqualTo( mapOf( @@ -105,22 +105,22 @@ class EditorConfigTest { @Test fun testSectionParsing() { - assertThat(EditorConfig.parseSection("*")).isEqualTo(listOf("*")) - assertThat(EditorConfig.parseSection("*.{js,py}")).isEqualTo(listOf("*.js", "*.py")) - assertThat(EditorConfig.parseSection("*.py")).isEqualTo(listOf("*.py")) - assertThat(EditorConfig.parseSection("Makefile")).isEqualTo(listOf("Makefile")) - assertThat(EditorConfig.parseSection("lib/**.js")).isEqualTo(listOf("lib/**.js")) - assertThat(EditorConfig.parseSection("{package.json,.travis.yml}")) + assertThat(EditorConfigInternal.parseSection("*")).isEqualTo(listOf("*")) + assertThat(EditorConfigInternal.parseSection("*.{js,py}")).isEqualTo(listOf("*.js", "*.py")) + assertThat(EditorConfigInternal.parseSection("*.py")).isEqualTo(listOf("*.py")) + assertThat(EditorConfigInternal.parseSection("Makefile")).isEqualTo(listOf("Makefile")) + assertThat(EditorConfigInternal.parseSection("lib/**.js")).isEqualTo(listOf("lib/**.js")) + assertThat(EditorConfigInternal.parseSection("{package.json,.travis.yml}")) .isEqualTo(listOf("package.json", ".travis.yml")) } @Test fun testMalformedSectionParsing() { - assertThat(EditorConfig.parseSection("")).isEqualTo(listOf("")) - assertThat(EditorConfig.parseSection(",*")).isEqualTo(listOf("", "*")) - assertThat(EditorConfig.parseSection("*,")).isEqualTo(listOf("*", "")) - assertThat(EditorConfig.parseSection("*.{js,py")).isEqualTo(listOf("*.js", "*.py")) - assertThat(EditorConfig.parseSection("*.{js,{py")).isEqualTo(listOf("*.js", "*.{py")) - assertThat(EditorConfig.parseSection("*.py}")).isEqualTo(listOf("*.py}")) + assertThat(EditorConfigInternal.parseSection("")).isEqualTo(listOf("")) + assertThat(EditorConfigInternal.parseSection(",*")).isEqualTo(listOf("", "*")) + assertThat(EditorConfigInternal.parseSection("*,")).isEqualTo(listOf("*", "")) + assertThat(EditorConfigInternal.parseSection("*.{js,py")).isEqualTo(listOf("*.js", "*.py")) + assertThat(EditorConfigInternal.parseSection("*.{js,{py")).isEqualTo(listOf("*.js", "*.{py")) + assertThat(EditorConfigInternal.parseSection("*.py}")).isEqualTo(listOf("*.py}")) } } diff --git a/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/IndentationRule.kt b/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/IndentationRule.kt index 055609684a..f102aebc04 100644 --- a/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/IndentationRule.kt +++ b/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/IndentationRule.kt @@ -131,7 +131,7 @@ class IndentationRule : Rule("indent"), Rule.Modifier.RestrictToRootLast { emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit ) { val editorConfig = node.getUserData(KtLint.EDITOR_CONFIG_USER_DATA_KEY)!! - if (editorConfig.indentStyle == EditorConfig.IntentStyle.TAB || editorConfig.indentSize <= 1) { + if (editorConfig.indentStyle == EditorConfig.IndentStyle.TAB || editorConfig.indentSize <= 1) { return } reset() diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/StandardRuleSetProvider.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/StandardRuleSetProvider.kt index 57fc025ee7..d6a938ac07 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/StandardRuleSetProvider.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/StandardRuleSetProvider.kt @@ -5,27 +5,23 @@ import com.pinterest.ktlint.core.RuleSetProvider class StandardRuleSetProvider : RuleSetProvider { + // Note: some of these rules may be disabled by default. See the default .editorconfig. override fun get(): RuleSet = RuleSet( "standard", - // disabled ("./mvnw clean verify" fails with "Internal Error") - // AnnotationRule(), + AnnotationRule(), ChainWrappingRule(), CommentSpacingRule(), FilenameRule(), FinalNewlineRule(), - // disabled until there is a way to suppress rules globally (https://git.io/fhxnm) - // PackageNameRule(), - // disabled until auto-correct is working properly - // (e.g. try formatting "if (true)\n return { _ ->\n _\n}") - // MultiLineIfElseRule(), + PackageNameRule(), + MultiLineIfElseRule(), IndentationRule(), MaxLineLengthRule(), ModifierOrderRule(), NoBlankLineBeforeRbraceRule(), NoConsecutiveBlankLinesRule(), NoEmptyClassBodyRule(), - // disabled until it's clear what to do in case of `import _.it` - // NoItParamInMultilineLambdaRule(), + NoItParamInMultilineLambdaRule(), NoLineBreakAfterElseRule(), NoLineBreakBeforeAssignmentRule(), NoMultipleSpacesRule(), @@ -33,11 +29,7 @@ class StandardRuleSetProvider : RuleSetProvider { NoTrailingSpacesRule(), NoUnitReturnRule(), NoUnusedImportsRule(), - // Disabling because it is now allowed by the Jetbrains styleguide, although it is still disallowed by - // the Android styleguide. - // Re-enable when there is a way to globally disable rules - // See discussion here: https://github.com/pinterest/ktlint/issues/48 - // NoWildcardImportsRule(), + NoWildcardImportsRule(), ParameterListWrappingRule(), SpacingAroundColonRule(), SpacingAroundCommaRule(), diff --git a/ktlint-test/src/main/kotlin/com/pinterest/ktlint/test/RuleExtension.kt b/ktlint-test/src/main/kotlin/com/pinterest/ktlint/test/RuleExtension.kt index d7b4d2f2a2..be247a6d1a 100644 --- a/ktlint-test/src/main/kotlin/com/pinterest/ktlint/test/RuleExtension.kt +++ b/ktlint-test/src/main/kotlin/com/pinterest/ktlint/test/RuleExtension.kt @@ -11,50 +11,43 @@ import org.assertj.core.util.diff.DiffUtils.generateUnifiedDiff fun Rule.lint(text: String, userData: Map = emptyMap(), script: Boolean = false): List { val res = ArrayList() val debug = debugAST() - val f: L = if (script) KtLint::lintScript else KtLint::lint - f( - text, - (if (debug) listOf(RuleSet("debug", DumpAST())) else emptyList()) + - listOf(RuleSet("standard", this@lint)), - userData - ) { e -> - if (debug) { - System.err.println("^^ lint error") - } - res.add(e) - } + KtLint.lint( + KtLint.Params( + text = text, + ruleSets = (if (debug) listOf(RuleSet("debug", DumpAST())) else emptyList()) + + listOf(RuleSet("standard", this@lint)), + userData = userData, + script = script, + cb = { e, _ -> + if (debug) { + System.err.println("^^ lint error") + } + res.add(e) + } + ) + ) return res } -private typealias L = ( - text: String, - ruleSets: Iterable, - userData: Map, - cb: (e: LintError) -> Unit -) -> Unit - fun Rule.format( text: String, userData: Map = emptyMap(), cb: (e: LintError, corrected: Boolean) -> Unit = { _, _ -> }, script: Boolean = false ): String { - val f: F = if (script) KtLint::formatScript else KtLint::format - return f( - text, - (if (debugAST()) listOf(RuleSet("debug", DumpAST())) else emptyList()) + - listOf(RuleSet("standard", this@format)), - userData, cb + return KtLint.format( + KtLint.Params( + text = text, + ruleSets = (if (debugAST()) listOf(RuleSet("debug", DumpAST())) else emptyList()) + + listOf(RuleSet("standard", this@format)), + userData = userData, + script = script, + cb = cb + ) + ) } -private typealias F = ( - text: String, - ruleSets: Iterable, - userData: Map, - cb: (e: LintError, corrected: Boolean) -> Unit -) -> String - fun Rule.diffFileLint(path: String, userData: Map = emptyMap()): String { val resourceText = getResourceAsText(path).replace("\r\n", "\n") val dividerIndex = resourceText.lastIndexOf("\n// expect\n") diff --git a/ktlint/build.gradle b/ktlint/build.gradle index 216f1e395c..cc5de307eb 100644 --- a/ktlint/build.gradle +++ b/ktlint/build.gradle @@ -43,5 +43,4 @@ dependencies { testImplementation deps.junit testImplementation deps.assertj - testImplementation deps.jimfs } diff --git a/ktlint/pom.xml b/ktlint/pom.xml index b4e15f2e69..9c5094d17b 100644 --- a/ktlint/pom.xml +++ b/ktlint/pom.xml @@ -175,12 +175,6 @@ ${assertj.version} test - - com.google.jimfs - jimfs - ${jimfs.version} - test - diff --git a/ktlint/src/main/kotlin/com/pinterest/ktlint/Main.kt b/ktlint/src/main/kotlin/com/pinterest/ktlint/Main.kt index 4812aac3f4..ca425a2b62 100644 --- a/ktlint/src/main/kotlin/com/pinterest/ktlint/Main.kt +++ b/ktlint/src/main/kotlin/com/pinterest/ktlint/Main.kt @@ -1,15 +1,12 @@ @file:JvmName("Main") package com.pinterest.ktlint -import com.pinterest.ktlint.core.KtLint import com.pinterest.ktlint.core.LintError import com.pinterest.ktlint.core.ParseException import com.pinterest.ktlint.core.Reporter import com.pinterest.ktlint.core.ReporterProvider import com.pinterest.ktlint.core.RuleExecutionException -import com.pinterest.ktlint.core.RuleSet import com.pinterest.ktlint.core.RuleSetProvider -import com.pinterest.ktlint.internal.EditorConfig import com.pinterest.ktlint.internal.GitPreCommitHookSubCommand import com.pinterest.ktlint.internal.GitPrePushHookSubCommand import com.pinterest.ktlint.internal.IntellijIDEAIntegration @@ -18,6 +15,7 @@ import com.pinterest.ktlint.internal.MavenDependencyResolver import com.pinterest.ktlint.internal.PrintASTSubCommand import com.pinterest.ktlint.internal.expandTilde import com.pinterest.ktlint.internal.fileSequence +import com.pinterest.ktlint.internal.formatFile import com.pinterest.ktlint.internal.lintFile import com.pinterest.ktlint.internal.location import com.pinterest.ktlint.internal.printHelpOrVersionUsage @@ -25,7 +23,6 @@ import java.io.File import java.io.IOException import java.io.PrintStream import java.net.URLDecoder -import java.nio.file.Path import java.nio.file.Paths import java.util.ArrayList import java.util.LinkedHashMap @@ -34,7 +31,6 @@ import java.util.Scanner import java.util.ServiceLoader import java.util.concurrent.ArrayBlockingQueue import java.util.concurrent.Callable -import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.Executors import java.util.concurrent.Future import java.util.concurrent.TimeUnit @@ -48,7 +44,6 @@ import org.eclipse.aether.repository.RemoteRepository import org.eclipse.aether.repository.RepositoryPolicy import org.eclipse.aether.repository.RepositoryPolicy.CHECKSUM_POLICY_IGNORE import org.eclipse.aether.repository.RepositoryPolicy.UPDATE_POLICY_NEVER -import org.jetbrains.kotlin.backend.common.onlyIf import picocli.CommandLine import picocli.CommandLine.Command import picocli.CommandLine.Option @@ -254,6 +249,7 @@ class KtlintCommandLine { exitProcess(0) } val start = System.currentTimeMillis() + // load 3rd party ruleset(s) (if any) val dependencyResolver = lazy(LazyThreadSafetyMode.NONE) { buildDependencyResolver() } if (!rulesets.isEmpty()) { @@ -279,18 +275,24 @@ class KtlintCommandLine { } val tripped = AtomicBoolean() val reporter = loadReporter(dependencyResolver) { tripped.get() } - val resolveUserData = userDataResolver() data class LintErrorWithCorrectionInfo(val err: LintError, val corrected: Boolean) + val userData = mapOf("android" to android.toString()) fun process(fileName: String, fileContent: String): List { if (debug) { val fileLocation = if (fileName != "") File(fileName).location(relative) else fileName System.err.println("[DEBUG] Checking $fileLocation") } val result = ArrayList() - val userData = resolveUserData(fileName) if (format) { val formattedFileContent = try { - format(fileName, fileContent, ruleSetProviders.map { it.second.get() }, userData) { err, corrected -> + formatFile( + fileName, + fileContent, + ruleSetProviders.map { it.second.get() }, + userData, + editorConfigPath, + debug + ) { err, corrected -> if (!corrected) { result.add(LintErrorWithCorrectionInfo(err, corrected)) tripped.set(true) @@ -310,7 +312,14 @@ class KtlintCommandLine { } } else { try { - lintFile(fileName, fileContent, ruleSetProviders.map { it.second.get() }, userData) { err -> + lintFile( + fileName, + fileContent, + ruleSetProviders.map { it.second.get() }, + userData, + editorConfigPath, + debug + ) { err -> result.add(LintErrorWithCorrectionInfo(err, false)) tripped.set(true) } @@ -358,50 +367,6 @@ class KtlintCommandLine { } } - private fun userDataResolver(): (String) -> Map { - val cliUserData = mapOf("android" to android.toString()) - if (editorConfigPath != null) { - val userData = ( - EditorConfig.of(File(editorConfigPath).canonicalPath) - ?.onlyIf({ debug }) { printEditorConfigChain(it) } - ?: emptyMap() - ) + cliUserData - return fun (fileName: String) = userData + ("file_path" to fileName) - } - val workdirUserData = lazy { - ( - EditorConfig.of(workDir) - ?.onlyIf({ debug }) { printEditorConfigChain(it) } - ?: emptyMap() - ) + cliUserData - } - val editorConfig = EditorConfig.cached() - val editorConfigSet = ConcurrentHashMap() - return fun (fileName: String): Map { - if (fileName == "") { - return workdirUserData.value - } - return ( - editorConfig.of(Paths.get(fileName).parent) - ?.onlyIf({ debug }) { - printEditorConfigChain(it) { - editorConfigSet.put(it.path, true) != true - } - } - ?: emptyMap() - ) + cliUserData + ("file_path" to fileName) - } - } - - private fun printEditorConfigChain(ec: EditorConfig, predicate: (EditorConfig) -> Boolean = { true }) { - for (lec in generateSequence(ec) { it.parent }.takeWhile(predicate)) { - System.err.println( - "[DEBUG] Discovered .editorconfig (${lec.path.parent.toFile().location(relative)})" + - " {${lec.entries.joinToString(", ")}}" - ) - } - } - private fun loadReporter(dependencyResolver: Lazy, tripped: () -> Boolean): Reporter { data class ReporterTemplate(val id: String, val artifact: String?, val config: Map, var output: String?) val tpls = (if (reporters.isEmpty()) listOf("plain") else reporters) @@ -644,19 +609,6 @@ class KtlintCommandLine { map } - private fun format( - fileName: String, - text: String, - ruleSets: Iterable, - userData: Map, - cb: (e: LintError, corrected: Boolean) -> Unit - ): String = - if (fileName.endsWith(".kt", ignoreCase = true)) { - KtLint.format(text, ruleSets, userData, cb) - } else { - KtLint.formatScript(text, ruleSets, userData, cb) - } - private fun java.net.URLClassLoader.addURLs(url: Iterable) { val method = java.net.URLClassLoader::class.java.getDeclaredMethod("addURL", java.net.URL::class.java) method.isAccessible = true diff --git a/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/FileUtils.kt b/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/FileUtils.kt index 7eb0df7561..8f0ac88847 100644 --- a/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/FileUtils.kt +++ b/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/FileUtils.kt @@ -1,8 +1,7 @@ package com.pinterest.ktlint.internal import com.github.shyiko.klob.Glob -import com.pinterest.ktlint.core.KtLint.lint -import com.pinterest.ktlint.core.KtLint.lintScript +import com.pinterest.ktlint.core.KtLint import com.pinterest.ktlint.core.LintError import com.pinterest.ktlint.core.RuleSet import java.io.File @@ -42,14 +41,50 @@ internal fun File.location( */ internal fun lintFile( fileName: String, - fileContent: String, - ruleSetList: List, + fileContents: String, + ruleSets: List, userData: Map = emptyMap(), + editorConfigPath: String? = null, + debug: Boolean = false, lintErrorCallback: (LintError) -> Unit = {} ) { - if (fileName.endsWith(".kt", ignoreCase = true)) { - lint(fileContent, ruleSetList, userData, lintErrorCallback) - } else { - lintScript(fileContent, ruleSetList, userData, lintErrorCallback) - } + KtLint.lint( + KtLint.Params( + fileName = fileName, + text = fileContents, + ruleSets = ruleSets, + userData = userData, + script = !fileName.endsWith(".kt", ignoreCase = true), + editorConfigPath = editorConfigPath, + cb = { e, _ -> + lintErrorCallback(e) + }, + debug = debug + ) + ) } + +/** + * Format a kotlin file or script file + */ +internal fun formatFile( + fileName: String, + fileContents: String, + ruleSets: Iterable, + userData: Map, + editorConfigPath: String?, + debug: Boolean, + cb: (e: LintError, corrected: Boolean) -> Unit +): String = + KtLint.format( + KtLint.Params( + fileName = fileName, + text = fileContents, + ruleSets = ruleSets, + userData = userData, + script = !fileName.endsWith(".kt", ignoreCase = true), + editorConfigPath = editorConfigPath, + cb = cb, + debug = debug + ) + ) diff --git a/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/IntellijIDEAIntegration.kt b/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/IntellijIDEAIntegration.kt index 6bd4679b38..d3ff09d774 100644 --- a/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/IntellijIDEAIntegration.kt +++ b/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/IntellijIDEAIntegration.kt @@ -1,6 +1,7 @@ package com.pinterest.ktlint.internal import com.github.shyiko.klob.Glob +import com.pinterest.ktlint.core.internal.EditorConfigInternal import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream import java.io.IOException @@ -26,7 +27,7 @@ object IntellijIDEAIntegration { if (!Files.isDirectory(workDir.resolve(".idea"))) { throw ProjectNotFoundException() } - val editorConfig: Map = EditorConfig.of(".") ?: emptyMap() + val editorConfig: Map = EditorConfigInternal.of(".") ?: emptyMap() val indentSize = editorConfig["indent_size"]?.toIntOrNull() ?: 4 val continuationIndentSize = editorConfig["continuation_indent_size"]?.toIntOrNull() ?: 4 val updates = if (local) {