fix access from tests to internal elements in production code in case of circular dependency

compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/modules/ModuleBuilder.kt

@@ -28,6 +28,7 @@ public class ModuleBuilder(
     private val sourceFiles = ArrayList<String>()
     private val classpathRoots = ArrayList<String>()
     private val javaSourceRoots = ArrayList<JavaRootPath>()
+    private val friendDirs = ArrayList<String>()
 
     public fun addSourceFiles(pattern: String) {
         sourceFiles.add(pattern)
@@ -41,7 +42,12 @@ public class ModuleBuilder(
         javaSourceRoots.add(rootPath)
     }
 
+    public fun addFriendDir(friendDir: String) {
+        friendDirs.add(friendDir)
+    }
+
     override fun getOutputDirectory(): String = outputDir
+    override fun getFriendPaths(): List<String> = friendDirs
     override fun getJavaSourceRoots(): List<JavaRootPath> = javaSourceRoots
     override fun getSourceFiles(): List<String> = sourceFiles
     override fun getClasspathRoots(): List<String> = classpathRoots

compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/modules/ModuleXmlParser.java

@@ -47,6 +47,7 @@ public class ModuleXmlParser {
     public static final String TYPE_PRODUCTION = "java-production";
     public static final String TYPE_TEST = "java-test";
     public static final String OUTPUT_DIR = "outputDir";
+    public static final String FRIEND_DIR = "friendDir";
     public static final String SOURCES = "sources";
     public static final String JAVA_SOURCE_ROOTS = "javaSourceRoots";
     public static final String JAVA_SOURCE_PACKAGE_PREFIX = "packagePrefix";
@@ -162,6 +163,10 @@ public class ModuleXmlParser {
                 String path = getAttribute(attributes, PATH, qName);
                 moduleBuilder.addSourceFiles(path);
             }
+            else if (FRIEND_DIR.equalsIgnoreCase(qName)) {
+                String path = getAttribute(attributes, PATH, qName);
+                moduleBuilder.addFriendDir(path);
+            }
             else if (CLASSPATH.equalsIgnoreCase(qName)) {
                 String path = getAttribute(attributes, PATH, qName);
                 moduleBuilder.addClasspathEntry(path);

compiler/cli/src/org/jetbrains/kotlin/cli/common/moduleVisibilityImpl.kt

@@ -18,13 +18,11 @@ package org.jetbrains.kotlin.cli.common
 
 import com.intellij.openapi.Disposable
 import com.intellij.openapi.project.Project
-import org.jetbrains.kotlin.cli.common.modules.ModuleXmlParser
 import org.jetbrains.kotlin.descriptors.*
 import org.jetbrains.kotlin.load.kotlin.ModuleVisibilityManager
 import org.jetbrains.kotlin.load.kotlin.getSourceElement
 import org.jetbrains.kotlin.load.kotlin.isContainedByCompiledPartOfOurModule
 import org.jetbrains.kotlin.modules.Module
-import org.jetbrains.kotlin.psi.KtFile
 import org.jetbrains.kotlin.resolve.lazy.descriptors.LazyPackageDescriptor
 import org.jetbrains.kotlin.resolve.source.KotlinSourceElement
 import org.jetbrains.kotlin.util.ModuleVisibilityHelper
@@ -44,37 +42,37 @@ class ModuleVisibilityHelperImpl : ModuleVisibilityHelper {
             }
 
         val moduleVisibilityManager = ModuleVisibilityManager.SERVICE.getInstance(project)
-
-        fun findModule(kotlinFile: KtFile): Module? = moduleVisibilityManager.chunk.firstOrNull { it.getSourceFiles().containsRaw(kotlinFile.virtualFile.path) }
-
-        val whatSource = getSourceElement(what)
-        if (whatSource is KotlinSourceElement) {
-            if (moduleVisibilityManager.chunk.size > 1 && fromSource is KotlinSourceElement) {
-                val fromSourceKotlinFile = fromSource.psi.getContainingKtFile()
-                val whatSourceKotlinFile = whatSource.psi.getContainingKtFile()
-                return findModule(whatSourceKotlinFile) === findModule(fromSourceKotlinFile)
-            }
-
-            return true
-        }
-
         moduleVisibilityManager.friendPaths.forEach {
             if (isContainedByCompiledPartOfOurModule(what, File(it))) return true
         }
 
         val modules = moduleVisibilityManager.chunk
 
-        val outputDirectories = modules.map { File(it.getOutputDirectory()) }
+        val whatSource = getSourceElement(what)
-        if (outputDirectories.isEmpty()) return isContainedByCompiledPartOfOurModule(what, null)
+        if (whatSource is KotlinSourceElement) {
+            if (modules.size > 1 && fromSource is KotlinSourceElement) {
+                return findModule(what, modules) === findModule(from, modules)
+            }
 
-        outputDirectories.forEach {
+            return true
-            if (isContainedByCompiledPartOfOurModule(what, it)) return true
         }
 
-        // Hack in order to allow access to internal elements in production code from tests
+        if (modules.isEmpty()) return false
-        if (modules.singleOrNull()?.getModuleType() == ModuleXmlParser.TYPE_TEST) return true
 
-        return false
+        return findModule(from, modules) === findModule(what, modules)
+    }
+
+    private fun findModule(descriptor: DeclarationDescriptor, modules: Collection<Module>): Module? {
+        val sourceElement = getSourceElement(descriptor)
+        if (sourceElement is KotlinSourceElement) {
+            return modules.singleOrNull() ?: modules.firstOrNull { sourceElement.psi.getContainingKtFile().virtualFile.path in it.getSourceFiles() }
+        }
+        else {
+            return modules.firstOrNull { module ->
+                isContainedByCompiledPartOfOurModule(descriptor, File(module.getOutputDirectory())) ||
+                module.getFriendPaths().any { isContainedByCompiledPartOfOurModule(descriptor, File(it))}
+            }
+        }
     }
 }
 

compiler/util/src/org/jetbrains/kotlin/modules/Module.kt

@@ -23,6 +23,8 @@ public interface Module {
 
     public fun getOutputDirectory(): String
 
+    public fun getFriendPaths(): List<String>
+
     public fun getSourceFiles(): List<String>
 
     public fun getClasspathRoots(): List<String>

jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinBuilderModuleScriptGenerator.java

@@ -32,6 +32,7 @@ import org.jetbrains.jps.builders.logging.ProjectBuilderLogger;
 import org.jetbrains.jps.incremental.CompileContext;
 import org.jetbrains.jps.incremental.ModuleBuildTarget;
 import org.jetbrains.jps.incremental.ProjectBuildException;
+import org.jetbrains.jps.model.java.JpsJavaExtensionService;
 import org.jetbrains.kotlin.config.IncrementalCompilation;
 import org.jetbrains.kotlin.modules.KotlinModuleXmlBuilder;
 
@@ -61,6 +62,11 @@ public class KotlinBuilderModuleScriptGenerator {
         ProjectBuilderLogger logger = context.getLoggingManager().getProjectBuilderLogger();
         for (ModuleBuildTarget target : chunk.getTargets()) {
             File outputDir = getOutputDirSafe(target);
+            List<File> friendDirs = new ArrayList<File>();
+            File friendDir = getFriendDirSafe(target);
+            if (friendDir != null) {
+                friendDirs.add(friendDir);
+            }
 
             List<File> moduleSources = new ArrayList<File>(
                     IncrementalCompilation.isEnabled()
@@ -85,7 +91,8 @@ public class KotlinBuilderModuleScriptGenerator {
                     findClassPathRoots(target),
                     (JavaModuleBuildTargetType) targetType,
                     // this excludes the output directories from the class path, to be removed for true incremental compilation
-                    outputDirs
+                    outputDirs,
+                    friendDirs
             );
         }
 
@@ -107,6 +114,17 @@ public class KotlinBuilderModuleScriptGenerator {
         return outputDir;
     }
 
+    @Nullable
+    public static File getFriendDirSafe(@NotNull ModuleBuildTarget target) throws ProjectBuildException {
+        if (!target.isTests()) return null;
+
+        File outputDirForProduction = JpsJavaExtensionService.getInstance().getOutputDirectory(target.getModule(), false);
+        if (outputDirForProduction == null) {
+            throw new ProjectBuildException("No output production directory found for " + target);
+        }
+        return outputDirForProduction;
+    }
+
     @NotNull
     private static Collection<File> findClassPathRoots(@NotNull ModuleBuildTarget target) {
         return ContainerUtil.filter(getAllDependencies(target).classes().getRoots(), new Condition<File>() {

jps-plugin/src/org/jetbrains/kotlin/modules/KotlinModuleXmlBuilder.java

@@ -24,6 +24,7 @@ import org.jetbrains.kotlin.utils.Printer;
 
 import java.io.File;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 
@@ -47,7 +48,8 @@ public class KotlinModuleXmlBuilder {
             List<JvmSourceRoot> javaSourceRoots,
             Collection<File> classpathRoots,
             JavaModuleBuildTargetType targetType,
-            Set<File> directoriesToFilterOut
+            Set<File> directoriesToFilterOut,
+            @NotNull List<File> friendDirs
     ) {
         assert !done : "Already done";
 
@@ -65,6 +67,10 @@ public class KotlinModuleXmlBuilder {
         );
         p.pushIndent();
 
+        for (File friendDir : friendDirs) {
+            p.println("<", FRIEND_DIR, " ", PATH, "=\"", getEscapedPath(friendDir), "\"/>");
+        }
+
         for (File sourceFile : sourceFiles) {
             p.println("<", SOURCES, " ", PATH, "=\"", getEscapedPath(sourceFile), "\"/>");
         }

jps-plugin/test/org/jetbrains/kotlin/jps/build/KotlinJpsBuildTest.kt

@@ -503,6 +503,21 @@ public class KotlinJpsBuildTest : AbstractKotlinJpsBuildTestCase() {
         checkWhen(touch("module2/src/b.kt"), null, packageClasses("module2", "module2/src/b.kt", "test.TestPackage"))
     }
 
+    public fun testCircularDependenciesSamePackageWithTests() {
+        initProject()
+        val result = makeAll()
+        result.assertSuccessful()
+
+        // Check that outputs are located properly
+        val facadeWithA = findFileInOutputDir(findModule("module1"), "test/AKt.class")
+        val facadeWithB = findFileInOutputDir(findModule("module2"), "test/BKt.class")
+        UsefulTestCase.assertSameElements(getMethodsOfClass(facadeWithA), "<clinit>", "a", "funA", "getA")
+        UsefulTestCase.assertSameElements(getMethodsOfClass(facadeWithB), "<clinit>", "b", "funB", "getB", "setB")
+
+        checkWhen(touch("module1/src/a.kt"), null, packageClasses("module1", "module1/src/a.kt", "test.TestPackage"))
+        checkWhen(touch("module2/src/b.kt"), null, packageClasses("module2", "module2/src/b.kt", "test.TestPackage"))
+    }
+
     public fun testInternalFromAnotherModule() {
         initProject()
         val result = makeAll()
@@ -517,6 +532,13 @@ public class KotlinJpsBuildTest : AbstractKotlinJpsBuildTestCase() {
         result.checkErrors()
     }
 
+    public fun testCircularDependenciesWrongInternalFromTests() {
+        initProject()
+        val result = makeAll()
+        result.assertFailed()
+        result.checkErrors()
+    }
+
     public fun testCircularDependencyWithReferenceToOldVersionLib() {
         initProject()
 

jps-plugin/test/org/jetbrains/kotlin/jvm/compiler/ClasspathOrderTest.kt

@@ -47,7 +47,8 @@ public class ClasspathOrderTest : TestCaseWithTmpdir() {
                 listOf(JvmSourceRoot(sourceDir)),
                 listOf(PathUtil.getKotlinPathsForDistDirectory().getRuntimePath()),
                 JavaModuleBuildTargetType.PRODUCTION,
-                setOf()
+                setOf(),
+                emptyList()
         ).asText().toString()
 
         val xml = File(tmpdir, "module.xml")

jps-plugin/test/org/jetbrains/kotlin/modules/KotlinModuleXmlGeneratorTest.java

@@ -34,7 +34,8 @@ public class KotlinModuleXmlGeneratorTest extends TestCase {
                 Collections.singletonList(new JvmSourceRoot(new File("java"), null)),
                 Arrays.asList(new File("cp1"), new File("cp2")),
                 JavaModuleBuildTargetType.PRODUCTION,
-                Collections.<File>emptySet()
+                Collections.<File>emptySet(),
+                Collections.<File>emptyList()
         ).asText().toString();
         KotlinTestUtils.assertEqualsToFile(new File("idea/testData/modules.xml/basic.xml"), actual);
     }
@@ -47,7 +48,8 @@ public class KotlinModuleXmlGeneratorTest extends TestCase {
                 Collections.<JvmSourceRoot>emptyList(),
                 Arrays.asList(new File("cp1"), new File("cp2")),
                 JavaModuleBuildTargetType.PRODUCTION,
-                Collections.singleton(new File("cp1"))
+                Collections.singleton(new File("cp1")),
+                Collections.<File>emptyList()
         ).asText().toString();
         KotlinTestUtils.assertEqualsToFile(new File("idea/testData/modules.xml/filtered.xml"), actual);
     }
@@ -61,7 +63,8 @@ public class KotlinModuleXmlGeneratorTest extends TestCase {
                 Collections.<JvmSourceRoot>emptyList(),
                 Arrays.asList(new File("cp1"), new File("cp2")),
                 JavaModuleBuildTargetType.PRODUCTION,
-                Collections.singleton(new File("cp1"))
+                Collections.singleton(new File("cp1")),
+                Collections.<File>emptyList()
         );
         builder.addModule(
                 "name2",
@@ -70,7 +73,8 @@ public class KotlinModuleXmlGeneratorTest extends TestCase {
                 Collections.<JvmSourceRoot>emptyList(),
                 Arrays.asList(new File("cp12"), new File("cp22")),
                 JavaModuleBuildTargetType.TEST,
-                Collections.singleton(new File("cp12"))
+                Collections.singleton(new File("cp12")),
+                Collections.<File>emptyList()
         );
         String actual = builder.asText().toString();
         KotlinTestUtils.assertEqualsToFile(new File("idea/testData/modules.xml/multiple.xml"), actual);

jps-plugin/testData/general/CircularDependenciesSamePackageWithTests/kotlinProject.ipr

@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project version="4">
+  <component name="CompilerConfiguration">
+    <option name="DEFAULT_COMPILER" value="Javac" />
+    <resourceExtensions />
+    <annotationProcessing>
+      <profile default="true" name="Default" enabled="false">
+        <processorPath useClasspath="true" />
+      </profile>
+    </annotationProcessing>
+  </component>
+  <component name="CopyrightManager" default="">
+    <module2copyright />
+  </component>
+  <component name="DependencyValidationManager">
+    <option name="SKIP_IMPORT_STATEMENTS" value="false" />
+  </component>
+  <component name="Encoding" useUTFGuessing="true" native2AsciiForPropertiesFiles="false" />
+  <component name="ProjectModuleManager">
+    <modules>
+      <module fileurl="file://$PROJECT_DIR$/module1/module1.iml" filepath="$PROJECT_DIR$/module1/module1.iml" />
+      <module fileurl="file://$PROJECT_DIR$/module2/module2.iml" filepath="$PROJECT_DIR$/module2/module2.iml" />
+    </modules>
+  </component>
+  <component name="ProjectRootManager" version="2" languageLevel="JDK_1_6" assert-keyword="true" jdk-15="true" project-jdk-name="IDEA_JDK" project-jdk-type="JavaSDK">
+    <output url="file://$PROJECT_DIR$/out" />
+  </component>
+  <component name="VcsDirectoryMappings">
+    <mapping directory="" vcs="" />
+  </component>
+</project>
+

jps-plugin/testData/general/CircularDependenciesSamePackageWithTests/module1/module1.iml

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+      <sourceFolder url="file://$MODULE_DIR$/test" isTestSource="true" />
+    </content>
+    <orderEntry type="jdk" jdkName="IDEA_JDK" jdkType="JavaSDK" />
+    <orderEntry type="sourceFolder" forTests="false" />
+    <orderEntry type="module" module-name="module2" />
+    <orderEntry type="library" name="KotlinRuntime" level="project" />
+  </component>
+</module>
+

jps-plugin/testData/general/CircularDependenciesSamePackageWithTests/module1/src/a.kt

@@ -0,0 +1,9 @@
+package test
+
+fun a() {
+
+}
+
+internal fun funA() {}
+
+val a = ""

jps-plugin/testData/general/CircularDependenciesSamePackageWithTests/module1/test/test_a_1.kt

@@ -0,0 +1,3 @@
+package test
+
+internal fun testFunA() {}

jps-plugin/testData/general/CircularDependenciesSamePackageWithTests/module1/test/test_a_2.kt

@@ -0,0 +1,6 @@
+package test
+
+fun foo() {
+    funA()
+    testFunA()
+}

jps-plugin/testData/general/CircularDependenciesSamePackageWithTests/module2/module2.iml

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+      <sourceFolder url="file://$MODULE_DIR$/test" isTestSource="true" />
+    </content>
+    <orderEntry type="jdk" jdkName="IDEA_JDK" jdkType="JavaSDK" />
+    <orderEntry type="sourceFolder" forTests="false" />
+    <orderEntry type="module" module-name="module1" />
+    <orderEntry type="library" name="KotlinRuntime" level="project" />
+  </component>
+</module>
+

jps-plugin/testData/general/CircularDependenciesSamePackageWithTests/module2/src/b.kt

@@ -0,0 +1,9 @@
+package test
+
+fun b() {
+
+}
+
+internal fun funB() {}
+
+var b = b()

jps-plugin/testData/general/CircularDependenciesSamePackageWithTests/module2/test/test_b_1.kt

@@ -0,0 +1,3 @@
+package test
+
+internal fun testFunB() {}

jps-plugin/testData/general/CircularDependenciesSamePackageWithTests/module2/test/test_b_2.kt

@@ -0,0 +1,6 @@
+package test
+
+fun bar() {
+    funB()
+    testFunB()
+}

jps-plugin/testData/general/CircularDependenciesWrongInternalFromTests/errors.txt

@@ -0,0 +1,4 @@
+Cannot access 'funA': it is 'internal' in 'test' at line 4, column 5
+Cannot access 'funB': it is 'internal' in 'test' at line 4, column 5
+Cannot access 'testFunA': it is 'internal' in 'test' at line 5, column 5
+Cannot access 'testFunB': it is 'internal' in 'test' at line 5, column 5

jps-plugin/testData/general/CircularDependenciesWrongInternalFromTests/kotlinProject.ipr

@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project version="4">
+  <component name="CompilerConfiguration">
+    <option name="DEFAULT_COMPILER" value="Javac" />
+    <resourceExtensions />
+    <annotationProcessing>
+      <profile default="true" name="Default" enabled="false">
+        <processorPath useClasspath="true" />
+      </profile>
+    </annotationProcessing>
+  </component>
+  <component name="CopyrightManager" default="">
+    <module2copyright />
+  </component>
+  <component name="DependencyValidationManager">
+    <option name="SKIP_IMPORT_STATEMENTS" value="false" />
+  </component>
+  <component name="Encoding" useUTFGuessing="true" native2AsciiForPropertiesFiles="false" />
+  <component name="ProjectModuleManager">
+    <modules>
+      <module fileurl="file://$PROJECT_DIR$/module1/module1.iml" filepath="$PROJECT_DIR$/module1/module1.iml" />
+      <module fileurl="file://$PROJECT_DIR$/module2/module2.iml" filepath="$PROJECT_DIR$/module2/module2.iml" />
+    </modules>
+  </component>
+  <component name="ProjectRootManager" version="2" languageLevel="JDK_1_6" assert-keyword="true" jdk-15="true" project-jdk-name="IDEA_JDK" project-jdk-type="JavaSDK">
+    <output url="file://$PROJECT_DIR$/out" />
+  </component>
+  <component name="VcsDirectoryMappings">
+    <mapping directory="" vcs="" />
+  </component>
+</project>
+

jps-plugin/testData/general/CircularDependenciesWrongInternalFromTests/module1/module1.iml

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+      <sourceFolder url="file://$MODULE_DIR$/test" isTestSource="true" />
+    </content>
+    <orderEntry type="jdk" jdkName="IDEA_JDK" jdkType="JavaSDK" />
+    <orderEntry type="sourceFolder" forTests="false" />
+    <orderEntry type="module" module-name="module2" />
+    <orderEntry type="library" name="KotlinRuntime" level="project" />
+  </component>
+</module>
+

jps-plugin/testData/general/CircularDependenciesWrongInternalFromTests/module1/src/a.kt

@@ -0,0 +1,9 @@
+package test
+
+fun a() {
+
+}
+
+internal fun funA() {}
+
+val a = ""

jps-plugin/testData/general/CircularDependenciesWrongInternalFromTests/module1/test/test_a_1.kt

@@ -0,0 +1,3 @@
+package test
+
+internal fun testFunA() {}

jps-plugin/testData/general/CircularDependenciesWrongInternalFromTests/module1/test/test_a_2.kt

@@ -0,0 +1,6 @@
+package test
+
+fun foo() {
+    funB()
+    testFunB()
+}

jps-plugin/testData/general/CircularDependenciesWrongInternalFromTests/module2/module2.iml

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+      <sourceFolder url="file://$MODULE_DIR$/test" isTestSource="true" />
+    </content>
+    <orderEntry type="jdk" jdkName="IDEA_JDK" jdkType="JavaSDK" />
+    <orderEntry type="sourceFolder" forTests="false" />
+    <orderEntry type="module" module-name="module1" />
+    <orderEntry type="library" name="KotlinRuntime" level="project" />
+  </component>
+</module>
+

jps-plugin/testData/general/CircularDependenciesWrongInternalFromTests/module2/src/b.kt

@@ -0,0 +1,9 @@
+package test
+
+fun b() {
+
+}
+
+internal fun funB() {}
+
+var b = b()

jps-plugin/testData/general/CircularDependenciesWrongInternalFromTests/module2/test/test_b_1.kt

@@ -0,0 +1,3 @@
+package test
+
+internal fun testFunB() {}

jps-plugin/testData/general/CircularDependenciesWrongInternalFromTests/module2/test/test_b_2.kt

@@ -0,0 +1,6 @@
+package test
+
+fun bar() {
+    funA()
+    testFunA()
+}