Recaf icon indicating copy to clipboard operation
Recaf copied to clipboard

fix: parser removing more than generics

Open Jydett opened this issue 1 year ago • 5 comments

Hello, firstly, thank you for this great tool !

I encounter an issue the other day similar to [this one](https://github.com/Col-E/Recaf/issues/584) where the comparison in this pseudo-class would be matched as a generic declaration and removed before being parsed as java code, thus resulting in java parsing errors.

decompiled code

class Clazz {
   public boolean test(int x) {
      int a = 0, b = 0;
      if (a < x && x > b) {
          return false;
      }
      return true;
   }
}

after generic type erasing

class Clazz {
   public boolean test(int x) {
      int a = 0, b = 0;
      if (a            b) {
          return false;
      }
      return true;
   }
}

I tried to fix the regular expression, but it would become massive, hard to understand and inefficient, so I rewrote the function as a simple syntax parser and added some tests.

Jydett avatar Nov 30 '23 08:11 Jydett

I'll switch this PR to draft as I discovered some a new edge case that will be hard to fix:

    public static void main(String[] args) {
        int a = 0, b = 0, c = 0, d = 0;
        boolean x = a < b & c > d;
    }

Jydett avatar Dec 01 '23 08:12 Jydett

@Jydett Hi, do you think this PR can solve the edge case you mentioned? #748

meiMingle avatar Dec 10 '23 15:12 meiMingle

@meiMingle Hi, i don't think so. If you want to check you can copy the test class to your PR and run it to validate its behaviour

Jydett avatar Dec 10 '23 16:12 Jydett

@Jydett I just used my newly created regular expression to run your test class and the only thing that failed was the following method, <T extends EntityLivingBase<X> | Foo> are you sure this syntax is correct? What environment (JDK version/language level) does it come from?

@Test
void removeGenericMethodDeclaration() {
String code = "<T extends EntityLivingBase<X> | Foo> foo() {}";
var res = filterGeneric(code);
assertEquals(" foo() {}", res);
}
```assertEquals(" foo() {}", res);
}

meiMingle avatar Dec 11 '23 10:12 meiMingle

@meiMingle You're right the correct syntax is <T extends EntityLivingBase<X> & Foo>

Jydett avatar Dec 11 '23 10:12 Jydett

Closing out as we're focusing on 4.x now, and its not using JavaParser as a backend.

Col-E avatar Jun 04 '24 09:06 Col-E