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.

Sign up for GitHub

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

Jump to bottom

Add configuration to add alternate trimming methods #6063

Merged
merged 1 commit into from
May 3, 2023
Merged

Add configuration to add alternate trimming methods #6063

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,9 @@ style:
MultilineRawStringIndentation:
active: false
indentSize: 4
trimmingMethods:
- 'trimIndent'
- 'trimMargin'
NestedClassesVisibility:
active: true
NewLineAtEndOfFile:
Expand Down Expand Up @@ -693,6 +696,9 @@ style:
active: false
TrimMultilineRawString:
active: false
trimmingMethods:
- 'trimIndent'
- 'trimMargin'
UnderscoresInNumericLiterals:
active: false
acceptableLength: 4
Expand Down
5 changes: 4 additions & 1 deletion .../src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineRawStringIndentation.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,13 @@ class MultilineRawStringIndentation(config: Config) : Rule(config) {
@Configuration("indentation size")
private val indentSize by config(4)

@Configuration("allows to provide a list of multiline string trimming methods")
private val trimmingMethods: List<String> by config(listOf("trimIndent", "trimMargin"))
Copy link
Member

Choose a reason for hiding this comment

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

Should we use the method matcher that we have on forbidden method call? I assume it is not needed. If someone really needs it we could add it later without any breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, @BraisGabin this will change the old behaviour, initially, we used to allow any multiline sting with any method with name trimMargin method at the end. Now since we are asking for the FunctionMatcher that would mean that we are checking for function signature and hence the above-mentioned usage will be reported by the rule.

Also adding FunctionMatcher will need bindingContext to get the FqName of trimming method

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't necessarily add bindingcontext here. IMHO, it doesn't add much value over using the current approach in this PR.


override fun visitStringTemplateExpression(expression: KtStringTemplateExpression) {
super.visitStringTemplateExpression(expression)

if (!expression.isRawStringWithLineBreak() || !expression.isTrimmed()) return
if (!expression.isRawStringWithLineBreak() || !expression.isTrimmed(trimmingMethods)) return

if (!expression.isSurroundedByLineBreaks()) {
report(
Expand Down
16 changes: 11 additions & 5 deletions ...s-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/TrimMultilineRawString.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Issue
import io.gitlab.arturbosch.detekt.api.Rule
import io.gitlab.arturbosch.detekt.api.Severity
import io.gitlab.arturbosch.detekt.api.config
import io.gitlab.arturbosch.detekt.api.internal.Configuration
import io.gitlab.arturbosch.detekt.rules.isConstant
import io.gitlab.arturbosch.detekt.rules.safeAs
import org.jetbrains.kotlin.psi.KtAnnotationEntry
Expand Down Expand Up @@ -54,10 +56,16 @@ class TrimMultilineRawString(val config: Config) : Rule(config) {
Debt.FIVE_MINS
)

@Configuration("allows to provide a list of multiline string trimming methods")
private val trimmingMethods: List<String> by config(listOf("trimIndent", "trimMargin"))

override fun visitStringTemplateExpression(expression: KtStringTemplateExpression) {
super.visitStringTemplateExpression(expression)

if (expression.isRawStringWithLineBreak() && !expression.isTrimmed() && !expression.isExpectedAsConstant()) {
if (expression.isRawStringWithLineBreak() &&
!expression.isTrimmed(trimmingMethods) &&
!expression.isExpectedAsConstant()
) {
report(
CodeSmell(
issue,
Expand All @@ -75,14 +83,14 @@ fun KtStringTemplateExpression.isRawStringWithLineBreak(): Boolean =
literalText != null && "\n" in literalText
}

fun KtStringTemplateExpression.isTrimmed(): Boolean {
fun KtStringTemplateExpression.isTrimmed(trimmingMethods: List<String>): Boolean {
val nextCall = getQualifiedExpressionForReceiver()
?.selectorExpression
?.safeAs<KtCallExpression>()
?.calleeExpression
?.text

return nextCall in trimFunctions
return nextCall in trimmingMethods
}

@Suppress("ReturnCount")
Expand All @@ -103,5 +111,3 @@ private fun KtStringTemplateExpression.isExpectedAsConstant(): Boolean {

return false
}

private val trimFunctions = listOf("trimIndent", "trimMargin")
69 changes: 49 additions & 20 deletions ...yle/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/TrimMultilineRawStringSpec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package io.gitlab.arturbosch.detekt.rules.style

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.test.TestConfig
import io.gitlab.arturbosch.detekt.test.compileAndLint
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
Expand All @@ -13,18 +14,46 @@ class TrimMultilineRawStringSpec {
@Test
fun `raises multiline raw strings without trim`() {
val code = """
val a = ${TQ}
val a = $TQ
Hello world!
${TQ}
$TQ
""".trimIndent()
subject.compileAndLint(code)
assertThat(subject.findings).hasSize(1)
}

@Test
fun `raises multiline raw strings with lenght`() {
fun `doesn't raise multiline raw strings with custom default configured trimIndent method`() {
fun String.trimIndent() = this
val code = """
val a = ${TQ}
val a = $TQ
Hello world!
$TQ.trimIndent()
""".trimIndent()
subject.compileAndLint(code)
assertThat(subject.findings).isEmpty()
}

@Test
fun `doesn't raise multiline raw strings with custom configured customTrim method`() {
val code = """
fun String.customTrim() = this
val a = $TQ
Hello world!
$TQ.customTrim()
""".trimIndent()
val findings = TrimMultilineRawString(
TestConfig(
"trimmingMethods" to "customTrim"
)
).compileAndLint(code)
assertThat(findings).isEmpty()
}

@Test
fun `raises multiline raw strings with length`() {
val code = """
val a = $TQ
Hello world!
${TQ}.length
""".trimIndent()
Expand All @@ -33,9 +62,9 @@ class TrimMultilineRawStringSpec {
}

@Test
fun `doesn't raise multiline raw strings without trimIndent`() {
fun `doesn't raise multiline raw strings with trimIndent`() {
val code = """
val a = ${TQ}
val a = $TQ
Hello world!
${TQ}.trimIndent()
""".trimIndent()
Expand All @@ -44,9 +73,9 @@ class TrimMultilineRawStringSpec {
}

@Test
fun `doesn't raise multiline raw strings without trimMargin`() {
fun `doesn't raise multiline raw strings with trimMargin`() {
val code = """
val a = ${TQ}
val a = $TQ
|Hello world!
${TQ}.trimMargin()
""".trimIndent()
Expand All @@ -55,9 +84,9 @@ class TrimMultilineRawStringSpec {
}

@Test
fun `doesn't raise multiline raw strings without trimMargin with parameter`() {
fun `doesn't raise multiline raw strings with trimMargin with parameter`() {
val code = """
val a = ${TQ}
val a = $TQ
>Hello world!
${TQ}.trimMargin(">")
""".trimIndent()
Expand Down Expand Up @@ -99,11 +128,11 @@ class TrimMultilineRawStringSpec {
val code = """
object O {
const val s =
${TQ}
$TQ
Given something
When something
Then something
${TQ}
$TQ
}
""".trimIndent()
subject.compileAndLint(code)
Expand All @@ -115,11 +144,11 @@ class TrimMultilineRawStringSpec {
val code = """
annotation class DisplayName(val s: String)
@DisplayName(
${TQ}
$TQ
Given something
When something
Then something
${TQ}
$TQ
)
class Foo
""".trimIndent()
Expand All @@ -132,11 +161,11 @@ class TrimMultilineRawStringSpec {
val code = """
annotation class DisplayName(
val s: String =
${TQ}
$TQ
Given something
When something
Then something
${TQ}
$TQ
)
""".trimIndent()
subject.compileAndLint(code)
Expand All @@ -148,11 +177,11 @@ class TrimMultilineRawStringSpec {
val code = """
fun foo(s: String) {}
val bar = foo(
${TQ}
$TQ
Given something
When something
Then something
${TQ}
$TQ
)
class Foo
""".trimIndent()
Expand All @@ -165,11 +194,11 @@ class TrimMultilineRawStringSpec {
val code = """
class Foo(
val s: String =
${TQ}
$TQ
Given something
When something
Then something
${TQ}
$TQ
)
""".trimIndent()
subject.compileAndLint(code)
Expand Down