"Goto definition" in decompiler mode may not match bytecode reference due to name shadowing

[MizzMaster]

While I was working on an obfuscated jar, I found a bug that causes you to find yourself in a random class that is not even relevant to what you would expect. Let me introduce a few images to explain.

I was on a pretty clean class and I though there was no problem to use "Goto definiton" option.

I used it and... I was here... I was expecting a Packet class but I found a Predicate, my mind just blowed first time I faced this bug

Fortunately, I found a way to find the actual class after I let go of my persistent thoughts about a "clean class". I just opened it in the assembler. It was the actual class. But it was in the default package. This is where the problem starts... As Java do not allow importing classes in default package from a non-default package class and the other way around (since Java 1.5), we can't import like that. So, CFR couldn't put the import statement of that class. But Recaf somehow recognizes this class as a same-package class I guess (because there is no import statement for that) and redirects me to the same-packaged class. (Both Predicate class named j1 and it's initializer class are in the net.minecraft package)

[Col-E]

Looks like the JavaParser resolve logic is failing in this case. I'll have to look and see why its resolving that way. Can you provide a sample?

[MizzMaster]

https://workupload.com/file/h4wWFDDQuek

[Col-E]

Yeah, validated my prior assumption. Because the context is based on the decompiled code and not the underlying bytecode it sees that we're in X package and pulls the matching class name from the package we're in, despite it pointing to the class by the same name in the default package (Which can't be done at source level) at the bytecode level.

I'd have to incorporate some double layer (source + bytecode) context system to address this since the information needed to resolve this simply cannot be represented in decompiled code. There's no good way to resolve bytecode instruction correlation to decompiled source code in any of the decompilers AFAIK, so it'd be some work to figure that out and put an implementation of that in Recaf.

[MizzMaster]

There's no good way to resolve bytecode instruction correlation to decompiled source code in any of the decompilers AFAIK

There might be, but for now you may can prevent this bug by simply checking the bytecode instructions if there is another class exist with the same Simple Name. (Compare what Recaf tries to redirect and what bytecode includes)

I'm going to explain here what I'm trying to say. (cuz my english is bad) As in my example jar, when you right click the SampleClass.class or SampleClass.test(), before Recaf pops up the options, Recaf should check all the bytecode instructions (of UnCreativeClassName.randomMethod) that contains a class name (LDC, INVOKESTATIC, etc.). If there is another class with exact Simple Name (SampleClass) comparing to what Recaf found in the decompiler mode (some/random/pack/SampleClass), then Recaf should warn the user somehow.

(Goto Definition option might become strikethrough text and might show a text box while hovering over that says something like "Destination may not be true, check the bytecode in order to be sure")

[Col-E]

That's the double layer approach, but warning instead of resolving.