Record support

[modmuss50]

Title, javadoc should be pulled from the field mappings similar to: FabricMC/fabric-loom#366

@liach not sure if this is something you fancy picking up, if not I'm happy to handle it.

[liach]

So just a note for me:
The docs for record component is kept on the field in tiny format, than the docs on method.

square/javapoet#829 would be a dependency too.

[modmuss50]

Worse case if javapoet doesnt support it (What I imagine will happen now) is it will just generate the record as a normal class with the methods and fields.

The fields would still have the docs on it so its not a cricial issue.

[liach]

Hmm, I actually need to check how javadoc tool generates docs for regular records. I've never actually written records

[liach]

Now with minecraft moving to 16, this has become an importance naturally.

[liach]

Created square/javapoet#840 which would add record support in javapoet. If mojang does add records in the next snapshot, you can consider hosting fabric's javapoet builds with record support on maven so we can properly deal with records.

[modmuss50]

Awesome 👏

Yes I can publish that java poet fork to our maven while the PR is being reviewed.

[liach]

Current status: https://github.com/FabricMC/yarn/runs/3625538513#step:6:197

So mojang has some stripped records. Also seems the generics on the record is stripped in cfr output, need to check status in class file (example: ahf = class_6497)

[liach]

@modmuss50 Turns out proguard seems to strip the signature on record class (but not parameters), so things like the min-max range (class_6497, ahf) does not appear generic in its header in the javap output, while the util.Lazy (ahg) appears generic like before.

Your motivation to ask mojang to keep record attribute is a good move 👍👍, since their stripping now removes some other (somewhat) helpful information for decompilers (and probably cannot be restored faithfully even with their obf map), besides changing the behavior of Class.isRecord() results before and after stripping.

For now, I guess we need to check which classes need their type parameters in signatures fixed before we can move on.

[liach]

A list:

• class_6561: <T> for the palette in PalettedContainer
• class_6497: <T extends Comparable<? super T>> (seems they used <T extends Comparable<T>>, which is more restrictive)
• class_6535: <T extends class_6534>
• class_2769$class_4933 aka net/minecraft/state/property/Property$Value: <T extends Comparable<T>>, required by Property<T>'s T

This seems to be the complete list of generic ones (determined by the type of the components and static factory methods). Should we have these classes' signatures patched? I think we can add this patch in the same place where we fix the record attribute.

[modmuss50]

Awesome thanks for looking into this. It seems like I will need to fork + publish our own javapoet with your record support, would be nice to get it into upstream though.

Im almost tempted to wait for the next snapshot to see if this improves the situation before undergoing a massive effort to get this data to yarn and the dev env, I imagine this is something that other mod loaders would like to have as well.

[liach]

Yeah, but having a javapoet published (or alternatively, build a jar and use it in this repo) is required no matter the records are more or less accessible, as otherwise it is a bit painstaking to generate record java files for javadoc tool no matter how they look in class files.

[liach]

@modmuss50 Would you mind creating a repo in fabricmc for our fork of javapoet and publish to maven from there (like you did for cfr)? I can pr the record stuff there and potentially add sealed class support when mojang officially migrates to Java 17.

[liach]

Since it passes my local tests after both the signature fix and the library bump + singular palette mapping prs are merged, considering this fixed.

[modmuss50]

Sorry for the delay, I have forked: https://github.com/FabricMC/javapoet

Feel free to PR your record support to this, I will get gradle and publishing setup soon.