Remapping hangs

[marvin-bitterlich]

I posted this issues at the ForgeGradle repo MinecraftForge/ForgeGradle#385

And they found out that it is this part of the process that makes problems.

Please check out the linked issue for possible information and if you need anything else, just ask.

[marvin-bitterlich]

I can confirm that the problem lies in this class:

package com.minecolonies.colony.materials;

/**
 * Exception for material handling. If this exception is thrown then their is a problem with the Material System's counting.
 * Created: December 15, 2015
 *
 * @author Colton
 */
class MaterialException extends RuntimeException
{
    MaterialException(String message)
    {
        super(message);
    }
}

As removing it made the build pass with no problems.

[Kamesuta]

I found that this problem occurs when I define a constructor of a class defined in a location that can not be reached from outside.

// A.java
public class A {
    public void example() {
        // class in method
        class B {
            // constructor of class
            public B() {
                // define constructor
            }
        }
    }
}

[boq]

Found same issue in #16 (noticed it's duplicate after posting). Well, derp... at least I got some logs.

[marvin-roesch]

I've encountered this issue myself only recently and investigated it further by attaching a debugger to a running instance of a mod Gradle build. It appears to be in fact caused by local classes which aren't empty. I've only tried with fields, but like @Kamesuta pointed out, it also happens with constructors, so I assume it's whenever there's something holding a reference to the containing class. @Kostronor's initial example seems to indicate that this is indeed a bigger issue and not limited to local classes, I wasn't able to reproduce the issue with the original example in my own environment, however.

I've created a minimal test case for this issue in this repo: PaleoCrafter/local-class-s2s. All that is required is to build the project and then see the retromapReplacedMain task silently close with exit code 1 after some progress. It also includes the original example, but commenting out the local class will make the build run just fine.

My investigations ultimately have led me to this line and as pointed out by @boq, this is causing the silent exit due to the double pipe in the string. The reason for this double pipe is this block, which gets an empty class name from resolving the IVariableBinding. This either is an issue with the underlying AST library or S2S, I'm not certain. I have also found out that the class name already is empty earlier up the pipeline, the first time I've found is here, when the class name is to be put in the log file. This would result in a useless line in the log, which can be seen by manually flushing the writer in that method.

I'm pinging @LexManos here because I'm not entirely sure how to resolve this issue, but a fix would be greatly appreciated considering that S2S can't consume arbitrary Java programs otherwise. A temporary solution for the local class case I've found is to simply use a nested class in the method's containing class rather than a local one.

A final note which might be relevant: I've had FG happily building projects containing local classes in MC1.9 and earlier. Hence, some change in S2S since then must have resulted in this issue.

[boq]

In my case it's this code that causes it. Seems similar to your findings. //This method is unused, my stacktrace also points to SymbolRangeEmitter.emitFieldRange.

Looks like Eclipse lib for parsing was recently updated (to support lambda), so regression may be in there.

EDIT: probably not regression, according to javadoc for ITypeBinding.getQualifiedName:

Local types (including anonymous classes) and members of local types do not have a fully qualified name. For these types, and array types thereof, this method returns an empty string.

[LexManos]

This is a rather low priority to fix because honestly I don't care. and I don't want to hack into the AST parser more then I have to. If someone wants to take a crack at it making sure to not break the old systems that'd be great. There is also the idea to move away from eclipse to another AST parser, Spoon is a candidate except last time I looked it still had some issues.

However, when it comes to these inner classes, we have some hacks in place to properly name them for normal non classes. We most likely need to expand that out to this special case as well. But, like I said, extremely low priority.

[boq]

Pushed my fix attempt in #17.