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 3557bf85c73..4dc2726a28c 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.idea.intentions.JetSelfTargetingRangeIntention import org.jetbrains.kotlin.psi.JetElement public abstract class IntentionBasedInspection( - protected val intentions: List>, + protected val intentions: List>, protected val problemText: String?, protected val elementType: Class ) : AbstractKotlinInspection() { - constructor(intention: JetSelfTargetingRangeIntention) : this(listOf(intention), null, intention.elementType) + + constructor(intention: JetSelfTargetingRangeIntention, additionalChecker: (TElement) -> Boolean = { true }) + : this(listOf(IntentionData(intention, additionalChecker)), null, intention.elementType) + + public data class IntentionData( + val intention: JetSelfTargetingRangeIntention, + 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( 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( /* 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( val intention: JetSelfTargetingRangeIntention, + 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( 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 00000000000..214eafac9cc --- /dev/null +++ b/idea/resources/inspectionDescriptions/ConvertToStringTemplate.html @@ -0,0 +1,5 @@ + + +This inspection reports string concatenation that can be converted to simple string template (one with no "${...}" entries). + + diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 2d42c6dd175..eccb1601479 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -933,6 +933,13 @@ level="INFO" /> + + ( + ConvertToStringTemplateIntention(), + { ConvertToStringTemplateIntention().isConversionResultSimple(it) } +) public class ConvertToStringTemplateIntention : JetSelfTargetingOffsetIndependentIntention(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 ece5ac89163..e3b8588db32 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.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) { 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 b2bd93faab0..e50d666ad9e 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.idea.inspections.IntentionBasedInspection import org.jetbrains.kotlin.psi.JetPackageDirective public class PackageDirectoryMismatchInspection: IntentionBasedInspection( - 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 00000000000..c790804144c --- /dev/null +++ b/idea/testData/intentions/convertToStringTemplate/inspectionData/expected.xml @@ -0,0 +1,114 @@ + + + noBracesSimpleFollowedByDot.kt + 3 + light_idea_test_case + + Convert string concatenation to string template + Convert concatenation to template + + + noBracesForLastSimpleExpression.kt + 3 + light_idea_test_case + + Convert string concatenation to string template + Convert concatenation to template + + + multilineString.kt + 3 + light_idea_test_case + + Convert string concatenation to string template + Convert concatenation to template + + + interpolateStringWithInt.kt + 2 + light_idea_test_case + + Convert string concatenation to string template + Convert concatenation to template + + + interpolateStringWithFloat.kt + 2 + light_idea_test_case + + Convert string concatenation to string template + Convert concatenation to template + + + interpolateDollarSign.kt + 2 + light_idea_test_case + + Convert string concatenation to string template + Convert concatenation to template + + + interpolateChar.kt + 2 + light_idea_test_case + + Convert string concatenation to string template + Convert concatenation to template + + + interpolate2Vals.kt + 4 + light_idea_test_case + + Convert string concatenation to string template + Convert concatenation to template + + + interpolate2StringConstants.kt + 2 + light_idea_test_case + + Convert string concatenation to string template + Convert concatenation to template + + + combinesNonStringsAsStrings2.kt + 2 + light_idea_test_case + + Convert string concatenation to string template + Convert concatenation to template + + + combinesNonStringsAsStrings.kt + 2 + light_idea_test_case + + Convert string concatenation to string template + Convert concatenation to template + + + combineEmptyStrings.kt + 2 + light_idea_test_case + + Convert string concatenation to string template + Convert concatenation to template + + + backslashNMultilineString.kt + 2 + light_idea_test_case + + Convert string concatenation to string template + Convert concatenation to template + + + doesNotCorruptExistingTemplate.kt + 4 + light_idea_test_case + + Convert string concatenation to string template + Convert concatenation to template + + \ 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 00000000000..10cea4c8255 --- /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 7f6314aa672..2ebab0d6aca 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 @@ 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"); diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index f294348c02e..853be2304f2 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 @@ public class IntentionTestGenerated extends AbstractIntentionTest { 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 97aa2c8f30f..2265730172b 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 4130d0bb3f9..b28081de91a 100644 --- a/j2k/testData/fileOrElement/issues/kt-1074.kt +++ b/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)") } } \ No newline at end of file diff --git a/j2k/testData/fileOrElement/overloads/Annotations.kt b/j2k/testData/fileOrElement/overloads/Annotations.kt index 7d3f8490591..59d2bb0cec4 100644 --- a/j2k/testData/fileOrElement/overloads/Annotations.kt +++ b/j2k/testData/fileOrElement/overloads/Annotations.kt @@ -30,6 +30,6 @@ class A { } 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 9fbf7956303..9cace0bc255 100644 --- a/j2k/testData/fileOrElement/overloads/Override.kt +++ b/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) { diff --git a/j2k/testData/fileOrElement/overloads/Simple.kt b/j2k/testData/fileOrElement/overloads/Simple.kt index 480ca2445e6..1bfaa3cf353 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 eabd97dc020..cb5ac308899 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() {