Fix for #814: enable global xforms (except Spock) during reconcile

Global xforms disabled for: - index - find JUnit tests - open in editor (aka become working copy) - select in explorer (aka test for extract class refactor) Global xforms enabled for: - compile - reconcile - resolve source reference - code minings (aka search) Note: #812 is disabled until Spock xform is run as part of reconcile Note: Scripts with @grab will now open in editor immediately and the grabs are processed asynchronously as part of reconcile operation Note: Global xforms that run during Conversion compiler phase are applied even if disabled by CompilerConfiguration setting
groovy · Jul 1, 2019 · 04673d9 · 04673d9
1 parent 650f314
commit 04673d9
Show file tree

Hide file tree

Showing 11 changed files with 38 additions and 69 deletions.
diff --git a/...ore.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/SpockInferencingTests.java b/...ore.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/SpockInferencingTests.java
@@ -22,6 +22,7 @@
 import org.eclipse.core.runtime.Path;
 import org.eclipse.jdt.core.JavaCore;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Test;
 
 public final class SpockInferencingTests extends InferencingTestSuite {
@@ -127,7 +128,7 @@ public void testPropertyCheck() throws Exception {
         assertType(source, offset, offset + 3, "java.lang.Integer");
     }
 
-    @Test // https://github.com/groovy/groovy-eclipse/issues/812
+    @Test @Ignore("see #814") // https://github.com/groovy/groovy-eclipse/issues/812
     public void testDataTableChecks() {
         String source =
             //@formatter:off

diff --git a/base/org.codehaus.groovy24/src/org/codehaus/groovy/transform/ASTTransformationVisitor.java b/base/org.codehaus.groovy24/src/org/codehaus/groovy/transform/ASTTransformationVisitor.java
@@ -60,7 +60,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.TreeSet;
 import java.util.concurrent.TimeUnit;
 
 /**
@@ -272,9 +271,6 @@ public static void addGlobalTransforms(ASTTransformationsContext context) {
 
     private static void doAddGlobalTransforms(ASTTransformationsContext context, boolean isFirstScan) {
         final CompilationUnit compilationUnit = context.getCompilationUnit();
-        // GRECLIPSE add
-        ensureGlobalTransformsAllowedInReconcileInitialized();
-        // GRECLIPSE end
         GroovyClassLoader transformLoader = compilationUnit.getTransformLoader();
         Map<String, URL> transformNames = new LinkedHashMap<String, URL>();
         try {
@@ -321,7 +317,10 @@ private static void doAddGlobalTransforms(ASTTransformationsContext context, boo
                                                 null,
                                                 null);
                                     }
-                                } else /*GRECLIPSE add*/if (compilationUnit.allowTransforms || globalTransformsAllowedInReconcile.contains(className))/*GRECLIPSE end*/{
+                                } else {
+                                    // GRECLIPSE add
+                                    if (compilationUnit.allowTransforms)
+                                    // GRECLIPSE end
                                     transformNames.put(className, service);
                                 }
                             }
@@ -405,20 +404,6 @@ private static boolean skipManifest(CompilationUnit compilationUnit, URL service
         }
         return false;
     }
