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

Fix false positive MultilineRawStringIndentation with tab indentation #5453

Merged
merged 1 commit into from Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -14,8 +14,11 @@ import io.gitlab.arturbosch.detekt.api.SourceLocation
import io.gitlab.arturbosch.detekt.api.TextLocation
import io.gitlab.arturbosch.detekt.api.config
import io.gitlab.arturbosch.detekt.api.internal.Configuration
import io.gitlab.arturbosch.detekt.rules.safeAs
import org.jetbrains.kotlin.com.intellij.psi.PsiFile
import org.jetbrains.kotlin.psi.KtElement
import org.jetbrains.kotlin.psi.KtLiteralStringTemplateEntry
import org.jetbrains.kotlin.psi.KtStringTemplateEntry
import org.jetbrains.kotlin.psi.KtStringTemplateExpression

/**
Expand Down Expand Up @@ -63,11 +66,9 @@ class MultilineRawStringIndentation(config: Config) : Rule(config) {
override fun visitStringTemplateExpression(expression: KtStringTemplateExpression) {
super.visitStringTemplateExpression(expression)

val text = expression.text
val lineCount = text.lines().count()
if (lineCount <= 1) return
if (!expression.isTrimmed()) return
if (!text.matches(rawStringRegex)) {
if (!expression.isRawStringWithLineBreak() || !expression.isTrimmed()) return

if (!expression.isSurroundedByLineBreaks()) {
report(
CodeSmell(
issue,
Expand All @@ -79,6 +80,7 @@ class MultilineRawStringIndentation(config: Config) : Rule(config) {
}

val lineAndColumn = getLineAndColumnInPsiFile(expression.containingFile, expression.textRange) ?: return
val lineCount = expression.text.lines().count()

expression.checkIndentation(
baseIndent = lineAndColumn.lineContent?.countIndent() ?: return,
Expand Down Expand Up @@ -167,9 +169,20 @@ private fun message(desiredIntent: Int, currentIndent: Int): String {
return "The indentation should be $desiredIntent but it is $currentIndent."
}

private val rawStringRegex = "\"{3}\n(.*\n)? *\"{3}".toRegex(RegexOption.DOT_MATCHES_ALL)
private fun KtStringTemplateExpression.isSurroundedByLineBreaks(): Boolean {
val entries = this.entries
return entries.takeWhile { it.isBlankOrLineBreak() }.any { it.text == "\n" } &&
entries.takeLastWhile { it.isBlankOrLineBreak() }.any { it.text == "\n" }
}

private fun KtStringTemplateEntry.isBlankOrLineBreak(): Boolean {
val text = safeAs<KtLiteralStringTemplateEntry>()?.text ?: return false
return text.all { it.isTabChar() } || text == "\n"
}

private fun Char.isTabChar() = this == ' ' || this == '\t'

private fun String.countIndent() = this.takeWhile { it == ' ' }.count()
private fun String.countIndent() = this.takeWhile { it.isTabChar() }.count()

private fun PsiFile.getLine(line: Int): String {
return text.lineSequence().drop(line - 1).first()
Expand Down
Expand Up @@ -61,7 +61,7 @@ class TrimMultilineRawString(val config: Config) : Rule(config) {
}

fun KtStringTemplateExpression.isRawStringWithLineBreak(): Boolean =
text.startsWith("\"\"\"") && entries.any {
text.startsWith("\"\"\"") && text.endsWith("\"\"\"") && entries.any {
val literalText = it.safeAs<KtLiteralStringTemplateEntry>()?.text
literalText != null && "\n" in literalText
}
Expand Down
@@ -1,6 +1,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.assertThat
import io.gitlab.arturbosch.detekt.test.compileAndLint
import org.junit.jupiter.api.Nested
Expand Down Expand Up @@ -310,6 +311,39 @@ class MultilineRawStringIndentationSpec {
assertThat(subject.findings)
.isEmpty()
}

@Test
fun `don't raise if it has no line breaks`() {
val code = """
val a = ${TQ}Hello world!$TQ.trimIndent()
""".trimIndent()
subject.compileAndLint(code)
assertThat(subject.findings)
.isEmpty()
}

@Test
fun `don't raise multiline raw strings if all have the correct indentation - tabs`() {
val code = """
fun respond(content: String, status: Int) {}
fun redirect(accessToken: String, refreshToken: String) {
${TAB}respond(
$TAB${TAB}content = $TQ
$TAB$TAB$TAB{
$TAB$TAB$TAB "access_token": "${'$'}{accessToken}",
$TAB$TAB$TAB "token_type": "fake_token_type",
$TAB$TAB$TAB "expires_in": 3600,
$TAB$TAB$TAB "refresh_token": "${'$'}{refreshToken}"
$TAB$TAB$TAB}
$TAB$TAB$TQ.trimIndent(),
$TAB${TAB}status = 302
$TAB)
}
""".trimIndent()
val subject = MultilineRawStringIndentation(TestConfig(mapOf("indentSize" to 1)))
subject.compileAndLint(code)
assertThat(subject.findings).isEmpty()
}
}

@Nested
Expand Down Expand Up @@ -366,3 +400,4 @@ class MultilineRawStringIndentationSpec {
}

private const val TQ = "\"\"\""
private const val TAB = "\t"