Recaf icon indicating copy to clipboard operation
Recaf copied to clipboard

Optimize `filterGenerics()`

Open meiMingle opened this issue 2 years ago • 4 comments

What's new

What's fixed

For the optimization of code like the following, code processed by the old filterGenerics method will cause it to be damaged, causing JavaParser to not parse correctly.

   static <T> Collection<T> a(Iterable<T> var0) {
      return (Collection<T>)var0;
   }

Then

   static                                 var0) {
      return (Collection   )var0;
   }

Now

   static  Collection  a(Iterable  var0) {
      return (Collection )var0;
   }

meiMingle avatar Dec 06 '23 09:12 meiMingle

<(,?(((\?\s+(extends|super)\s+)?[^<>+\-*/%=&|!~^:(){}?'"]*)))+>|<\s*\?\s*>

@Col-E Hello, I created a new regular expression that should work for most situations so far. But maybe the expression is a little complicated, which will cause OOM when using the classes provided by jregex. This problem does not occur when using java.util.regex. I'm wondering if I should replace the jregex library. Should it be replaced by java.util.regex or RegExodus like the one used in 4x? What are the advantages of RegExodus?

meiMingle avatar Dec 11 '23 08:12 meiMingle

@Col-E Currently I only run the regular expressions here using java.util.regex to avoid large-scale changes and avoid OOM caused by Jregex. I think it's probably a relatively good compromise.

meiMingle avatar Dec 11 '23 10:12 meiMingle

@meiMingle I've added this test that fails with this PR:

	@Test
	void dontRemoveGenericInString() {
		String code = "String x = \"Foo<X>\";";
		var res = filterGeneric(code);
		assertEquals(code, res);
	}

The expression is here in a string and should not be filtered out; but it is. Do you think you can fix it ?

Jydett avatar Dec 11 '23 10:12 Jydett

@Jydett You are right, I thought about it for a while but didn't find a reliable way to handle this situation.

meiMingle avatar Dec 12 '23 03:12 meiMingle

I realize this may not be a good solution

meiMingle avatar Mar 29 '24 13:03 meiMingle