Add kotlin.stdlib to module-info when configure Maven project (KT-19207)

#KT-19207 In Progress

idea/idea-maven/src/org/jetbrains/kotlin/idea/maven/PomFile.kt

@@ -16,6 +16,8 @@
 
 package org.jetbrains.kotlin.idea.maven
 
+import com.intellij.openapi.application.ApplicationManager
+import com.intellij.openapi.diagnostic.Logger
 import com.intellij.openapi.module.Module
 import com.intellij.openapi.roots.ModuleRootManager
 import com.intellij.openapi.roots.SourceFolder
@@ -33,6 +35,7 @@ import org.jetbrains.idea.maven.dom.MavenDomUtil
 import org.jetbrains.idea.maven.dom.model.*
 import org.jetbrains.idea.maven.model.MavenId
 import org.jetbrains.idea.maven.model.MavenPlugin
+import org.jetbrains.idea.maven.project.MavenProject
 import org.jetbrains.idea.maven.project.MavenProjectsManager
 import org.jetbrains.idea.maven.utils.MavenArtifactScope
 import org.jetbrains.jps.model.java.JavaSourceRootType
@@ -214,7 +217,16 @@ class PomFile private constructor(val xmlFile: XmlFile, val domModel: MavenDomPr
     fun addJavacExecutions(module: Module, kotlinPlugin: MavenDomPlugin) {
         val javacPlugin = ensurePluginAfter(addPlugin(MavenId("org.apache.maven.plugins", "maven-compiler-plugin", null)), kotlinPlugin)
 
-        val project = MavenProjectsManager.getInstance(module.project).findProject(module) ?: error("Can't find maven project for $module")
+        val project: MavenProject =
+                MavenProjectsManager.getInstance(module.project).findProject(module) ?:
+                run {
+                    if (ApplicationManager.getApplication().isUnitTestMode) {
+                        LOG.warn("WARNING: Bad project configuration in tests. Javac execution configuration was skipped.")
+                        return
+                    }
+                    error("Can't find maven project for $module")
+                }
+
         val plugin = project.findPlugin("org.apache.maven.plugins", "maven-compiler-plugin")
 
         if (isExecutionEnabled(plugin, "default-compile")) {
@@ -501,6 +513,8 @@ class PomFile private constructor(val xmlFile: XmlFile, val domModel: MavenDomPr
     }
 
     companion object {
+        private val LOG = Logger.getInstance(PomFile::class.java)
+
         fun forFileOrNull(xmlFile: XmlFile): PomFile? = MavenDomUtil.getMavenDomProjectModel(xmlFile.project, xmlFile.virtualFile)?.let { PomFile(xmlFile, it) }
 
         @Deprecated("We shouldn't use phase but additional compiler configuration in most cases")

idea/idea-maven/src/org/jetbrains/kotlin/idea/maven/configuration/KotlinJavaMavenConfigurator.kt

@@ -18,7 +18,10 @@ package org.jetbrains.kotlin.idea.maven.configuration
 
 import com.intellij.openapi.module.Module
 import com.intellij.openapi.roots.ModuleRootManager
+import com.intellij.psi.PsiFile
 import org.jetbrains.idea.maven.dom.model.MavenDomPlugin
+import org.jetbrains.kotlin.idea.configuration.NotificationMessageCollector
+import org.jetbrains.kotlin.idea.configuration.addStdlibToJavaModuleInfo
 import org.jetbrains.kotlin.idea.configuration.hasKotlinJvmRuntimeInScope
 import org.jetbrains.kotlin.idea.maven.PomFile
 import org.jetbrains.kotlin.idea.versions.getDefaultJvmTarget
@@ -28,17 +31,14 @@ import org.jetbrains.kotlin.resolve.jvm.platform.JvmPlatform
 
 class KotlinJavaMavenConfigurator : KotlinMavenConfigurator(KotlinJavaMavenConfigurator.TEST_LIB_ID, false, KotlinJavaMavenConfigurator.NAME, KotlinJavaMavenConfigurator.PRESENTABLE_TEXT) {
 
-    override fun isKotlinModule(module: Module): Boolean {
+    override fun isKotlinModule(module: Module) =
-        return hasKotlinJvmRuntimeInScope(module)
+            hasKotlinJvmRuntimeInScope(module)
-    }
 
-    override fun isRelevantGoal(goalName: String): Boolean {
+    override fun isRelevantGoal(goalName: String) =
-        return goalName == PomFile.KotlinGoals.Compile
+            goalName == PomFile.KotlinGoals.Compile
-    }
 
-    override fun getStdlibArtifactId(module: Module, version: String): String {
+    override fun getStdlibArtifactId(module: Module, version: String): String =
-        return getStdlibArtifactId(ModuleRootManager.getInstance(module).sdk, version)
+            getStdlibArtifactId(ModuleRootManager.getInstance(module).sdk, version)
-    }
 
     override fun createExecutions(pomFile: PomFile, kotlinPlugin: MavenDomPlugin, module: Module) {
         createExecution(pomFile, kotlinPlugin, PomFile.DefaultPhases.Compile, PomFile.KotlinGoals.Compile, module, false)
@@ -53,6 +53,15 @@ class KotlinJavaMavenConfigurator : KotlinMavenConfigurator(KotlinJavaMavenConfi
         }
     }
 
+    override fun configureModule(module: Module, file: PsiFile, version: String, collector: NotificationMessageCollector): Boolean {
+        if (!super.configureModule(module, file, version, collector)) {
+            return false
+        }
+
+        addStdlibToJavaModuleInfo(module, collector)
+        return true
+    }
+
     override val targetPlatform: TargetPlatform
         get() = JvmPlatform
 

idea/idea-maven/src/org/jetbrains/kotlin/idea/maven/configuration/KotlinMavenConfigurator.kt

@@ -118,7 +118,7 @@ abstract class KotlinMavenConfigurator
             for (module in excludeMavenChildrenModules(project, dialog.modulesToConfigure)) {
                 val file = findModulePomFile(module)
                 if (file != null && canConfigureFile(file)) {
-                    changePomFile(module, file, dialog.kotlinVersion, collector)
+                    configureModule(module, file, dialog.kotlinVersion, collector)
                     OpenFileAction.openFile(file.virtualFile, project)
                 }
                 else {
@@ -137,20 +137,23 @@ abstract class KotlinMavenConfigurator
     protected abstract fun createExecutions(pomFile: PomFile, kotlinPlugin: MavenDomPlugin, module: Module)
     protected abstract fun getStdlibArtifactId(module: Module, version: String): String
 
-    fun changePomFile(
+    open fun configureModule(module: Module, file: PsiFile, version: String, collector: NotificationMessageCollector): Boolean =
+            changePomFile(module, file, version, collector)
+
+    private fun changePomFile(
             module: Module,
             file: PsiFile,
             version: String,
-            collector: NotificationMessageCollector) {
+            collector: NotificationMessageCollector
-
+    ): Boolean {
         val virtualFile = file.virtualFile ?: error("Virtual file should exists for psi file " + file.name)
         val domModel = MavenDomUtil.getMavenDomProjectModel(module.project, virtualFile)
         if (domModel == null) {
             showErrorMessage(module.project, null)
-            return
+            return false
         }
 
-        val pom = PomFile.forFileOrNull(file as XmlFile) ?: return
+        val pom = PomFile.forFileOrNull(file as XmlFile) ?: return false
         pom.addProperty(KOTLIN_VERSION_PROPERTY, version)
 
         pom.addDependency(MavenId(GROUP_ID, getStdlibArtifactId(module, version), "\${$KOTLIN_VERSION_PROPERTY}"), MavenArtifactScope.COMPILE, null, false, null)
@@ -176,6 +179,7 @@ abstract class KotlinMavenConfigurator
         CodeInsightUtilCore.forcePsiPostprocessAndRestoreElement<PsiFile>(file)
 
         collector.addMessage(virtualFile.path + " was modified")
+        return true
     }
 
     protected open fun configurePlugin(pom: PomFile, plugin: MavenDomPlugin, module: Module, version: String) {

idea/idea-maven/test/org/jetbrains/kotlin/idea/maven/configuration/AbstractMavenConfigureProjectByChangingFileTest.kt

@@ -18,10 +18,15 @@ package org.jetbrains.kotlin.idea.maven.configuration
 
 import com.intellij.openapi.command.WriteCommandAction
 import com.intellij.openapi.module.Module
+import com.intellij.openapi.projectRoots.Sdk
 import com.intellij.psi.PsiFile
 import org.jetbrains.idea.maven.model.MavenConstants
 import org.jetbrains.kotlin.idea.configuration.AbstractConfigureProjectByChangingFileTest
 import org.jetbrains.kotlin.idea.configuration.NotificationMessageCollector
+import org.jetbrains.kotlin.idea.test.PluginTestCaseBase
+import org.jetbrains.kotlin.test.KotlinTestUtils
+import org.jetbrains.kotlin.test.KotlinTestUtils.isAllFilesPresentTest
+import java.io.File
 
 abstract class AbstractMavenConfigureProjectByChangingFileTest : AbstractConfigureProjectByChangingFileTest<KotlinMavenConfigurator>() {
     fun doTestWithMaven(path: String) {
@@ -36,7 +41,21 @@ abstract class AbstractMavenConfigureProjectByChangingFileTest : AbstractConfigu
 
     override fun runConfigurator(module: Module, file: PsiFile, configurator: KotlinMavenConfigurator, version: String, collector: NotificationMessageCollector) {
         WriteCommandAction.runWriteCommandAction(module.project) {
-            configurator.changePomFile(module, file, version, collector)
+            configurator.configureModule(module, file, version, collector)
         }
     }
+
+    override fun getProjectJDK(): Sdk {
+        if (!isAllFilesPresentTest(getTestName(false))) {
+            val root = KotlinTestUtils.getTestsRoot(this::class.java)
+            val dir = KotlinTestUtils.getTestDataFileName(this::class.java, name)
+
+            val pomFile = File("$root/$dir", MavenConstants.POM_XML)
+            if (pomFile.readText().contains("<target>9</target>")) {
+                return PluginTestCaseBase.mockJdk9()
+            }
+        }
+
+        return super.getProjectJDK()
+    }
 }

idea/idea-maven/test/org/jetbrains/kotlin/idea/maven/configuration/MavenConfigureProjectByChangingFileTestGenerated.java

@@ -79,6 +79,12 @@ public class MavenConfigureProjectByChangingFileTestGenerated extends AbstractMa
             String fileName = KotlinTestUtils.navigationMetadata("idea/idea-maven/testData/configurator/jvm/simpleProjectSnapshot/");
             doTestWithMaven(fileName);
         }
+
+        @TestMetadata("withJava9ModuleInfo")
+        public void testWithJava9ModuleInfo() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/idea-maven/testData/configurator/jvm/withJava9ModuleInfo/");
+            doTestWithMaven(fileName);
+        }
     }
 
     @TestMetadata("idea/idea-maven/testData/configurator/js")

idea/idea-maven/testData/configurator/jvm/withJava9ModuleInfo/module-info.java

@@ -0,0 +1,3 @@
+module TestModule {
+
+}

idea/idea-maven/testData/configurator/jvm/withJava9ModuleInfo/module-info_after.java

@@ -0,0 +1,4 @@
+module TestModule {
+    requires kotlin.stdlib;
+
+}

idea/idea-maven/testData/configurator/jvm/withJava9ModuleInfo/pom.xml

@@ -0,0 +1,41 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <groupId>maventest</groupId>
+    <artifactId>maventest</artifactId>
+    <version>1.0-SNAPSHOT</version>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-compiler-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <id>compile</id>
+                        <phase>compile</phase>
+                        <goals>
+                            <goal>compile</goal>
+                        </goals>
+                    </execution>
+                    <execution>
+                        <id>testCompile</id>
+                        <phase>test-compile</phase>
+                        <goals>
+                            <goal>testCompile</goal>
+                        </goals>
+                    </execution>
+                </executions>
+                <configuration>
+                    <source>9</source>
+                    <target>9</target>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>
+
+
+</project>

idea/idea-maven/testData/configurator/jvm/withJava9ModuleInfo/pom_after.xml

@@ -0,0 +1,84 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <groupId>maventest</groupId>
+    <artifactId>maventest</artifactId>
+    <version>1.0-SNAPSHOT</version>
+
+    <properties>
+        <kotlin.version>$VERSION$</kotlin.version>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.jetbrains.kotlin</groupId>
+            <artifactId>kotlin-stdlib-jre8</artifactId>
+            <version>${kotlin.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.jetbrains.kotlin</groupId>
+            <artifactId>kotlin-test</artifactId>
+            <version>${kotlin.version}</version>
+            <scope>test</scope>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <sourceDirectory></sourceDirectory>
+        <plugins>
+            <plugin>
+                <groupId>org.jetbrains.kotlin</groupId>
+                <artifactId>kotlin-maven-plugin</artifactId>
+                <version>${kotlin.version}</version>
+                <executions>
+                    <execution>
+                        <id>compile</id>
+                        <phase>compile</phase>
+                        <goals>
+                            <goal>compile</goal>
+                        </goals>
+                    </execution>
+                    <execution>
+                        <id>test-compile</id>
+                        <phase>test-compile</phase>
+                        <goals>
+                            <goal>test-compile</goal>
+                        </goals>
+                    </execution>
+                </executions>
+                <configuration>
+                    <jvmTarget>1.8</jvmTarget>
+                </configuration>
+            </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-compiler-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <id>compile</id>
+                        <phase>compile</phase>
+                        <goals>
+                            <goal>compile</goal>
+                        </goals>
+                    </execution>
+                    <execution>
+                        <id>testCompile</id>
+                        <phase>test-compile</phase>
+                        <goals>
+                            <goal>testCompile</goal>
+                        </goals>
+                    </execution>
+                </executions>
+                <configuration>
+                    <source>9</source>
+                    <target>9</target>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>
+
+
+</project>

idea/src/org/jetbrains/kotlin/idea/configuration/ConfigureKotlinInProjectUtils.kt

@@ -284,17 +284,18 @@ private class LibraryKindSearchScope(val module: Module,
     }
 }
 
-fun addStdlibToJavaModuleInfo(module: Module, collector: NotificationMessageCollector) {
+fun addStdlibToJavaModuleInfo(module: Module, collector: NotificationMessageCollector): Boolean {
-    if (module.sdk?.version?.isAtLeast(JavaSdkVersion.JDK_1_9) != true) return
+    if (module.sdk?.version?.isAtLeast(JavaSdkVersion.JDK_1_9) != true) return false
 
     val project = module.project
-    val javaModule: PsiJavaModule = findFirstPsiJavaModule(module) ?: return
+    val javaModule: PsiJavaModule = findFirstPsiJavaModule(module) ?: return false
 
     val success = WriteCommandAction.runWriteCommandAction(project, Computable<Boolean> {
         KotlinAddRequiredModuleFix.addModuleRequirement(javaModule, KOTLIN_STDLIB_MODULE_NAME)
     })
 
-    if (success) {
+    if (!success) return false
-        collector.addMessage("Added ${KOTLIN_STDLIB_MODULE_NAME} requirement to module-info in ${module.name}")
+
-    }
+    collector.addMessage("Added $KOTLIN_STDLIB_MODULE_NAME requirement to module-info in ${module.name}")
+    return true
 }

idea/tests/org/jetbrains/kotlin/idea/configuration/AbstractConfigureProjectByChangingFileTest.java

@@ -18,18 +18,33 @@ package org.jetbrains.kotlin.idea.configuration;
 
 import com.intellij.openapi.application.ApplicationManager;
 import com.intellij.openapi.module.Module;
+import com.intellij.openapi.module.ModuleType;
+import com.intellij.openapi.projectRoots.Sdk;
+import com.intellij.openapi.roots.OrderRootType;
+import com.intellij.openapi.util.io.FileUtilRt;
+import com.intellij.openapi.vfs.CharsetToolkit;
 import com.intellij.psi.PsiFile;
+import com.intellij.psi.PsiJavaModule;
+import com.intellij.psi.search.FilenameIndex;
+import com.intellij.psi.search.GlobalSearchScope;
 import com.intellij.testFramework.LightCodeInsightTestCase;
+import com.intellij.testFramework.LightProjectDescriptor;
+import com.intellij.util.containers.ContainerUtil;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.jetbrains.kotlin.idea.core.script.ScriptDependenciesUpdaterKt;
 import org.jetbrains.kotlin.test.InTextDirectivesUtils;
 import org.jetbrains.kotlin.test.KotlinTestUtils;
 
 import java.io.File;
+import java.io.IOException;
+import java.util.Objects;
 
 public abstract class AbstractConfigureProjectByChangingFileTest<C extends KotlinProjectConfigurator> extends LightCodeInsightTestCase {
     private static final String DEFAULT_VERSION = "default_version";
 
+    private PsiFile moduleInfoFile;
+
     @Override
     protected void setUp() throws Exception {
         super.setUp();
@@ -39,12 +54,15 @@ public abstract class AbstractConfigureProjectByChangingFileTest<C extends Kotli
     @Override
     protected void tearDown() throws Exception {
         ScriptDependenciesUpdaterKt.setScriptDependenciesUpdaterDisabled(ApplicationManager.getApplication(), false);
+        moduleInfoFile = null;
         super.tearDown();
     }
 
     protected void doTest(@NotNull String beforeFile, @NotNull String afterFile, @NotNull C configurator) throws Exception {
         configureByFile(beforeFile);
 
+        prepareModuleInfoFile(beforeFile);
+
         String versionFromFile = InTextDirectivesUtils.findStringWithPrefixes(getFile().getText(), "// VERSION:");
         String version = versionFromFile != null ? versionFromFile : DEFAULT_VERSION;
 
@@ -55,6 +73,35 @@ public abstract class AbstractConfigureProjectByChangingFileTest<C extends Kotli
         collector.showNotification();
 
         KotlinTestUtils.assertEqualsToFile(new File(afterFile), getFile().getText().replace(version, "$VERSION$"));
+
+        checkModuleInfoFile(beforeFile);
+    }
+
+    private void prepareModuleInfoFile(@NotNull String beforeFile) throws IOException {
+        File file = new File(beforeFile);
+        String parent = file.getParent();
+        File moduleInfo = new File(parent, PsiJavaModule.MODULE_INFO_FILE);
+        moduleInfoFile = null;
+        if (moduleInfo.exists()) {
+            String fileText = FileUtilRt.loadFile(moduleInfo, CharsetToolkit.UTF8, true);
+            createAndSaveFile(PsiJavaModule.MODULE_INFO_FILE, fileText);
+
+            PsiFile[] moduleInfoFiles =
+                    FilenameIndex.getFilesByName(getProject(), PsiJavaModule.MODULE_INFO_FILE, GlobalSearchScope.allScope(getProject()));
+            assertTrue(PsiJavaModule.MODULE_INFO_FILE + " should be present in index", moduleInfoFiles.length == 1);
+            moduleInfoFile = moduleInfoFiles[0];
+        }
+    }
+
+    private void checkModuleInfoFile(@NotNull String beforeFile) {
+        File file = new File(beforeFile);
+        String parent = file.getParent();
+
+        if (moduleInfoFile != null) {
+            String afterFileName = PsiJavaModule.MODULE_INFO_FILE.replace(".java", "_after.java");
+            commitAllDocuments();
+            KotlinTestUtils.assertEqualsToFile(new File(parent, afterFileName), moduleInfoFile.getText());
+        }
     }
 
     protected abstract void runConfigurator(
@@ -69,4 +116,60 @@ public abstract class AbstractConfigureProjectByChangingFileTest<C extends Kotli
     protected String getTestDataPath() {
         return "";
     }
+
+    @NotNull
+    @Override
+    protected LightProjectDescriptor getProjectDescriptor() {
+        return new SimpleLightProjectDescriptor(getModuleType(), getProjectJDK());
+    }
+
+    private static class SimpleLightProjectDescriptor extends LightProjectDescriptor {
+        @NotNull private final ModuleType myModuleType;
+        @Nullable private final Sdk mySdk;
+
+        SimpleLightProjectDescriptor(@NotNull ModuleType moduleType, @Nullable Sdk sdk) {
+            myModuleType = moduleType;
+            mySdk = sdk;
+        }
+
+        @NotNull
+        @Override
+        public ModuleType getModuleType() {
+            return myModuleType;
+        }
+
+        @Nullable
+        @Override
+        public Sdk getSdk() {
+            return mySdk;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+
+            SimpleLightProjectDescriptor that = (SimpleLightProjectDescriptor)o;
+
+            if (!myModuleType.equals(that.myModuleType)) return false;
+            return areJdksEqual(that.getSdk());
+        }
+
+        @Override
+        public int hashCode() {
+            return myModuleType.hashCode();
+        }
+
+        private boolean areJdksEqual(Sdk newSdk) {
+            if (mySdk == null || newSdk == null) return mySdk == newSdk;
+
+            if (!Objects.equals(mySdk.getVersionString(), newSdk.getVersionString())) {
+                return false;
+            }
+
+            String[] myUrls = mySdk.getRootProvider().getUrls(OrderRootType.CLASSES);
+            String[] newUrls = newSdk.getRootProvider().getUrls(OrderRootType.CLASSES);
+            return ContainerUtil.newHashSet(myUrls).equals(ContainerUtil.newHashSet(newUrls));
+        }
+    }
 }