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

@@ -3,9 +3,11 @@ platforms:
   ubuntu1604:
     test_targets:
     - "//:all_tests"
+    - "//examples/dagger/..."
   ubuntu1804:
     test_targets:
     - "//:all_tests"
+    - "//examples/dagger/..."
   rbe_ubuntu1604:
     test_targets:
     - "--"
@@ -14,10 +16,12 @@ 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)
     - "--strategy=KotlinCompile=remote"
   macos:
     test_targets:
     - "//:all_tests"
+    - "//examples/dagger/..."

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

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(

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"],
+)

examples/dagger/BUILD → 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"],
-)

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")
+        }
     }
 }

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
+}

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",
+    ],
+)

examples/dagger/src/coffee/ElectricHeater.kt → 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
     }
 }

examples/dagger/src/coffee/Heater.kt → 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()
 }

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",
+    ],
+)

examples/dagger/src/coffee/Pump.kt → 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()
 }

examples/dagger/src/coffee/PumpModule.kt → 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
 }

examples/dagger/src/coffee/Thermosiphon.kt → 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()
         }
     }
 }