From c72a35f1d240f6e994957eb61bfb078436c7b3ed Mon Sep 17 00:00:00 2001 From: sschr15 Date: Tue, 6 Aug 2024 16:55:04 -0500 Subject: [PATCH] Fix StringBuilder-based single character issues --- plugins/kotlin/.gitignore | 1 + plugins/kotlin/build.gradle | 19 +++ .../kotlin/expr/KFunctionExprent.java | 23 +++- .../org/vineflower/kotlin/KotlinTests.java | 2 + .../pkg/TestClassicStringInterpolation.dec | 118 ++++++++++++++++++ .../pkg/TestClassicStringInterpolation.kt | 24 ++++ .../decompiler/SingleClassesTestBase.java | 1 + 7 files changed, 184 insertions(+), 4 deletions(-) create mode 100644 plugins/kotlin/testData/results/pkg/TestClassicStringInterpolation.dec create mode 100644 plugins/kotlin/testData/src/ktold/pkg/TestClassicStringInterpolation.kt diff --git a/plugins/kotlin/.gitignore b/plugins/kotlin/.gitignore index 7dc657844a..9f83c1522d 100644 --- a/plugins/kotlin/.gitignore +++ b/plugins/kotlin/.gitignore @@ -1,4 +1,5 @@ /build/ /out/ /testData/classes/kt/ +/testData/classes/ktold/ bin \ No newline at end of file diff --git a/plugins/kotlin/build.gradle b/plugins/kotlin/build.gradle index d906f1f5f1..c3b729b4fc 100644 --- a/plugins/kotlin/build.gradle +++ b/plugins/kotlin/build.gradle @@ -13,6 +13,11 @@ archivesBaseName = 'vineflower-kotlin' sourceSets { testDataKotlin.kotlin.srcDirs files("testData/src/kt/") + testDataOldKotlin.kotlin.srcDirs files("testData/src/ktold/") +} + +configurations { + testDataOldKotlinImplementation.extendsFrom testDataKotlinImplementation } dependencies { @@ -35,6 +40,20 @@ compileTestDataKotlinKotlin { } testDataClasses.dependsOn(testDataKotlinClasses) +compileTestDataOldKotlinKotlin { + destinationDirectory = file("testData/classes/ktold") + kotlinOptions { + compilerOptions { + freeCompilerArgs = [ // Kotlin 2.0 defaults to invokedynamic implementations. Use class implementations instead. + "-Xlambdas=class", + "-Xsam-conversions=class", + "-Xstring-concat=inline", + ] + } + } +} +testDataClasses.dependsOn(testDataOldKotlinClasses) + jar { enabled = false } diff --git a/plugins/kotlin/src/main/java/org/vineflower/kotlin/expr/KFunctionExprent.java b/plugins/kotlin/src/main/java/org/vineflower/kotlin/expr/KFunctionExprent.java index d638fd4af9..29859aff89 100644 --- a/plugins/kotlin/src/main/java/org/vineflower/kotlin/expr/KFunctionExprent.java +++ b/plugins/kotlin/src/main/java/org/vineflower/kotlin/expr/KFunctionExprent.java @@ -1,6 +1,7 @@ package org.vineflower.kotlin.expr; import org.jetbrains.java.decompiler.main.DecompilerContext; +import org.jetbrains.java.decompiler.main.extern.IFernflowerLogger; import org.jetbrains.java.decompiler.main.extern.IFernflowerPreferences; import org.jetbrains.java.decompiler.modules.decompiler.exps.*; import org.jetbrains.java.decompiler.modules.decompiler.vars.CheckTypesResult; @@ -201,10 +202,24 @@ public TextBuffer toJava(int indent) { case STR_CONCAT -> { buf.append('"'); for (Exprent expr : lstOperands) { - if (expr instanceof ConstExprent constExpr && VarType.VARTYPE_STRING.equals(constExpr.getExprType())) { - boolean ascii = DecompilerContext.getOption(IFernflowerPreferences.ASCII_STRING_CHARACTERS); - String value = ConstExprent.convertStringToJava((String) constExpr.getValue(), ascii); - buf.append(value.replace("$", "\\$")); + if (expr instanceof ConstExprent constExpr) { + // Strings can be directly placed into the resulting string, but other constants are a little more touchy. + // Kotlin will inline any primitive constants. If any are found, warn on the presence of such cases as that + // implies that either the compiler or decompiler is not working as expected. + if (VarType.VARTYPE_STRING.equals(constExpr.getConstType())) { + boolean ascii = DecompilerContext.getOption(IFernflowerPreferences.ASCII_STRING_CHARACTERS); + String value = ConstExprent.convertStringToJava((String) constExpr.getValue(), ascii); + buf.append(value.replace("$", "\\$")); + } else if (VarType.VARTYPE_CHAR.equals(constExpr.getConstType())) { + // The compiler uses `StringBuilder.append(char)` on single characters when using StringBuilder + // instead of makeConcatWithConstants. Inline the character directly - intentional behavior. + buf.append((char) (int) constExpr.getValue()); + } else { + if (VarType.isPrimitive(constExpr.getConstType())) { + DecompilerContext.getLogger().writeMessage("Primitive constant type in string concatenation: " + constExpr.getConstType(), IFernflowerLogger.Severity.WARN); + } + buf.append("${").append(constExpr.toJava(indent)).append("}"); + } } else if (expr instanceof VarExprent var) { buf.append("$").append(var.toJava(indent)); } else { diff --git a/plugins/kotlin/src/test/java/org/vineflower/kotlin/KotlinTests.java b/plugins/kotlin/src/test/java/org/vineflower/kotlin/KotlinTests.java index af2e758f0e..56831a4f1a 100644 --- a/plugins/kotlin/src/test/java/org/vineflower/kotlin/KotlinTests.java +++ b/plugins/kotlin/src/test/java/org/vineflower/kotlin/KotlinTests.java @@ -7,6 +7,7 @@ import java.nio.file.Path; import static org.jetbrains.java.decompiler.SingleClassesTestBase.TestDefinition.Version.KOTLIN; +import static org.jetbrains.java.decompiler.SingleClassesTestBase.TestDefinition.Version.KOTLIN_OLD; public class KotlinTests extends SingleClassesTestBase { @@ -86,5 +87,6 @@ private void registerKotlinTests() { register(KOTLIN, "TestConstructors"); register(KOTLIN, "TestContracts"); register(KOTLIN, "TestStringInterpolation"); + register(KOTLIN_OLD, "TestClassicStringInterpolation"); } } diff --git a/plugins/kotlin/testData/results/pkg/TestClassicStringInterpolation.dec b/plugins/kotlin/testData/results/pkg/TestClassicStringInterpolation.dec new file mode 100644 index 0000000000..33a12203f5 --- /dev/null +++ b/plugins/kotlin/testData/results/pkg/TestClassicStringInterpolation.dec @@ -0,0 +1,118 @@ +package pkg + +class TestClassicStringInterpolation { + public final val x: Int = 5 + + + public fun stringInterpolation(x: Int, y: String) { + System.out.println("$x $y");// 5 + }// 6 + + public fun testConstant(x: Int) { + System.out.println("$x 5");// 9 + }// 10 + + public fun testExpression(x: Int) { + System.out.println("$x ${x + 1}");// 13 + }// 14 + + public fun testProperty() { + System.out.println("${this.x}!");// 18 + }// 19 + + public fun testLiteralClass() { + System.out.println("${TestClassicStringInterpolation::class.java}!");// 22 + }// 23 +} + +class 'pkg/TestClassicStringInterpolation' { + method 'stringInterpolation (ILjava/lang/String;)V' { + d 7 + 16 7 + 1a 7 + 1b 7 + 1c 7 + 1d 7 + 1e 7 + 1f 7 + 21 7 + 22 7 + 23 7 + 24 8 + } + + method 'testConstant (I)V' { + 7 11 + 10 11 + 11 11 + 12 11 + 13 11 + 14 11 + 15 11 + 17 11 + 18 11 + 19 11 + 1a 12 + } + + method 'testExpression (I)V' { + 7 15 + 10 15 + 11 15 + 12 15 + 16 15 + 17 15 + 18 15 + 19 15 + 1a 15 + 1b 15 + 1d 15 + 1e 15 + 1f 15 + 20 16 + } + + method 'testProperty ()V' { + 7 19 + 8 19 + 9 19 + a 19 + 13 19 + 14 19 + 15 19 + 16 19 + 17 19 + 18 19 + 1a 19 + 1b 19 + 1c 19 + 1d 20 + } + + method 'testLiteralClass ()V' { + 7 23 + 8 23 + 11 23 + 12 23 + 13 23 + 14 23 + 15 23 + 16 23 + 18 23 + 19 23 + 1a 23 + 1b 24 + } +} + +Lines mapping: +5 <-> 8 +6 <-> 9 +9 <-> 12 +10 <-> 13 +13 <-> 16 +14 <-> 17 +18 <-> 20 +19 <-> 21 +22 <-> 24 +23 <-> 25 diff --git a/plugins/kotlin/testData/src/ktold/pkg/TestClassicStringInterpolation.kt b/plugins/kotlin/testData/src/ktold/pkg/TestClassicStringInterpolation.kt new file mode 100644 index 0000000000..5ed50b57fa --- /dev/null +++ b/plugins/kotlin/testData/src/ktold/pkg/TestClassicStringInterpolation.kt @@ -0,0 +1,24 @@ +package pkg + +class TestClassicStringInterpolation { + fun stringInterpolation(x: Int, y: String) { + println("$x $y") + } + + fun testConstant(x: Int) { + println("$x ${5}") + } + + fun testExpression(x: Int) { + println("$x ${x + 1}") + } + + val x = 5 + fun testProperty() { + println("$x!") + } + + fun testLiteralClass() { + println("${TestClassicStringInterpolation::class.java}!") + } +} diff --git a/testFixtures/org/jetbrains/java/decompiler/SingleClassesTestBase.java b/testFixtures/org/jetbrains/java/decompiler/SingleClassesTestBase.java index 59b2d8f033..f735378c26 100644 --- a/testFixtures/org/jetbrains/java/decompiler/SingleClassesTestBase.java +++ b/testFixtures/org/jetbrains/java/decompiler/SingleClassesTestBase.java @@ -289,6 +289,7 @@ public enum Version { JAVA_21_PREVIEW(21, "preview", "Preview"), GROOVY("groovy", "Groovy"), KOTLIN("kt", "Kotlin"), + KOTLIN_OLD("ktold", "Kotlin (old defaults)"), SCALA("scala", "Scala"), JASM("jasm", "Custom (jasm)"), ;