Refactor and rename transform classes

src/main/java/net/md_5/specialsource/JarMapping.java

@@ -28,14 +28,15 @@
  */
 package net.md_5.specialsource;
 
+import net.md_5.specialsource.transformer.MavenShade;
 import net.md_5.specialsource.srg.CompactSrgWriter;
 import net.md_5.specialsource.srg.SrgWriter;
 import net.md_5.specialsource.srg.ISrgWriter;
 import net.md_5.specialsource.provider.IInheritanceProvider;
-import net.md_5.specialsource.transformer.CSVMappingTransformer;
+import net.md_5.specialsource.transformer.MinecraftCodersPack;
-import net.md_5.specialsource.transformer.MethodDescriptorTransformer;
+import net.md_5.specialsource.transformer.MethodDescriptor;
-import net.md_5.specialsource.transformer.ChainTransformer;
+import net.md_5.specialsource.transformer.ChainingTransformer;
-import net.md_5.specialsource.transformer.JarMappingLoadTransformer;
+import net.md_5.specialsource.transformer.MappingTransformer;
 import java.io.*;
 import java.util.*;
 
@@ -140,8 +141,8 @@ public class JarMapping {
         File methodsCsv = URLDownloader.getLocalFile(dirname + sep + "methods.csv");
         File packagesCsv = URLDownloader.getLocalFile(dirname + sep + "packages.csv"); // FML repackaging, optional
 
-        CSVMappingTransformer outputTransformer;
+        MinecraftCodersPack outputTransformer;
-        JarMappingLoadTransformer inputTransformer;
+        MappingTransformer inputTransformer;
 
         if (numericSrgNames) {
             // Wants numeric "srg" names -> descriptive "csv" names. To accomplish this:
@@ -150,14 +151,14 @@ public class JarMapping {
             // 3. result: mcp->srg, similar to MCP ./reobfuscate --srgnames
             JarMapping chainMappings = new JarMapping();
             chainMappings.loadMappingsDir(dirname, reverse, false/*ignoreCsv*/, false/*numeric*/);
-            inputTransformer = new ChainTransformer(new JarRemapper(chainMappings));
+            inputTransformer = new ChainingTransformer(new JarRemapper(chainMappings));
             ignoreCsv = true; // keep numeric srg as output
         } else {
             inputTransformer = null;
         }
 
         if (fieldsCsv.exists() && methodsCsv.exists()) {
-            outputTransformer = new CSVMappingTransformer(ignoreCsv ? null : fieldsCsv, ignoreCsv ? null : methodsCsv, packagesCsv);
+            outputTransformer = new MinecraftCodersPack(ignoreCsv ? null : fieldsCsv, ignoreCsv ? null : methodsCsv, packagesCsv);
         } else {
             outputTransformer = null;
         }
@@ -167,11 +168,6 @@ public class JarMapping {
         }
     }
 
-    @Deprecated
-    public void loadMappingsDir(String dirname, boolean reverse, boolean ignoreCsv) throws IOException {
-        loadMappingsDir(dirname, reverse, ignoreCsv, false);
-    }
-
     public void loadMappings(File file) throws IOException {
         loadMappings(new BufferedReader(new FileReader(file)), null, null, false);
     }
@@ -189,15 +185,15 @@ public class JarMapping {
      */
     public void loadMappings(String filename, boolean reverse, boolean numericSrgNames, String inShadeRelocation, String outShadeRelocation) throws IOException {
         // Optional shade relocation, on input or output names
-        JarMappingLoadTransformer inputTransformer = null;
+        MappingTransformer inputTransformer = null;
-        JarMappingLoadTransformer outputTransformer = null;
+        MappingTransformer outputTransformer = null;
 
         if (inShadeRelocation != null) {
-            inputTransformer = new ShadeRelocationSimulator(inShadeRelocation);
+            inputTransformer = new MavenShade(inShadeRelocation);
         }
 
         if (outShadeRelocation != null) {
-            outputTransformer = new ShadeRelocationSimulator(outShadeRelocation);
+            outputTransformer = new MavenShade(outShadeRelocation);
         }
 
         if (new File(filename).isDirectory() || filename.endsWith("/")) {
@@ -229,12 +225,12 @@ public class JarMapping {
      * input/output transformations)
      * @throws IOException
      */
-    public void loadMappings(BufferedReader reader, JarMappingLoadTransformer inputTransformer, JarMappingLoadTransformer outputTransformer, boolean reverse) throws IOException {
+    public void loadMappings(BufferedReader reader, MappingTransformer inputTransformer, MappingTransformer outputTransformer, boolean reverse) throws IOException {
         if (inputTransformer == null) {
-            inputTransformer = ShadeRelocationSimulator.IDENTITY;
+            inputTransformer = MavenShade.IDENTITY;
         }
         if (outputTransformer == null) {
-            outputTransformer = ShadeRelocationSimulator.IDENTITY;
+            outputTransformer = MavenShade.IDENTITY;
         }
 
         String line;
@@ -256,7 +252,7 @@ public class JarMapping {
     /**
      * Parse a 'csrg' mapping format line and populate the data structures
      */
-    private void parseCsrgLine(String line, JarMappingLoadTransformer inputTransformer, JarMappingLoadTransformer outputTransformer, boolean reverse) throws IOException {
+    private void parseCsrgLine(String line, MappingTransformer inputTransformer, MappingTransformer outputTransformer, boolean reverse) throws IOException {
         if (reverse) {
             throw new IllegalArgumentException("csrg reversed not supported"); // TODO: reverse csg (need to lookup remapped classes)
         }
@@ -293,7 +289,7 @@ public class JarMapping {
      * Parse a standard 'srg' mapping format line and populate the data
      * structures
      */
-    private void parseSrgLine(String line, JarMappingLoadTransformer inputTransformer, JarMappingLoadTransformer outputTransformer, boolean reverse) throws IOException {
+    private void parseSrgLine(String line, MappingTransformer inputTransformer, MappingTransformer outputTransformer, boolean reverse) throws IOException {
         String[] tokens = line.split(" ");
         String kind = tokens[0];
 
@@ -444,7 +440,7 @@ public class JarMapping {
             String key = oldMethod.owner + "/" + oldMethod.name + " " + oldMethod.descriptor;
             methods.put(key, newMethod.name);
 
-            MethodDescriptorTransformer methodDescriptorTransformer = new MethodDescriptorTransformer(null, classes);
+            MethodDescriptor methodDescriptorTransformer = new MethodDescriptor(null, classes);
             String oldDescriptor = methodDescriptorTransformer.transform(oldMethod.descriptor);
 
             if (full || !(oldMethod.name + " " + oldDescriptor).equals(newMethod.name + " " + newMethod.descriptor)) {

src/main/java/net/md_5/specialsource/JarRemapper.java

@@ -61,7 +61,7 @@ public class JarRemapper extends Remapper {
         return mapTypeName(typeName, jarMapping.packages, jarMapping.classes, typeName);
     }
 
-    public static String mapTypeName(String typeName, LinkedHashMap<String, String> packageMap, Map<String, String> classMap, String defaultIfUnmapped) {
+    public static String mapTypeName(String typeName, Map<String, String> packageMap, Map<String, String> classMap, String defaultIfUnmapped) {
         int index = typeName.indexOf('$');
         String key = (index == -1) ? typeName : typeName.substring(0, index);
         String mapped = mapClassName(key, packageMap, classMap);
@@ -72,7 +72,7 @@ public class JarRemapper extends Remapper {
     /**
      * Helper method to map a class name by package (prefix) or class (exact)
      */
-    private static String mapClassName(String className, LinkedHashMap<String, String> packageMap, Map<String, String> classMap) {
+    private static String mapClassName(String className, Map<String, String> packageMap, Map<String, String> classMap) {
         if (packageMap != null) {
             Iterator<String> iter = packageMap.keySet().iterator();
             while (iter.hasNext()) {

src/main/java/net/md_5/specialsource/transformer/ChainingTransformer.java

@@ -31,17 +31,18 @@ package net.md_5.specialsource.transformer;
 import net.md_5.specialsource.JarRemapper;
 
 /**
- * Load a mapping 'chained' through another mapping
+ * Load a mapping 'chained' through another mapping.
  */
-public class ChainTransformer extends JarMappingLoadTransformer {
+public class ChainingTransformer extends MappingTransformer {
 
     private final JarRemapper jarRemapper;
+    private final MethodDescriptor methodTransformer;
 
-    public ChainTransformer(JarRemapper jarRemapper) {
+    public ChainingTransformer(JarRemapper jarRemapper) {
         this.jarRemapper = jarRemapper;
+        this.methodTransformer = new MethodDescriptor(jarRemapper.jarMapping.packages, jarRemapper.jarMapping.classes);
     }
 
-    // TODO: make this less lame
     @Override
     public String transformClassName(String className) {
         return jarRemapper.map(className);
@@ -59,7 +60,6 @@ public class ChainTransformer extends JarMappingLoadTransformer {
 
     @Override
     public String transformMethodDescriptor(String oldDescriptor) {
-        MethodDescriptorTransformer methodDescriptorTransformer = new MethodDescriptorTransformer(jarRemapper.jarMapping.packages, jarRemapper.jarMapping.classes);
+        return methodTransformer.transform(oldDescriptor);
-        return methodDescriptorTransformer.transform(oldDescriptor);
     }
 }

src/main/java/net/md_5/specialsource/transformer/MappingTransformer.java

@@ -33,7 +33,7 @@ package net.md_5.specialsource.transformer;
  *
  * TODO: refactor with JarRemapper?
  */
-public abstract class JarMappingLoadTransformer {
+public abstract class MappingTransformer {
 
     public String transformClassName(String className) {
         return className;

src/main/java/net/md_5/specialsource/transformer/MavenShade.java

@@ -26,23 +26,21 @@
  * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
  * POSSIBILITY OF SUCH DAMAGE.
  */
-package net.md_5.specialsource;
+package net.md_5.specialsource.transformer;
 
-import net.md_5.specialsource.transformer.MethodDescriptorTransformer;
-import net.md_5.specialsource.transformer.JarMappingLoadTransformer;
 import java.util.*;
 
 /**
  * Simulate a small subset of the maven-shade-plugin class relocation
  * functionality
  */
-public class ShadeRelocationSimulator extends JarMappingLoadTransformer {
+public class MavenShade extends MappingTransformer {
 
     public LinkedHashMap<String, String> relocations = new LinkedHashMap<String, String>();
     // No relocations
-    public static final ShadeRelocationSimulator IDENTITY = new ShadeRelocationSimulator();
+    public static final MavenShade IDENTITY = new MavenShade();
 
-    private ShadeRelocationSimulator() {
+    private MavenShade() {
     }
 
     /**
@@ -50,7 +48,7 @@ public class ShadeRelocationSimulator extends JarMappingLoadTransformer {
      *
      * @param relocations
      */
-    public ShadeRelocationSimulator(Map<String, String> relocations) {
+    public MavenShade(Map<String, String> relocations) {
         for (Map.Entry<String, String> entry : relocations.entrySet()) {
             this.relocations.put(toInternalName(entry.getKey()), toInternalName(entry.getValue()));
         }
@@ -62,7 +60,7 @@ public class ShadeRelocationSimulator extends JarMappingLoadTransformer {
      *
      * @param list
      */
-    public ShadeRelocationSimulator(List<String> list) {
+    public MavenShade(List<String> list) {
         for (String pair : list) {
             int index = pair.indexOf("=");
             if (index == -1) {
@@ -75,7 +73,7 @@ public class ShadeRelocationSimulator extends JarMappingLoadTransformer {
         }
     }
 
-    public ShadeRelocationSimulator(String string) {
+    public MavenShade(String string) {
         this(Arrays.asList(string.split(",")));
     }
 
@@ -98,7 +96,7 @@ public class ShadeRelocationSimulator extends JarMappingLoadTransformer {
 
     @Override
     public String transformMethodDescriptor(String oldDescriptor) {
-        MethodDescriptorTransformer methodDescriptorTransformer = new MethodDescriptorTransformer(relocations, null);
+        MethodDescriptor methodDescriptorTransformer = new MethodDescriptor(relocations, null);
         return methodDescriptorTransformer.transform(oldDescriptor);
     }
 

src/main/java/net/md_5/specialsource/transformer/MethodDescriptor.java

@@ -28,16 +28,15 @@
  */
 package net.md_5.specialsource.transformer;
 
-import java.util.LinkedHashMap;
 import java.util.Map;
 import net.md_5.specialsource.JarRemapper;
 
-public class MethodDescriptorTransformer {
+public class MethodDescriptor {
 
-    private LinkedHashMap<String, String> packageMap;
+    private Map<String, String> packageMap;
     private Map<String, String> classMap;
 
-    public MethodDescriptorTransformer(LinkedHashMap<String, String> packageMap, Map<String, String> classMap) {
+    public MethodDescriptor(Map<String, String> packageMap, Map<String, String> classMap) {
         this.packageMap = packageMap;
         this.classMap = classMap;
     }

src/main/java/net/md_5/specialsource/transformer/MinecraftCodersPack.java

@@ -29,6 +29,7 @@
 package net.md_5.specialsource.transformer;
 
 import au.com.bytecode.opencsv.CSVReader;
+import com.google.common.base.Preconditions;
 
 import java.io.File;
 import java.io.FileReader;
@@ -38,19 +39,16 @@ import java.util.Map;
 import net.md_5.specialsource.JarRemapper;
 
 /**
- * For reading a .srg through MCP's fields.csv and methods.csv Maps func_### and
+ * For reading a srg through MCP's fields.csv and methods.csv Maps func_### and
- * field_### in input srg to "descriptive" names
+ * field_### in input srg to "descriptive" names.
  */
-public class CSVMappingTransformer extends JarMappingLoadTransformer {
+public class MinecraftCodersPack extends MappingTransformer {
 
-    private final Map<String, String> fieldMap; // numeric srg name field_### -> descriptive csv name
+    private final Map<String, String> fieldMap = new HashMap<String, String>(); // numeric srg name field_### -> descriptive csv name
-    private final Map<String, String> methodMap; // numeric srg name func_### -> descriptive csv name
+    private final Map<String, String> methodMap = new HashMap<String, String>(); // numeric srg name func_### -> descriptive csv name
     private final Map<String, String> classPackageMap; // class src name -> repackaged full class name
 
-    public CSVMappingTransformer(File fieldsCsv, File methodsCsv, File packagesCsv) throws IOException {
+    public MinecraftCodersPack(File fieldsCsv, File methodsCsv, File packagesCsv) throws IOException {
-        fieldMap = new HashMap<String, String>();
-        methodMap = new HashMap<String, String>();
-
         if (fieldsCsv != null && fieldsCsv.exists()) {
             readIntoMap(fieldsCsv, fieldMap);
         }
@@ -65,12 +63,8 @@ public class CSVMappingTransformer extends JarMappingLoadTransformer {
 
             Map<String, String> packages = new HashMap<String, String>();
             readIntoMap(packagesCsv, packages);
-
             for (Map.Entry<String, String> entry : packages.entrySet()) {
-                String classSimpleName = entry.getKey();
+                classPackageMap.put("net/minecraft/src/" + entry.getKey(), entry.getValue() + "/" + entry.getKey());
-                String newPackageName = entry.getValue();
-
-                classPackageMap.put("net/minecraft/src/" + classSimpleName, newPackageName + "/" + classSimpleName);
             }
         } else {
             // flat package (vanilla MCP)
@@ -79,33 +73,40 @@ public class CSVMappingTransformer extends JarMappingLoadTransformer {
     }
 
     private void readIntoMap(File file, Map<String, String> map) throws IOException {
-        CSVReader csvReader = new CSVReader(new FileReader(file));
+        FileReader fileReader = null;
-        String[] line;
+        CSVReader csvReader = null;
+        try {
+            fileReader = new FileReader(file);
+            csvReader = new CSVReader(fileReader);
 
+            String[] line;
             while ((line = csvReader.readNext()) != null) {
                 if (line.length == 0) {
                     continue;
                 }
-
+                Preconditions.checkArgument(line.length >= 2, "Invalid csv line: %s", (Object) line);
-            if (line.length < 2) {
+                map.put(line[0], line[1]);
-                throw new IllegalArgumentException("Invalid csv line: " + line);
+            }
+        } finally {
+            if (csvReader != null) {
+                csvReader.close();
+            }
+            if (fileReader != null) {
+                fileReader.close();
             }
-
-            String key = line[0];
-            String value = line[1];
-
-            map.put(key, value);
         }
     }
 
     @Override
     public String transformFieldName(String className, String fieldName) {
-        return fieldMap.containsKey(fieldName) ? fieldMap.get(fieldName) : fieldName;
+        String mapped = fieldMap.get(fieldName);
+        return (mapped != null) ? mapped : fieldName;
     }
 
     @Override
     public String transformMethodName(String className, String methodName, String methodDescriptor) {
-        return methodMap.containsKey(methodName) ? methodMap.get(methodName) : methodName;
+        String mapped = methodMap.get(methodName);
+        return (mapped != null) ? mapped : methodName;
     }
 
     @Override
@@ -114,12 +115,8 @@ public class CSVMappingTransformer extends JarMappingLoadTransformer {
             return className;
         }
 
-        String newPackage = classPackageMap.get(className);
+        String mapped = classPackageMap.get(className);
-        if (newPackage == null) {
+        return (mapped != null) ? JarRemapper.mapTypeName(className, null, classPackageMap, className) : className;
-            return className;
-        }
-
-        return JarRemapper.mapTypeName(className, null, classPackageMap, className);
     }
 
     @Override
@@ -128,7 +125,7 @@ public class CSVMappingTransformer extends JarMappingLoadTransformer {
             return oldDescriptor;
         }
 
-        MethodDescriptorTransformer methodDescriptorTransformer = new MethodDescriptorTransformer(null, classPackageMap);
+        MethodDescriptor methodDescriptorTransformer = new MethodDescriptor(null, classPackageMap);
         return methodDescriptorTransformer.transform(oldDescriptor);
     }
 }