Refactor: make fragments resolver consistent with variant resolver

libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/GradleProjectModuleBuilder.kt

@@ -108,7 +108,7 @@ class GradleProjectModuleBuilder(private val addInferredSourceSetVisibilityAsExp
             extension.sourceSets.forEach { sourceSet ->
                 val existingVariant = fragments.filterIsInstance<BasicKotlinModuleVariant>().find { it.fragmentName == sourceSet.name }
                 val fragment = existingVariant ?: BasicKotlinModuleFragment(this@apply, sourceSet.name).also { fragments.add(it) }
-                fragment.kotlinSourceDirectories = sourceSet.kotlin.sourceDirectories.toList()
+                fragment.kotlinSourceRoots = sourceSet.kotlin.sourceDirectories.toList()
 
                 // FIXME: Kotlin/Native implementation-effective-api dependencies are missing here. Introduce dependency scopes
                 project.configurations.getByName(sourceSet.apiConfigurationName).allDependencies.forEach {

libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/pm20/DependencyResolution.kt

@@ -65,7 +65,7 @@ class FragmentDependenciesDiscovery(
     private val project: Project,
     private val expansion: InternalDependencyExpansion,
     private val moduleResolver: ModuleDependencyResolver,
-    private val fragmentResolver: KotlinModuleFragmentResolver
+    private val fragmentsResolver: KotlinModuleFragmentsResolver
 ) : DependencyDiscovery {
     override fun discoverDependencies(fragment: KotlinModuleFragment): Iterable<ModuleDependency> {
         require(fragment.containingModule.representsProject(project))
@@ -93,11 +93,13 @@ class FragmentDependenciesDiscovery(
             val module = moduleResolver.resolveDependency(component.toModuleDependency())
                 ?: buildStubModule(component, component.variants.singleOrNull()?.displayName ?: "default")
 
-            val visibleFragments = fragmentResolver.getChosenFragments(fragment, module)
+            val newTransitiveDeps = when (val fragmentsResolution = fragmentsResolver.getChosenFragments(fragment, module)) {
-            val newTransitiveDeps =
+                is FragmentResolution.ChosenFragments ->
-                visibleFragments.chosenFragments.flatMap { it.declaredModuleDependencies }.mapNotNull { dependency ->
+                    fragmentsResolution.visibleFragments.flatMap { it.declaredModuleDependencies }.mapNotNull { dependency ->
-                    componentByModuleDependency[dependency].takeIf { component -> component !in visited }
+                        componentByModuleDependency[dependency].takeIf { component -> component !in visited }
-                }
+                    }
+                else -> emptyList()
+            }
 
             resolvedComponentQueue.addAll(newTransitiveDeps)
         }

libraries/tools/kotlin-project-model/src/main/kotlin/KotlinModuleFragmentResolver.kt

@@ -1,45 +0,0 @@
-/*
- * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors.
- * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
- */
-
-package org.jetbrains.kotlin.project.model
-
-import org.jetbrains.kotlin.project.model.utils.variantsContainingFragment
-
-interface KotlinModuleFragmentResolver {
-    fun getChosenFragments(dependingFragment: KotlinModuleFragment, dependencyModule: KotlinModule): KotlinChosenFragments
-}
-
-class KotlinChosenFragments(
-    val module: KotlinModule,
-    val chosenFragments: Iterable<KotlinModuleFragment>,
-    val variantMatchingResults: Iterable<VariantResolution>
-)
-
-class DefaultKotlinModuleFragmentResolver(
-    private val variantResolver: ModuleVariantResolver
-) : KotlinModuleFragmentResolver {
-    override fun getChosenFragments(dependingFragment: KotlinModuleFragment, dependencyModule: KotlinModule): KotlinChosenFragments {
-        val dependingModule = dependingFragment.containingModule
-        val containingVariants = dependingModule.variantsContainingFragment(dependingFragment)
-
-        val chosenVariants = containingVariants.map { variantResolver.getChosenVariant(it, dependencyModule) }
-
-        val chosenFragments = chosenVariants.map { variantResolution ->
-            when (variantResolution) {
-                is VariantResolution.VariantMatch -> variantResolution.chosenVariant.refinesClosure
-                else -> emptySet()
-            }
-        }
-
-        val result = if (chosenFragments.isEmpty())
-            emptyList<KotlinModuleFragment>()
-        else chosenFragments
-            // Note this emulates the existing behavior that is lenient wrt to unresolved modules, but gives imprecise results. TODO revisit
-            .filter { it.isNotEmpty() }
-            .reduce { acc, it -> acc.intersect(it) }
-
-        return KotlinChosenFragments(dependencyModule, result, chosenVariants)
-    }
-}

libraries/tools/kotlin-project-model/src/main/kotlin/KotlinModuleFragmentsResolver.kt

@@ -0,0 +1,61 @@
+/*
+ * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.project.model
+
+import org.jetbrains.kotlin.project.model.utils.variantsContainingFragment
+
+interface KotlinModuleFragmentsResolver {
+    fun getChosenFragments(
+        requestingFragment: KotlinModuleFragment,
+        dependencyModule: KotlinModule
+    ): FragmentResolution
+}
+
+sealed class FragmentResolution(val requestingFragment: KotlinModuleFragment, val dependencyModule: KotlinModule) {
+    class ChosenFragments(
+        requestingFragment: KotlinModuleFragment,
+        dependencyModule: KotlinModule,
+        val visibleFragments: Iterable<KotlinModuleFragment>,
+        val variantResolutions: Iterable<VariantResolution>
+    ) : FragmentResolution(requestingFragment, dependencyModule)
+
+    class NotRequested(requestingFragment: KotlinModuleFragment, dependencyModule: KotlinModule) :
+        FragmentResolution(requestingFragment, dependencyModule)
+
+    // TODO: think about restricting calls with the type system to avoid partial functions in resolvers?
+    class Unknown(requestingFragment: KotlinModuleFragment, dependencyModule: KotlinModule) :
+        FragmentResolution(requestingFragment, dependencyModule)
+}
+
+class DefaultKotlinModuleFragmentsResolver(
+    private val variantResolver: ModuleVariantResolver
+) : KotlinModuleFragmentsResolver {
+    override fun getChosenFragments(
+        requestingFragment: KotlinModuleFragment,
+        dependencyModule: KotlinModule
+    ): FragmentResolution.ChosenFragments {
+        val dependingModule = requestingFragment.containingModule
+        val containingVariants = dependingModule.variantsContainingFragment(requestingFragment)
+
+        val chosenVariants = containingVariants.map { variantResolver.getChosenVariant(it, dependencyModule) }
+
+        val chosenFragments = chosenVariants.map { variantResolution ->
+            when (variantResolution) {
+                is VariantResolution.VariantMatch -> variantResolution.chosenVariant.refinesClosure
+                else -> emptySet()
+            }
+        }
+
+        val result = if (chosenFragments.isEmpty())
+            emptyList<KotlinModuleFragment>()
+        else chosenFragments
+            // Note this emulates the existing behavior that is lenient wrt to unresolved modules, but gives imprecise results. TODO revisit
+            .filter { it.isNotEmpty() }
+            .reduce { acc, it -> acc.intersect(it) }
+
+        return FragmentResolution.ChosenFragments(requestingFragment, dependencyModule, result, chosenVariants)
+    }
+}

libraries/tools/kotlin-project-model/src/main/kotlin/ModuleVariantResolver.kt

@@ -60,6 +60,7 @@ sealed class VariantResolution(
         val chosenVariant: KotlinModuleVariant
     ) : VariantResolution(requestingVariant, dependencyModule)
 
+    // TODO: think about restricting calls with the type system to avoid partial functions in resolvers?
     class Unknown(requestingVariant: KotlinModuleVariant, dependencyModule: KotlinModule) :
         VariantResolution(requestingVariant, dependencyModule)
 

libraries/tools/kotlin-project-model/src/test/kotlin/org/jetbrains/kotlin/project/model/DefaultKotlinModuleFragmentsResolverTest.kt

@@ -5,16 +5,15 @@
 
 package org.jetbrains.kotlin.project.model
 
-import org.junit.Assume
 import org.junit.Assume.assumeTrue
 import org.junit.Test
 import kotlin.test.assertEquals
 
-class DefaultKotlinModuleFragmentResolverTest {
+class DefaultKotlinModuleFragmentsResolverTest {
     val moduleFoo = simpleModule("foo")
     val moduleBar = simpleModule("bar")
 
-    val fragmentResolver = DefaultKotlinModuleFragmentResolver(MatchVariantsByExactAttributes())
+    val fragmentResolver = DefaultKotlinModuleFragmentsResolver(MatchVariantsByExactAttributes())
 
     @Test
     fun testFragmentVisibility() {
@@ -30,7 +29,7 @@ class DefaultKotlinModuleFragmentResolverTest {
         moduleBar.fragments.forEach { consumingFragment ->
             val result = fragmentResolver.getChosenFragments(consumingFragment, moduleFoo)
             val expected = expectedVisibleFragments.getValue(consumingFragment.fragmentName.removeSuffix("Main").removeSuffix("Test"))
-            assertEquals(expected, result.chosenFragments.map { it.fragmentName }.toSet())
+            assertEquals(expected, result.visibleFragments.map { it.fragmentName }.toSet())
         }
     }
 
@@ -45,7 +44,7 @@ class DefaultKotlinModuleFragmentResolverTest {
         assumeTrue(MatchVariantsByExactAttributes().getChosenVariant(dependingModule.variant("linuxMain"), moduleFoo) is NoVariantMatch)
 
         val (commonMainResult, jsAndLinuxResult) = listOf("commonMain", "jsAndLinuxMain").map {
-            fragmentResolver.getChosenFragments(dependingModule.fragment(it), moduleFoo).chosenFragments.map { it.fragmentName }.toSet()
+            fragmentResolver.getChosenFragments(dependingModule.fragment(it), moduleFoo).visibleFragments.map { it.fragmentName }.toSet()
         }
 
         assertEquals(setOf("commonMain", "jvmAndJsMain"), commonMainResult)