Inline Javatemplate in RefasterTemplateProcessor #57
Conversation
|
@knutwannheden As expected
|
| final JavaTemplate before = Semantics.expression(this, "before", (String[] strings) -> String.join(", ", strings)).build(); | ||
| final JavaTemplate after = Semantics.expression(this, "after", (String[] strings) -> String.join(":", strings)).build(); | ||
| final JavaTemplate before = JavaTemplate | ||
| .builder("String.join(\", \", #{strings:any(java.lang.String[])})") |
There was a problem hiding this comment.
Surprised the String reference isn't qualified here. Will look into that separately.
There was a problem hiding this comment.
I see, we have an explicit exception for java.lang:
It does make the template code a bit more readable, but it could cause problems when the Java class an after template gets embedded into is in a package which has a class by the same name, which is unusual but not unheard of.
I am a bit on the fence on this one. java.lang is a special case also in the ShortenFullyQualifiedTypeReferences recipe (for basically the same reason). But I think in probably around 90% of the cases the name would end up getting shortened, because the compilation unit already has another (unqualified) reference to the same type in which case the recipe knows it can shorten the reference.
As I pointed out before, this isn't directly related to your PR. It is just something that caught my eye.
There was a problem hiding this comment.
Yes we can tackle this separately; there's a few other items too I'd been looking to do after this lands, since any changes previously were likely to conflict with this one.
| .build(); | ||
| final JavaTemplate after = JavaTemplate | ||
| .builder("String.join(\":\", #{strings:any(java.lang.String[])})") | ||
| .build(); |
There was a problem hiding this comment.
The only downside I see with this refactoring is that the generated code is now harder to read and that the Semantics mechanism isn't visible anymore. But I think that is acceptable.
There was a problem hiding this comment.
Yes the main motivator for me was PRs like this one that aim to use Java 9+ constructs in recipes, which previously meant that code was in the generated recipes as well(likely?) bumping the Java version required to run the recipe.
With this change I think we should be good to have the generated recipes included with the other Java 8 compatible classes.
WIP to replace directly create JavaTemplate in RefasterTemplateProcessor instead of indirection.