Always emit fully qualified code from TemplateProcessor

[knutwannheden]

TemplateProcessor now always emits fully qualified type, field and method references regardless of what the input Java code looks like. The idea being that then a ShortenFullyQualifiedTypeReferences will remove unnecessary full qualifications.

[knutwannheden]

@timtebeek Whether we actually want this needs to be discussed. But this is in any case how I had imagined this to be working. Note that we no longer generate any JavaTemplate.Builder#imports() calls.

I also haven't verified if this actually works correctly and whether the fully qualified references get shortened as expected.

[timtebeek]

I like how you've pulled out TemplateCode; should help if we are to aim for reuse. Could it make sense to also pull in templateCode.replace("\\", "\\\\").replace("\"", "\\\"") and creation of JavaTemplate.builder + classpath into a static method there? Then we're very near being able to use this directly in RefasterTemplateProcessor too I believe (haven't checked).

Nice improvement not to have to use imports on templates, although I've not yet looked at the effect on the replacements we end up doing. I have a slight concern for cases like Mockito.times(1) which are most commonly static imports, and not sure we handle those now. That was why I'd aimed for static imports in #53 originally. I've since spun out #56 to make both easier to review (or only partially merge).

[knutwannheden]

What worries me about templates that rely on imports (and static imports) is that we don't have anything to deal with conflict resolution.

At the moment it would indeed be better to make the code generating the full JavaTemplate.builder() call chain reusable and in a first step only use the fully qualified name generation for Refaster, as we only there have the shortening of qualified references integrated.

I can amend this PR with that.

[knutwannheden]

@timtebeek I've refactored it to the point now, where a full JavaTemplate.Builder expression gets produced (assuming certain imports) and it could be directly embedded into the Refaster processor. For this we may want to introduce an option to specify whether fully qualified names should be produced or not (for Refaster we would want this).

[timtebeek]

Looks good to me; thanks for doing this!

[knutwannheden]

The TemplateProcessor should hopefully still work the same way as before. In a next step we could now try inlining the JavaTemplate into the class emitted by the RefasterTemplateProcessor and there only generate fully qualified names.

[timtebeek]

Sounds good to me, although such a change in RefasterTemplateProcessor would also affect the TemplateProcessor right? As that currently operates on

rewrite-templating/src/main/java/org/openrewrite/java/template/processor/TemplateProcessor.java

Line 76 in 7516697

if (("expression".equals(name) || "statement".equals(name)) && tree.getArguments().size() == 3) {

[knutwannheden]

I don't think it would affect TemplateProcessor, but I am not entirely sure either. Although, there might be some more of the code in TemplateProcessor that we might want to reuse.

[knutwannheden]

I can try to create a PR and then you can maybe tell me how I can easily test that against the error-prone-support code to make sure it still works correctly.

[timtebeek]

Sure; the flow isn't very advanced, as we use Gradle whereas they use Maven, so what I ended up with is:

1. ./gradlew pTML on rewrite-templating
2. Checkout my PR at error-prone-support
3. Change the version rewrite-templating version number to 1.4.0-SNAPSHOT
4. Run mvnd -U clean verify -Dverification.warn -DskipTests -e
5. Inspect error-prone-contrib/target/generated-sources/annotations/tech/picnic/errorprone/refasterrules

All in all that takes ~2 minutes or so; not too bad.