Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump KtLint to 0.44.0 and add UnnecessaryParenthesesBeforeTrailingLamda rule #4630

Merged
merged 8 commits into from Mar 19, 2022
Merged
Expand Up @@ -26,7 +26,7 @@ class SpekTestDiscoverySpec(private val env: KotlinCoreEnvironment) {
val s = "simple"
val p = Paths.get("")
val f = File("")
"""
"""
)

assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()
Expand Down
1 change: 1 addition & 0 deletions detekt-core/src/main/resources/deprecation.properties
Expand Up @@ -8,3 +8,4 @@ style>UnderscoresInNumericLiterals>acceptableDecimalLength=Use `acceptableLength
style>UnnecessaryAbstractClass>excludeAnnotatedClasses=Use `ignoreAnnotated` instead
style>UseDataClass>excludeAnnotatedClasses=Use `ignoreAnnotated` instead
formatting>Indentation>continuationIndentSize=`continuationIndentSize` is ignored by KtLint and will have no effect
formatting>ParameterListWrapping>indentSize=`indentSize` is ignored by KtLint and will have no effect
Expand Up @@ -170,7 +170,7 @@ class SuppressionSpec {
println("FAILED TEST")
}
}
"""
"""
)
val rule = TestRule()
rule.visitFile(ktFile)
Expand All @@ -188,7 +188,7 @@ class SuppressionSpec {
println("FAILED TEST")
}
}
"""
"""
)
val rule = TestRule()
rule.visitFile(ktFile)
Expand All @@ -206,7 +206,7 @@ class SuppressionSpec {
println("FAILED TEST")
}
}
"""
"""
)
val rule = TestRule()
rule.visitFile(ktFile)
Expand All @@ -228,7 +228,7 @@ class SuppressionSpec {
println("FAILED TEST")
}
}
"""
"""
)
val rule = TestRule(TestConfig(mutableMapOf("aliases" to "[MyTest]")))
rule.visitFile(ktFile)
Expand Down Expand Up @@ -314,7 +314,7 @@ private fun isSuppressedBy(annotation: String, argument: String): Boolean {
val annotated = """
@$annotation("$argument")
class Test
"""
"""
val file = compileContentForTest(annotated)
val annotatedClass = file.children.first { it is KtClass } as KtAnnotated
return annotatedClass.isSuppressedBy("Test", setOf("alias"))
Expand Down
Expand Up @@ -37,7 +37,7 @@ class SupportConfigValidationSpec {
my_additional_properties:
magic_number: 7
magic_string: 'Hello World'
"""
"""
)
createProcessingSettings(testDir, config).use {
assertThatCode { checkConfiguration(it, spec.getDefaultConfiguration()) }
Expand All @@ -56,7 +56,7 @@ class SupportConfigValidationSpec {
TooManyFunctions:
# This property is tested via the SampleConfigValidator
active: 1 # should be true
"""
"""
)
createProcessingSettings(testDir, config).use {
assertThatCode { checkConfiguration(it, spec.getDefaultConfiguration()) }
Expand Down Expand Up @@ -86,7 +86,7 @@ class SupportConfigValidationSpec {
my_additional_properties:
magic_number: 7
magic_string: 'Hello World'
"""
"""
)
createProcessingSettings(testDir, config).use {
assertThatCode { checkConfiguration(it, spec.getDefaultConfiguration()) }
Expand Down
6 changes: 5 additions & 1 deletion detekt-formatting/build.gradle.kts
Expand Up @@ -13,6 +13,7 @@ dependencies {
implementation(libs.ktlint.rulesetExperimental) {
exclude(group = "org.jetbrains.kotlin")
}
implementation(libs.ktlint.microutilsKotlinLoggingJvm)

testImplementation(projects.detektTest)
testImplementation(libs.assertj)
Expand All @@ -22,7 +23,10 @@ tasks.build { finalizedBy(":detekt-generator:generateDocumentation") }

val depsToPackage = setOf(
"org.ec4j.core",
"com.pinterest.ktlint"
"com.pinterest.ktlint",
"com.pinterest.ktlint",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fuuuu

"io.github.microutils",
"org.slf4j",
)

tasks.jar {
Expand Down
@@ -1,6 +1,9 @@
package io.gitlab.arturbosch.detekt.formatting

import com.pinterest.ktlint.core.EditorConfig.Companion.fromMap
import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.Rule.VisitorModifier.RunAsLateAsPossible
import com.pinterest.ktlint.core.Rule.VisitorModifier.RunOnRootNodeOnly
import com.pinterest.ktlint.core.api.FeatureInAlphaState
import com.pinterest.ktlint.core.api.UsesEditorConfigProperties
import io.github.detekt.psi.fileName
Expand Down Expand Up @@ -37,6 +40,12 @@ abstract class FormattingRule(config: Config) : Rule(config) {
protected val isAndroid
get() = FormattingProvider.android.value(ruleSetConfig)

val runOnRootNodeOnly
get() = RunOnRootNodeOnly in wrapping.visitorModifiers

val runAsLateAsPossible
get() = RunAsLateAsPossible in wrapping.visitorModifiers

private var positionByOffset: (offset: Int) -> Pair<Int, Int> by SingleAssign()
private var root: KtFile by SingleAssign()

Expand Down Expand Up @@ -77,6 +86,13 @@ abstract class FormattingRule(config: Config) : Rule(config) {
if (ruleShouldOnlyRunOnFileNode(node)) {
return
}

// KtLint 0.44.0 is assuming that KtLint.EDITOR_CONFIG_USER_DATA_KEY is available on all the nodes.
// If not, it crashes with a NPE. Here we're patching their behavior.
if (node.getUserData(KtLint.EDITOR_CONFIG_USER_DATA_KEY) == null) {
node.putUserData(KtLint.EDITOR_CONFIG_USER_DATA_KEY, fromMap(emptyMap()))
}

wrapping.visit(node, autoCorrect) { offset, message, _ ->
val (line, column) = positionByOffset(offset)
val location = Location(
Expand All @@ -102,5 +118,5 @@ abstract class FormattingRule(config: Config) : Rule(config) {
TextLocation(node.startOffset, node.psi.endOffset)

private fun ruleShouldOnlyRunOnFileNode(node: ASTNode) =
wrapping is com.pinterest.ktlint.core.Rule.Modifier.RestrictToRoot && node !is FileASTNode
runOnRootNodeOnly && node !is FileASTNode
}
@@ -1,8 +1,5 @@
package io.gitlab.arturbosch.detekt.formatting

import com.pinterest.ktlint.core.Rule.Modifier.Last
import com.pinterest.ktlint.core.Rule.Modifier.RestrictToRoot
import com.pinterest.ktlint.core.Rule.Modifier.RestrictToRootLast
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.MultiRule
import io.gitlab.arturbosch.detekt.api.Rule
Expand Down Expand Up @@ -48,6 +45,7 @@ import io.gitlab.arturbosch.detekt.formatting.wrappers.SpacingBetweenDeclaration
import io.gitlab.arturbosch.detekt.formatting.wrappers.SpacingBetweenDeclarationsWithComments
import io.gitlab.arturbosch.detekt.formatting.wrappers.StringTemplate
import io.gitlab.arturbosch.detekt.formatting.wrappers.TrailingComma
import io.gitlab.arturbosch.detekt.formatting.wrappers.UnnecessaryParenthesesBeforeTrailingLambda
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.JavaDummyElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.JavaDummyHolder
Expand Down Expand Up @@ -105,6 +103,7 @@ class KtLintMultiRule(config: Config = Config.empty) : MultiRule() {
SpacingBetweenDeclarationsWithAnnotations(config),
SpacingBetweenDeclarationsWithComments(config),
TrailingComma(config),
UnnecessaryParenthesesBeforeTrailingLambda(config),
)

override fun visit(root: KtFile) {
Expand All @@ -121,11 +120,10 @@ class KtLintMultiRule(config: Config = Config.empty) : MultiRule() {
val runLastOnRoot = mutableListOf<FormattingRule>()
val runLast = mutableListOf<FormattingRule>()
for (rule in activeRules.filterIsInstance<FormattingRule>()) {
when (rule.wrapping) {
is Last -> runLast.add(rule)
// RestrictToRootLast implements RestrictToRoot, so we have to perform this check first
is RestrictToRootLast -> runLastOnRoot.add(rule)
is RestrictToRoot -> runFirstOnRoot.add(rule)
when {
rule.runOnRootNodeOnly && rule.runAsLateAsPossible -> runLastOnRoot.add(rule)
rule.runOnRootNodeOnly -> runFirstOnRoot.add(rule)
rule.runAsLateAsPossible -> runLast.add(rule)
else -> other.add(rule)
}
}
Expand Down
Expand Up @@ -8,7 +8,6 @@ import io.gitlab.arturbosch.detekt.api.internal.ActiveByDefault
import io.gitlab.arturbosch.detekt.api.internal.AutoCorrectable
import io.gitlab.arturbosch.detekt.api.internal.Configuration
import io.gitlab.arturbosch.detekt.formatting.FormattingRule
import io.gitlab.arturbosch.detekt.formatting.INDENT_SIZE_KEY
import io.gitlab.arturbosch.detekt.formatting.MAX_LINE_LENGTH_KEY

/**
Expand All @@ -21,14 +20,15 @@ class ParameterListWrapping(config: Config) : FormattingRule(config) {
override val wrapping = ParameterListWrappingRule()
override val issue = issueFor("Detects mis-aligned parameter lists")

@Configuration("indentation size")
private val indentSize by config(4)
Comment on lines -24 to -25
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be deprecated instead


@Configuration("maximum line length")
private val maxLineLength: Int by configWithAndroidVariants(120, 100)

@Configuration("indentation size")
@Deprecated("`indentSize` is ignored by KtLint and will have no effect")
@Suppress("UnusedPrivateMember")
private val indentSize by config(4)

override fun overrideEditorConfig() = mapOf(
INDENT_SIZE_KEY to indentSize,
MAX_LINE_LENGTH_KEY to maxLineLength
)
}
@@ -0,0 +1,18 @@
package io.gitlab.arturbosch.detekt.formatting.wrappers

import com.pinterest.ktlint.core.api.FeatureInAlphaState
import com.pinterest.ktlint.ruleset.experimental.UnnecessaryParenthesesBeforeTrailingLambdaRule
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.internal.AutoCorrectable
import io.gitlab.arturbosch.detekt.formatting.FormattingRule

/**
* See <a href="https://ktlint.github.io/#rule-spacing">ktlint-website</a> for documentation.
*/
@OptIn(FeatureInAlphaState::class)
@AutoCorrectable(since = "1.20.0")
class UnnecessaryParenthesesBeforeTrailingLambda(config: Config) : FormattingRule(config) {

override val wrapping = UnnecessaryParenthesesBeforeTrailingLambdaRule()
override val issue = issueFor("Ensures there are no unnecessary parentheses before a trailing lambda")
}
4 changes: 3 additions & 1 deletion detekt-formatting/src/main/resources/config/config.yml
Expand Up @@ -87,7 +87,6 @@ formatting:
ParameterListWrapping:
active: true
autoCorrect: true
indentSize: 4
maxLineLength: 120
SpacingAroundAngleBrackets:
active: false
Expand Down Expand Up @@ -136,3 +135,6 @@ formatting:
autoCorrect: true
allowTrailingComma: false
allowTrailingCommaOnCallSite: false
UnnecessaryParenthesesBeforeTrailingLambda:
active: false
autoCorrect: true
Expand Up @@ -26,7 +26,7 @@ class FinalNewlineSpec {
"""
fun main() = Unit

"""
"""
)

assertThat(findings).isEmpty()
Expand All @@ -39,7 +39,7 @@ class FinalNewlineSpec {
"""
fun main() = Unit

"""
"""
)

assertThat(findings).hasSize(1)
Expand Down
Expand Up @@ -85,7 +85,7 @@ class FormattingRuleSpec {
"""
fun main()
= Unit
""",
""",
expectedPath
)

Expand Down
Expand Up @@ -54,5 +54,26 @@ class IndentationSpec {
val code = "fun main() {\n println()\n}"
assertThat(subject.lint(code)).isEmpty()
}

@Nested
inner class `parameter list indent size equals 1` {

val code = """
fun f(
a: Int
) {}
""".trimIndent()

@Test
fun `reports wrong indent size`() {
assertThat(subject.lint(code)).hasSize(1)
}

@Test
fun `does not report when using an indentation level config of 1`() {
val config = TestConfig("indentSize" to "1")
assertThat(Indentation(config).lint(code)).isEmpty()
}
}
}
}
@@ -1,8 +1,5 @@
package io.gitlab.arturbosch.detekt.formatting

import com.pinterest.ktlint.core.Rule.Modifier.Last
import com.pinterest.ktlint.core.Rule.Modifier.RestrictToRoot
import com.pinterest.ktlint.core.Rule.Modifier.RestrictToRootLast
import io.github.detekt.test.utils.compileContentForTest
import io.gitlab.arturbosch.detekt.api.Config
import org.assertj.core.api.Assertions.assertThat
Expand All @@ -20,15 +17,15 @@ class KtLintMultiRuleSpec {
ktlintRule.visitFile(compileContentForTest(""))
val sortedRules = ktlintRule.getSortedRules()
assertThat(sortedRules).isNotEmpty
assertThat(sortedRules.indexOfFirst { it.wrapping is RestrictToRoot })
assertThat(sortedRules.indexOfFirst { it.runOnRootNodeOnly })
.isGreaterThan(-1)
.isLessThan(sortedRules.indexOfFirst { it.wrapping !is RestrictToRoot })
assertThat(sortedRules.indexOfFirst { it.wrapping !is RestrictToRoot })
.isLessThan(sortedRules.indexOfFirst { !it.runOnRootNodeOnly })
assertThat(sortedRules.indexOfFirst { !it.runOnRootNodeOnly })
.isGreaterThan(-1)
.isLessThan(sortedRules.indexOfFirst { it.wrapping is RestrictToRootLast })
assertThat(sortedRules.indexOfFirst { it.wrapping is RestrictToRootLast })
.isLessThan(sortedRules.indexOfFirst { it.runOnRootNodeOnly && it.runAsLateAsPossible })
assertThat(sortedRules.indexOfFirst { it.runOnRootNodeOnly && it.runAsLateAsPossible })
.isGreaterThan(-1)
.isLessThan(sortedRules.indexOfFirst { it.wrapping is Last })
.isLessThan(sortedRules.indexOfFirst { it.runAsLateAsPossible && !it.runOnRootNodeOnly })
}
}
}
Expand Up @@ -20,27 +20,6 @@ class ParameterListWrappingSpec {
@Nested
inner class `ParameterListWrapping rule` {

@Nested
inner class `indent size equals 1` {

val code = """
fun f(
a: Int
) {}
""".trimIndent()

@Test
fun `reports wrong indent size`() {
assertThat(subject.lint(code)).hasSize(1)
}

@Test
fun `does not report when using an indentation level config of 1`() {
val config = TestConfig("indentSize" to "1")
assertThat(ParameterListWrapping(config).lint(code)).isEmpty()
}
}

@Test
fun `does not report correct ParameterListWrapping level`() {
val code = """
Expand Down
@@ -0,0 +1,32 @@
package io.gitlab.arturbosch.detekt.formatting

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.formatting.wrappers.UnnecessaryParenthesesBeforeTrailingLambda
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test

/**
* Test cases were used directly from KtLint to verify the wrapper rule:
*
* https://github.com/pinterest/ktlint/blob/master/ktlint-ruleset-experimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/UnnecessaryParenthesesBeforeTrailingLambdaRuleTest.kt
*/
class UnnecessaryParenthesesBeforeTrailingLambdaSpec {

@Test
fun `reports unnecessary parentheses before trailing lambda`() {
val code = """
fun countDash(input: String) =
"some-string".count() { it == '-' }
""".trimIndent()
assertThat(UnnecessaryParenthesesBeforeTrailingLambda(Config.empty).lint(code)).hasSize(1)
}

@Test
fun `does not report unnecessary parentheses before trailing lambda`() {
val code = """
fun countDash(input: String) =
"some-string".count { it == '-' }
""".trimIndent()
assertThat(UnnecessaryParenthesesBeforeTrailingLambda(Config.empty).lint(code)).isEmpty()
}
}