From 6b67001ac5b2f77ede762dbbcf8d584ab437f98d Mon Sep 17 00:00:00 2001 From: Daniil Ovchinnikov Date: Mon, 15 Apr 2019 19:53:57 +0300 Subject: [PATCH] [groovy] don't pass `groovyProjectName` property into groovy-eclipse compiler (IDEA-207921) --- .../incremental/groovy/GreclipseBuilder.java | 58 ++++++++++--------- .../jps/incremental/groovy/GreclipseMain.java | 20 +------ .../groovy/RepositoryTestLibrary.groovy | 2 +- .../jetbrains/plugins/groovy/TestLibrary.java | 5 ++ .../groovy/compiler/GrEclipse2415Test.groovy | 14 +++++ .../groovy/compiler/GrEclipse2416Test.groovy | 23 ++++++++ .../groovy/compiler/GrEclipseTestBase.groovy | 35 +++++++++++ .../groovy/compiler/GroovyCompilerTest.groovy | 34 +---------- .../compiler/GroovyCompilerTestCase.groovy | 2 +- 9 files changed, 113 insertions(+), 80 deletions(-) create mode 100644 plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GrEclipse2415Test.groovy create mode 100644 plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GrEclipse2416Test.groovy create mode 100644 plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GrEclipseTestBase.groovy diff --git a/plugins/groovy/jps-plugin/src/org/jetbrains/jps/incremental/groovy/GreclipseBuilder.java b/plugins/groovy/jps-plugin/src/org/jetbrains/jps/incremental/groovy/GreclipseBuilder.java index 9cb3ba64aed3..ec06c58b4ee2 100644 --- a/plugins/groovy/jps-plugin/src/org/jetbrains/jps/incremental/groovy/GreclipseBuilder.java +++ b/plugins/groovy/jps-plugin/src/org/jetbrains/jps/incremental/groovy/GreclipseBuilder.java @@ -1,18 +1,4 @@ -/* - * Copyright 2000-2014 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * 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 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ +// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package org.jetbrains.jps.incremental.groovy; import com.intellij.openapi.application.PathManager; @@ -226,7 +212,7 @@ public class GreclipseBuilder extends ModuleLevelBuilder { synchronized (ourGlobalEnvironmentLock) { try { System.setProperty(GroovyRtConstants.GROOVY_TARGET_BYTECODE, bytecodeTarget); - return performCompilationInner(args, out, err, outputs, context, chunk); + return performCompilationInner(args, out, err, outputs, context); } finally { System.clearProperty(GroovyRtConstants.GROOVY_TARGET_BYTECODE); @@ -234,34 +220,50 @@ public class GreclipseBuilder extends ModuleLevelBuilder { } } - return performCompilationInner(args, out, err, outputs, context, chunk); + return performCompilationInner(args, out, err, outputs, context); } private boolean performCompilationInner(List args, StringWriter out, StringWriter err, Map> outputs, - CompileContext context, ModuleChunk chunk) { + CompileContext context) { + final ClassLoader jpsLoader = Thread.currentThread().getContextClassLoader(); try { + // We have to set context class loader in order because greclipse will create child GroovyClassLoader, + // and will use context class loader as parent. + // + // Here's what happens if we leave jpsLoader: + // 1. org.codehaus.groovy.transform.ASTTransformationCollectorCodeVisitor + // is loaded with GreclipseMain's class loader, i.e. myGreclipseLoader; + // 2. org.codehaus.groovy.transform.ASTTransformation inside ASTTransformationCollectorCodeVisitor.verifyClass + // is loaded with ASTTransformationCollectorCodeVisitor' loader, i.e. myGreclipseLoader; + // 3. transformation GroovyClassLoader is created with context class loader (jpsLoader) as a parent; + // 4. some CoolTransform implements ASTTransformation is loaded with GroovyClassLoader; + // 5. ASTTransformation supertype of CoolTransform is loaded with GroovyClassLoader too; + // 6. GroovyClassLoader asks its parent, which is jpsLoader, it doesn't know about ASTTransformation + // => GroovyClassLoader loads ASTTransformation by itself; + // 7. there are two different ASTTransformation class instances + // => we get ASTTransformation.class.isAssignableFrom(klass) = false + // => compilation fails with error. + // + // If we set context classloader here, then in the 6th step parent loader will be myGreclipseLoader, + // and ASTTransformation class will be returned from myGreclipseLoader, and the compilation won't fail. + Thread.currentThread().setContextClassLoader(myGreclipseLoader); Class mainClass = Class.forName(GreclipseMain.class.getName(), true, myGreclipseLoader); - Constructor constructor = mainClass.getConstructor(PrintWriter.class, PrintWriter.class, Map.class, Map.class); + Constructor constructor = mainClass.getConstructor(PrintWriter.class, PrintWriter.class, Map.class); Method compileMethod = mainClass.getMethod("compile", String[].class); - HashMap customDefaultOptions = ContainerUtil.newHashMap(); - // without this greclipse won't load AST transformations - customDefaultOptions.put("org.eclipse.jdt.core.compiler.groovy.groovyClassLoaderPath", getClasspathString(chunk)); - - // used by greclipse to cache transform loaders - // names should be different for production & tests - customDefaultOptions.put("org.eclipse.jdt.core.compiler.groovy.groovyProjectName", chunk.getPresentableShortName()); - - Object main = constructor.newInstance(new PrintWriter(out), new PrintWriter(err), customDefaultOptions, outputs); + Object main = constructor.newInstance(new PrintWriter(out), new PrintWriter(err), outputs); return (Boolean)compileMethod.invoke(main, new Object[]{ArrayUtil.toStringArray(args)}); } catch (Exception e) { context.processMessage(CompilerMessage.createInternalBuilderError(getPresentableName(), e)); return false; } + finally { + Thread.currentThread().setContextClassLoader(jpsLoader); + } } private static List createCommandLine(CompileContext context, diff --git a/plugins/groovy/jps-plugin/src/org/jetbrains/jps/incremental/groovy/GreclipseMain.java b/plugins/groovy/jps-plugin/src/org/jetbrains/jps/incremental/groovy/GreclipseMain.java index a6446c53f9f6..6369f8c06479 100644 --- a/plugins/groovy/jps-plugin/src/org/jetbrains/jps/incremental/groovy/GreclipseMain.java +++ b/plugins/groovy/jps-plugin/src/org/jetbrains/jps/incremental/groovy/GreclipseMain.java @@ -1,18 +1,4 @@ -/* - * Copyright 2000-2014 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * 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 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ +// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package org.jetbrains.jps.incremental.groovy; import org.eclipse.jdt.internal.compiler.ClassFile; @@ -30,8 +16,8 @@ import java.util.Map; public class GreclipseMain extends Main { private final Map> myOutputs; - public GreclipseMain(PrintWriter outWriter, PrintWriter errWriter, Map customDefaultOptions, Map> outputs) { - super(new PrintWriter(outWriter), new PrintWriter(errWriter), false, customDefaultOptions, null); + public GreclipseMain(PrintWriter outWriter, PrintWriter errWriter, Map> outputs) { + super(new PrintWriter(outWriter), new PrintWriter(errWriter), false, null, null); myOutputs = outputs; } diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/RepositoryTestLibrary.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/RepositoryTestLibrary.groovy index 9902651c5680..315f75a4f485 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/RepositoryTestLibrary.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/RepositoryTestLibrary.groovy @@ -52,7 +52,7 @@ final class RepositoryTestLibrary implements TestLibrary { model.findLibraryOrderEntry(library).scope = myDependencyScope } - private static Collection loadRoots(Project project, String coordinates) { + static Collection loadRoots(Project project, String coordinates) { def libraryProperties = new RepositoryLibraryProperties(coordinates, true) def roots = JarRepositoryManager.loadDependenciesModal(project, libraryProperties, false, false, null, remoteRepositoryDescriptions) assert !roots.isEmpty() diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/TestLibrary.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/TestLibrary.java index 81e4cf663a9c..8da191c93f78 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/TestLibrary.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/TestLibrary.java @@ -3,10 +3,15 @@ package org.jetbrains.plugins.groovy; import com.intellij.openapi.module.Module; import com.intellij.openapi.roots.ModifiableRootModel; +import com.intellij.openapi.roots.ModuleRootModificationUtil; import org.jetbrains.annotations.NotNull; public interface TestLibrary { + default void addTo(@NotNull Module module) { + ModuleRootModificationUtil.updateModel(module, model -> addTo(module, model)); + } + void addTo(@NotNull Module module, @NotNull ModifiableRootModel model); @NotNull diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GrEclipse2415Test.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GrEclipse2415Test.groovy new file mode 100644 index 000000000000..985f42ae33ec --- /dev/null +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GrEclipse2415Test.groovy @@ -0,0 +1,14 @@ +// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package org.jetbrains.plugins.groovy.compiler + +import com.intellij.project.IntelliJProjectConfiguration +import groovy.transform.CompileStatic + +@CompileStatic +class GrEclipse2415Test extends GrEclipseTestBase { + + @Override + protected String getGrEclipsePath() { + return IntelliJProjectConfiguration.getProjectLibraryClassesRootPaths("Groovy-Eclipse-Batch")[0] + } +} diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GrEclipse2416Test.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GrEclipse2416Test.groovy new file mode 100644 index 000000000000..9ef592cd3857 --- /dev/null +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GrEclipse2416Test.groovy @@ -0,0 +1,23 @@ +// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package org.jetbrains.plugins.groovy.compiler + +import com.intellij.openapi.module.Module +import com.intellij.openapi.vfs.JarFileSystem +import groovy.transform.CompileStatic +import org.jetbrains.plugins.groovy.GroovyProjectDescriptors +import org.jetbrains.plugins.groovy.RepositoryTestLibrary + +@CompileStatic +class GrEclipse2416Test extends GrEclipseTestBase { + + @Override + protected String getGrEclipsePath() { + def jarRoot = RepositoryTestLibrary.loadRoots(project, "org.codehaus.groovy:groovy-eclipse-batch:2.4.16-01")[0].file + return JarFileSystem.instance.getVirtualFileForJar(jarRoot).path + } + + @Override + protected void addGroovyLibrary(Module to) { + GroovyProjectDescriptors.LIB_GROOVY_2_4.addTo(to) + } +} diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GrEclipseTestBase.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GrEclipseTestBase.groovy new file mode 100644 index 000000000000..fc8df13983eb --- /dev/null +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GrEclipseTestBase.groovy @@ -0,0 +1,35 @@ +// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package org.jetbrains.plugins.groovy.compiler + +import com.intellij.compiler.CompilerConfiguration +import com.intellij.compiler.CompilerConfigurationImpl +import com.intellij.openapi.projectRoots.JavaSdkVersion +import com.intellij.openapi.projectRoots.JavaSdkVersionUtil +import com.intellij.openapi.roots.ModuleRootManager +import groovy.transform.CompileStatic + +@CompileStatic +abstract class GrEclipseTestBase extends GroovyCompilerTest { + + protected abstract String getGrEclipsePath() + + @Override + protected void setUp() { + super.setUp() + ((CompilerConfigurationImpl)CompilerConfiguration.getInstance(project)).defaultCompiler = new GreclipseIdeaCompiler(project) + GreclipseIdeaCompilerSettings.getSettings(project).greclipsePath = grEclipsePath + } + + @Override + void runTest() { + if (JavaSdkVersionUtil.getJavaSdkVersion(ModuleRootManager.getInstance(myModule).sdk)?.isAtLeast(JavaSdkVersion.JDK_10)) { + println "Groovy-Eclipse doesn't support Java 10+ yet" + return + } + super.runTest() + } + + protected List chunkRebuildMessage(String builder) { + return [] + } +} diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GroovyCompilerTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GroovyCompilerTest.groovy index 450c6e6d32ec..ad2ab7f5fba7 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GroovyCompilerTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GroovyCompilerTest.groovy @@ -2,7 +2,6 @@ package org.jetbrains.plugins.groovy.compiler import com.intellij.compiler.CompilerConfiguration -import com.intellij.compiler.CompilerConfigurationImpl import com.intellij.compiler.server.BuildManager import com.intellij.execution.executors.DefaultRunExecutor import com.intellij.execution.impl.DefaultJavaProgramRunner @@ -18,9 +17,6 @@ import com.intellij.openapi.compiler.options.ExcludeEntryDescription import com.intellij.openapi.compiler.options.ExcludesConfiguration import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.module.Module -import com.intellij.openapi.projectRoots.JavaSdkVersion -import com.intellij.openapi.projectRoots.JavaSdkVersionUtil -import com.intellij.openapi.roots.ModuleRootManager import com.intellij.openapi.roots.ModuleRootModificationUtil import com.intellij.openapi.util.Key import com.intellij.openapi.util.Ref @@ -1052,33 +1048,5 @@ class Bar {}''' protected List chunkRebuildMessage(String builder) { return ['Builder "' + builder + '" requested rebuild of module chunk "mainModule"'] } - } - - static class EclipseTest extends GroovyCompilerTest { - @Override - protected void setUp() { - super.setUp() - - ((CompilerConfigurationImpl)CompilerConfiguration.getInstance(project)).defaultCompiler = new GreclipseIdeaCompiler(project) - - def jarPath = IntelliJProjectConfiguration.getProjectLibraryClassesRootPaths("Groovy-Eclipse-Batch")[0] - - GreclipseIdeaCompilerSettings.getSettings(project).greclipsePath = jarPath - } - - @Override - void runTest() { - if (JavaSdkVersionUtil.getJavaSdkVersion(ModuleRootManager.getInstance(myModule).sdk)?.isAtLeast(JavaSdkVersion.JDK_10)) { - println "Groovy-Eclipse doesn't support Java 10+ yet" - return - } - - super.runTest() - } - - protected List chunkRebuildMessage(String builder) { - return [] - } - } -} \ No newline at end of file +} diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GroovyCompilerTestCase.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GroovyCompilerTestCase.groovy index a088d1fe9e29..9ab11b9d2503 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GroovyCompilerTestCase.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/compiler/GroovyCompilerTestCase.groovy @@ -83,7 +83,7 @@ abstract class GroovyCompilerTestCase extends JavaCodeInsightFixtureTestCase imp super.runTest() } - protected static void addGroovyLibrary(final Module to) { + protected void addGroovyLibrary(final Module to) { File jar = BundledGroovy.getBundledGroovyFile() PsiTestUtil.addLibrary(to, "groovy", jar.getParent(), jar.getName()) }