Add support for adding missing and removing unused imports

This adds a post-process step which automatically adds unambiguous imports,
removes unused imports and sorts the import list (formatting matches standard
IntelliJ settings).
This will preserve line count across versions at all cost.

Java only for now because it's a lot more tricky with Kotlin and we don't yet
use Kotlin ourselves (and won't be preprocessing it in the future either).

6 files changed, 536 insertions, 15 deletions

diff --git a/src/test/kotlin/com/replaymod/gradle/remap/imports/TestJavaAutoImports.kt b/src/test/kotlin/com/replaymod/gradle/remap/imports/TestJavaAutoImports.kt
new file mode 100644
index 0000000..f41767f
--- /dev/null
+++ b/src/test/kotlin/com/replaymod/gradle/remap/imports/TestJavaAutoImports.kt
@@ -0,0 +1,344 @@
+package com.replaymod.gradle.remap.imports
+
+import com.replaymod.gradle.remap.util.TestData
+import io.kotest.matchers.shouldBe
+import org.junit.jupiter.api.Test
+
+class TestJavaAutoImports {
+    @Test
+    fun `should remove unused imports`() {
+        TestData.remap("""
+            package test;
+            
+            import java.util.ArrayList;
+            
+            class Test {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            
+            
+            class Test {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should add unambiguous missing imports from JDK`() {
+        TestData.remap("""
+            package test;
+            
+            
+            
+            class Test extends ArrayList {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            import java.util.ArrayList;
+            
+            class Test extends ArrayList {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should add unambiguous missing imports from remapped classpath`() {
+        TestData.remap("""
+            package test;
+            
+            
+            
+            class Test extends B {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            import b.pkg.B;
+            
+            class Test extends B {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should not add unambiguous missing imports from original classpath`() {
+        TestData.remap("""
+            package test;
+            
+            
+            
+            class Test extends A {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            
+            
+            class Test extends A {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should add unambiguous missing imports from the same source set`() {
+        val original = """
+            package test;
+            
+            
+            
+            class Test extends Sample {
+            }
+        """.trimIndent()
+        TestData.transformer.remap(mapOf(
+            "test.java" to original,
+            "my/Sample.java" to "package my; public class Sample {}"
+        ))["test.java"]!!.first shouldBe """
+            package test;
+            
+            import my.Sample;
+            
+            class Test extends Sample {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should preserve existing ambiguous imports`() {
+        TestData.remap("""
+            package test;
+            
+            import java.awt.List;
+            
+            class Test implements List {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            import java.awt.List;
+            
+            class Test implements List {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should add new imports in empty lines`() {
+        TestData.remap("""
+            package test;
+            
+            
+            
+            
+            class Test extends ArrayList implements Closeable {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            import java.io.Closeable;
+            import java.util.ArrayList;
+            
+            class Test extends ArrayList implements Closeable {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should add new imports in place of removed imports`() {
+        TestData.remap("""
+            package test;
+            
+            import test.Unused1;
+            import test.Unused2;
+            
+            class Test extends ArrayList implements Closeable {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            import java.io.Closeable;
+            import java.util.ArrayList;
+            
+            class Test extends ArrayList implements Closeable {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `preserves star imports`() {
+        TestData.remap("""
+            package test;
+            
+            import test.Unused1;
+            import java.util.*;
+            
+            class Test extends ArrayList implements Closeable {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            import java.io.Closeable;
+            import java.util.*;
+            
+            class Test extends ArrayList implements Closeable {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should not import classes from the current package`() {
+        TestData.remap("""
+            package b.pkg;
+            
+            class Test extends B implements BInterface {
+            }
+        """.trimIndent()) shouldBe """
+            package b.pkg;
+            
+            class Test extends B implements BInterface {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should not import self`() {
+        TestData.remap("test/Test.java", """
+            package test;
+            
+            public class Test {
+                Test inner;
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            public class Test {
+                Test inner;
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should not import java-lang package`() {
+        TestData.remap("test/Test.java", """
+            package test;
+            
+            public class Test {
+                Object inner;
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            public class Test {
+                Object inner;
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should not import own inner classes`() {
+        TestData.remap("""
+            package test;
+            
+            class Test {
+                TestInner inner;
+                public class TestInner {}
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            class Test {
+                TestInner inner;
+                public class TestInner {}
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should not import generic types`() {
+        TestData.remap("""
+            package test;
+            
+            class Test<B> {
+                B inner;
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            class Test<B> {
+                B inner;
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should not import variable references`() {
+        TestData.remap("""
+            package test;
+            
+            class Test {
+                Object B;
+                { B = null; }
+                { Object BParent; BParent = B; }
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            class Test {
+                Object B;
+                { B = null; }
+                { Object BParent; BParent = B; }
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should not import types that look like fields`() {
+        val content = """
+            package test;
+            
+            import a.pkg.A;
+            
+            class Test extends A {
+                { conflictingField.method(); }
+            }
+        """.trimIndent()
+        TestData.transformer.remap(mapOf(
+            "test/Test.java" to content,
+            "c/conflictingField.java" to "package c; public class conflictingField {}"
+        ))["test/Test.java"]!!.first shouldBe """
+            package test;
+            
+            import b.pkg.B;
+            
+            class Test extends B {
+                { conflictingField.method(); }
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should not touch static imports (yet)`() {
+        TestData.remap("""
+            package test;
+            
+            import test.Unused1;
+            import test.Unused2;
+            
+            import static test.Unused.method;
+            
+            class Test extends ArrayList implements Closeable {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            import java.io.Closeable;
+            import java.util.ArrayList;
+            
+            import static test.Unused.method;
+            
+            class Test extends ArrayList implements Closeable {
+            }
+        """.trimIndent()
+    }
+}
\ No newline at end of file
diff --git a/src/test/kotlin/com/replaymod/gradle/remap/imports/TestJavaAutoImportsFormatting.kt b/src/test/kotlin/com/replaymod/gradle/remap/imports/TestJavaAutoImportsFormatting.kt
new file mode 100644
index 0000000..848eff8
--- /dev/null
+++ b/src/test/kotlin/com/replaymod/gradle/remap/imports/TestJavaAutoImportsFormatting.kt
@@ -0,0 +1,142 @@
+package com.replaymod.gradle.remap.imports
+
+import com.replaymod.gradle.remap.util.TestData
+import io.kotest.matchers.shouldBe
+import org.junit.jupiter.api.Test
+
+class TestJavaAutoImportsFormatting {
+    @Test
+    fun `should separate java(x) from other imports with an empty line if possible`() {
+        TestData.remap("""
+            package test;
+            
+            
+            
+            
+            
+            
+            class Test extends ArrayList implements Closeable, BInterface {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            import b.pkg.BInterface;
+            
+            import java.io.Closeable;
+            import java.util.ArrayList;
+            
+            class Test extends ArrayList implements Closeable, BInterface {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should put new imports in single line if necessary to preserve original line count`() {
+        TestData.remap("""
+            package test;
+            
+            import test.Unused1;
+            import test.Unused2;
+            
+            class Test extends ArrayList implements Closeable, BInterface {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            import b.pkg.BInterface;
+            import java.io.Closeable; import java.util.ArrayList;
+            
+            class Test extends ArrayList implements Closeable, BInterface {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should always leave line after imports`() {
+        TestData.remap("""
+            package test;
+            
+            
+            class Test extends ArrayList implements Closeable {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            import java.io.Closeable; import java.util.ArrayList;
+            
+            class Test extends ArrayList implements Closeable {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should put imports in same line as package if required`() {
+        TestData.remap("""
+            package test;
+            
+            class Test extends ArrayList implements Closeable {
+            }
+        """.trimIndent()) shouldBe """
+            package test; import java.io.Closeable; import java.util.ArrayList;
+            
+            class Test extends ArrayList implements Closeable {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should remove unused imports from shared lines`() {
+        TestData.remap("""
+            package test;
+            
+            import java.io.Closeable; import java.util.ArrayList;
+            
+            class Test {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            
+            
+            class Test {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should remove unused imports from end of shared lines`() {
+        TestData.remap("""
+            package test;
+            
+            import java.io.Closeable; import java.util.ArrayList;
+            
+            class Test implements Closeable {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            import java.io.Closeable;
+            
+            class Test implements Closeable {
+            }
+        """.trimIndent()
+    }
+
+    @Test
+    fun `should remove unused imports from start of shared lines`() {
+        TestData.remap("""
+            package test;
+            
+            import java.io.Closeable; import java.util.ArrayList;
+            
+            class Test extends ArrayList {
+            }
+        """.trimIndent()) shouldBe """
+            package test;
+            
+            import java.util.ArrayList;
+            
+            class Test extends ArrayList {
+            }
+        """.trimIndent()
+    }
+}
\ No newline at end of file
diff --git a/src/test/kotlin/com/replaymod/gradle/remap/imports/TestKotlinAutoImports.kt b/src/test/kotlin/com/replaymod/gradle/remap/imports/TestKotlinAutoImports.kt
new file mode 100644
index 0000000..58e2a02
--- /dev/null
+++ b/src/test/kotlin/com/replaymod/gradle/remap/imports/TestKotlinAutoImports.kt
@@ -0,0 +1,26 @@
+package com.replaymod.gradle.remap.imports
+
+import com.replaymod.gradle.remap.util.TestData
+import io.kotest.matchers.shouldBe
+import org.junit.jupiter.api.Test
+
+class TestKotlinAutoImports {
+    @Test
+    fun `should not touch Kotlin files (yet)`() {
+        TestData.remap("test.kt", """
+            package test
+            
+            import java.util.ArrayList
+            
+            class Test {
+            }
+        """.trimIndent()) shouldBe """
+            package test
+            
+            import java.util.ArrayList
+            
+            class Test {
+            }
+        """.trimIndent()
+    }
+}
\ No newline at end of file
diff --git a/src/test/kotlin/com/replaymod/gradle/remap/mapper/mixin/TestMixinAnnotation.kt b/src/test/kotlin/com/replaymod/gradle/remap/mapper/mixin/TestMixinAnnotation.kt
index bc7cd8f..729b00d 100644
--- a/src/test/kotlin/com/replaymod/gradle/remap/mapper/mixin/TestMixinAnnotation.kt
+++ b/src/test/kotlin/com/replaymod/gradle/remap/mapper/mixin/TestMixinAnnotation.kt
@@ -8,9 +8,11 @@ class TestMixinAnnotation {
     @Test
     fun `remaps with class target`() {
         TestData.remap("""
+            import org.spongepowered.asm.mixin.Shadow;
             @org.spongepowered.asm.mixin.Mixin(a.pkg.A.class)
             class MixinA { @Shadow private int aField; }
         """.trimIndent()) shouldBe """
+            import org.spongepowered.asm.mixin.Shadow;
             @org.spongepowered.asm.mixin.Mixin(b.pkg.B.class)
             class MixinA { @Shadow private int bField; }
         """.trimIndent()
@@ -19,9 +21,11 @@ class TestMixinAnnotation {
     @Test
     fun `remaps with string target`() {
         TestData.remap("""
+            import org.spongepowered.asm.mixin.Shadow;
             @org.spongepowered.asm.mixin.Mixin(targets = "a.pkg.A")
             class MixinA { @Shadow private int aField; }
         """.trimIndent()) shouldBe """
+            import org.spongepowered.asm.mixin.Shadow;
             @org.spongepowered.asm.mixin.Mixin(targets = "b.pkg.B")
             class MixinA { @Shadow private int bField; }
         """.trimIndent()
@@ -31,9 +35,11 @@ class TestMixinAnnotation {
     fun `remaps with inner class string target separated by dot`() {
         // FIXME should probably keep the dot?
         TestData.remap("""
+            import org.spongepowered.asm.mixin.Shadow;
             @org.spongepowered.asm.mixin.Mixin(targets = "a.pkg.A.Inner")
             class MixinA { @Shadow private int aField; }
         """.trimIndent()) shouldBe """
+            import org.spongepowered.asm.mixin.Shadow;
             @org.spongepowered.asm.mixin.Mixin(targets = "b.pkg.B${'$'}Inner")
             class MixinA { @Shadow private int bField; }
         """.trimIndent()
@@ -42,9 +48,11 @@ class TestMixinAnnotation {
     @Test
     fun `remaps with inner class string target separated by dollar`() {
         TestData.remap("""
+            import org.spongepowered.asm.mixin.Shadow;
             @org.spongepowered.asm.mixin.Mixin(targets = "a.pkg.A${'$'}Inner")
             class MixinA { @Shadow private int aField; }
         """.trimIndent()) shouldBe """
+            import org.spongepowered.asm.mixin.Shadow;
             @org.spongepowered.asm.mixin.Mixin(targets = "b.pkg.B${'$'}Inner")
             class MixinA { @Shadow private int bField; }
         """.trimIndent()
diff --git a/src/test/kotlin/com/replaymod/gradle/remap/mapper/mixin/TestMixinInjections.kt b/src/test/kotlin/com/replaymod/gradle/remap/mapper/mixin/TestMixinInjections.kt
index da68496..65fbc69 100644
--- a/src/test/kotlin/com/replaymod/gradle/remap/mapper/mixin/TestMixinInjections.kt
+++ b/src/test/kotlin/com/replaymod/gradle/remap/mapper/mixin/TestMixinInjections.kt
@@ -181,16 +181,14 @@ class TestMixinInjections {
     @Test
     fun `remaps @At target`() {
         TestData.remap("""
-            import org.spongepowered.asm.mixin.injection.At;
-            import org.spongepowered.asm.mixin.injection.Inject;
+            import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject;
             @org.spongepowered.asm.mixin.Mixin(a.pkg.A.class)
             class MixinA {
                 @Inject(method = "aMethod", at = @At(target = "La/pkg/A;aInterfaceMethod()V"))
                 private void test() {}
             }
         """.trimIndent()) shouldBe """
-            import org.spongepowered.asm.mixin.injection.At;
-            import org.spongepowered.asm.mixin.injection.Inject;
+            import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject;
             @org.spongepowered.asm.mixin.Mixin(b.pkg.B.class)
             class MixinA {
                 @Inject(method = "bMethod", at = @At(target = "Lb/pkg/B;bInterfaceMethod()V"))
@@ -202,16 +200,14 @@ class TestMixinInjections {
     @Test
     fun `remaps @At target without mappings for target`() {
         TestData.remap("""
-            import org.spongepowered.asm.mixin.injection.At;
-            import org.spongepowered.asm.mixin.injection.Inject;
+            import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject;
             @org.spongepowered.asm.mixin.Mixin(a.pkg.A.class)
             class MixinA {
                 @Inject(method = "aMethod", at = @At(target = "La/pkg/A;unmappedOverloaded(La/pkg/A;)V"))
                 private void test() {}
             }
         """.trimIndent()) shouldBe """
-            import org.spongepowered.asm.mixin.injection.At;
-            import org.spongepowered.asm.mixin.injection.Inject;
+            import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject;
             @org.spongepowered.asm.mixin.Mixin(b.pkg.B.class)
             class MixinA {
                 @Inject(method = "bMethod", at = @At(target = "Lb/pkg/B;unmappedOverloaded(Lb/pkg/B;)V"))
@@ -223,8 +219,7 @@ class TestMixinInjections {
     @Test
     fun `remaps @At target in constant`() {
         TestData.remap("""
-            import org.spongepowered.asm.mixin.injection.At;
-            import org.spongepowered.asm.mixin.injection.Inject;
+            import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject;
             @org.spongepowered.asm.mixin.Mixin(a.pkg.A.class)
             class MixinA {
                 private static final String TARGET = "La/pkg/A;aInterfaceMethod()V";
@@ -234,8 +229,7 @@ class TestMixinInjections {
                 private void test2() {}
             }
         """.trimIndent()) shouldBe """
-            import org.spongepowered.asm.mixin.injection.At;
-            import org.spongepowered.asm.mixin.injection.Inject;
+            import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject;
             @org.spongepowered.asm.mixin.Mixin(b.pkg.B.class)
             class MixinA {
                 private static final String TARGET = "Lb/pkg/B;bInterfaceMethod()V";
diff --git a/src/test/kotlin/com/replaymod/gradle/remap/util/TestData.kt b/src/test/kotlin/com/replaymod/gradle/remap/util/TestData.kt
index c5b8851..d1ed905 100644
--- a/src/test/kotlin/com/replaymod/gradle/remap/util/TestData.kt
+++ b/src/test/kotlin/com/replaymod/gradle/remap/util/TestData.kt
@@ -48,14 +48,21 @@ object TestData {
             findClasspathEntry("BMarkerKt"),
         )
         patternAnnotation = "remap.Pattern"
+        manageImports = true
     }
 
-    fun remap(content: String, patternsBefore: String = "", patternsAfter: String = ""): String = transformer.remap(mapOf(
-        "test.java" to content,
+    fun remap(content: String): String =
+        remap("test.java", content)
+    fun remap(fileName: String, content: String): String =
+        remap(fileName, content, "", "")
+    fun remap(content: String, patternsBefore: String, patternsAfter: String): String =
+        remap("test.java", content, patternsBefore, patternsAfter)
+    fun remap(fileName: String, content: String, patternsBefore: String, patternsAfter: String): String = transformer.remap(mapOf(
+        fileName to content,
         "pattern.java" to "class Patterns {\n$patternsBefore\n}",
     ), mapOf(
         "pattern.java" to "class Patterns {\n$patternsAfter\n}",
-    ))["test.java"]!!.first
+    ))[fileName]!!.first
     fun remapWithErrors(content: String) = transformer.remap(mapOf("test.java" to content))["test.java"]!!
 
     fun remapKt(content: String): String = transformer.remap(mapOf("test.kt" to content))["test.kt"]!!.first