Renaming plugin (#334)

* first draft

* Improve import collector locking

* Deduplicate part of the buildscript

* Small fixup

build.gradle

@@ -1,6 +1,7 @@
 // Copyright 2000-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 
-import org.jetbrains.java.decompiler.build.JasmCompile
+import org.vineflower.build.JasmCompile
+import org.vineflower.build.TestDataRuntimesProvider
 
 plugins {
   id 'jacoco'
@@ -172,25 +173,6 @@ compileTestDataScalaScala {
 }
 testDataClasses.dependsOn(testDataScalaClasses)
 
-class TestDataRuntimesProvider implements CommandLineArgumentProvider {
-    @Nested
-    final MapProperty<String, JavaLauncher> launchers
-
-    @Inject
-    TestDataRuntimesProvider(final ObjectFactory objects) {
-        this.launchers = objects.mapProperty(Integer, JavaLauncher)
-    }
-
-    @Override
-    Iterable<String> asArguments() {
-        def result = []
-        this.launchers.get().each { k, v ->
-            result << "-Djava.${k}.home=${v.metadata.installationPath.asFile.absolutePath}"
-        }
-        return result
-    }
-}
-
 test {
   maxHeapSize = "512M"
 

buildSrc/build.gradle

@@ -1,4 +1,7 @@
-apply plugin: 'java'
+plugins {
+  id 'java'
+  id 'groovy-gradle-plugin'
+}
 
 repositories {
   mavenCentral()

buildSrc/src/main/groovy/org/vineflower/build/TestDataRuntimesProvider.groovy

@@ -0,0 +1,28 @@
+package org.vineflower.build
+
+import org.gradle.api.model.ObjectFactory
+import org.gradle.api.provider.MapProperty
+import org.gradle.api.tasks.Nested
+import org.gradle.jvm.toolchain.JavaLauncher
+import org.gradle.process.CommandLineArgumentProvider
+
+import javax.inject.Inject
+
+public class TestDataRuntimesProvider implements CommandLineArgumentProvider {
+  @Nested
+  final MapProperty<String, JavaLauncher> launchers
+
+  @Inject
+  public TestDataRuntimesProvider(final ObjectFactory objects) {
+    this.launchers = objects.mapProperty(Integer, JavaLauncher)
+  }
+
+  @Override
+  Iterable<String> asArguments() {
+    def result = []
+    this.launchers.get().each { k, v ->
+      result << "-Djava.${k}.home=${v.metadata.installationPath.asFile.absolutePath}"
+    }
+    return result
+  }
+}

buildSrc/src/main/java/org/jetbrains/java/decompiler/build/JasmCompile.java → buildSrc/src/main/java/org/vineflower/build/JasmCompile.java

@@ -1,4 +1,4 @@
-package org.jetbrains.java.decompiler.build;
+package org.vineflower.build;
 
 import org.gradle.api.file.DirectoryProperty;
 import org.gradle.api.model.ReplacedBy;
@@ -7,12 +7,9 @@
 import org.gradle.api.tasks.OutputDirectory;
 import org.gradle.api.tasks.SourceTask;
 import org.gradle.api.tasks.TaskAction;
-import org.gradle.api.tasks.compile.CompileOptions;
-import org.gradle.work.InputChanges;
 import org.openjdk.asmtools.jasm.Main;
 
 import java.io.File;
-import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.List;

buildSrc/src/main/java/org/jetbrains/java/decompiler/build/JasmCompileOptions.java → buildSrc/src/main/java/org/vineflower/build/JasmCompileOptions.java

@@ -1,4 +1,4 @@
-package org.jetbrains.java.decompiler.build;
+package org.vineflower.build;
 
 import org.gradle.api.tasks.Input;
 import org.gradle.api.tasks.compile.AbstractOptions;

plugins/variable-renaming/.gitignore

@@ -0,0 +1,4 @@
+/build/
+/out/
+testData/classes/java*/
+bin

plugins/variable-renaming/build.gradle

@@ -0,0 +1,61 @@
+import org.vineflower.build.TestDataRuntimesProvider
+
+archivesBaseName = 'vineflower-variable-renaming'
+
+dependencies {
+  implementation project(":")
+  testImplementation testFixtures(project(":"))
+}
+
+task testDataClasses {
+  group = 'build'
+}
+testClasses.dependsOn(testDataClasses)
+
+void createJavaTestDataSet(int version, String suffix = "", List<String> compilerArgs = []) {
+  sourceSets.create("testDataJava${version}${suffix}") {
+    it.java.srcDirs file("testData/src/java${version}${suffix.toLowerCase()}")
+  }
+  tasks.getByName("compileTestDataJava${version}${suffix}Java") {
+    destinationDirectory = file("testData/classes/java${version}${suffix.toLowerCase()}")
+    if (project.isArm && version > 8 && version < 11) {
+      // On ARM systems, a more limited set of JVM versions are available
+      // We'll accept the `--release` flag so development is at least somewhat possible
+      javaCompiler = javaToolchains.compilerFor {
+        languageVersion = JavaLanguageVersion.of(11)
+      }
+      options.release = version
+    } else {
+      javaCompiler = javaToolchains.compilerFor {
+        languageVersion = JavaLanguageVersion.of(version)
+      }
+    }
+
+    options.compilerArgs = compilerArgs
+  }
+  testDataClasses.dependsOn("testDataJava${version}${suffix}Classes")
+}
+
+def testJavaRuntimes = [:]
+
+[8].forEach { version ->
+  def runtimeVersion = isArm && version > 8 && version < 11 ? 11 : version
+  createJavaTestDataSet(version)
+  testJavaRuntimes[version] = javaToolchains.launcherFor {
+    languageVersion = JavaLanguageVersion.of(runtimeVersion)
+  }
+}
+
+test {
+  maxHeapSize = "512M"
+
+  systemProperty "DOT_EXPORT_DIR", System.getProperty("DOT_EXPORT_DIR", null)
+  systemProperty "DOT_ERROR_EXPORT_DIR", System.getProperty("DOT_ERROR_EXPORT_DIR", null)
+  systemProperty "VALIDATE_DECOMPILED_CODE", System.getProperty("VALIDATE_DECOMPILED_CODE", "false")
+
+  def provider = objects.newInstance(TestDataRuntimesProvider)
+  testJavaRuntimes.each { k, v ->
+    provider.launchers.put(k, v)
+  }
+  jvmArgumentProviders << provider
+}

plugins/variable-renaming/gradle.properties

@@ -0,0 +1 @@
+does_shadow=false

src/org/jetbrains/java/decompiler/util/JADNameProvider.java → plugins/variable-renaming/src/main/java/org/vineflower/variablerenaming/JADNameProvider.java

@@ -13,11 +13,10 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.jetbrains.java.decompiler.util;
+package org.vineflower.variablerenaming;
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -27,19 +26,23 @@
 import java.util.regex.Pattern;
 
 import org.jetbrains.java.decompiler.code.CodeConstants;
+import org.jetbrains.java.decompiler.main.DecompilerContext;
+import org.jetbrains.java.decompiler.main.extern.IFernflowerPreferences;
 import org.jetbrains.java.decompiler.main.extern.IVariableNameProvider;
 import org.jetbrains.java.decompiler.main.extern.IVariableNamingFactory;
+import org.jetbrains.java.decompiler.modules.decompiler.ExprProcessor;
 import org.jetbrains.java.decompiler.modules.decompiler.vars.VarVersionPair;
 import org.jetbrains.java.decompiler.struct.StructMethod;
 import org.jetbrains.java.decompiler.struct.gen.MethodDescriptor;
 import org.jetbrains.java.decompiler.struct.gen.VarType;
+import org.jetbrains.java.decompiler.util.Pair;
 
 public class JADNameProvider implements IVariableNameProvider {
-  private HashMap<String, Holder> last = null;
-  private HashMap<String, String> remap = null;
+  private HashMap<String, Holder> last;
+  private HashMap<String, String> remap;
   private final HashMap<Integer, String> parameters = new HashMap<>();
-  private StructMethod method = null;
-  private boolean renameParameters = false;
+  private final StructMethod method;
+  private final boolean renameParameters;
   private static final Pattern CAPS_START = Pattern.compile("^[A-Z]");
   private static final Pattern ARRAY = Pattern.compile("(\\[|\\.\\.\\.)");
 
@@ -100,7 +103,7 @@ public Holder(int t1, boolean skip_zero, List<String> names) {
   }
 
   @Override
-  public Map<VarVersionPair,String> rename(Map<VarVersionPair, String> entries) {
+  public Map<VarVersionPair,String> rename(Map<VarVersionPair, Pair<VarType, String>> entries) {
     int params = 0;
     if ((this.method.getAccessFlags() & CodeConstants.ACC_STATIC) != CodeConstants.ACC_STATIC) {
       params++;
@@ -116,7 +119,7 @@ public Map<VarVersionPair,String> rename(Map<VarVersionPair, String> entries) {
 
     Map<VarVersionPair, String> result = new LinkedHashMap<>();
     for (VarVersionPair ver : keys) {
-      String type = cleanType(entries.get(ver));
+      String type = cleanType(entries.get(ver).b);
       if ("this".equals(type)) {
         continue;
       }
@@ -201,24 +204,20 @@ else if (remap.containsKey(type)) {
   }
 
   @Override
-  public String renameParameter(int flags, String type, String name, int index) {
+  public String renameParameter(int flags, VarType type, String name, int index) {
+    String typeName = ExprProcessor.getCastTypeName(type);
     if (!this.renameParameters) {
       return IVariableNameProvider.super.renameParameter(flags, type, name, index);
     }
 
-    return this.parameters.computeIfAbsent(index, k -> getNewName(cleanType(type)));
+    return this.parameters.computeIfAbsent(index, k -> getNewName(cleanType(typeName)));
   }
 
   public static class JADNameProviderFactory implements IVariableNamingFactory {
-    private final boolean renameParameters;
-
-    public JADNameProviderFactory(boolean renameParameters) {
-      this.renameParameters = renameParameters;
-    }
-
     @Override
     public IVariableNameProvider createFactory(StructMethod method) {
-      return new JADNameProvider(renameParameters, method);
+      return new JADNameProvider(DecompilerContext.getOption(VariableRenamingOptions.RENAME_PARAMETERS)
+        || DecompilerContext.getOption(VariableRenamingOptions.USE_JAD_PARAMETER_NAMING), method);
     }
   }
 }

plugins/variable-renaming/src/main/java/org/vineflower/variablerenaming/Renamers.java

@@ -0,0 +1,18 @@
+package org.vineflower.variablerenaming;
+
+import org.jetbrains.java.decompiler.main.extern.IVariableNamingFactory;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class Renamers {
+  private static final Map<String, IVariableNamingFactory> PROVIDERS = new HashMap<>();
+
+  public static void registerProvider(String name, IVariableNamingFactory factory) {
+    PROVIDERS.put(name, factory);
+  }
+
+  public static IVariableNamingFactory get(String name) {
+    return PROVIDERS.get(name);
+  }
+}