-
-    private static Set<String> globalTransformsAllowedInReconcile = null;
-
-    private static void ensureGlobalTransformsAllowedInReconcileInitialized() {
-        if (globalTransformsAllowedInReconcile == null) {
-            globalTransformsAllowedInReconcile = new TreeSet<String>();
-            globalTransformsAllowedInReconcile.add("groovy.grape.GrabAnnotationTransformation");
-            String transformNames = System.getProperty("greclipse.globalTransformsInReconcile", "");
-            for (String transformName : transformNames.split(",")) {
-                globalTransformsAllowedInReconcile.add(transformName.trim());
-            }
-            globalTransformsAllowedInReconcile.remove("");
-        }
-    }
     // GRECLIPSE end
 
     private static void addPhaseOperationsForGlobalTransforms(CompilationUnit compilationUnit,

diff --git a/base/org.codehaus.groovy25/src/org/codehaus/groovy/transform/ASTTransformationVisitor.java b/base/org.codehaus.groovy25/src/org/codehaus/groovy/transform/ASTTransformationVisitor.java
@@ -61,7 +61,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.TreeSet;
 import java.util.concurrent.TimeUnit;
 
 /**
@@ -267,9 +266,6 @@ public static void addGlobalTransforms(ASTTransformationsContext context) {
 
     private static void doAddGlobalTransforms(ASTTransformationsContext context, boolean isFirstScan) {
         final CompilationUnit compilationUnit = context.getCompilationUnit();
-        // GRECLIPSE add
-        ensureGlobalTransformsAllowedInReconcileInitialized();
-        // GRECLIPSE end
         GroovyClassLoader transformLoader = compilationUnit.getTransformLoader();
         Map<String, URL> transformNames = new LinkedHashMap<String, URL>();
         try {
@@ -314,7 +310,10 @@ private static void doAddGlobalTransforms(ASTTransformationsContext context, boo
                                                 null,
                                                 null);
                                     }
-                                } else /*GRECLIPSE add*/if (compilationUnit.allowTransforms || globalTransformsAllowedInReconcile.contains(className))/*GRECLIPSE end*/{
+                                } else {
+                                    // GRECLIPSE add
+                                    if (compilationUnit.allowTransforms)
+                                    // GRECLIPSE end
                                     transformNames.put(className, service);
                                 }
                             }
@@ -379,20 +378,6 @@ private static boolean skipManifest(CompilationUnit compilationUnit, URL service
         }
         return false;
     }
-
-    private static Set<String> globalTransformsAllowedInReconcile = null;
-
-    private static void ensureGlobalTransformsAllowedInReconcileInitialized() {
-        if (globalTransformsAllowedInReconcile == null) {
-            globalTransformsAllowedInReconcile = new TreeSet<>();
-            globalTransformsAllowedInReconcile.add("groovy.grape.GrabAnnotationTransformation");
-            String transformNames = System.getProperty("greclipse.globalTransformsInReconcile", "");
-            for (String transformName : transformNames.split(",")) {
-                globalTransformsAllowedInReconcile.add(transformName.trim());
-            }
-            globalTransformsAllowedInReconcile.remove("");
-        }
-    }
     // GRECLIPSE end
 
     private static void addPhaseOperationsForGlobalTransforms(CompilationUnit compilationUnit,

diff --git a/base/org.codehaus.groovy30/src/org/codehaus/groovy/transform/ASTTransformationVisitor.java b/base/org.codehaus.groovy30/src/org/codehaus/groovy/transform/ASTTransformationVisitor.java
@@ -61,7 +61,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.TreeSet;
 import java.util.concurrent.TimeUnit;
 
 /**
@@ -267,9 +266,6 @@ public static void addGlobalTransforms(ASTTransformationsContext context) {
 
     private static void doAddGlobalTransforms(ASTTransformationsContext context, boolean isFirstScan) {
         final CompilationUnit compilationUnit = context.getCompilationUnit();
-        // GRECLIPSE add
-        ensureGlobalTransformsAllowedInReconcileInitialized();
-        // GRECLIPSE end
         GroovyClassLoader transformLoader = compilationUnit.getTransformLoader();
         Map<String, URL> transformNames = new LinkedHashMap<String, URL>();
         try {
@@ -314,7 +310,10 @@ private static void doAddGlobalTransforms(ASTTransformationsContext context, boo
                                                 null,
                                                 null);
                                     }
-                                } else /*GRECLIPSE add*/if (compilationUnit.allowTransforms || globalTransformsAllowedInReconcile.contains(className))/*GRECLIPSE end*/{
+                                } else {
+                                    // GRECLIPSE add
+                                    if (compilationUnit.allowTransforms)
+                                    // GRECLIPSE end
                                     transformNames.put(className, service);
                                 }
                             }
@@ -379,20 +378,6 @@ private static boolean skipManifest(CompilationUnit compilationUnit, URL service
         }
         return false;
     }
