From 0780508f02730b5d010dc9521d0534d6ba4deddd Mon Sep 17 00:00:00 2001
From: Christian Gruber <gruber@squareup.com>
Date: Fri, 4 Oct 2019 15:52:58 -0700
Subject: [PATCH] Add more robust support for friend declarations of a library
 or test on another compilation unit, causing it to (a) share a module
 definition, and (b) have access to the internal members of that library. This
 expands the current feature which exposes this to test only, and generalizes
 it.

It also addresses an issue in kotlinc (https://youtrack.jetbrains.com/issue/KT-34277) whereby `-Xfriends-path` is interpreted using different path separators between JS and JVM. The present code uses File.pathSeparator to obtain the platform-specific separator. However, the JVM backend uses a system which separates path elements with a comma, as a list.

There's also some application of ktlint and buildifier on some files involved in this PR.

Finally, the dagger example is partitioned into smaller pieces, to illustrate and validate the use of friend declarations, directly and transitively.
---
 .bazelci/presubmit.yml                        |  4 ++
 README.md                                     | 43 ++++++++++++++++--
 WORKSPACE                                     |  4 +-
 examples/dagger/BUILD.bazel                   | 21 +++++++++
 .../dagger/{BUILD => src/coffee/BUILD.bazel}  | 41 ++++-------------
 examples/dagger/src/coffee/CoffeeMaker.kt     |  7 ++-
 .../dagger/src/coffee/DripCoffeeModule.kt     | 22 ++++++++--
 examples/dagger/src/heating/BUILD.bazel       | 24 ++++++++++
 .../src/{coffee => heating}/ElectricHeater.kt | 15 ++++++-
 .../dagger/src/{coffee => heating}/Heater.kt  | 13 +++---
 examples/dagger/src/pumping/BUILD.bazel       | 25 +++++++++++
 .../dagger/src/{coffee => pumping}/Pump.kt    |  4 +-
 .../src/{coffee => pumping}/PumpModule.kt     |  4 +-
 .../src/{coffee => pumping}/Thermosiphon.kt   | 10 +++--
 examples/dagger/src/time/BUILD.bazel          | 20 +++++++++
 examples/dagger/src/time/Delayer.kt           | 21 +++++++++
 examples/dagger/test/coffee/BUILD.bazel       | 22 ++++++++++
 examples/dagger/test/coffee/BasicTest.kt      | 21 +++++++++
 examples/dagger/test/coffee/BasicTestUtil.kt  |  5 +++
 kotlin/internal/defs.bzl                      |  1 +
 kotlin/internal/jvm/android.bzl               | 10 +++--
 kotlin/internal/jvm/compile.bzl               | 44 ++++++++++++-------
 kotlin/internal/jvm/jvm.bzl                   | 11 ++++-
 kotlin/internal/repositories/repositories.bzl |  4 +-
 .../tasks/jvm/KotlinJvmTaskExecutor.kt        | 19 +++++---
 .../bazel/kotlin/KotlinAssertionTestCase.kt   |  4 +-
 26 files changed, 331 insertions(+), 88 deletions(-)
 create mode 100644 examples/dagger/BUILD.bazel
 rename examples/dagger/{BUILD => src/coffee/BUILD.bazel} (57%)
 create mode 100644 examples/dagger/src/heating/BUILD.bazel
 rename examples/dagger/src/{coffee => heating}/ElectricHeater.kt (71%)
 rename examples/dagger/src/{coffee => heating}/Heater.kt (79%)
 create mode 100644 examples/dagger/src/pumping/BUILD.bazel
 rename examples/dagger/src/{coffee => pumping}/Pump.kt (93%)
 rename examples/dagger/src/{coffee => pumping}/PumpModule.kt (93%)
 rename examples/dagger/src/{coffee => pumping}/Thermosiphon.kt (78%)
 create mode 100644 examples/dagger/src/time/BUILD.bazel
 create mode 100644 examples/dagger/src/time/Delayer.kt
 create mode 100644 examples/dagger/test/coffee/BUILD.bazel
 create mode 100644 examples/dagger/test/coffee/BasicTest.kt
 create mode 100644 examples/dagger/test/coffee/BasicTestUtil.kt

diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
index 7b2bba37e..6e5d5b6c9 100644
--- a/.bazelci/presubmit.yml
+++ b/.bazelci/presubmit.yml
@@ -3,9 +3,11 @@ platforms:
   ubuntu1604:
     test_targets:
     - "//:all_tests"
+    - "//examples/dagger/..."
   ubuntu1804:
     test_targets:
     - "//:all_tests"
+    - "//examples/dagger/..."
   rbe_ubuntu1604:
     test_targets:
     - "--"
@@ -14,6 +16,7 @@ platforms:
     # execution compatible, do not run them for now.
     - "//src/test/kotlin/io/bazel/kotlin:KotlinJvmFriendsVisibilityTest"
     - "//src/test/kotlin/io/bazel/kotlin:KotlinJvmBasicAssertionTest"
+    - "//examples/dagger/..."
     test_flags:
     # Override the default worker strategy for remote builds (worker strategy
     # cannot be used with remote builds)
@@ -21,3 +24,4 @@ platforms:
   macos:
     test_targets:
     - "//:all_tests"
+    - "//examples/dagger/..."
diff --git a/README.md b/README.md
index d700fea1f..ecdb89f07 100644
--- a/README.md
+++ b/README.md
@@ -39,7 +39,7 @@ Key changes:
 
 * Replace the macros with three basic rules. `kt_jvm_binary`, `kt_jvm_library` and `kt_jvm_test`.
 * Android rules. `kt_android_library` and `kt_android_binary`
-* Friend support for tests (supports access to `internal` types and functions)
+* Friend support for tests and other use-cases (supports access to `internal` types and functions)
 * Use a single `deps` attribute instead of `java_dep` and `dep`.
 * Add support for the following standard java rules attributes:
   * `data`
@@ -124,8 +124,8 @@ in your `WORKSPACE` file (or import from a `.bzl` file:
 ```
 load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kotlin_repositories")
 
-KOTLIN_VERSION = "1.3.31"
-KOTLINC_RELEASE_SHA = "107325d56315af4f59ff28db6837d03c2660088e3efeb7d4e41f3e01bb848d6a"
+KOTLIN_VERSION = "1.3.50"
+KOTLINC_RELEASE_SHA = "69424091a6b7f52d93eed8bba2ace921b02b113dbb71388d704f8180a6bdc6ec"
 
 KOTLINC_RELEASE = {
     "urls": [
@@ -137,6 +137,43 @@ KOTLINC_RELEASE = {
 kotlin_repositories(compiler_release = KOTLINC_RELEASE)
 ```
 
+## Friends/Internal support
+
+The rules support kotlin `internal` visibility (usually accessible only to the compilation unit)
+by allowing a library or test to specify that it is friends with another library like so:
+
+```
+kt_jvm_library(
+    name = "foo",
+    srcs = glob(["*.kt"]),
+    friend = "//some/other:target",
+    # ...
+)
+```
+
+> Note: declaring friends of a kt_android_library requires adding _kt to the target, e.g.
+> `//some/other:target_kt`.  This is because the kt_android_library rule is a macro with a
+> `kt_jvm_library` under the hood, and the surfaced rule is an android_rule which doesn't have
+> kotlin providers. This will be fixed in the future.
+
+This grants access to `internal` members of `//some/other:target`. A library can only be friends
+with one other library, which must be a kotlin-aware target. It inherits the module name from that
+library. Only one friend can be declared, because of the assumptions made here about module
+membership.  Since friendship can be transitive (see below), this constrains the visibility so it
+does not become an arbitrarily growing lattice of trust, defeating the purpose.
+
+Very common use-cases for this are:
+
+  * tests and test-libraries depending on internals of the system under test.
+  * clusters of targets that represent one logical unit, with public, private,
+     fake, testing-utilities, configuration, or other targets that comprise the
+     whole unit.
+
+Friendship has limited transitivity. Consider projects `C`, `B`, and `A` with a dependency
+relationship `C->B->A`.  If `C` declares friendship in `B`, and `B` declares friendship with `A`,
+then they are all treated as logically one module for `internal` purposes. However it doesn't
+skip. Once the line of friendship is broken, a separate module is presumed by kotlinc. 
+
 # Bazel Kotlin Rules compatibility
 
 Which version of *rules_kotlin* can you use with which version of Bazel (best
diff --git a/WORKSPACE b/WORKSPACE
index 9a64eadac..8271a853e 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -35,9 +35,9 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive", "http_jar")
 
 http_archive(
     name = "bazel_skylib",
-    urls = ["https://github.com/bazelbuild/bazel-skylib/archive/0.8.0.tar.gz"],
-    strip_prefix = "bazel-skylib-0.8.0",
     sha256 = "2ea8a5ed2b448baf4a6855d3ce049c4c452a6470b1efd1504fdb7c1c134d220a",
+    strip_prefix = "bazel-skylib-0.8.0",
+    urls = ["https://github.com/bazelbuild/bazel-skylib/archive/0.8.0.tar.gz"],
 )
 
 http_jar(
diff --git a/examples/dagger/BUILD.bazel b/examples/dagger/BUILD.bazel
new file mode 100644
index 000000000..d24199900
--- /dev/null
+++ b/examples/dagger/BUILD.bazel
@@ -0,0 +1,21 @@
+# Copyright 2018 The Bazel Authors. All rights reserved.
+#
+# 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.
+package(default_visibility = ["//examples/dagger:__subpackages__"])
+
+java_binary(
+    name = "coffee_app",
+    main_class = "coffee.CoffeeApp",
+    visibility = ["//visibility:public"],
+    runtime_deps = ["//examples/dagger/src/coffee"],
+)
diff --git a/examples/dagger/BUILD b/examples/dagger/src/coffee/BUILD.bazel
similarity index 57%
rename from examples/dagger/BUILD
rename to examples/dagger/src/coffee/BUILD.bazel
index 8829b100e..4ed03d872 100644
--- a/examples/dagger/BUILD
+++ b/examples/dagger/src/coffee/BUILD.bazel
@@ -11,31 +11,8 @@
 # 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.
-package(default_visibility = ["//visibility:private"])
-
 load("//kotlin:kotlin.bzl", "kt_jvm_library")
 
-java_plugin(
-    name = "dagger_plugin",
-    processor_class = "dagger.internal.codegen.ComponentProcessor",
-    deps = [
-        "//third_party/jvm/com/google/dagger",
-        "//third_party/jvm/com/google/dagger:dagger_compiler",
-        "//third_party/jvm/com/google/dagger:dagger_producers",
-        "//third_party/jvm/javax/inject:javax_inject",
-        "@//third_party/jvm/com/google/guava",
-    ],
-)
-
-java_library(
-    name = "dagger_lib",
-    exported_plugins = ["dagger_plugin"],
-    exports = [
-        "//third_party/jvm/com/google/dagger",
-        "//third_party/jvm/javax/inject:javax_inject",
-    ],
-)
-
 # Generate a srcjar to validate intellij plugin correctly attaches it.
 genrule(
     name = "tea_lib_src",
@@ -54,20 +31,18 @@ rm TeaPot.kt
 )
 
 kt_jvm_library(
-    name = "coffee_lib",
-    srcs = glob(["src/**"]) + [
+    name = "coffee",
+    srcs = glob(["*.kt"]) + [
         # Adding a file ending with .srcjar is how code generation patterns are implemented.
         ":tea_lib_src",
     ],
+    friend = "//examples/dagger/src/heating",
+    visibility = ["//examples/dagger:__subpackages__"],
     deps = [
-        ":dagger_lib",
+        "//examples/dagger/src/heating",
+        "//examples/dagger/src/pumping",
+        "//examples/dagger/src/time",
+        "//third_party:dagger",
         "//third_party/jvm/org/jetbrains/kotlinx:kotlinx_coroutines_core",
     ],
 )
-
-java_binary(
-    name = "coffee_app",
-    main_class = "coffee.CoffeeApp",
-    visibility = ["//visibility:public"],
-    runtime_deps = [":coffee_lib"],
-)
diff --git a/examples/dagger/src/coffee/CoffeeMaker.kt b/examples/dagger/src/coffee/CoffeeMaker.kt
index 7ccd2ee41..6ce1237d0 100644
--- a/examples/dagger/src/coffee/CoffeeMaker.kt
+++ b/examples/dagger/src/coffee/CoffeeMaker.kt
@@ -16,9 +16,11 @@
 package coffee
 
 import dagger.Lazy
+import heating.Heater
+import javax.inject.Inject
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
-import javax.inject.Inject
+import pumping.Pump
 
 class CoffeeMaker @Inject internal constructor(
     // Create a possibly costly heater only when we use it.
@@ -34,5 +36,8 @@ class CoffeeMaker @Inject internal constructor(
             println(" [_]P coffee! [_]P ")
             heater.get().off()
         }
+        withContext(Dispatchers.Default) {
+            if (heater.get().isOn) throw IllegalStateException("Heater should be off")
+        }
     }
 }
diff --git a/examples/dagger/src/coffee/DripCoffeeModule.kt b/examples/dagger/src/coffee/DripCoffeeModule.kt
index a4717ebdc..097e646e7 100644
--- a/examples/dagger/src/coffee/DripCoffeeModule.kt
+++ b/examples/dagger/src/coffee/DripCoffeeModule.kt
@@ -15,16 +15,30 @@
  */
 package coffee
 
+import dagger.Binds
 import dagger.Module
 import dagger.Provides
-
+import heating.ElectricHeater
+import heating.Heater
 import javax.inject.Singleton
+import pumping.PumpModule
+import time.Delayer
 
-@Module(includes = arrayOf(PumpModule::class))
+@Module(includes = arrayOf(PumpModule::class, Bindings::class))
 internal class DripCoffeeModule {
     @Provides
     @Singleton
-    fun provideHeater(): Heater {
-        return ElectricHeater()
+    fun provideDelayer(): Delayer {
+        return object : Delayer {
+            override fun delay() {
+                Thread.sleep(1000)
+            }
+        }
     }
 }
+
+@Module
+abstract class Bindings {
+    @Binds @Singleton
+    internal abstract fun bindHeater(heater: ElectricHeater): Heater
+}
diff --git a/examples/dagger/src/heating/BUILD.bazel b/examples/dagger/src/heating/BUILD.bazel
new file mode 100644
index 000000000..26e726172
--- /dev/null
+++ b/examples/dagger/src/heating/BUILD.bazel
@@ -0,0 +1,24 @@
+# Copyright 2019 The Bazel Authors. All rights reserved.
+#
+# 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.
+load("//kotlin:kotlin.bzl", "kt_jvm_library")
+
+kt_jvm_library(
+    name = "heating",
+    srcs = glob(["*.kt"]),
+    visibility = ["//examples/dagger:__subpackages__"],
+    deps = [
+        "//examples/dagger/src/time",
+        "//third_party:dagger",
+    ],
+)
diff --git a/examples/dagger/src/coffee/ElectricHeater.kt b/examples/dagger/src/heating/ElectricHeater.kt
similarity index 71%
rename from examples/dagger/src/coffee/ElectricHeater.kt
rename to examples/dagger/src/heating/ElectricHeater.kt
index eebdb1dc1..e0428613a 100644
--- a/examples/dagger/src/coffee/ElectricHeater.kt
+++ b/examples/dagger/src/heating/ElectricHeater.kt
@@ -13,17 +13,28 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package coffee
+package heating
+
+import javax.inject.Inject
+import time.Delayer
+
+class ElectricHeater
+    @Inject constructor(private val delayer: Delayer) : Heater() {
 
-internal class ElectricHeater : Heater {
     override var isHot: Boolean = false
+    override var isOn: Boolean = false
 
     override fun on() {
+        isOn = true
         println("~ ~ ~ heating ~ ~ ~")
+        delayer.delay()
         this.isHot = true
     }
 
     override fun off() {
+        this.isOn = false
+        println("~ ~ ~ cooling ~ ~ ~")
+        delayer.delay()
         this.isHot = false
     }
 }
diff --git a/examples/dagger/src/coffee/Heater.kt b/examples/dagger/src/heating/Heater.kt
similarity index 79%
rename from examples/dagger/src/coffee/Heater.kt
rename to examples/dagger/src/heating/Heater.kt
index cebb61ff5..9db0e6e3c 100644
--- a/examples/dagger/src/coffee/Heater.kt
+++ b/examples/dagger/src/heating/Heater.kt
@@ -13,10 +13,13 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package coffee
+package heating
 
-internal interface Heater {
-    val isHot: Boolean
-    fun on()
-    fun off()
+abstract class Heater {
+    abstract val isHot: Boolean
+
+    internal abstract val isOn: Boolean
+
+    abstract fun on()
+    abstract fun off()
 }
diff --git a/examples/dagger/src/pumping/BUILD.bazel b/examples/dagger/src/pumping/BUILD.bazel
new file mode 100644
index 000000000..9eca1ca1e
--- /dev/null
+++ b/examples/dagger/src/pumping/BUILD.bazel
@@ -0,0 +1,25 @@
+# Copyright 2019 The Bazel Authors. All rights reserved.
+#
+# 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.
+load("//kotlin:kotlin.bzl", "kt_jvm_library")
+
+kt_jvm_library(
+    name = "pumping",
+    srcs = glob(["*.kt"]),
+    visibility = ["//examples/dagger:__subpackages__"],
+    deps = [
+        "//examples/dagger/src/heating",
+        "//examples/dagger/src/time",
+        "//third_party:dagger",
+    ],
+)
diff --git a/examples/dagger/src/coffee/Pump.kt b/examples/dagger/src/pumping/Pump.kt
similarity index 93%
rename from examples/dagger/src/coffee/Pump.kt
rename to examples/dagger/src/pumping/Pump.kt
index 6796ee000..2918ec2eb 100644
--- a/examples/dagger/src/coffee/Pump.kt
+++ b/examples/dagger/src/pumping/Pump.kt
@@ -13,8 +13,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package coffee
+package pumping
 
-internal interface Pump {
+interface Pump {
     fun pump()
 }
diff --git a/examples/dagger/src/coffee/PumpModule.kt b/examples/dagger/src/pumping/PumpModule.kt
similarity index 93%
rename from examples/dagger/src/coffee/PumpModule.kt
rename to examples/dagger/src/pumping/PumpModule.kt
index 8a499e7c5..9e2e60b4a 100644
--- a/examples/dagger/src/coffee/PumpModule.kt
+++ b/examples/dagger/src/pumping/PumpModule.kt
@@ -13,13 +13,13 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package coffee
+package pumping
 
 import dagger.Binds
 import dagger.Module
 
 @Module
-internal abstract class PumpModule {
+abstract class PumpModule {
     @Binds
     internal abstract fun providePump(pump: Thermosiphon): Pump
 }
diff --git a/examples/dagger/src/coffee/Thermosiphon.kt b/examples/dagger/src/pumping/Thermosiphon.kt
similarity index 78%
rename from examples/dagger/src/coffee/Thermosiphon.kt
rename to examples/dagger/src/pumping/Thermosiphon.kt
index b70633520..a5d273495 100644
--- a/examples/dagger/src/coffee/Thermosiphon.kt
+++ b/examples/dagger/src/pumping/Thermosiphon.kt
@@ -13,16 +13,20 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package coffee
+package pumping
 
+import heating.Heater
 import javax.inject.Inject
+import time.Delayer
 
-internal class Thermosiphon @Inject
-constructor(private val heater: Heater) : Pump {
+internal class Thermosiphon
+    @Inject constructor(private val heater: Heater, private val delayer: Delayer) :
+    Pump {
 
     override fun pump() {
         if (heater.isHot) {
             println("=> => pumping => =>")
+            delayer.delay()
         }
     }
 }
diff --git a/examples/dagger/src/time/BUILD.bazel b/examples/dagger/src/time/BUILD.bazel
new file mode 100644
index 000000000..f358b6371
--- /dev/null
+++ b/examples/dagger/src/time/BUILD.bazel
@@ -0,0 +1,20 @@
+# Copyright 2019 The Bazel Authors. All rights reserved.
+#
+# 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.
+load("//kotlin:kotlin.bzl", "kt_jvm_library")
+
+kt_jvm_library(
+    name = "time",
+    srcs = glob(["*.kt"]),
+    visibility = ["//examples/dagger:__subpackages__"],
+)
diff --git a/examples/dagger/src/time/Delayer.kt b/examples/dagger/src/time/Delayer.kt
new file mode 100644
index 000000000..bf4c63c4c
--- /dev/null
+++ b/examples/dagger/src/time/Delayer.kt
@@ -0,0 +1,21 @@
+/*
+ * Copyright 2018 The Bazel Authors. All rights reserved.
+ *
+ * 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.
+ */
+package time
+
+/** Introduces a delay (which can be overridden in testing */
+interface Delayer {
+    fun delay(): Unit
+}
diff --git a/examples/dagger/test/coffee/BUILD.bazel b/examples/dagger/test/coffee/BUILD.bazel
new file mode 100644
index 000000000..914187067
--- /dev/null
+++ b/examples/dagger/test/coffee/BUILD.bazel
@@ -0,0 +1,22 @@
+load("//kotlin:kotlin.bzl", "kt_jvm_library", "kt_jvm_test")
+
+kt_jvm_library(
+    name = "testlib",
+    srcs = ["BasicTestUtil.kt"],
+    friend = "//examples/dagger/src/heating",
+    deps = [
+        "//examples/dagger/src/heating",
+    ],
+)
+
+kt_jvm_test(
+    name = "BasicTest",
+    srcs = ["BasicTest.kt"],
+    friends = [":testlib"],  # old syntax
+    deps = [
+        ":testlib",
+        "//examples/dagger/src/heating",
+        "//third_party/jvm/com/google/truth",
+        "//third_party/jvm/junit",
+    ],
+)
diff --git a/examples/dagger/test/coffee/BasicTest.kt b/examples/dagger/test/coffee/BasicTest.kt
new file mode 100644
index 000000000..a64f69e1f
--- /dev/null
+++ b/examples/dagger/test/coffee/BasicTest.kt
@@ -0,0 +1,21 @@
+package coffee
+
+import com.google.common.truth.Truth.assertThat
+import heating.ElectricHeater
+import org.junit.Test
+import time.Delayer
+
+class BasicTest {
+    @Test fun FooTest() {
+        val heater = ElectricHeater(object : Delayer {
+            override fun delay() {
+                println("fake delay")
+            }
+        })
+        assertThat(isHeaterOn(heater)).isFalse()
+        heater.on()
+        assertThat(isHeaterOn(heater)).isTrue()
+        heater.off()
+        assertThat(heater.isOn).isFalse()
+    }
+}
diff --git a/examples/dagger/test/coffee/BasicTestUtil.kt b/examples/dagger/test/coffee/BasicTestUtil.kt
new file mode 100644
index 000000000..2a6f01d8e
--- /dev/null
+++ b/examples/dagger/test/coffee/BasicTestUtil.kt
@@ -0,0 +1,5 @@
+package coffee
+
+import heating.Heater
+
+internal fun isHeaterOn(heater: Heater): Boolean = heater.isOn
diff --git a/kotlin/internal/defs.bzl b/kotlin/internal/defs.bzl
index 62690d553..463b3d92c 100644
--- a/kotlin/internal/defs.bzl
+++ b/kotlin/internal/defs.bzl
@@ -21,6 +21,7 @@ KT_COMPILER_REPO = "com_github_jetbrains_kotlin"
 KtJvmInfo = provider(
     fields = {
         "module_name": "the module name",
+        "friend_paths": "The target(s) that this library can see the internals of.",
         "srcs": "the source files. [intelij-aspect]",
         "outputs": "output jars produced by this rule. [intelij-aspect]",
     },
diff --git a/kotlin/internal/jvm/android.bzl b/kotlin/internal/jvm/android.bzl
index db75ebcc9..c8b8ae9ef 100644
--- a/kotlin/internal/jvm/android.bzl
+++ b/kotlin/internal/jvm/android.bzl
@@ -16,7 +16,7 @@ load(
     _kt_jvm_library = "kt_jvm_library",
 )
 
-def _kt_android_artifact(name, srcs = [], deps = [], plugins = [], **kwargs):
+def _kt_android_artifact(name, srcs = [], deps = [], plugins = [], friend = None, **kwargs):
     """Delegates Android related build attributes to the native rules but uses the Kotlin builder to compile Java and
     Kotlin srcs. Returns a sequence of labels that a wrapping macro should export.
     """
@@ -36,8 +36,10 @@ def _kt_android_artifact(name, srcs = [], deps = [], plugins = [], **kwargs):
         srcs = srcs,
         deps = base_deps + [base_name],
         plugins = plugins,
-        testonly = kwargs.get("testonly", default=0),
-        visibility = ["//visibility:private"],
+        testonly = kwargs.get("testonly", default = 0),
+        # must be public to be referenced as friends.
+        # TODO: rework this into a proper android provider giving rule, so we can avoid all this.
+        visibility = ["//visibility:public"],
     )
     return [base_name, kt_name]
 
@@ -49,5 +51,5 @@ def kt_android_library(name, exports = [], visibility = None, **kwargs):
         name = name,
         exports = exports + _kt_android_artifact(name, **kwargs),
         visibility = visibility,
-        testonly = kwargs.get("testonly", default=0),
+        testonly = kwargs.get("testonly", default = 0),
     )
diff --git a/kotlin/internal/jvm/compile.bzl b/kotlin/internal/jvm/compile.bzl
index 6b9a554b6..fe69fbc18 100644
--- a/kotlin/internal/jvm/compile.bzl
+++ b/kotlin/internal/jvm/compile.bzl
@@ -137,6 +137,21 @@ def _partition_srcs(srcs):
         src_jars = depset(src_jars),
     )
 
+def _handle_legacy_friends(ctx):
+    # TODO extract and move this into common. Need to make it generic first.
+    friend = getattr(ctx.attr, "friend", None)
+
+    # Reduce to a single friend if more than one is set (old API)
+    friends = getattr(ctx.attr, "friends", [])
+    if len(friends) == 1:
+        if bool(friend):
+            fail("Can only use friend or friends, but not both. Please only set `friend=`")
+        print("`friends=` is deprecated, please use `friend=` with a single target")
+        friend = friends[0]
+    if len(friends) > 1:
+        fail("only one friend is possible")
+    return friend
+
 def kt_jvm_compile_action(ctx, rule_kind, output_jar):
     """This macro sets up a compile action for a Kotlin jar.
 
@@ -153,24 +168,23 @@ def kt_jvm_compile_action(ctx, rule_kind, output_jar):
     if not srcs.kt and not srcs.java and not srcs.src_jars:
         fail("no sources provided")
 
-    # TODO extract and move this into common. Need to make it generic first.
-    friends = getattr(ctx.attr, "friends", [])
-    deps = [d[JavaInfo] for d in friends + ctx.attr.deps] + [toolchain.jvm_stdlibs]
+    friend = _handle_legacy_friends(ctx)
+    deps = [d[JavaInfo] for d in ctx.attr.deps] + [toolchain.jvm_stdlibs] + ([friend[JavaInfo]] if bool(friend) else [])
     compile_jars = java_common.merge(deps).transitive_compile_time_jars
-
-    if len(friends) == 0:
-        module_name = _utils.derive_module_name(ctx)
-        friend_paths = depset()
-    elif len(friends) == 1:
-        if friends[0][_KtJvmInfo] == None:
+    if bool(friend):
+        if friend[_KtJvmInfo] == None:
             fail("only kotlin dependencies can be friends")
         elif ctx.attr.module_name:
             fail("if friends has been set then module_name cannot be provided")
         else:
-            friend_paths = depset([j.path for j in friends[0][JavaInfo].compile_jars.to_list()])
-            module_name = friends[0][_KtJvmInfo].module_name
+            friend_paths = depset(
+                direct = [x.class_jar.path for x in friend[JavaInfo].outputs.jars],
+                transitive = [friend[_KtJvmInfo].friend_paths],
+            )
+            module_name = friend[_KtJvmInfo].module_name
     else:
-        fail("only one friend is possible")
+        module_name = _utils.derive_module_name(ctx)
+        friend_paths = depset()
 
     classes_directory = _declare_output_directory(ctx, "jvm", "classes")
     generated_classes_directory = _declare_output_directory(ctx, "jvm", "generated_classes")
@@ -187,8 +201,7 @@ def kt_jvm_compile_action(ctx, rule_kind, output_jar):
     args.add("--output", output_jar)
     args.add("--kotlin_output_jdeps", ctx.outputs.jdeps)
     args.add("--kotlin_output_srcjar", ctx.outputs.srcjar)
-
-    args.add("--kotlin_friend_paths", "\n".join(friend_paths.to_list()))
+    args.add("--kotlin_friend_paths", "\n".join([d for d in friend_paths.to_list()]))
 
     args.add_all("--classpath", compile_jars)
     args.add_all("--sources", srcs.all_srcs, omit_if_empty = True)
@@ -232,7 +245,7 @@ def kt_jvm_compile_action(ctx, rule_kind, output_jar):
         progress_message = progress_message,
         input_manifests = input_manifests,
         env = {
-            "LC_CTYPE": "en_US.UTF-8" # For Java source files
+            "LC_CTYPE": "en_US.UTF-8",  # For Java source files
         },
     )
 
@@ -250,6 +263,7 @@ def kt_jvm_compile_action(ctx, rule_kind, output_jar):
         kt = _KtJvmInfo(
             srcs = ctx.files.srcs,
             module_name = module_name,
+            friend_paths = friend_paths,
             # intelij aspect needs this.
             outputs = struct(
                 jdeps = ctx.outputs.jdeps,
diff --git a/kotlin/internal/jvm/jvm.bzl b/kotlin/internal/jvm/jvm.bzl
index aa1d0925d..9e8e2b452 100644
--- a/kotlin/internal/jvm/jvm.bzl
+++ b/kotlin/internal/jvm/jvm.bzl
@@ -161,6 +161,11 @@ _common_attr = utils.add_dicts(
             default = [],
             allow_files = False,
         ),
+        "friend": attr.label(
+            doc = """A single Kotlin dep which allows this code to access internal members of the given dependency.
+             Currently uses the output jar of the module -- i.e., exported deps won't be included.""",
+            providers = [JavaInfo, _KtJvmInfo],
+        ),
         "resources": attr.label_list(
             doc = """A list of files that should be include in a Java jar.""",
             default = [],
@@ -267,8 +272,10 @@ kt_jvm_test = rule(
             allow_files = True,
         ),
         "friends": attr.label_list(
-            doc = """A single Kotlin dep which allows the test code access to internal members. Currently uses the output
-            jar of the module -- i.e., exported deps won't be included.""",
+            doc = """A single Kotlin dep which allows this code to access internal members of the given dependency.
+            Currently uses the output jar of the module -- i.e., exported deps won't be included.
+
+            DEPRECATED - PLEASE USE `friend=` instead.""",
             default = [],
             providers = [JavaInfo, _KtJvmInfo],
         ),
diff --git a/kotlin/internal/repositories/repositories.bzl b/kotlin/internal/repositories/repositories.bzl
index 0d00fddf9..e2831bddf 100644
--- a/kotlin/internal/repositories/repositories.bzl
+++ b/kotlin/internal/repositories/repositories.bzl
@@ -32,9 +32,9 @@ _BAZEL_JAVA_LAUNCHER_VERSION = "0.28.1"
 
 _KOTLIN_CURRENT_COMPILER_RELEASE = {
     "urls": [
-        "https://github.com/JetBrains/kotlin/releases/download/v1.3.21/kotlin-compiler-1.3.21.zip",
+        "https://github.com/JetBrains/kotlin/releases/download/v1.3.50/kotlin-compiler-1.3.50.zip",
     ],
-    "sha256": "dbc7fbed67e0fa9a2f2ef6efd89fc1ef8d92daa38bb23c1f23914869084deb56",
+    "sha256": "69424091a6b7f52d93eed8bba2ace921b02b113dbb71388d704f8180a6bdc6ec",
 }
 
 def github_archive(name, repo, commit, build_file_content = None, sha256 = None):
diff --git a/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinJvmTaskExecutor.kt b/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinJvmTaskExecutor.kt
index 77385179b..60ce50c36 100644
--- a/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinJvmTaskExecutor.kt
+++ b/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinJvmTaskExecutor.kt
@@ -28,6 +28,12 @@ import java.nio.file.Paths
 import javax.inject.Inject
 import javax.inject.Singleton
 
+/**
+ * Due to an inconsistency in the handling of -Xfriends-path, jvm uses a comma (property list
+ * separator), js uses the system path separator.
+ */
+const val X_FRIENDS_PATH_SEPARATOR = ","
+
 @Singleton
 class KotlinJvmTaskExecutor @Inject internal constructor(
     private val compiler: KotlinToolchain.KotlincInvoker,
@@ -96,15 +102,18 @@ class KotlinJvmTaskExecutor @Inject internal constructor(
      */
     private fun JvmCompilationTask.getCommonArgs(): MutableList<String> {
         val args = mutableListOf<String>()
-        val friendPaths= info.friendPathsList.map { Paths.get(it).toAbsolutePath() }
+        val friendPaths = info.friendPathsList.map { Paths.get(it).toAbsolutePath() }
+        val cp = inputs.joinedClasspath
+            .split(File.pathSeparator)
+            .map { Paths.get(it).toAbsolutePath() }
+            .joinToString(File.pathSeparator)
         args.addAll(
-            "-cp", inputs.joinedClasspath,
+            "-cp", cp,
             "-api-version", info.toolchainInfo.common.apiVersion,
             "-language-version", info.toolchainInfo.common.languageVersion,
             "-jvm-target", info.toolchainInfo.jvm.jvmTarget,
-            "-Xfriend-paths=${friendPaths.joinToString(File.pathSeparator)}"
+            "-Xfriend-paths=${friendPaths.joinToString(X_FRIENDS_PATH_SEPARATOR)}"
         )
-
         args
             .addAll("-module-name", info.moduleName)
             .addAll("-d", directories.classes)
@@ -228,5 +237,3 @@ class KotlinJvmTaskExecutor @Inject internal constructor(
             it.build()
         }
 }
-
-
diff --git a/src/test/kotlin/io/bazel/kotlin/KotlinAssertionTestCase.kt b/src/test/kotlin/io/bazel/kotlin/KotlinAssertionTestCase.kt
index ecaff2c50..6ff8cb756 100644
--- a/src/test/kotlin/io/bazel/kotlin/KotlinAssertionTestCase.kt
+++ b/src/test/kotlin/io/bazel/kotlin/KotlinAssertionTestCase.kt
@@ -120,7 +120,7 @@ abstract class BasicAssertionTestCase {
         ProcessBuilder().command("bash", "-c", Paths.get(executable).fileName.toString())
             .also { it.directory(executable.resolveDirectory()) }
             .start().let {
-                it.waitFor(5, TimeUnit.SECONDS)
+                it.waitFor(10, TimeUnit.SECONDS)
                 assert(it.exitValue() == 0) {
                     throw TestCaseFailedException(
                         description = description,
@@ -132,4 +132,4 @@ abstract class BasicAssertionTestCase {
 
     protected open fun String.resolveDirectory(): File =
         trimStart('/').split("/").let { File(it.take(it.size - 1).joinToString(File.separator)) }
-}
\ No newline at end of file
+}