Potential fix for var keyword breaking linemappings with Loom

[Draylar]

Obligatory disclaimer: I only looked at Mercury & Loom for the first time this week, and as such I am not confident that this change does not break other features. Thanks for your patience in review!

---

Version Details

Loom: 1.7-SNAPSHOT
Mercury: 0.4.1
Minecraft: 1.20.1
Yarn: 1.20.1+build.10
Loader: 0.15.10

Issue Explanation

We have a class in a project that always has linemapping issues - you can consistently "lose" breakpoints in IntelliJ 100% of the time by placing a breakpoint in any method within this class. The breakpoint is always consistently offset by a constant amount. I went on a goose chase yesterday to figure out why this was happening, and traced it to down to a quirk in Mercury with the var keyword.

AFAIK: When Loom publishes or imports(?) projects, the remapSourcesJar task is used to map the project to Intermediary (and then re-map back to Yarn when you pull the library back into your project). This task uses Mercury under the hood to swap out identifiers based on the mappings set. I notice after some poking around that the import statements from the source code and output sources (that go through Mercury once or twice) do not match. There are 2 reproducible issues here:

1. if you use var with a primitive type, Mercury will attempt to import the primitive type when producing remapped jar files. You can reproduce this easily by creating a new Fabric project and running the remapSourcesJar task with code like this:

public void foo() {
    var test = 5.0;
    System.out.println(test);
}

If you check the output sources jar, you will find an additional import added to your class. This import will appear (and show as an error) in any projects that include your library as a dependency. Additionally, this import will shift all line-mapping down by 1, meaning you can no longer breakpoint anywhere in the class.

import D;

2. if you use var with a type that needs an import, and do not explicitly import the type, Mercury will inject the type in output sources jar. Every import added this way will shift your linemappings down by 1:

(Left = fixed patch, Right = the same class, but every additional import was added by Mercury due to the presence of var keyword. Both of these library (non-vanilla) classes were piped through remapSourcesJar. The fixed patch is a 1:1 mirror of the original source code for the project.)

Issue TL;DR

1. We have a 1,000 LOC class (from a private Fabric/Yarn library) that fails to breakpoint 100% of the time in any method
2. Using var in this class breaks linemapping for the entire class (and thus all breakpoints)
3. Writing var foo = 5.0 (for any primitive type) will result in an import statement being added for the primitive (eg. import D;) when running remapSourcesJar, which will offset linemappings once for every primitive type used in the class.
4. Writing var foo = MethodThatReturnsType.get() without explicitly importing the type will result in an extra import being added by Mercury, which will also shift linemaps down by 1 (can no longer debug the class)
5. Following step 4 while explicitly importing the type (even if you can compile without the import) will result in the issue being fixed, because the source and output classes have the same number of imports

Issue Fix

This issue is fixed by moving the addImport call within the isVar check; it prevents imports from being forcibly added when the var keyword is used. As far as I can tell from testing, this has not caused any additional issues (and fixes the issues described above - our problematic 1,000 LOC class with 15 var implicitly imported types can now perfectly breakpoint at any method).

[modmuss50]

Hi, many thanks for looking into this its been an issue for a while. Im not familar with this code so im unable to say if its the correct fix or not but it looks good to me. I will merge this and test in loom.