Skip to content

Commit

Permalink
Add support for disabled_rules property in .editorconfig for globally…
Browse files Browse the repository at this point in the history 
… disabling rules (pinterest#503)

* Add support for disabled_rules property to .editorconfig for globally disabling rules

Fixes pinterest#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
  • Loading branch information
@shashachu
shashachu authored Jul 18, 2019
1 parent b96391e commit a599844
Show file tree
Hide file tree
Showing 18 changed files with 341 additions and 278 deletions.
5 changes: 5 additions & 0 deletions .editorconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions ktlint-core/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ dependencies {

testImplementation deps.junit
testImplementation deps.assertj
testImplementation deps.jimfs
}
6 changes: 6 additions & 0 deletions ktlint-core/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@
<version>${assertj.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.jimfs</groupId>
<artifactId>jimfs</artifactId>
<version>${jimfs.version}</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
10 changes: 6 additions & 4 deletions ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/EditorConfig.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -17,8 +19,8 @@ interface EditorConfig {
companion object {
fun fromMap(map: Map<String, String>): 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
Expand Down
210 changes: 107 additions & 103 deletions ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -37,10 +43,32 @@ object KtLint {
val EDITOR_CONFIG_USER_DATA_KEY = Key<EditorConfig>("EDITOR_CONFIG")
val ANDROID_USER_DATA_KEY = Key<Boolean>("ANDROID")
val FILE_PATH_USER_DATA_KEY = Key<String>("FILE_PATH")
val DISABLED_RULES = Key<Set<String>>("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<RuleSet>,
val userData: Map<String, String> = 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 {
Expand Down Expand Up @@ -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<RuleSet>, cb: (e: LintError) -> Unit) {
lint(text, ruleSets, emptyMap(), cb, script = false)
}

fun lint(text: String, ruleSets: Iterable<RuleSet>, userData: Map<String, String>, 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<RuleSet>, cb: (e: LintError) -> Unit) {
lint(text, ruleSets, emptyMap(), cb, script = true)
}

fun lintScript(
text: String,
ruleSets: Iterable<RuleSet>,
userData: Map<String, String>,
cb: (e: LintError) -> Unit
) {
lint(text, ruleSets, userData, cb, script = true)
}

private fun lint(
text: String,
ruleSets: Iterable<RuleSet>,
userData: Map<String, String>,
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<LintError>()
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)) {
Expand All @@ -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<String, String> {
if (editorConfigPath != null) {
val userData = (
EditorConfigInternal.of(File(editorConfigPath).canonicalPath)
?.onlyIf({ debug }) { printEditorConfigChain(it) }
?: emptyMap<String, String>()
)
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<String, String>()
)
}
val editorConfig = EditorConfigInternal.cached()
val editorConfigSet = ConcurrentHashMap<Path, Boolean>()
return fun (fileName: String?): Map<String, String> {
if (fileName == null) {
return emptyMap()
}

if (fileName == "<text>") {
return workdirUserData.value
}
return (
editorConfig.of(Paths.get(fileName).parent)
?.onlyIf({ debug }) {
printEditorConfigChain(it) {
editorConfigSet.put(it.path, true) != true
}
}
?: emptyMap<String, String>()
) + ("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<String, String>) {
Expand All @@ -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<RuleSet>,
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<Pair<String, Rule>>()
val fqrs = mutableListOf<Pair<String, Rule>>()
Expand All @@ -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
Expand Down Expand Up @@ -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<Int, Int> {
var i = -1
val e = text.length
Expand Down Expand Up @@ -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<RuleSet>, cb: (e: LintError, corrected: Boolean) -> Unit): String =
format(text, ruleSets, emptyMap<String, String>(), cb, script = false)

fun format(
text: String,
ruleSets: Iterable<RuleSet>,
userData: Map<String, String>,
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<RuleSet>,
cb: (e: LintError, corrected: Boolean) -> Unit
): String =
format(text, ruleSets, emptyMap(), cb, script = true)

fun formatScript(
text: String,
ruleSets: Iterable<RuleSet>,
userData: Map<String, String>,
cb: (e: LintError, corrected: Boolean) -> Unit
): String = format(text, ruleSets, userData, cb, script = true)

private fun format(
text: String,
ruleSets: Iterable<RuleSet>,
userData: Map<String, String>,
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)
Expand All @@ -378,7 +382,7 @@ object KtLint {
}
if (tripped) {
val errors = mutableListOf<Pair<LintError, Boolean>>()
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)) {
Expand All @@ -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, String>): String {
Expand Down
Loading

0 comments on commit a599844

Please sign in to comment.