[groovy] don't pass groovyProjectName property into groovy-eclipse compiler (IDEA-207921)

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<String> args,
                                           StringWriter out,
                                           StringWriter err,
                                           Map<String, List<String>> 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<String, Object> 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<String> createCommandLine(CompileContext context,

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<String, List<String>> myOutputs;
 
-  public GreclipseMain(PrintWriter outWriter, PrintWriter errWriter, Map customDefaultOptions, Map<String, List<String>> outputs) {
-    super(new PrintWriter(outWriter), new PrintWriter(errWriter), false, customDefaultOptions, null);
+  public GreclipseMain(PrintWriter outWriter, PrintWriter errWriter, Map<String, List<String>> outputs) {
+    super(new PrintWriter(outWriter), new PrintWriter(errWriter), false, null, null);
     myOutputs = outputs;
   }
 

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<OrderRoot> loadRoots(Project project, String coordinates) {
+  static Collection<OrderRoot> loadRoots(Project project, String coordinates) {
     def libraryProperties = new RepositoryLibraryProperties(coordinates, true)
     def roots = JarRepositoryManager.loadDependenciesModal(project, libraryProperties, false, false, null, remoteRepositoryDescriptions)
     assert !roots.isEmpty()

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

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]
+  }
+}

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)
+  }
+}

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<String> chunkRebuildMessage(String builder) {
+    return []
+  }
+}

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<String> 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<String> chunkRebuildMessage(String builder) {
-      return []
-    }
-  }
-}
+}

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())
   }