sfPlayer1 / Matcher

Tool for tracking elements in obfuscated Java archives across releases
GNU General Public License v3.0
75 stars 41 forks source link

Fix inner classes failing to format #15

Closed Chocohead closed 1 year ago

Chocohead commented 4 years ago

The situation where this happens arises when CFR decompiles an inner class which captures some locals as well as calling a super constructor. Right now this results in JavaParser noticing the class is invalid and failing to format:

parse error: matcher.srcprocess.SrcDecorator$SrcParseException: Parsing failed
    at matcher.srcprocess.SrcDecorator.decorate(SrcDecorator.java:119)
    at matcher.gui.tab.SourcecodeTab.lambda$1(SourcecodeTab.java:98)
    at matcher.gui.Gui$1.call(Gui.java:249)
    at javafx.concurrent.Task$TaskCallable.call(Task.java:1423)
    at java.util.concurrent.FutureTask.run(Unknown Source)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
    at java.lang.Thread.run(Unknown Source)
Caused by: com.github.javaparser.ParseProblemException: (line 11,col 33) Parse error. Found "super", expected "}"
Problem stacktrace : 
  com.github.javaparser.GeneratedJavaParser.generateParseException(GeneratedJavaParser.java:10315)
  com.github.javaparser.GeneratedJavaParser.jj_consume_token(GeneratedJavaParser.java:10174)
  com.github.javaparser.GeneratedJavaParser.ConstructorDeclaration(GeneratedJavaParser.java:1537)
  com.github.javaparser.GeneratedJavaParser.ClassOrInterfaceBodyDeclaration(GeneratedJavaParser.java:922)
  com.github.javaparser.GeneratedJavaParser.ClassOrInterfaceBody(GeneratedJavaParser.java:845)
  com.github.javaparser.GeneratedJavaParser.ClassOrInterfaceDeclaration(GeneratedJavaParser.java:407)
  com.github.javaparser.GeneratedJavaParser.CompilationUnit(GeneratedJavaParser.java:153)
  com.github.javaparser.JavaParser.parse(JavaParser.java:135)
  com.github.javaparser.JavaParser.simplifiedParse(JavaParser.java:334)
  com.github.javaparser.JavaParser.parse(JavaParser.java:306)
  matcher.srcprocess.SrcDecorator.decorate(SrcDecorator.java:72)
  matcher.gui.tab.SourcecodeTab.lambda$1(SourcecodeTab.java:98)
  matcher.gui.Gui$1.call(Gui.java:249)
  javafx.concurrent.Task$TaskCallable.call(Task.java:1423)
  java.util.concurrent.FutureTask.run(Unknown Source)
  java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
  java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
  java.lang.Thread.run(Unknown Source)

    at com.github.javaparser.JavaParser.simplifiedParse(JavaParser.java:338)
    at com.github.javaparser.JavaParser.parse(JavaParser.java:306)
    at matcher.srcprocess.SrcDecorator.decorate(SrcDecorator.java:72)
    ... 7 more
decompiled source:
/*
 * Decompiled with CFR 0.149.
 */
import net.minecraft.server.MinecraftServer;

public final class bq
extends Thread {
    final /* synthetic */ MinecraftServer a;

    public bq(String string, MinecraftServer minecraftServer) {
        this.a = minecraftServer;
        super(string);
    }

    public void run() {
        this.a.run();
    }
}

Far from ideal as then jumping to class members doesn't work. This fix aims to shift the super call up to where it is valid for the sake of JavaParser. It comments out the original position rather than completely removing it just to give an idea what changed:

/*
 * Decompiled with CFR 0.149.
 */
import net.minecraft.server.MinecraftServer;

public final class bq extends Thread {
    public bq(String string, MinecraftServer minecraftServer) {
        //// Matcher moved

        super(string);
        this.a = minecraftServer;
    /* super(string); */
    }

    public void run() {
        this.a.run();
    }

    final MinecraftServer /* synthetic */
    a;
}

Think the comments throw the formatting off a little, the actual reparsed source looks more like:

public final class bq extends Thread {
    final MinecraftServer /* synthetic */ a;

    public bq(String string, MinecraftServer minecraftServer) {
super(string); // Matcher moved
        this.a = minecraftServer;
/* super(string); */
    }

    public void run() {
        this.a.run();
    }
}

Little bit of a finger cross fix approach to get it, but I don't see a quicker solution with what the exception provides, and it does allow jumping to the different parts of the class properly.

NebelNidas commented 1 year ago

Can this PR be merged? It's still relevant

liach commented 1 year ago

Alternatively we can wait for https://openjdk.org/jeps/8300786 to come true :trollface:

sfPlayer1 commented 1 year ago

merged via #26