From 04673d95852747a4089eec1b4d256e0fa96f9c7c Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Mon, 1 Jul 2019 14:21:56 -0500 Subject: [PATCH] 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 --- .../tests/search/SpockInferencingTests.java | 3 ++- .../transform/ASTTransformationVisitor.java | 23 ++++--------------- .../transform/ASTTransformationVisitor.java | 23 ++++--------------- .../transform/ASTTransformationVisitor.java | 23 ++++--------------- ...tiplexingSourceElementRequestorParser.java | 10 ++++---- .../internal/compiler/ast/GroovyParser.java | 15 +++++++++++- .../core/CompilationUnitProblemFinder.java | 2 +- .../core/CompilationUnitProblemFinder.java | 2 +- .../core/CompilationUnitProblemFinder.java | 2 +- .../core/CompilationUnitProblemFinder.java | 2 +- .../core/CompilationUnitProblemFinder.java | 2 +- 11 files changed, 38 insertions(+), 69 deletions(-) diff --git a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/SpockInferencingTests.java b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/SpockInferencingTests.java index 7c25f37c7d..1fd89cb7ea 100644 --- a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/SpockInferencingTests.java +++ b/base-test/org.eclipse.jdt.groovy.core.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 index 73653a4521..9934f453af 100644 --- 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 transformNames = new LinkedHashMap(); 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 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.groovy25/src/org/codehaus/groovy/transform/ASTTransformationVisitor.java b/base/org.codehaus.groovy25/src/org/codehaus/groovy/transform/ASTTransformationVisitor.java index 3f2f9a7c3f..d7e9eeb091 100644 --- 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 transformNames = new LinkedHashMap(); 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 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 index 3f2f9a7c3f..d7e9eeb091 100644 --- 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 transformNames = new LinkedHashMap(); 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 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.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/integration/internal/MultiplexingSourceElementRequestorParser.java b/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/integration/internal/MultiplexingSourceElementRequestorParser.java index e8dbeb4d84..38d52acb36 100644 --- a/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/integration/internal/MultiplexingSourceElementRequestorParser.java +++ b/base/org.eclipse.jdt.groovy.core/src/org/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/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyParser.java b/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyParser.java index 1ad01bc694..837655d01b 100644 --- a/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyParser.java +++ b/base/org.eclipse.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 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/jdt-patch/e410/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java b/jdt-patch/e410/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java index 033cc99d2f..9a42148587 100644 --- a/jdt-patch/e410/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java +++ b/jdt-patch/e410/org.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/jdt-patch/e411/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java b/jdt-patch/e411/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java index 033cc99d2f..9a42148587 100644 --- a/jdt-patch/e411/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java +++ b/jdt-patch/e411/org.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/jdt-patch/e412/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java b/jdt-patch/e412/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java index 033cc99d2f..9a42148587 100644 --- a/jdt-patch/e412/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java +++ b/jdt-patch/e412/org.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/jdt-patch/e48/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java b/jdt-patch/e48/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java index 1cf628bc6a..77b1318881 100644 --- a/jdt-patch/e48/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java +++ b/jdt-patch/e48/org.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/jdt-patch/e49/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java b/jdt-patch/e49/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java index 033cc99d2f..9a42148587 100644 --- a/jdt-patch/e49/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnitProblemFinder.java +++ b/jdt-patch/e49/org.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 } }