KT-5419 J2K: convert string concatenation to string template + implemented corresponding inspection #KT-5419 Fixed
diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/inspections/IntentionBasedInspection.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/inspections/IntentionBasedInspection.kt index 3557bf8..4dc2726 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/inspections/IntentionBasedInspection.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/inspections/IntentionBasedInspection.kt
@@ -33,11 +33,18 @@ 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 @@ 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 @@ /* 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 @@ 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?) {
diff --git a/idea/resources/inspectionDescriptions/ConvertToStringTemplate.html b/idea/resources/inspectionDescriptions/ConvertToStringTemplate.html new file mode 100644 index 0000000..214eafa --- /dev/null +++ b/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>
diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 2d42c6d..eccb160 100644 --- a/idea/src/META-INF/plugin.xml +++ b/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"
diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertToStringTemplateIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertToStringTemplateIntention.kt index 1d1c619..52f70e2 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertToStringTemplateIntention.kt +++ b/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 (!KotlinBuiltIns.isString(element.analyze().getType(element))) return false + if (!isApplicableToNoParentCheck(element)) return false - val left = element.getLeft() ?: return false - val right = element.getRight() ?: return false - return !PsiUtilCore.hasErrorElementChild(left) && !PsiUtilCore.hasErrorElementChild(right) + val parent = element.getParent() + if (parent is JetBinaryExpression && isApplicableToNoParentCheck(parent)) return false + + return true } override fun applyTo(element: JetBinaryExpression, editor: Editor) { - val parent = element.getParent() - 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)) + applyTo(element) } - private fun fold(left: JetExpression?, right: String): String { - val needsBraces = !right.isEmpty() && right.first() != '$' && right.first().isJavaIdentifierPart() + public fun applyTo(element: JetBinaryExpression): JetStringTemplateExpression { + return element.replaced(buildReplacement(element)) + } - if (left is JetBinaryExpression && isApplicableTo(left)) { - val leftRight = buildText(left.getRight(), needsBraces) - return fold(left.getLeft(), leftRight + right) + public fun isConversionResultSimple(expression: JetBinaryExpression): Boolean { + return buildReplacement(expression).getEntries().none { it is JetBlockStringTemplateEntry } + } + + 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) - return "\"$leftText$right\"" + val leftText = buildText(left, forceBraces) + return factory.createExpression("\"$leftText$right\"") as JetStringTemplateExpression } }
diff --git a/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessor.kt index ece5ac8..e3b8588 100644 --- a/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessor.kt +++ b/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessor.kt
@@ -26,6 +26,7 @@ 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 @@ } } + + 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) {
diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/changePackage/PackageDirectoryMismatchInspection.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/changePackage/PackageDirectoryMismatchInspection.kt index b2bd93fa..e50d666 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/changePackage/PackageDirectoryMismatchInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/changePackage/PackageDirectoryMismatchInspection.kt
@@ -20,7 +20,7 @@ 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() ) \ No newline at end of file
diff --git a/idea/testData/intentions/convertToStringTemplate/inspectionData/expected.xml b/idea/testData/intentions/convertToStringTemplate/inspectionData/expected.xml new file mode 100644 index 0000000..c790804 --- /dev/null +++ b/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> \ No newline at end of file
diff --git a/idea/testData/intentions/convertToStringTemplate/inspectionData/inspections.test b/idea/testData/intentions/convertToStringTemplate/inspectionData/inspections.test new file mode 100644 index 0000000..10cea4c --- /dev/null +++ b/idea/testData/intentions/convertToStringTemplate/inspectionData/inspections.test
@@ -0,0 +1 @@ +// INSPECTION_CLASS: org.jetbrains.kotlin.idea.intentions.ConvertToStringTemplateInspection
diff --git a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/JetInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/JetInspectionTestGenerated.java index 7f6314a..2ebab0d 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/JetInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/JetInspectionTestGenerated.java
@@ -55,6 +55,12 @@ 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");
diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index f294348..853be23 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java
@@ -4167,6 +4167,7 @@ String fileName = JetTestUtils.navigationMetadata("idea/testData/intentions/convertToStringTemplate/tricky.kt"); doTest(fileName); } + } @TestMetadata("idea/testData/intentions/declarations")
diff --git a/j2k/testData/fileOrElement/assertStatement/withStringDetail2.kt b/j2k/testData/fileOrElement/assertStatement/withStringDetail2.kt index 97aa2c8..2265730 100644 --- a/j2k/testData/fileOrElement/assertStatement/withStringDetail2.kt +++ b/j2k/testData/fileOrElement/assertStatement/withStringDetail2.kt
@@ -1 +1 @@ -assert(true) { "string details:" + x } \ No newline at end of file +assert(true) { "string details:$x" } \ No newline at end of file
diff --git a/j2k/testData/fileOrElement/issues/kt-1074.kt b/j2k/testData/fileOrElement/issues/kt-1074.kt index 4130d0b..b28081d 100644 --- a/j2k/testData/fileOrElement/issues/kt-1074.kt +++ b/j2k/testData/fileOrElement/issues/kt-1074.kt
@@ -3,10 +3,10 @@ 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)") } } \ No newline at end of file
diff --git a/j2k/testData/fileOrElement/overloads/Annotations.kt b/j2k/testData/fileOrElement/overloads/Annotations.kt index 7d3f849..59d2bb0 100644 --- a/j2k/testData/fileOrElement/overloads/Annotations.kt +++ b/j2k/testData/fileOrElement/overloads/Annotations.kt
@@ -30,6 +30,6 @@ } public fun f(p: Int) { - println("p = " + p) + println("p = $p") } }
diff --git a/j2k/testData/fileOrElement/overloads/Override.kt b/j2k/testData/fileOrElement/overloads/Override.kt index 9fbf795..9cace0b 100644 --- a/j2k/testData/fileOrElement/overloads/Override.kt +++ b/j2k/testData/fileOrElement/overloads/Override.kt
@@ -5,7 +5,7 @@ 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) {
diff --git a/j2k/testData/fileOrElement/overloads/Simple.kt b/j2k/testData/fileOrElement/overloads/Simple.kt index 480ca24..1bfaa3c 100644 --- a/j2k/testData/fileOrElement/overloads/Simple.kt +++ b/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 {
diff --git a/j2k/testData/fileOrElement/overloads/Synchronized.kt b/j2k/testData/fileOrElement/overloads/Synchronized.kt index eabd97d..cb5ac30 100644 --- a/j2k/testData/fileOrElement/overloads/Synchronized.kt +++ b/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() {