Conform finalness of overwrites with target, and provide more fine-grained control of overwrite access flags

[Niko-sk2x]

For example if I have the following mixin:

@Mixin(Biome.class)
public abstract class MixinBiomeTemperatureConfig {
    @Overwrite
    public final float getTemperature(BlockPos pos) {
        // code here
    }
}

And another mod has an access transformer that makes this method non-final, overriding this method will fail in that mod. This currently causes https://github.com/micdoodle8/Galacticraft/issues/3764

Special casing forge access transformers won't fix the above issue, because galacticraft is using a much older way of using access transformers, before forge searched for them automatically in META-INF.

[Mumfrey]

Yeah this is not a new problem, I think the specific issue is with final here, since the visibility issue has been trodden and re-trodden. The main discussion for overwrites wrt visibility was on #189 which was itself a result of sanity check behaviour added to fix #167. In some ways this issue is orthogonal to #293 in that it's a different aspect of how the contract of final can be somewhat opaque for overwrites and cause unexpected behaviour.

Personally I'm tempted to tweak the logic here to preserve the finalness of the overwritten method, but much like my more general concerns with how Access Transformers fundamentally break class contracts - especially with regard to virtual method call semantics - I feel like providing a tool to mixin author to declare when they don't want automatic conformance is fair. Since if program correctness depends on the method being final, having it unexpectedly un-finalled should be a detectable event.

This would also allow for other visibility changes to be selectively inhibited when necessary.

It would be nice to end up with something like

    @Overwrite
    @PreserveAttributes(MethodAttributes.FINAL)
    public final float getTemperature(BlockPos pos) {
        // code here
    }

which would selectively inhibit the automatic convergence and instead raise an error condition to indicate that the change to the target is incompatible with the method contract. Another option would be to overload the @Final annotation, but it already has overloaded responsibilities as it is and I'm not particularly keen on pushing it further. Besides, using an @PreserveAttributes or similar allows things like final to be enforced in both states rather than just one.

[radfast]

Thank you both, this type of principled approach is very much appreciated.

My own view is that none of the methods in Minecraft code as modded by Forge should be final. The only reason for making a method final is to prevent modification, which just makes life harder for modders.