Validation for parameter name conflicts in source.

[alcatrazEscapee]

Motivation

• Parchment's goal is to produce valid, usable mappings to all consumers.
• Forge, one of the major targets for Parchment's mappings applies source patches to the decompiled Minecraft source code. This requires that the decompile is able to recompile without issue or error.
• Notably, neither Fabric or Sponge make this requirement (and as such, are not affected by the issues I'm about to list)

Issues

There are a couple different types of possible parameter naming issues that could arise. This is by no means comprehensive, but I will try and motivate the reasoning behind different types of issues.

1. Local variable name conflicts

A parameter name and local variable name (as named by the decompiler) match:

void foo(Bar bar /* paramA */ ) {
    Bar bar = ...;
}

Solution:

In order to solve this, one first needs to make assumptions about the possible set of local variable names that the decompiler spits out. For example, in Forge, using ForgeFlower, local variable names will never contain a capital letter. Then, all parameter names must be made in such a way that it could not possibly be a local variable name.

2. Lambda parameter name conflicts

A parameter name in a method, conflicts with another parameter name from a different lambda method.

void foo(Bar bar /* paramA */ ) {
    Consumer<Bar> = bar /* paramB */ => {
        ...
    };
}

Solution:

In order to completely avoid this, one needs to know where lambda methods are used, in order to know what possible parameter names can conflict with each other.

3. Anonymous classes causing method parameter conflicts

Parameter names from two unrelated methods conflict, due to the instantiation of an anonymous class:

class classA {
    void foo(Bar bar /* paramA */ ) {
        new ClassB() {
            @Override void baz(Bar bar /* paramB */ ) {
                ...
            }
        };
    }
}

Solution:

In a similar manner to problem 2., one needs to know what methods could possibly conflict with each other, from methods present in anonymous classes.

Notably: In 1.17's MCPConfig format, the presence of parameter names in overridden methods not requiring their parameters to be identical to the super method makes this issue a lot easier to not appear accidentally (i.e. if someone's mapping super class parameters and doesn't notice a conflict that would be created in a subclass.

4. Anonymous class parent field conflicts.

This is a bit of a unique case:

abstract class Parent {
    String foo;

    Parent(String foo) { this.foo = foo; }
    String getFoo();
}

// Somewhere else
Parent create(String pFoo) {
    return new Parent("from constructor") {
        @Override
        String getFoo()
        {
            return pFoo;
        }
    };
}

// We want this to return "from parameter"
// If `pFoo` is named `foo`, it won't.
create("from parameter").getFoo()

This is a unique case. If the pFoo parameter is named foo, then it will change the semantics of this method, as java will prefer using the field name instead of the parameter. But, if the parameter was intended to be referenced, then there is no way to get it to reference that over the field.

The root issue is that local variables from outer scopes, are lower priority than fields in inner scopes, which causes this issue. This is currently unsolved in Parchment.

---

Final Musings

With the current iteration of the mappings standards for Parchment, we solve issue 1. by enforcing a p prefix followed by an uppercase letter (i.e. the p[A-Z][A-Za-z0-9]* regex). We made the decision that, in the absence of perfect tooling to handle 2. and 3., we would have to be vigilant in reviewing parameter names to not introduce name conflicts of this kind.

In my own project, which generates automatic names for parameters, in 1.16, I ran into all of the above issues. I eventually solved 1. and 2., and partially solved 3. to a level that was manageable to fix by hand. This involved basically, ensuring that:

• No parameter in a lambda method, or an anonymous class, could be the same as any other parameter in said class.
• As a result, parameters in some classes ended up being named level1_, level2_, level3_ etc. due to overzealous conflict avoidance.

This issue is the result of internal discussions in the ParchmentMC discord here, and here