-
-    private static Set<String> globalTransformsAllowedInReconcile = null;
-
-    private static void ensureGlobalTransformsAllowedInReconcileInitialized() {
-        if (globalTransformsAllowedInReconcile == null) {
-            globalTransformsAllowedInReconcile = new TreeSet<>();
-            globalTransformsAllowedInReconcile.add("groovy.grape.GrabAnnotationTransformation");
-            String transformNames = System.getProperty("greclipse.globalTransformsInReconcile", "");
-            for (String transformName : transformNames.split(",")) {
-                globalTransformsAllowedInReconcile.add(transformName.trim());
-            }
-            globalTransformsAllowedInReconcile.remove("");
-        }
-    }
     // GRECLIPSE end
 
     private static void addPhaseOperationsForGlobalTransforms(CompilationUnit compilationUnit,

diff --git a/...rg/codehaus/jdt/groovy/integration/internal/MultiplexingSourceElementRequestorParser.java b/...rg/codehaus/jdt/groovy/integration/internal/MultiplexingSourceElementRequestorParser.java
@@ -5,7 +5,7 @@
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- *     http://www.apache.org/licenses/LICENSE-2.0
+ *     https://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -99,13 +99,13 @@ public void reset() {
     public CompilationUnitDeclaration parseCompilationUnit(ICompilationUnit compilationUnit, boolean fullParse, IProgressMonitor progressMonitor) {
         if (ContentTypeUtils.isGroovyLikeFileName(compilationUnit.getFileName())) {
             // ASSUMPTIONS:
-            // 1) there is no difference between a diet and full parse in the groovy works, so can ignore the fullParse parameter
-            // 2) parsing is for the entire CU (ie- from character 0, to compilationUnit.getContents().length)
-            // 3) nodesToCategories map is not necessary. I think it has something to do with JavaDoc, but not sure
+            // 1) parsing is for the entire CU (ie- from character 0, to compilationUnit.getContents().length)
+            // 2) nodesToCategories map is not necessary. I think it has something to do with JavaDoc, but not sure
 
+            boolean disableGlobalXforms = !fullParse || optimizeStringLiterals;
             // FIXASC Is it ok to use a new parser here everytime? If we don't we sometimes recurse back into the first one.
             // FIXASC ought to reuse to ensure types end up in same groovy CU
-            GroovyParser groovyParser = new GroovyParser(this.groovyParser.requestor, options, problemReporter, false, true);
+            GroovyParser groovyParser = new GroovyParser(this.groovyParser.requestor, options, problemReporter, !disableGlobalXforms, true);
             CompilationResult compilationResult = new CompilationResult(compilationUnit, 0, 0, options.maxProblemsPerUnit);
             GroovyCompilationUnitDeclaration compUnitDecl = groovyParser.dietParse(compilationUnit, compilationResult);
 

diff --git a/...lipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyParser.java b/...lipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyParser.java
@@ -5,7 +5,7 @@
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- *     http://www.apache.org/licenses/LICENSE-2.0
+ *     https://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -15,8 +15,10 @@
  */
 package org.codehaus.jdt.groovy.internal.compiler.ast;
 
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Supplier;
 
@@ -114,6 +116,17 @@ public GroovyParser(Object requestor, CompilerOptions compilerOptions, ProblemRe
         this.unitFactory = () -> {
             CompilerConfiguration compilerConfiguration = GroovyLanguageSupport.newCompilerConfiguration(compilerOptions, problemReporter);
             GroovyClassLoader[] classLoaders = loaderFactory.getGroovyClassLoaders(compilerConfiguration);
+            // https://github.com/groovy/groovy-eclipse/issues/814
+            if (allowTransforms && isReconcile) {
+                // disable Spock transform until inferencing supports it
+                Set<String> xforms = new HashSet<>();
+                xforms.add("org.spockframework.compiler.SpockTransform");
+                Optional.ofNullable(
+                    compilerConfiguration.getDisabledGlobalASTTransformations()
+                ).ifPresent(xforms::addAll);
+                compilerConfiguration.setDisabledGlobalASTTransformations(xforms);
+            }
+
             CompilationUnit unit = new CompilationUnit(
                 compilerConfiguration,
                 null, // CodeSource

diff --git a/...rg.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java b/...rg.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java
@@ -366,7 +366,7 @@ public static CompilationUnitDeclaration process(
 	public void initializeParser() {
 		// GROOVY edit
 		//this.parser = new CommentRecorderParser(this.problemReporter, this.options.parseLiteralExpressionsAsConstants);
-		this.parser = LanguageSupportFactory.getParser(this, this.lookupEnvironment == null ? null : this.lookupEnvironment.globalOptions, this.problemReporter, this.options.parseLiteralExpressionsAsConstants, LanguageSupportFactory.CommentRecorderParserVariant + 1);
+		this.parser = LanguageSupportFactory.getParser(this, this.lookupEnvironment == null ? null : this.lookupEnvironment.globalOptions, this.problemReporter, this.options.parseLiteralExpressionsAsConstants, LanguageSupportFactory.CommentRecorderParserVariant);
 		// GROOVY end
 	}
 }
diff --git a/...rg.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java b/...rg.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java
@@ -366,7 +366,7 @@ public static CompilationUnitDeclaration process(
 	public void initializeParser() {
 		// GROOVY edit
 		//this.parser = new CommentRecorderParser(this.problemReporter, this.options.parseLiteralExpressionsAsConstants);
-		this.parser = LanguageSupportFactory.getParser(this, this.lookupEnvironment == null ? null : this.lookupEnvironment.globalOptions, this.problemReporter, this.options.parseLiteralExpressionsAsConstants, LanguageSupportFactory.CommentRecorderParserVariant + 1);
+		this.parser = LanguageSupportFactory.getParser(this, this.lookupEnvironment == null ? null : this.lookupEnvironment.globalOptions, this.problemReporter, this.options.parseLiteralExpressionsAsConstants, LanguageSupportFactory.CommentRecorderParserVariant);
 		// GROOVY end
 	}
 }
diff --git a/...rg.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java b/...rg.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java
@@ -366,7 +366,7 @@ public static CompilationUnitDeclaration process(
 	public void initializeParser() {
 		// GROOVY edit
 		//this.parser = new CommentRecorderParser(this.problemReporter, this.options.parseLiteralExpressionsAsConstants);
-		this.parser = LanguageSupportFactory.getParser(this, this.lookupEnvironment == null ? null : this.lookupEnvironment.globalOptions, this.problemReporter, this.options.parseLiteralExpressionsAsConstants, LanguageSupportFactory.CommentRecorderParserVariant + 1);
+		this.parser = LanguageSupportFactory.getParser(this, this.lookupEnvironment == null ? null : this.lookupEnvironment.globalOptions, this.problemReporter, this.options.parseLiteralExpressionsAsConstants, LanguageSupportFactory.CommentRecorderParserVariant);
 		// GROOVY end
 	}
 }
diff --git a/...rg.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java b/...rg.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java
@@ -363,7 +363,7 @@ public static CompilationUnitDeclaration process(
 	public void initializeParser() {
 		// GROOVY edit
 		//this.parser = new CommentRecorderParser(this.problemReporter, this.options.parseLiteralExpressionsAsConstants);
-		this.parser = LanguageSupportFactory.getParser(this, this.lookupEnvironment == null ? null : this.lookupEnvironment.globalOptions, this.problemReporter, this.options.parseLiteralExpressionsAsConstants, LanguageSupportFactory.CommentRecorderParserVariant + 1);
+		this.parser = LanguageSupportFactory.getParser(this, this.lookupEnvironment == null ? null : this.lookupEnvironment.globalOptions, this.problemReporter, this.options.parseLiteralExpressionsAsConstants, LanguageSupportFactory.CommentRecorderParserVariant);
 		// GROOVY end
 	}
 }
diff --git a/...rg.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java b/...rg.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java
@@ -366,7 +366,7 @@ public static CompilationUnitDeclaration process(
 	public void initializeParser() {
 		// GROOVY edit
 		//this.parser = new CommentRecorderParser(this.problemReporter, this.options.parseLiteralExpressionsAsConstants);
-		this.parser = LanguageSupportFactory.getParser(this, this.lookupEnvironment == null ? null : this.lookupEnvironment.globalOptions, this.problemReporter, this.options.parseLiteralExpressionsAsConstants, LanguageSupportFactory.CommentRecorderParserVariant + 1);
+		this.parser = LanguageSupportFactory.getParser(this, this.lookupEnvironment == null ? null : this.lookupEnvironment.globalOptions, this.problemReporter, this.options.parseLiteralExpressionsAsConstants, LanguageSupportFactory.CommentRecorderParserVariant);
 		// GROOVY end
 	}
 }