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

Rule 3.14.2 string template format #305

Merged
merged 8 commits into from
Sep 24, 2020
10 changes: 9 additions & 1 deletion diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -243,4 +243,12 @@
- name: TYPE_ALIAS
enabled: true
configuration:
typeReferenceLength: '25' # max length of type reference
typeReferenceLength: '25' # max length of type reference
# Inspection that checks if string template has redundant curly braces
- name: STRING_TEMPLATE_CURLY_BRACES
enabled: true
configuration: {}
# Inspection that checks if string template has redundant quotes
- name: STRING_TEMPLATE_QUOTES
enabled: true
configuration: {}
4 changes: 4 additions & 0 deletions diktat-rules/src/main/kotlin/generated/WarningNames.kt
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,8 @@ object WarningNames {
const val LOCAL_VARIABLE_EARLY_DECLARATION: String = "LOCAL_VARIABLE_EARLY_DECLARATION"

const val TYPE_ALIAS: String = "TYPE_ALIAS"

const val STRING_TEMPLATE_CURLY_BRACES: String = "STRING_TEMPLATE_CURLY_BRACES"

const val STRING_TEMPLATE_QUOTES: String = "STRING_TEMPLATE_QUOTES"
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S
WRONG_MULTIPLE_MODIFIERS_ORDER(true, "sequence of modifiers is incorrect"),
LOCAL_VARIABLE_EARLY_DECLARATION(false, "local variables should be declared close to the line where they are first used"),
TYPE_ALIAS(false, "variable's type is too complex and should be replaced with typealias"),
STRING_TEMPLATE_CURLY_BRACES(true, "string template has redundant curly braces"),
STRING_TEMPLATE_QUOTES(true, "string template has redundant quotes"),
;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy
::KdocFormatting,
::FileNaming,
::PackageNaming,
::StringTemplateFormatRule,
::FileSize,
::IdentifierNaming,
::LocalVariablesRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package org.cqfn.diktat.ruleset.rules

import com.pinterest.ktlint.core.Rule
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.IDENTIFIER
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
import com.pinterest.ktlint.core.ast.ElementType.SHORT_STRING_TEMPLATE_ENTRY
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.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

/**
* In String templates there should not be redundant curly braces. In case of using a not complex statement (one argument)
* there should not be curly braces.
*
* FixMe: The important caveat here: in "$foo" kotlin compiler adds implicit call to foo.toString() in case foo type is not string.
*/
class StringTemplateFormatRule(private val configRules: List<RulesConfig>) : Rule("string-template-format") {
private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit)
private var isFixMode: Boolean = false

override fun visit(node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) {
emitWarn = emit
isFixMode = autoCorrect

when (node.elementType) {
LONG_STRING_TEMPLATE_ENTRY -> handleLongStringTemplate(node)
SHORT_STRING_TEMPLATE_ENTRY -> handleShortStringTemplate(node)
}
}

@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()) {
Warnings.STRING_TEMPLATE_CURLY_BRACES.warnAndFix(configRules, emitWarn, isFixMode, node.text, node.startOffset, node) {
val identifierName = node.findChildByType(REFERENCE_EXPRESSION)
if (identifierName != null) {
val shortTemplate = CompositeElement(SHORT_STRING_TEMPLATE_ENTRY)
val reference = CompositeElement(REFERENCE_EXPRESSION)

node.treeParent.addChild(shortTemplate, node)
shortTemplate.addChild(LeafPsiElement(SHORT_TEMPLATE_ENTRY_START, "$"), null)
shortTemplate.addChild(reference)
reference.addChild(LeafPsiElement(IDENTIFIER, identifierName.text))
node.treeParent.removeChild(node)
} else {
val stringTemplate = node.treeParent
val appropriateText = node.text.trim('$', '{', '}')
stringTemplate.addChild(LeafPsiElement(LITERAL_STRING_TEMPLATE_ENTRY, appropriateText), node)
stringTemplate.removeChild(node)
}
}
}
}

