Feature/Issue: Compile-time checks to store metadata useful for runtime debugging of AT name conflicts

[lukebemish]

NeoForged has plans to move to Mojmaps at runtime; obviously, this offers countless advantages. However, there are some pitfalls that can arise. Most notably, this means that vanilla classes will have human-readable names at runtime. This increases the chance of a collision between an existing private vanilla method and a mod method added on a subclass. This can lead to one of two issues. Assuming that Minecraft has some class A with a private instance method someMethod, and that mod firstmod creates a subclass of A that happens to habe a method named someMethod (this does not override the super method normally, as the super method is private), and mod secondmod ATs someMethod to be public for unrelated reasons, if the descriptors of the two someMethods line up:

• the two methods have different behaviour, resulting in subtle bugs, possibly without logged information, where the "subclass" implementation (not meant to override the superclass) is used when the superclass is needed
• ... or, especially if the parameters have generic parameters, and those differ between the two, it all crashes with a ClassCastException with a log that implicates neither mod involved.

The only reliable way to prevent a mod from falling prey to this is to either not AT private instance methods (even on final classes, as someone else could AT the final away), which is unreasonable to enforce given that this is one of the most useful cases for ATs as it allows overriding private methods, or to not make instance methods that share names and descriptors with private vanilla methods - which is probably a good idea, though unreasonable to enforce as doing so is perfectly valid java normally.

This is likely a low priority issue - the number of situations where this pops up is likely very low. However, the chances that it will, eventually, pop up in production are not necessarily that low, given that some modders definitely copy and slightly modify private helper methods from vanilla classes, and given that you can find a reason to AT practically any method in Minecraft if you try hard enough. With this in mind, I propose the following solution, which works as a combined runtime/compile-time checker:

• at compile time, each class in the mod is checked for any instance method that shares a name and descriptor with a private instance method in a super class or interface that comes from a library that could be ATed by something unknown at runtime - these are methods which could potentially get unintentionally "promoted" to override the super method if an AT is involved.
• that list of methods is stored in a file in the mod jar, as the ATs which might be present are not known at compile time. A warning is also shown to the developer, encouraging them to rename the methods in question - while this would not be enforced in any way, the easiest way to avoid this issue ever happening is to not have methods with these sorts of name conflicts.
• at runtime, the lists are read from every mod jar. When a private instance method is ATed, it is compared to the lists to look for conflicts. If a conflict is present, the AT is not applied and a descrptive error is shown in the log, detailing both which method was ATed, which mod the AT came from, which method conflicted, and which mod the conflicting method came from.

This solution allows for easy diagnosis of this variety of issue at runtime, while placing no burdens on mod developers, whether they use ATs or not.

To clarify, as this was apparently not well communicated elsewhere: I do not believe that this issue is likely to be anywhere near common enough that it should delay the implementation of runtime MojMaps in NeoForge, or change what ATs are allowed to do. I simply believe that the potential severity of issues that could arise, and, more importantly, the difficulty in tracking down the issue-causing mods in question (likely to be much harder than diagnosing mixin or coremod issues, in my opinion), addressing it eventually is likely a good idea.

For an example reproducing this issue (using srg names, so unlikely to ever occur at runtime given the mangled nature of the method names), see this discussion on discord: https://discord.com/channels/313125603924639766/1136320550168436798/1152344220867248189. This was a simple, proof-of-concept example designed to crash - a naturally arising example may cause unexpected, undefined behaviour instead. Notably, the stack trace produced by the crash contained no information pointing towards either of the mods in question, and the most useful debugging information ("I clicked this thing before it crashed") pointed only to the mod with the conflicting method, not the mod with the AT.

[lukebemish]

To not leave information stuck on discord, example provided in the linked discord conversation involves the following class in a mod with no ATs:

public class CustomItem extends InstrumentItem {

    public CustomItem(Properties properties, TagKey<Instrument> tagKey) {
        super(properties, tagKey);
    }

    @Override
    public @NotNull ItemStack getDefaultInstance() {
        return create(this, BuiltInRegistries.INSTRUMENT.getHolderOrThrow(Instruments.CALL_GOAT_HORN));
    }

    // making this `getInstrument` (a human readable name) in 1.20.2, with mojmaps at runtime, would cause
    // the same issue - hence why this is a worry now when it was not so much in the past.
    protected Optional<ResourceLocation> m_220134_(ItemStack stack) {
        var tag = stack.getTag();
        if (tag != null && tag.contains("instrument", Tag.TAG_STRING)) {
            var instrument = tag.getString("instrument");
            return Optional.ofNullable(ResourceLocation.tryParse(instrument));
        } else {
            return Optional.empty();
        }
    }
}

(In mojang official mappings for 1.20.1, m_220134_ is getInstrument, a common enough name that a modder might reasonably use it for their own method that happens to have the same signature). The other mod does nothing at all related, but contains an AT to InstrumentItem.getInstrument, normally private, making it public. This crashes as soon as the superclass getInstrument is invoked, as it invokes the subclass version, which would not normally override but does due to the AT, which then further returns an Optional containing the wrong sort of thing - a ResourceLocation instead of a Holder - which leads to a class cast exception when an access to that Optional is attempted. Due to that access happening entirely in the super class, the stack trace never mentions the CustomItem class.