rewrite icon indicating copy to clipboard operation
rewrite copied to clipboard

Problem with inner Class named '$' in JavaTemplate

Open MBoegers opened this issue 2 years ago • 5 comments

What version of OpenRewrite are you using?

I am using current main (Commit: d53b36febdc9ba609ae64295bcaa54b9fe4169b8) rewrite-java 8.5.0-SNAPSHOT

How are you running OpenRewrite?

Initially via tests in PR https://github.com/openrewrite/rewrite-migrate-java/pull/285 Reproduced with test in rewrite-java directly.

What is the smallest, simplest way to reproduce the problem?

Reference an inner class with name $ inside a JavaTemplate

What did you expect to see?

JavaTemplate has to deal with $ as name of inner classes. They resutl into A$$as FQN when defined as A.$.

What did you see instead?

__P__.<error>

What is the full stack trace of any errors you encountered?

none

Are you interested in contributing a fix to OpenRewrite?

Yes, as it blocks my PR :D

MBoegers avatar Oct 16 '23 15:10 MBoegers

see https://github.com/openrewrite/rewrite/compare/main...MBoegers:rewrite:3623-%24-as-inner-class#diff-4c3061e863c60e5a5391eb3c2b843b66b4113096c6527d0c09029d3338484b9e for reproducer

MBoegers avatar Oct 16 '23 15:10 MBoegers

I may found the Spot, org.openrewrite.java.internal.template.Substitutions L155 replaces all $ which are used as delimiter

Same for JavaType#build Line 572

MBoegers avatar Jan 09 '24 14:01 MBoegers

Nice find! sorry this didn't get a lot of traction before; it's hard to get to everything! What do you want to do now? Do you need input on anything?

timtebeek avatar Jan 09 '24 20:01 timtebeek

First of all, thanks an no worries. Its rarely used and I mentioned working on it ;). I just documented my find as I try to understand the purpose of $ in OpenRewrite. It seems like $ is wider used in OpenRewrite than specified the JLS and that leads to sime wierd edge cases. Would it be a hight value target to comply OpenRewrites usage of $ with the JLS?

MBoegers avatar Jan 09 '24 20:01 MBoegers

I think I made some progress.

  • TypeUtils.toFullyQualifiedName seem to handle $ better (see TypeUtilsTest L 150 following)
  • All occurrences of String.replace("$", ".") in rewrite-java* now use TypeUtils.toFullyQualifiedName (namely Substitutions and RemoveUnusedImports) with green tests

I have a strange behavior in JavaTemplateSubstitutionTest.anyForInnerClass. If I run the test I get

class A {
   void foo() {
-      $ test = new $();
+      $ test = new A.$();
     }
 //...
}

If I change Line 125 to new A.$() which also should be correct, I get

java.lang.IllegalStateException: AST contains missing or invalid type information
NewClass->NamedVariable->VariableDeclarations->Block->MethodDeclaration->Block->ClassDeclaration->CompilationUnit
/*~~(NewClass type is missing or malformed)~~>*/new A.$()
	at org.openrewrite.java.Assertions.assertValidTypes(Assertions.java:92)
	at org.openrewrite.java.Assertions.validateTypes(Assertions.java:62)
	at org.openrewrite.java.Assertions$$Lambda$440/0x00000008001ecfa8.accept(Unknown Source)
	at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:466)
	at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:132)
	at org.openrewrite.java.JavaTemplateSubstitutionsTest.anyForInnerClass(JavaTemplateSubstitutionsTest.java:89)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

That makes me feel like I miss a thing here. Any suggestions?

MBoegers avatar Jan 09 '24 22:01 MBoegers