@Suppress("UnsafeCallOnNullableType")
private fun handleShortStringTemplate(node: ASTNode) {
val identifierName = node.findChildByType(REFERENCE_EXPRESSION)?.text

if (identifierName != null && node.treeParent.text.trim('"', '$') == identifierName) {
Warnings.STRING_TEMPLATE_QUOTES.warnAndFix(configRules, emitWarn, isFixMode, node.text, node.startOffset, node) {
val identifier = node.findChildByType(REFERENCE_EXPRESSION)!!.copyElement()
// node.treeParent is String template that we need to delete
node.treeParent.treeParent.addChild(identifier, node.treeParent)
node.treeParent.treeParent.removeChild(node.treeParent)
}
}
}
}
10 changes: 9 additions & 1 deletion diktat-rules/src/main/resources/diktat-analysis-huawei.yml
Original file line number Diff line number Diff line change
Expand Up @@ -243,4 +243,12 @@
- name: TYPE_ALIAS
enabled: true
configuration:
typeReferenceLength: '25' # max length of type reference
typeReferenceLength: '25' # max length of type reference
# Inspection that checks if string template has redundant curly braces
- name: STRING_TEMPLATE_CURLY_BRACES
enabled: true
configuration: {}
# Inspection that checks if string template has redundant quotes
- name: STRING_TEMPLATE_QUOTES
enabled: true
configuration: {}
10 changes: 9 additions & 1 deletion diktat-rules/src/main/resources/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -245,4 +245,12 @@
- name: TYPE_ALIAS
enabled: true
configuration:
typeReferenceLength: '25' # max length of type reference
typeReferenceLength: '25' # max length of type reference
# Inspection that checks if string template has redundant curly braces
- name: STRING_TEMPLATE_CURLY_BRACES
enabled: true
configuration: {}
# Inspection that checks if string template has redundant quotes
- name: STRING_TEMPLATE_QUOTES
enabled: true
configuration: {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.cqfn.diktat.ruleset.chapter3

import generated.WarningNames
import org.cqfn.diktat.ruleset.rules.StringTemplateFormatRule
import org.cqfn.diktat.util.FixTestBase
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test

class StringTemplateRuleFixTest : FixTestBase("test/paragraph3/string_template", ::StringTemplateFormatRule) {
@Test
fun `should fix enum order`() {
fixAndCompare("StringTemplateExpected.kt", "StringTemplateTest.kt")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
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
import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
import org.cqfn.diktat.ruleset.rules.StringTemplateFormatRule
import org.cqfn.diktat.util.LintTestBase
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test

class StringTemplateRuleWarnTest : LintTestBase(::StringTemplateFormatRule) {

private val ruleId = "$DIKTAT_RULE_SET_ID:string-template-format"

@Test
@Tag(STRING_TEMPLATE_CURLY_BRACES)
fun `long string template good example`() {
lintMethod(
"""
|class Some {
| val template = "${'$'}{::String} ${'$'}{asd.moo()}"
|}
""".trimMargin()
)
}

@Test
@Tag(STRING_TEMPLATE_CURLY_BRACES)
fun `long string template bad example`() {
lintMethod(
"""
|class Some {
| val template = "${'$'}{a} ${'$'}{asd.moo()}"
| val some = "${'$'}{1.0}"
|}
""".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)
)
}

@Test
@Tag(STRING_TEMPLATE_QUOTES)
fun `short string template bad example`() {
lintMethod(
"""
|class Some {
| val template = "${'$'}a"
| val z = a
|}
""".trimMargin(),
LintError(2, 20, ruleId, "${Warnings.STRING_TEMPLATE_QUOTES.warnText()} ${'$'}a", true)
)
}

@Test
@Tag(STRING_TEMPLATE_CURLY_BRACES)
fun `long string template bad example 2`() {
lintMethod(
"""
|class Some {
| fun some() {
| val s = "abs"
| println("${'$'}{s}.length is ${'$'}{s.length}")
|
| }
|}
""".trimMargin(),
LintError(4, 17, ruleId, "${Warnings.STRING_TEMPLATE_CURLY_BRACES.warnText()} ${'$'}{s}", true)
)
}

@Test
@Tag(STRING_TEMPLATE_QUOTES)
fun `should not trigger`() {
lintMethod(
"""
|class Some {
| fun some() {
| val price = ""${'"'}
| ${'$'}9.99
| ""${'"'}
| }
|}
""".trimMargin()
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package test.paragraph3.string_template

class SomeClass {
fun someFunc() {
val x = "asd"

val z = x

val m = "1.0"

val template = "$x, ${asd.moo()}"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package test.paragraph3.string_template

class SomeClass {
fun someFunc() {
val x = "asd"

val z = "$x"

val m = "${1.0}"

val template = "${x}, ${asd.moo()}"
}
}
2 changes: 2 additions & 0 deletions info/available-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,6 @@
| 3 | 3.2 | WRONG_DECLARATIONS_ORDER | Check: if order of enum values or constant property inside companion isn't correct | yes | - | - |
| 3 | 3.11 | ANNOTATION_NEW_LINE | Check: warns if annotation not on a new single line | yes | - | - |
| 3 | 3.2 | WRONG_MULTIPLE_MODIFIERS_ORDER | Check: if multiple modifiers sequence is in the wrong order | yes | - | - |
| 3 | 3.14.2 | STRING_TEMPLATE_CURLY_BRACES | Check: warns if there is redundant curly braces in string template<br> Fix: deletes curly braces | yes | - | + |
| 3 | 3.14.2 | STRING_TEMPLATE_QUOTES | Check: warns if there are redundant quotes in string template<br> Fix: deletes quotes and $ symbol | yes | - | + |
| 4 | 4.2.2| TYPE_ALIAS | Check: if type reference of property is longer than expected | yes | typeReferenceLength | -|