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

String template curly braces bugfix #427

Merged
merged 12 commits into from
Oct 27, 2020
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
package org.cqfn.diktat.ruleset.rules

import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.ElementType.BINARY_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.BINARY_WITH_TYPE
import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.CLOSING_QUOTE
import com.pinterest.ktlint.core.ast.ElementType.COLONCOLON
import com.pinterest.ktlint.core.ast.ElementType.DOT_QUALIFIED_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.FLOAT_CONSTANT
import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER
import com.pinterest.ktlint.core.ast.ElementType.INTEGER_CONSTANT
import com.pinterest.ktlint.core.ast.ElementType.LITERAL_STRING_TEMPLATE_ENTRY
import com.pinterest.ktlint.core.ast.ElementType.LONG_STRING_TEMPLATE_ENTRY
import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION
Expand All @@ -12,6 +19,8 @@ import com.pinterest.ktlint.core.ast.ElementType.SHORT_TEMPLATE_ENTRY_START
import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.ruleset.constants.Warnings
import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType
import org.cqfn.diktat.ruleset.utils.getFirstChildWithType
import org.cqfn.diktat.ruleset.utils.hasAnyChildOfTypes
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.CompositeElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
Expand Down Expand Up @@ -41,8 +50,7 @@ class StringTemplateFormatRule(private val configRules: List<RulesConfig>) : Rul
@Suppress("UnsafeCallOnNullableType")
private fun handleLongStringTemplate(node: ASTNode) {
// Checking if in long templates {a.foo()} there are function calls or class toString call
if (node.findAllNodesWithSpecificType(COLONCOLON).isEmpty() &&
node.findAllNodesWithSpecificType(DOT_QUALIFIED_EXPRESSION).isEmpty()) {
if (bracesCanBeOmitted(node)) {
Warnings.STRING_TEMPLATE_CURLY_BRACES.warnAndFix(configRules, emitWarn, isFixMode, node.text, node.startOffset, node) {
val identifierName = node.findChildByType(REFERENCE_EXPRESSION)
if (identifierName != null) {
Expand Down Expand Up @@ -77,4 +85,20 @@ class StringTemplateFormatRule(private val configRules: List<RulesConfig>) : Rul
}
}
}

@Suppress("UnsafeCallOnNullableType")
private fun bracesCanBeOmitted(node: ASTNode): Boolean {
val onlyOneRefExpr = node
.findAllNodesWithSpecificType(REFERENCE_EXPRESSION)
.singleOrNull()
?.treeParent
?.elementType == LONG_STRING_TEMPLATE_ENTRY
return if (onlyOneRefExpr) {
!(node.treeNext.text.first().isLetterOrDigit() // checking if first letter is valid
|| node.treeNext.text.startsWith("_"))
|| node.treeNext.elementType == CLOSING_QUOTE
} else {
node.hasAnyChildOfTypes(FLOAT_CONSTANT, INTEGER_CONSTANT) // it also fixes "${1.0}asd" cases
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import org.junit.jupiter.api.Test

class StringTemplateRuleFixTest : FixTestBase("test/paragraph3/string_template", ::StringTemplateFormatRule) {
@Test
@Tag(WarningNames.STRING_TEMPLATE_CURLY_BRACES)
fun `should fix enum order`() {
fixAndCompare("StringTemplateExpected.kt", "StringTemplateTest.kt")
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package org.cqfn.diktat.ruleset.chapter3

import com.pinterest.ktlint.core.LintError
import generated.WarningNames
import generated.WarningNames.STRING_TEMPLATE_CURLY_BRACES
import generated.WarningNames.STRING_TEMPLATE_QUOTES
import org.cqfn.diktat.ruleset.constants.Warnings
Expand All @@ -22,6 +21,7 @@ class StringTemplateRuleWarnTest : LintTestBase(::StringTemplateFormatRule) {
"""
|class Some {
| val template = "${'$'}{::String} ${'$'}{asd.moo()}"
| val some = "${'$'}{foo as Foo}"
Copy link
Member

@orchestr7 orchestr7 Oct 20, 2020

Choose a reason for hiding this comment

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

not much tests for a regression

|}
""".trimMargin()
)
Expand All @@ -35,10 +35,16 @@ class StringTemplateRuleWarnTest : LintTestBase(::StringTemplateFormatRule) {
|class Some {
| val template = "${'$'}{a} ${'$'}{asd.moo()}"
| val some = "${'$'}{1.0}"
| val another = "${'$'}{1}"
| val singleLetterCase = "${'$'}{ref}"
| val digitsWithLetters = "${'$'}{1.0}asd"
|}
""".trimMargin(),
LintError(2, 20, ruleId, "${Warnings.STRING_TEMPLATE_CURLY_BRACES.warnText()} ${'$'}{a}", true),
LintError(3, 16, ruleId, "${Warnings.STRING_TEMPLATE_CURLY_BRACES.warnText()} ${'$'}{1.0}", true)
LintError(3, 16, ruleId, "${Warnings.STRING_TEMPLATE_CURLY_BRACES.warnText()} ${'$'}{1.0}", true),
LintError(4, 19, ruleId, "${Warnings.STRING_TEMPLATE_CURLY_BRACES.warnText()} ${'$'}{1}", true),
LintError(5, 28, ruleId, "${Warnings.STRING_TEMPLATE_CURLY_BRACES.warnText()} ${'$'}{ref}", true),
LintError(6, 29, ruleId, "${Warnings.STRING_TEMPLATE_CURLY_BRACES.warnText()} ${'$'}{1.0}", true)
)
}

Expand All @@ -58,14 +64,13 @@ class StringTemplateRuleWarnTest : LintTestBase(::StringTemplateFormatRule) {

@Test
@Tag(STRING_TEMPLATE_CURLY_BRACES)
fun `long string template bad example 2`() {
fun `should trigger on dot after braces`() {
lintMethod(
"""
|class Some {
| fun some() {
| val s = "abs"
| println("${'$'}{s}.length is ${'$'}{s.length}")
|
| }
|}
""".trimMargin(),
Expand All @@ -83,6 +88,21 @@ class StringTemplateRuleWarnTest : LintTestBase(::StringTemplateFormatRule) {
| val price = ""${'"'}
| ${'$'}9.99
| ""${'"'}
| val some = "${'$'}{index + 1}"
| }
|}
""".trimMargin()
)
}

@Test
@Tag(STRING_TEMPLATE_CURLY_BRACES)
fun `underscore after braces - braces should not be removed`() {
lintMethod(
"""
|class Some {
| fun some() {
| val copyTestFile = File("${'$'}{testFile()} copy ${'$'}{testFile}_copy")
| }
|}
""".trimMargin()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,13 @@ class SomeClass {
val m = "1.0"

val template = "$x, ${asd.moo()}"

val binExpr = "${foo as Foo}"

val trippleQuotes = """$x"""

val test = """${'$'}"""

val test2= "${'$'}1"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,13 @@ class SomeClass {
val m = "${1.0}"

val template = "${x}, ${asd.moo()}"

val binExpr = "${foo as Foo}"

val trippleQuotes = """${x}"""

val test = """${'$'}"""

val test2= "${'$'}1"
}
}