Improperly rewrites inside switch case statements

[j-baker]

Shadow Version

6.1.0

Gradle Version

6.8.0

Expected Behavior

If I am relocating "com.example.Foo" to "relocated.com.example.Foo" then:

switch (someString) {
    case "com.example.Foo": return true;
    default: return false;
}

should be rewritten to something effectively equivalent to either:

switch (someString) {
    case "relocated.com.example.Foo": return true;
    default: return false;
}

or leave the code block unchanged.

Actual Behavior

In real life, Java compiles such a switch-case to something equivalent to

byte branch = -1;
switch (someString.hashCode()) {
    case 718948661:
        if (someString.equals("com.example.Foo")) {
            branch = 0;
        }
        break;
}
switch (branch) {
    case 0: return true;
    default: return false;
}

and what the shadower will do is rewrite it to something roughly equivalent to

byte branch = -1;
switch (someString.hashCode()) {
    case 718948661:
        if (someString.equals("relocated.com.example.Foo")) {
            branch = 0;
        }
        break;
}
switch (branch) {
    case 0: return true;
    default: return false;
}

thus making the code unreachable because it hasn't rewritten the hashcode.

I believe that in such a case, the shadower should not attempt a rewrite of the string constant.

[j-baker]

In terms of specific cases, I'm trying to relocate this formatter: https://github.com/palantir/palantir-java-format/ and am finding that the shadow plugin incorrectly relocates the 'final' in this block

https://github.com/palantir/palantir-java-format/blob/develop/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java#L2256

to "myrelocatedpackage.final", which along with making the code unreachable also breaks the semantics.

[johnrengelman]

Ooof. I’m not sure how we’d tackle that at all.
If someone wants to go digging in the Maven Shade bug tracker and see if it exhibits the same problem, that would be wonderful.

[fcurts]

related: oracle/graal#1644

[fcurts]

This is a known limitation of ASM: https://gitlab.ow2.org/asm/asm/-/issues/317882

Handling this in the shadow plugin seems a good idea. String switches are compiled differently by javac and the Eclipse compiler. Thus, the implementation of this feature must depend on the implementation of the Java compiler(s), whereas ASM only depends on the JVM specification.
Closing this as "Won't Fix".

[johnrengelman]

Will need to find some way to detect this issue and throw a warning for users....or fail the step since it's invalid.
At least need to document this in Shadow's docs.

[mahmoudyusuf94]

I am facing a similar problem:
in my build.gradle I have the following:

relocate 'io.netty','thirdparty.io.netty'

I found string literals affected by this shading which caused some runtime errors with the netty dependency.

The following string literal is defined in the original dependency:
String staticLibName = "netty_tcnative"

In the shaded jar, this string literal is changed to:
String staticLibName = "thirdparty.netty_tcnative";

Is there any way I can prevent this behavior of changing the string literals?

[Goooler]

Skipping string relocations has been landed in #1401.