KT-5419 J2K: convert string concatenation to string template + implemented corresponding inspection

#KT-5419 Fixed

idea/idea-analysis/src/org/jetbrains/kotlin/idea/inspections/IntentionBasedInspection.kt

@@ -33,11 +33,18 @@ import org.jetbrains.kotlin.idea.intentions.JetSelfTargetingRangeIntention
 import org.jetbrains.kotlin.psi.JetElement
 
 public abstract class IntentionBasedInspection<TElement : JetElement>(
-        protected val intentions: List<JetSelfTargetingRangeIntention<TElement>>,
+        protected val intentions: List<IntentionBasedInspection.IntentionData<TElement>>,
         protected val problemText: String?,
         protected val elementType: Class<TElement>
 ) : AbstractKotlinInspection() {
-    constructor(intention: JetSelfTargetingRangeIntention<TElement>) : this(listOf(intention), null, intention.elementType)
+
+    constructor(intention: JetSelfTargetingRangeIntention<TElement>, additionalChecker: (TElement) -> Boolean = { true })
+        : this(listOf(IntentionData(intention, additionalChecker)), null, intention.elementType)
+
+    public data class IntentionData<TElement : JetElement>(
+            val intention: JetSelfTargetingRangeIntention<TElement>,
+            val additionalChecker: (TElement) -> Boolean = { true }
+    )
 
     override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor {
         return object : PsiElementVisitor() {
@@ -49,16 +56,17 @@ public abstract class IntentionBasedInspection<TElement : JetElement>(
 
                 val ranges = intentions
                         .map {
-                            it.applicabilityRange(targetElement)?.let { range ->
+                            val range = it.intention.applicabilityRange(targetElement)?.let { range ->
                                 val elementRange = targetElement.getTextRange()
                                 assert(range in elementRange) { "Wrong applicabilityRange() result for $it - should be within element's range" }
                                 range.shiftRight(-elementRange.getStartOffset())
                             }
+                            if (range != null && it.additionalChecker(targetElement)) range else null
                         }
                         .filterNotNull()
                 if (ranges.isEmpty()) return
 
-                val fixes = intentions.map { IntentionBasedQuickFix(it, targetElement) }.toTypedArray()
+                val fixes = intentions.map { IntentionBasedQuickFix(it.intention, it.additionalChecker, targetElement) }.toTypedArray()
                 val rangeInElement = ranges.fold(ranges.first()) { result, range -> result.union(range) }
                 holder.registerProblem(targetElement, problemText ?: fixes.first().getName(), problemHighlightType, rangeInElement, *fixes)
             }
@@ -71,8 +79,10 @@ public abstract class IntentionBasedInspection<TElement : JetElement>(
     /* we implement IntentionAction to provide isAvailable which will be used to hide outdated items and make sure we never call 'invoke' for such item */
     private class IntentionBasedQuickFix<TElement : JetElement>(
             val intention: JetSelfTargetingRangeIntention<TElement>,
+            val additionalChecker: (TElement) -> Boolean,
             targetElement: TElement
     ): LocalQuickFixOnPsiElement(targetElement), IntentionAction {
+
         // store text into variable because intention instance is shared and may change its text later
         private val _text = intention.getText()
 
@@ -86,7 +96,7 @@ public abstract class IntentionBasedInspection<TElement : JetElement>(
 
         override fun isAvailable(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement): Boolean {
             assert(startElement == endElement)
-            return intention.applicabilityRange(startElement as TElement) != null
+            return intention.applicabilityRange(startElement as TElement) != null && additionalChecker(startElement)
         }
 
         override fun invoke(project: Project, editor: Editor?, file: PsiFile?) {

idea/resources/inspectionDescriptions/ConvertToStringTemplate.html

@@ -0,0 +1,5 @@
+<html>
+<body>
+This inspection reports string concatenation that can be converted to simple string template (one with no "${...}" entries).
+</body>
+</html>

idea/src/META-INF/plugin.xml

@@ -933,6 +933,13 @@
                      level="INFO"
         />
 
+      <localInspection implementationClass="org.jetbrains.kotlin.idea.intentions.ConvertToStringTemplateInspection"
+                       displayName="Convert string concatenation to string template"
+                       groupName="Kotlin"
+                       enabledByDefault="true"
+                       level="INFO"
+          />
+
     <localInspection implementationClass="org.jetbrains.kotlin.idea.intentions.conventionNameCalls.ExplicitGetInspection"
                      displayName="Explicit 'get'"
                      groupName="Kotlin"

idea/src/org/jetbrains/kotlin/idea/intentions/ConvertToStringTemplateIntention.kt

@@ -16,57 +16,69 @@
 
 package org.jetbrains.kotlin.idea.intentions
 
-import org.jetbrains.kotlin.psi.JetBinaryExpression
 import com.intellij.openapi.editor.Editor
-import org.jetbrains.kotlin.resolve.BindingContextUtils
-import org.jetbrains.kotlin.psi.JetExpression
-import org.jetbrains.kotlin.psi.JetPsiFactory
-import org.jetbrains.kotlin.psi.JetSimpleNameExpression
-import org.jetbrains.kotlin.psi.JetConstantExpression
-import org.jetbrains.kotlin.psi.JetStringTemplateExpression
-import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator
-import org.jetbrains.kotlin.resolve.DelegatingBindingTrace
-import org.jetbrains.kotlin.resolve.constants.IntegerValueTypeConstant
-import org.jetbrains.kotlin.lexer.JetTokens
-import org.jetbrains.kotlin.psi.JetPsiUtil
 import com.intellij.openapi.util.text.StringUtil
 import com.intellij.psi.util.PsiUtilCore
 import org.jetbrains.kotlin.builtins.KotlinBuiltIns
 import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.idea.core.replaced
+import org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection
+import org.jetbrains.kotlin.lexer.JetTokens
+import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.resolve.constants.IntegerValueTypeConstant
+import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator
 
+public class ConvertToStringTemplateInspection : IntentionBasedInspection<JetBinaryExpression>(
+        ConvertToStringTemplateIntention(),
+        { ConvertToStringTemplateIntention().isConversionResultSimple(it) }
+)
 
 public class ConvertToStringTemplateIntention : JetSelfTargetingOffsetIndependentIntention<JetBinaryExpression>(javaClass(), "Convert concatenation to template") {
     override fun isApplicableTo(element: JetBinaryExpression): Boolean {
-        if (element.getOperationToken() != JetTokens.PLUS) return false
+        if (!isApplicableToNoParentCheck(element)) return false
-        if (!KotlinBuiltIns.isString(element.analyze().getType(element))) return false
 
-        val left = element.getLeft() ?: return false
+        val parent = element.getParent()
-        val right = element.getRight() ?: return false
+        if (parent is JetBinaryExpression && isApplicableToNoParentCheck(parent)) return false
-        return !PsiUtilCore.hasErrorElementChild(left) && !PsiUtilCore.hasErrorElementChild(right)
+
+        return true
     }
 
     override fun applyTo(element: JetBinaryExpression, editor: Editor) {
-        val parent = element.getParent()
+        applyTo(element)
-        if (parent is JetBinaryExpression && isApplicableTo(parent)) {
-            return applyTo(parent, editor)
-        }
-
-        val rightText = buildText(element.getRight(), false)
-        val text = fold(element.getLeft(), rightText)
-
-        element.replace(JetPsiFactory(element).createExpression(text))
     }
 
-    private fun fold(left: JetExpression?, right: String): String {
+    public fun applyTo(element: JetBinaryExpression): JetStringTemplateExpression {
-        val needsBraces = !right.isEmpty() && right.first() != '$' && right.first().isJavaIdentifierPart()
+        return element.replaced(buildReplacement(element))
+    }
 
-        if (left is JetBinaryExpression && isApplicableTo(left)) {
+    public fun isConversionResultSimple(expression: JetBinaryExpression): Boolean {
-            val leftRight = buildText(left.getRight(), needsBraces)
+        return buildReplacement(expression).getEntries().none { it is JetBlockStringTemplateEntry }
-            return fold(left.getLeft(), leftRight + right)
+    }
+
+    private fun isApplicableToNoParentCheck(expression: JetBinaryExpression): Boolean {
+        if (expression.getOperationToken() != JetTokens.PLUS) return false
+        if (!KotlinBuiltIns.isString(expression.analyze().getType(expression))) return false
+
+        val left = expression.getLeft() ?: return false
+        val right = expression.getRight() ?: return false
+        return !PsiUtilCore.hasErrorElementChild(left) && !PsiUtilCore.hasErrorElementChild(right)
+    }
+
+    private fun buildReplacement(expression: JetBinaryExpression): JetStringTemplateExpression {
+        val rightText = buildText(expression.getRight(), false)
+        return fold(expression.getLeft(), rightText, JetPsiFactory(expression))
+    }
+
+    private fun fold(left: JetExpression?, right: String, factory: JetPsiFactory): JetStringTemplateExpression {
+        val forceBraces = !right.isEmpty() && right.first() != '$' && right.first().isJavaIdentifierPart()
+
+        if (left is JetBinaryExpression && isApplicableToNoParentCheck(left)) {
+            val leftRight = buildText(left.getRight(), forceBraces)
+            return fold(left.getLeft(), leftRight + right, factory)
         }
         else {
-            val leftText = buildText(left, needsBraces)
+            val leftText = buildText(left, forceBraces)
-            return "\"$leftText$right\""
+            return factory.createExpression("\"$leftText$right\"") as JetStringTemplateExpression
         }
     }
 

idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessor.kt

@@ -26,6 +26,7 @@ import org.jetbrains.kotlin.diagnostics.Diagnostic
 import org.jetbrains.kotlin.diagnostics.Errors
 import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
 import org.jetbrains.kotlin.idea.caches.resolve.resolveImportReference
+import org.jetbrains.kotlin.idea.intentions.ConvertToStringTemplateIntention
 import org.jetbrains.kotlin.idea.intentions.IfNullToElvisIntention
 import org.jetbrains.kotlin.idea.intentions.RemoveExplicitTypeArgumentsIntention
 import org.jetbrains.kotlin.idea.intentions.SimplifyNegatedBinaryExpressionIntention
@@ -158,6 +159,17 @@ public class J2kPostProcessor(private val formatCode: Boolean) : PostProcessor {
 
                 }
             }
+
+            override fun visitBinaryExpression(expression: JetBinaryExpression) {
+                val intention = ConvertToStringTemplateIntention()
+                if (intention.isApplicableTo(expression) && intention.isConversionResultSimple(expression)) {
+                    val result = intention.applyTo(expression)
+                    result.accept(this)
+                }
+                else {
+                    super.visitBinaryExpression(expression)
+                }
+            }
         })
 
         for (typeArgs in redundantTypeArgs) {

idea/src/org/jetbrains/kotlin/idea/refactoring/move/changePackage/PackageDirectoryMismatchInspection.kt

@@ -20,7 +20,7 @@ import org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection
 import org.jetbrains.kotlin.psi.JetPackageDirective
 
 public class PackageDirectoryMismatchInspection: IntentionBasedInspection<JetPackageDirective>(
-        listOf(MoveFileToPackageMatchingDirectoryIntention(), ChangePackageToMatchDirectoryIntention()),
+        listOf(IntentionBasedInspection.IntentionData(MoveFileToPackageMatchingDirectoryIntention()), IntentionBasedInspection.IntentionData(ChangePackageToMatchDirectoryIntention())),
         "Package directive doesn't match file location",
         javaClass()
 )

idea/testData/intentions/convertToStringTemplate/inspectionData/expected.xml

@@ -0,0 +1,114 @@
+<problems>
+    <problem>
+        <file>noBracesSimpleFollowedByDot.kt</file>
+        <line>3</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="noBracesSimpleFollowedByDot.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Convert string concatenation to string template</problem_class>
+        <description>Convert concatenation to template</description>
+    </problem>
+    <problem>
+        <file>noBracesForLastSimpleExpression.kt</file>
+        <line>3</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="noBracesForLastSimpleExpression.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Convert string concatenation to string template</problem_class>
+        <description>Convert concatenation to template</description>
+    </problem>
+    <problem>
+        <file>multilineString.kt</file>
+        <line>3</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="multilineString.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Convert string concatenation to string template</problem_class>
+        <description>Convert concatenation to template</description>
+    </problem>
+    <problem>
+        <file>interpolateStringWithInt.kt</file>
+        <line>2</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="interpolateStringWithInt.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Convert string concatenation to string template</problem_class>
+        <description>Convert concatenation to template</description>
+    </problem>
+    <problem>
+        <file>interpolateStringWithFloat.kt</file>
+        <line>2</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="interpolateStringWithFloat.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Convert string concatenation to string template</problem_class>
+        <description>Convert concatenation to template</description>
+    </problem>
+    <problem>
+        <file>interpolateDollarSign.kt</file>
+        <line>2</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="interpolateDollarSign.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Convert string concatenation to string template</problem_class>
+        <description>Convert concatenation to template</description>
+    </problem>
+    <problem>
+        <file>interpolateChar.kt</file>
+        <line>2</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="interpolateChar.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Convert string concatenation to string template</problem_class>
+        <description>Convert concatenation to template</description>
+    </problem>
+    <problem>
+        <file>interpolate2Vals.kt</file>
+        <line>4</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="interpolate2Vals.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Convert string concatenation to string template</problem_class>
+        <description>Convert concatenation to template</description>
+    </problem>
+    <problem>
+        <file>interpolate2StringConstants.kt</file>
+        <line>2</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="interpolate2StringConstants.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Convert string concatenation to string template</problem_class>
+        <description>Convert concatenation to template</description>
+    </problem>
+    <problem>
+        <file>combinesNonStringsAsStrings2.kt</file>
+        <line>2</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="combinesNonStringsAsStrings2.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Convert string concatenation to string template</problem_class>
+        <description>Convert concatenation to template</description>
+    </problem>
+    <problem>
+        <file>combinesNonStringsAsStrings.kt</file>
+        <line>2</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="combinesNonStringsAsStrings.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Convert string concatenation to string template</problem_class>
+        <description>Convert concatenation to template</description>
+    </problem>
+    <problem>
+        <file>combineEmptyStrings.kt</file>
+        <line>2</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="combineEmptyStrings.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Convert string concatenation to string template</problem_class>
+        <description>Convert concatenation to template</description>
+    </problem>
+    <problem>
+        <file>backslashNMultilineString.kt</file>
+        <line>2</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="backslashNMultilineString.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Convert string concatenation to string template</problem_class>
+        <description>Convert concatenation to template</description>
+    </problem>
+    <problem>
+        <file>doesNotCorruptExistingTemplate.kt</file>
+        <line>4</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="doesNotCorruptExistingTemplate.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Convert string concatenation to string template</problem_class>
+        <description>Convert concatenation to template</description>
+    </problem>
+</problems>

idea/testData/intentions/convertToStringTemplate/inspectionData/inspections.test

@@ -0,0 +1 @@
+// INSPECTION_CLASS: org.jetbrains.kotlin.idea.intentions.ConvertToStringTemplateInspection

idea/tests/org/jetbrains/kotlin/idea/codeInsight/JetInspectionTestGenerated.java

@@ -55,6 +55,12 @@ public class JetInspectionTestGenerated extends AbstractJetInspectionTest {
             doTest(fileName);
         }
 
+        @TestMetadata("convertToStringTemplate/inspectionData/inspections.test")
+        public void testConvertToStringTemplate_inspectionData_Inspections_test() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("idea/testData/intentions/convertToStringTemplate/inspectionData/inspections.test");
+            doTest(fileName);
+        }
+
         @TestMetadata("deprecatedCallableAddReplaceWith/inspectionData/inspections.test")
         public void testDeprecatedCallableAddReplaceWith_inspectionData_Inspections_test() throws Exception {
             String fileName = JetTestUtils.navigationMetadata("idea/testData/intentions/deprecatedCallableAddReplaceWith/inspectionData/inspections.test");

idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java

@@ -4167,6 +4167,7 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
             String fileName = JetTestUtils.navigationMetadata("idea/testData/intentions/convertToStringTemplate/tricky.kt");
             doTest(fileName);
         }
+
     }
 
     @TestMetadata("idea/testData/intentions/declarations")

j2k/testData/fileOrElement/assertStatement/withStringDetail2.kt

@@ -1 +1 @@
-assert(true) { "string details:" + x }
+assert(true) { "string details:$x" }

j2k/testData/fileOrElement/issues/kt-1074.kt

@@ -3,10 +3,10 @@ package demo
 object Test {
     fun subListRangeCheck(fromIndex: Int, toIndex: Int, size: Int) {
         if (fromIndex < 0)
-            throw IndexOutOfBoundsException("fromIndex = " + fromIndex)
+            throw IndexOutOfBoundsException("fromIndex = $fromIndex")
         if (toIndex > size)
-            throw IndexOutOfBoundsException("toIndex = " + toIndex)
+            throw IndexOutOfBoundsException("toIndex = $toIndex")
         if (fromIndex > toIndex)
-            throw IllegalArgumentException("fromIndex(" + fromIndex + ") > toIndex(" + toIndex + ")")
+            throw IllegalArgumentException("fromIndex($fromIndex) > toIndex($toIndex)")
     }
 }

j2k/testData/fileOrElement/overloads/Annotations.kt

@@ -30,6 +30,6 @@ class A {
     }
 
     public fun f(p: Int) {
-        println("p = " + p)
+        println("p = $p")
     }
 }

j2k/testData/fileOrElement/overloads/Override.kt

@@ -5,7 +5,7 @@ interface I {
 
 open class A : I {
     override fun foo(i: Int, c: Char, s: String) {
-        println("foo" + i + c + s)
+        println("foo$i$c$s")
     }
 
     public fun foo(i: Int, c: Char) {

j2k/testData/fileOrElement/overloads/Simple.kt

@@ -1,6 +1,6 @@
 class A {
     jvmOverloads fun foo(i: Int, c: Char = 'a', s: String = "") {
-        println("foo" + i + c + s)
+        println("foo$i$c$s")
     }
 
     jvmOverloads fun bar(s: String? = null): Int {

j2k/testData/fileOrElement/overloads/Synchronized.kt

@@ -1,6 +1,6 @@
 class A {
     public fun foo(p: Int) {
-        println("p = [" + p + "]")
+        println("p = [$p]")
     }
 
     synchronized public fun foo() {