ParchmentMC / Lodestone

Gradle plugin which can generate metadata based on the releases of Minecraft.
GNU Lesser General Public License v2.1
0 stars 3 forks source link

Explicitly-defined non-simple record getters aren't detected #8

Open sciwhiz12 opened 6 months ago

sciwhiz12 commented 6 months ago

In record classes, record components have an associated field and getter (which the JLS calls the accessor method), defined with the same name as the component. Both the field and the getter are mandatory, and are defined implicitly.

However, while the field's implicit definition is fixed, the getter can be explicitly defined by the programmer. This explicit definition has no restrictions on its containing code; the getter can do anything a method does, without needing to return the value of the record component/field.

This poses a problem for Lodestone, which currently detects getters (for any field, not just records) with a bytecode-based heuristic: a getter is something that contains a return <field> statement and nothing more (what I term a "simple getter"). This heuristic fails to cover the case of records with explicitly-defined non-simple getters.

For example: in 24w11a, the record PotionContents (with obfuscated name cuc) contains a customEffects component with an obfuscated name of g, which is followed by the corresponding field. However, the corresponding getter has a different obfuscated name of e (return type of List<MobInstanceEffect or List<bpx>), and is non-simple: the getter transforms the list before returning it, thus not following the simple return <field> heuristic.

This causes no getters to be attached to the record field, which causes a crash later down the line when the record info converter assume that there is at least one getter associated with the field (and thus, associated with the record).


The fix is not simple, though. The metadata extraction is done on the obfuscated JAR file, which means there is no access to the Mojmap name. Without access to the Mojmap name, there is no fool-proof way to detect non-simple getters for a record field, since the obfuscated name of the getter can differ from the obfuscated name of the record component.

One possibility here is a heuristic that looks for an instance method that accepts no parameters and returns the same type as the record component (and ignoring the name), but that is not fool-proof since there might be multiple methods with no parameters and the same return type, differentiated only by name.

The best way to resolve this would be to access the Mojmap names during metadata extraction and use that to determine the real getter for the component, but that requires a substantial refactor of Lodestone.

sciwhiz12 commented 6 months ago

It seems the heuristic I outlined above works on 24w11a and onwards (up to 24w14a, which is the latest snapshot as of writing). I'll be applying that fix for now, but this issue will stay open until we definitively fix the problem with the substantial refactor I mentioned.