JS: support internal visibility from friend modules

Friend modules should be provided using the -Xfriend-modules flag in the same format as -libraries. No manual configuration required for JPS, Gradle and Maven plugins. Friend modules could be switched off using the -Xfriend-modules-disabled flag. Doing that will * prevent internal declarations from being exported, * values provided by -Xfriend-modules ignored, * raise a compilation error on attemps to use internal declarations from other modules Fixes #KT-15135 and #KT-16568.
ohmerhe · May 4, 2017 · 2e9a598 · 2e9a598
1 parent 7bbf986
commit 2e9a598
Show file tree

Hide file tree

Showing 48 changed files with 767 additions and 135 deletions.
diff --git a/...r/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/K2JSCompilerArguments.java b/...r/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/K2JSCompilerArguments.java
@@ -79,9 +79,20 @@ public class K2JSCompilerArguments extends CommonCompilerArguments {
     // Advanced options
 
     @GradleOption(DefaultValues.BooleanFalseDefault.class)
-    @Argument(value = "-Xtypedarrays", description = "Translate primitive arrays to JS typed arrays")
+    @Argument(value = "-Xtyped-arrays", description = "Translate primitive arrays to JS typed arrays")
     public boolean typedArrays;
 
+    @GradleOption(DefaultValues.BooleanFalseDefault.class)
+    @Argument(value = "-Xfriend-modules-disabled", description = "Disable internal declaration export")
+    public boolean friendModulesDisabled;
+
+    @Argument(
+            value = "-Xfriend-modules",
+            valueDescription = "<path>",
+            description = "Paths to friend modules"
+    )
+    public String friendModules;
+
     @NotNull
     public static K2JSCompilerArguments createDefaultInstance() {
         K2JSCompilerArguments arguments = new K2JSCompilerArguments();

diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/js/K2JSCompiler.java b/compiler/cli/src/org/jetbrains/kotlin/cli/js/K2JSCompiler.java
@@ -274,11 +274,19 @@ protected void setupPlatformSpecificArgumentsAndServices(
             ContainerUtil.addAll(libraries, ArraysKt.filterNot(arguments.libraries.split(File.pathSeparator), String::isEmpty));
         }
 
+        configuration.put(JSConfigurationKeys.LIBRARIES, libraries);
+
+
         if (arguments.typedArrays) {
             configuration.put(JSConfigurationKeys.TYPED_ARRAYS_ENABLED, true);
         }
 
-        configuration.put(JSConfigurationKeys.LIBRARIES, libraries);
+        configuration.put(JSConfigurationKeys.FRIEND_PATHS_DISABLED, arguments.friendModulesDisabled);
+
+        if (!arguments.friendModulesDisabled && arguments.friendModules != null) {
+            List<String> friendPaths = ArraysKt.filterNot(arguments.friendModules.split(File.pathSeparator), String::isEmpty);
+            configuration.put(JSConfigurationKeys.FRIEND_PATHS, friendPaths);
+        }
 
         String moduleKindName = arguments.moduleKind;
         ModuleKind moduleKind = moduleKindName != null ? moduleKindMap.get(moduleKindName) : ModuleKind.PLAIN;

diff --git a/compiler/testData/cli/js/jsExtraHelp.out b/compiler/testData/cli/js/jsExtraHelp.out
@@ -1,6 +1,8 @@
 Usage: kotlinc-js <options> <source files>
 where advanced options include:
-  -Xtypedarrays              Translate primitive arrays to JS typed arrays
+  -Xtyped-arrays             Translate primitive arrays to JS typed arrays
+  -Xfriend-modules-disabled  Disable internal declaration export
+  -Xfriend-modules=<path>    Paths to friend modules
   -Xno-inline                Disable method inlining
   -Xrepeat=<count>           Repeat compilation (for performance analysis)
   -Xskip-metadata-version-check Load classes with bad metadata version anyway (incl. pre-release classes)
@@ -14,4 +16,4 @@ where advanced options include:
                              Enable coroutines or report warnings or errors on declarations and use sites of 'suspend' modifier
 
 Advanced options are non-standard and may be changed or removed without any notice.
-OK
+OK
diff --git a/compiler/testData/codegen/box/mangling/internal.kt b/compiler/testData/codegen/box/mangling/internal.kt
@@ -0,0 +1,37 @@
+// MODULE: lib
+// FILE: lib.kt
+
+package lib
+
+internal fun foo() = 1
+
+internal val bar = 2
+
+internal class A {
+    internal fun baz(a: Int): Int {
+        return a * 10
+    }
+
+    internal val foo = 3
+
+    internal inner class B {
+        internal fun foo() = 4
+    }
+}
+
+// MODULE: main(lib)(lib)
+// FILE: main.kt
+
+package main
+
+import lib.*
+
+fun box(): String {
+    if (foo() != 1) return "fail 1: ${foo()}"
+    if (bar != 2) return "fail 2: ${bar}"
+    val a = A()
+    if (a.baz(10) != 100) return "fail 3: ${a.baz(10)}"
+    if (a.foo != 3) return "fail 4: ${a.foo}"
+    if (a.B().foo() != 4) return "fail 5: ${a.B().foo()}"
+    return "OK"
+}
diff --git a/compiler/tests-common/org/jetbrains/kotlin/checkers/KotlinMultiFileTestWithJava.java b/compiler/tests-common/org/jetbrains/kotlin/checkers/KotlinMultiFileTestWithJava.java
@@ -46,10 +46,12 @@ public abstract class KotlinMultiFileTestWithJava<M, F> extends KotlinTestWithEn
     public class ModuleAndDependencies {
         final M module;
         final List<String> dependencies;
+        final List<String> friends;
 
-        ModuleAndDependencies(M module, List<String> dependencies) {
+        ModuleAndDependencies(M module, List<String> dependencies, List<String> friends) {
             this.module = module;
             this.dependencies = dependencies;
+            this.friends = friends;
         }
     }
 
@@ -129,9 +131,9 @@ public F createFile(
                     }
 
                     @Override
-                    public M createModule(@NotNull String name, @NotNull List<String> dependencies) {
+                    public M createModule(@NotNull String name, @NotNull List<String> dependencies, @NotNull List<String> friends) {
                         M module = createTestModule(name);
-                        ModuleAndDependencies oldValue = modules.put(name, new ModuleAndDependencies(module, dependencies));
+                        ModuleAndDependencies oldValue = modules.put(name, new ModuleAndDependencies(module, dependencies, friends));
                         assert oldValue == null : "Module " + name + " declared more than once";
 
                         return module;

diff --git a/compiler/tests-common/org/jetbrains/kotlin/test/KotlinTestUtils.java b/compiler/tests-common/org/jetbrains/kotlin/test/KotlinTestUtils.java
@@ -118,7 +118,7 @@ public class KotlinTestUtils {
     private static final String MODULE_DELIMITER = ",\\s*";
 
     private static final Pattern FILE_OR_MODULE_PATTERN = Pattern.compile(
-            "(?://\\s*MODULE:\\s*([^()\\n]+)(?:\\(([^()]+(?:" + MODULE_DELIMITER + "[^()]+)*)\\))?\\s*)?" +
+            "(?://\\s*MODULE:\\s*([^()\\n]+)(?:\\(([^()]+(?:" + MODULE_DELIMITER + "[^()]+)*)\\))?\\s*(?:\\(([^()]+(?:" + MODULE_DELIMITER + "[^()]+)*)\\))?\\s*)?" +
             "//\\s*FILE:\\s*(.*)$", Pattern.MULTILINE);
     private static final Pattern DIRECTIVE_PATTERN = Pattern.compile("^//\\s*!([\\w_]+)(:\\s*(.*)$)?", Pattern.MULTILINE);
 
@@ -610,7 +610,7 @@ public static boolean compileKotlinWithJava(
 
     public interface TestFileFactory<M, F> {
         F createFile(@Nullable M module, @NotNull String fileName, @NotNull String text, @NotNull Map<String, String> directives);
-        M createModule(@NotNull String name, @NotNull List<String> dependencies);
+        M createModule(@NotNull String name, @NotNull List<String> dependencies, @NotNull List<String> friends);
     }
 
     public static abstract class TestFileFactoryNoModules<F> implements TestFileFactory<Void, F> {
@@ -628,7 +628,7 @@ public final F createFile(
         public abstract F create(@NotNull String fileName, @NotNull String text, @NotNull Map<String, String> directives);
 
         @Override
-        public Void createModule(@NotNull String name, @NotNull List<String> dependencies) {
+        public Void createModule(@NotNull String name, @NotNull List<String> dependencies, @NotNull List<String> friends) {
             return null;
         }
     }
@@ -651,12 +651,13 @@ public static <M, F> List<F> createTestFiles(String testFileName, String expecte
             while (true) {
                 String moduleName = matcher.group(1);
                 String moduleDependencies = matcher.group(2);
+                String moduleFriends = matcher.group(3);
                 if (moduleName != null) {
                     hasModules = true;
-                    module = factory.createModule(moduleName, parseDependencies(moduleDependencies));
+                    module = factory.createModule(moduleName, parseModuleList(moduleDependencies), parseModuleList(moduleFriends));
                 }
 
-                String fileName = matcher.group(3);
+                String fileName = matcher.group(4);
                 int start = processedChars;
 
                 boolean nextFileExists = matcher.find();
@@ -681,7 +682,7 @@ public static <M, F> List<F> createTestFiles(String testFileName, String expecte
         }
 
         if (isDirectiveDefined(expectedText, "WITH_COROUTINES")) {
-            M supportModule = hasModules ? factory.createModule("support", Collections.emptyList()) : null;
+            M supportModule = hasModules ? factory.createModule("support", Collections.emptyList(), Collections.emptyList()) : null;
             testFiles.add(factory.createFile(supportModule,
                                              "CoroutineUtil.kt",
                                              "import kotlin.coroutines.experimental.*\n" +
@@ -715,7 +716,7 @@ public static <M, F> List<F> createTestFiles(String testFileName, String expecte
         return testFiles;
     }
 
-    private static List<String> parseDependencies(@Nullable String dependencies) {
+    private static List<String> parseModuleList(@Nullable String dependencies) {
         if (dependencies == null) return Collections.emptyList();
         return StringsKt.split(dependencies, Pattern.compile(MODULE_DELIMITER), 0);
     }

diff --git a/...er/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/...er/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java
@@ -10535,6 +10535,12 @@ public void testFun() throws Exception {
             doTest(fileName);
         }
 
+        @TestMetadata("internal.kt")
+        public void testInternal() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/mangling/internal.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("internalOverride.kt")
         public void testInternalOverride() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/mangling/internalOverride.kt");

diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java
@@ -10535,6 +10535,12 @@ public void testFun() throws Exception {
             doTest(fileName);
         }
 
+        @TestMetadata("internal.kt")
+        public void testInternal() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/mangling/internal.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("internalOverride.kt")
         public void testInternalOverride() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/mangling/internalOverride.kt");

diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java
@@ -10535,6 +10535,12 @@ public void testFun() throws Exception {
             doTest(fileName);
         }
 
+        @TestMetadata("internal.kt")
+        public void testInternal() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/mangling/internal.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("internalOverride.kt")
         public void testInternalOverride() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/mangling/internalOverride.kt");

diff --git a/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/ModuleDescriptorImpl.kt b/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/ModuleDescriptorImpl.kt
@@ -97,6 +97,10 @@ class ModuleDescriptorImpl @JvmOverloads constructor(
         setDependencies(ModuleDependenciesImpl(descriptors, emptySet()))
     }
 
+    fun setDependencies(descriptors: List<ModuleDescriptorImpl>, friends: Set<ModuleDescriptorImpl>) {
+        setDependencies(ModuleDependenciesImpl(descriptors, friends))
+    }
+
     override fun shouldSeeInternalsOf(targetModule: ModuleDescriptor): Boolean {
         return this == targetModule || targetModule in dependencies!!.modulesWhoseInternalsAreVisible
     }

diff --git a/jps-plugin/jps-tests/test/org/jetbrains/kotlin/jps/build/KotlinJpsBuildTest.kt b/jps-plugin/jps-tests/test/org/jetbrains/kotlin/jps/build/KotlinJpsBuildTest.kt
@@ -373,6 +373,11 @@ class KotlinJpsBuildTest : AbstractKotlinJpsBuildTestCase() {
         makeAll().assertSuccessful()
     }
 
+    fun testKotlinJavaScriptInternalFromSpecialRelatedModule() {
+        initProject(JS_STDLIB)
+        makeAll().assertSuccessful()
+    }
+
     fun testKotlinJavaScriptProjectWithTests() {
         initProject(JS_STDLIB)
         makeAll().assertSuccessful()

diff --git a/jps-plugin/src/org/jetbrains/kotlin/compilerRunner/JpsKotlinCompilerRunner.kt b/jps-plugin/src/org/jetbrains/kotlin/compilerRunner/JpsKotlinCompilerRunner.kt
@@ -82,6 +82,7 @@ class JpsKotlinCompilerRunner : KotlinCompilerRunner<JpsCompilerEnvironment>() {
             environment: JpsCompilerEnvironment,
             sourceFiles: Collection<File>,
             libraries: List<String>,
+            friendModules: List<String>,
             outputFile: File
     ) {
         log.debug("K2JS: common arguments: " + ArgumentUtils.convertArgumentsToStringList(commonArguments))
@@ -90,7 +91,7 @@ class JpsKotlinCompilerRunner : KotlinCompilerRunner<JpsCompilerEnvironment>() {
         val arguments = mergeBeans(commonArguments, XmlSerializerUtil.createCopy(k2jsArguments))
         log.debug("K2JS: merged arguments: " + ArgumentUtils.convertArgumentsToStringList(arguments))
 
-        setupK2JsArguments(outputFile, sourceFiles, libraries, arguments)
+        setupK2JsArguments(outputFile, sourceFiles, libraries, friendModules, arguments)
         log.debug("K2JS: arguments after setup" + ArgumentUtils.convertArgumentsToStringList(arguments))
 
         withCompilerSettings(compilerSettings) {
@@ -203,13 +204,14 @@ class JpsKotlinCompilerRunner : KotlinCompilerRunner<JpsCompilerEnvironment>() {
         }
     }
 
-    private fun setupK2JsArguments(_outputFile: File, sourceFiles: Collection<File>, _libraries: List<String>, settings: K2JSCompilerArguments) {
+    private fun setupK2JsArguments(_outputFile: File, sourceFiles: Collection<File>, _libraries: List<String>, _friendModules: List<String>, settings: K2JSCompilerArguments) {
         with(settings) {
             noStdlib = true
             freeArgs = sourceFiles.map { it.path }
             outputFile = _outputFile.path
             metaInfo = true
             libraries = _libraries.joinToString(File.pathSeparator)
+            friendModules = _friendModules.joinToString(File.pathSeparator)
         }
     }
 

diff --git a/jps-plugin/src/org/jetbrains/kotlin/jps/build/JpsJsModuleUtils.kt b/jps-plugin/src/org/jetbrains/kotlin/jps/build/JpsJsModuleUtils.kt
@@ -51,25 +51,30 @@ object JpsJsModuleUtils {
                 if (module.moduleType != JpsJavaModuleType.INSTANCE) return
 
                 if ((module != target.module || target.isTests) && module.sourceRoots.any { it.rootType == JavaSourceRootType.SOURCE}) {
-                    addTarget(module, JavaModuleBuildTargetType.PRODUCTION)
+                    addTarget(module, isTests = false)
                 }
 
                 if (module != target.module && target.isTests && module.sourceRoots.any { it.rootType == JavaSourceRootType.TEST_SOURCE}) {
-                    addTarget(module, JavaModuleBuildTargetType.TEST)
+                    addTarget(module, isTests = true)
                 }
             }
 
-            fun addTarget(module: JpsModule, targetType: JavaModuleBuildTargetType) {
-                val moduleBuildTarget = ModuleBuildTarget(module, targetType)
-                val outputDir = KotlinBuilderModuleScriptGenerator.getOutputDirSafe(moduleBuildTarget)
-                val metaInfoFile = getOutputMetaFile(outputDir, module.name, targetType.isTests)
+            fun addTarget(module: JpsModule, isTests: Boolean) {
+                val metaInfoFile = getOutputMetaFile(module, isTests)
                 if (metaInfoFile.exists()) {
                     result.add(metaInfoFile.absolutePath)
                 }
             }
         })
     }
 
+    @JvmStatic
+    fun getOutputMetaFile(module: JpsModule, isTests: Boolean): File {
+        val moduleBuildTarget = ModuleBuildTarget(module, if (isTests) JavaModuleBuildTargetType.TEST else JavaModuleBuildTargetType.PRODUCTION)
+        val outputDir = KotlinBuilderModuleScriptGenerator.getOutputDirSafe(moduleBuildTarget)
+        return getOutputMetaFile(outputDir, module.name, isTests)
+    }
+
     @JvmStatic
     fun getOutputFile(outputDir: File, moduleName: String, isTests: Boolean)
             = File(outputDir, moduleName + suffix(isTests) + KotlinJavascriptMetadataUtils.JS_EXT)

diff --git a/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinBuilder.kt b/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinBuilder.kt
@@ -57,6 +57,7 @@ import org.jetbrains.kotlin.daemon.common.isDaemonEnabled
 import org.jetbrains.kotlin.incremental.*
 import org.jetbrains.kotlin.incremental.components.LookupTracker
 import org.jetbrains.kotlin.jps.JpsKotlinCompilerSettings
+import org.jetbrains.kotlin.jps.build.JpsJsModuleUtils.getOutputMetaFile
 import org.jetbrains.kotlin.jps.incremental.*
 import org.jetbrains.kotlin.load.kotlin.incremental.components.IncrementalCache
 import org.jetbrains.kotlin.load.kotlin.incremental.components.IncrementalCompilationComponents
@@ -661,8 +662,13 @@ class KotlinBuilder : ModuleLevelBuilder(BuilderCategory.SOURCE_PROCESSOR) {
         val compilerSettings = JpsKotlinCompilerSettings.getCompilerSettings(representativeModule)
         val k2JsArguments = JpsKotlinCompilerSettings.getK2JsCompilerArguments(representativeModule)
 
+        val friendPaths = KotlinBuilderModuleScriptGenerator.getProductionModulesWhichInternalsAreVisible(representativeTarget).mapNotNull {
+            val file = getOutputMetaFile(it, false)
+            if (file.exists()) file.absolutePath.toString() else null
+        }
+
         val compilerRunner = JpsKotlinCompilerRunner()
-        compilerRunner.runK2JsCompiler(commonArguments, k2JsArguments, compilerSettings, environment, sourceFiles, libraries, outputFile)
+        compilerRunner.runK2JsCompiler(commonArguments, k2JsArguments, compilerSettings, environment, sourceFiles, libraries, friendPaths, outputFile)
         return environment.outputItemsCollector
     }
 

diff --git a/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinBuilderModuleScriptGenerator.kt b/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinBuilderModuleScriptGenerator.kt
@@ -132,18 +132,19 @@ object KotlinBuilderModuleScriptGenerator {
     fun getOutputDirSafe(target: ModuleBuildTarget): File =
             target.outputDir ?: throw ProjectBuildException("No output directory found for " + target)
 
-    private fun getAdditionalOutputDirsWhereInternalsAreVisible(target: ModuleBuildTarget): List<File> {
-        if (!target.isTests) return emptyList()
+    fun getProductionModulesWhichInternalsAreVisible(from: ModuleBuildTarget): List<JpsModule> {
+        if (!from.isTests) return emptyList()
 
-        val result = SmartList<File>()
+        val result = SmartList<JpsModule>(from.module)
+        result.addIfNotNull(getRelatedProductionModule(from.module))
 
-        result.addIfNotNull(JpsJavaExtensionService.getInstance().getOutputDirectory(target.module, false))
+        return result
+    }
 
-        getRelatedProductionModule(target.module)?.let {
-            result.addIfNotNull(JpsJavaExtensionService.getInstance().getOutputDirectory(it, false))
+    fun getAdditionalOutputDirsWhereInternalsAreVisible(target: ModuleBuildTarget): List<File> {
+        return getProductionModulesWhichInternalsAreVisible(target).mapNotNullTo(SmartList<File>()) {
+            JpsJavaExtensionService.getInstance().getOutputDirectory(it, false)
         }
-
-        return result
     }
 
     private fun findClassPathRoots(target: ModuleBuildTarget): Collection<File> {