Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
sciwhiz12 opened this issue Apr 10, 2024 · 1 comment
Open

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

sciwhiz12 opened this issue Apr 10, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@sciwhiz12
Copy link
Member

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 sciwhiz12 added the bug Something isn't working label Apr 10, 2024
@sciwhiz12
Copy link
Member Author

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.

sciwhiz12 added a commit that referenced this issue Apr 10, 2024
A simple getter is a getter which contains only "return <field>". The
heuristic for detecting field getters can't detect anything more complex
than that.

This means that non-simple getters for record fields aren't detected,
causing failure later on due to the assumption that a record component
always has a getter.

To fix that, add a simple heuristic to detect getters for record
components, by finding one method that has no parameters and returns the
same type as the record component. It only considers the parameters and
return type (e.g., the method descriptor) as the metadata extraction
works on obfuscated names, and the obf name of the record component may
be different from the obf name of the corresponding getter.

Also add a check to throw an exception when a getter could not be found
for a record component, to easily detect these problems in the future.

Partially addresses #8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant