Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#1354] Revert experimental GroovyTempate.compile code to fix template issue #1368

Merged
merged 3 commits into from
Jan 28, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 15 additions & 23 deletions framework/src/play/templates/GroovyTemplate.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
import java.io.File;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;

Expand All @@ -16,6 +18,7 @@
import org.codehaus.groovy.control.CompilationUnit.IGroovyClassOperation;
import org.codehaus.groovy.control.CompilerConfiguration;
import org.codehaus.groovy.control.MultipleCompilationErrorsException;
import org.codehaus.groovy.control.Phases;
import org.codehaus.groovy.control.SourceUnit;
import org.codehaus.groovy.control.messages.ExceptionMessage;
import org.codehaus.groovy.control.messages.Message;
Expand Down Expand Up @@ -139,34 +142,23 @@ public void compile() {
CompilationUnit compilationUnit = new CompilationUnit(compilerConfiguration);
compilationUnit.addSource(
new SourceUnit(name, compiledSource, compilerConfiguration, tClassLoader, compilationUnit.getErrorCollector()));

// The following approach to adding the phase operation replaces the original
// reflection based approach commented out lower down. This appears to be the
// canonical approach and possibly has only been made available in the v3.x
// stream but it differs in two ways from the reflection based approach and it's
// not clear if and what the impact is:
// 1. It does NOT guarantee an empty list of OUTPUT phases operations to begin with.
// 2. The new phase operation is added to the start and not the end.
// See https://github.com/apache/groovy/blob/GROOVY_3_0_6/src/main/java/org/codehaus/groovy/control/CompilationUnit.java#L349
compilationUnit.addPhaseOperation(new IGroovyClassOperation() {

Maximillian13 marked this conversation as resolved.
Show resolved Hide resolved
// Play needs to handle writing the generated Groovy class to the file system but the Groovy
// compilation unit by default adds it's own output phase operation to do this that cannot
// be replaced using the available public methods. Until Groovy provides this capability
// it's necessary to access the compilation unit directly using reflection to replace the
// default output operation with the Play Groovy class handler.
Field phasesF = compilationUnit.getClass().getDeclaredField("phaseOperations");
phasesF.setAccessible(true);
Collection[] phases = (Collection[]) phasesF.get(compilationUnit);
LinkedList<IGroovyClassOperation> output = new LinkedList<>();
phases[Phases.OUTPUT] = output;
output.add(new IGroovyClassOperation() {
@Override
public void call(GroovyClass gclass) {
groovyClassesForThisTemplate.add(gclass);
}
});

// TOOD: Remove once the above replacement logic has been confirmed.
// Field phasesF = compilationUnit.getClass().getDeclaredField("phaseOperations");
// phasesF.setAccessible(true);
// Collection[] phases = (Collection[]) phasesF.get(compilationUnit);
// LinkedList<IGroovyClassOperation> output = new LinkedList<>();
// phases[Phases.OUTPUT] = output;
// output.add(new IGroovyClassOperation() {
// @Override
// public void call(GroovyClass gclass) {
// groovyClassesForThisTemplate.add(gclass);
// }
// });

compilationUnit.compile();
// ouf
Expand Down