Allow multiple kt_plugin_cfg (#1353)

* Allow multiple kt_plugin_cfg

Since multiple plugin configurations can be provided to a simple compilation unit, adding a merge_cfgs hook before finalizing the configurations ensure the plugin author can merge configurations into a format that the compiler plugin can consume.

This is a precursor to adding kapt options.

* make buildifier.check match current warnings

Author: restingbull

BUILD

@@ -87,8 +87,28 @@ buildifier(
     name = "buildifier.check",
     exclude_patterns = [
         "./.git/*",
+        "./.ijwb/*",
     ],
     lint_mode = "warn",
+    lint_warnings = [
+        "-confusing-name",
+        "-constant-glob",
+        "-duplicated-name",
+        "-function-docstring",
+        "-function-docstring-args",
+        "-function-docstring-header",
+        "-module-docstring",
+        "-name-conventions",
+        "-no-effect",
+        "-constant-glob",
+        "-provider-params",
+        "-print",
+        "-rule-impl-return",
+        "-bzl-visibility",
+        "-unnamed-macro",
+        "-uninitialized",
+        "-unreachable",
+    ],
     mode = "diff",
 )
 

kotlin/internal/jvm/compile.bzl

@@ -213,7 +213,7 @@ def _new_plugins_from(targets):
         cfg = t[_KtPluginConfiguration]
         if cfg.id not in all_plugins:
             cfgs_without_plugin.append("%s: %s" % (t.label, cfg.id))
-        all_plugin_cfgs[cfg.id] = cfg
+        all_plugin_cfgs.setdefault(cfg.id, []).append(cfg)
 
     if cfgs_without_plugin:
         fail("has plugin configurations without corresponding plugins: %s" % cfgs_without_plugin)
@@ -231,7 +231,7 @@ def _new_plugin_from(all_cfgs, plugins_for_phase):
         classpath.append(p.classpath)
         options.extend(p.options)
         if p.id in all_cfgs:
-            cfg = all_cfgs[p.id]
+            cfg = p.merge_cfgs(p, all_cfgs[p.id])
             classpath.append(cfg.classpath)
             data.append(cfg.data)
             options.extend(cfg.options)

kotlin/internal/jvm/impl.bzl

@@ -15,8 +15,6 @@
 load("@rules_java//java:defs.bzl", "JavaInfo", "JavaPluginInfo", "java_common")
 load(
     "//kotlin/internal:defs.bzl",
-    "KtCompilerPluginOption",
-    "KtPluginConfiguration",
     _KspPluginInfo = "KspPluginInfo",
     _KtCompilerPluginInfo = "KtCompilerPluginInfo",
     _KtJvmInfo = "KtJvmInfo",
@@ -30,6 +28,7 @@ load(
     "//kotlin/internal/utils:utils.bzl",
     _utils = "utils",
 )
+load("//src/main/starlark/core/plugin:common.bzl", "plugin_common")
 load("//third_party:jarjar.bzl", "jarjar_action")
 
 # borrowed from skylib to avoid adding that to the release.
@@ -392,37 +391,6 @@ def _reshade_embedded_kotlinc_jars(target, ctx, jars, deps):
         ],
     )
 
-def _resolve_plugin_options(id, string_list_dict, expand_location):
-    """
-    Resolves plugin options from a string dict to a dict of strings.
-
-    Args:
-        id: the plugin id
-        string_list_dict: a dict of list[string].
-    Returns:
-        a dict of strings
-    """
-    options = []
-    for (k, vs) in string_list_dict.items():
-        for v in vs:
-            if "=" in k:
-                fail("kotlin compiler option keys cannot contain the = symbol")
-            value = k + "=" + expand_location(v) if v else k
-            options.append(KtCompilerPluginOption(id = id, value = value))
-    return options
-
-# This is naive reference implementation for resolving configurations.
-# A more complicated plugin will need to provide its own implementation.
-def _resolve_plugin_cfg(info, options, deps, expand_location):
-    ji = java_common.merge([dep[JavaInfo] for dep in deps if JavaInfo in dep])
-    classpath = depset(ji.runtime_output_jars, transitive = [ji.transitive_runtime_jars])
-    return KtPluginConfiguration(
-        id = info.id,
-        options = _resolve_plugin_options(info.id, options, expand_location),
-        classpath = classpath,
-        data = depset(),
-    )
-
 def kt_compiler_plugin_impl(ctx):
     plugin_id = ctx.attr.id
 
@@ -444,7 +412,7 @@ def kt_compiler_plugin_impl(ctx):
     classpath = depset(info.runtime_output_jars, transitive = [info.transitive_runtime_jars])
 
     # TODO(1035): Migrate kt_compiler_plugin.options to string_list_dict
-    options = _resolve_plugin_options(plugin_id, {k: [v] for (k, v) in ctx.attr.options.items()}, ctx.expand_location)
+    options = plugin_common.resolve_plugin_options(plugin_id, {k: [v] for (k, v) in ctx.attr.options.items()}, ctx.expand_location)
 
     return [
         DefaultInfo(files = classpath),
@@ -454,7 +422,8 @@ def kt_compiler_plugin_impl(ctx):
             options = options,
             stubs = ctx.attr.stubs_phase,
             compile = ctx.attr.compile_phase,
-            resolve_cfg = _resolve_plugin_cfg,
+            resolve_cfg = plugin_common.resolve_cfg,
+            merge_cfgs = plugin_common.merge_cfgs,
         ),
     ]
 

src/main/starlark/core/plugin/common.bzl

@@ -0,0 +1,54 @@
+load("@rules_java//java/common:java_common.bzl", "java_common")
+load("@rules_java//java/common:java_info.bzl", "JavaInfo")
+load(":providers.bzl", "KtCompilerPluginOption", "KtPluginConfiguration")
+
+# This is naive reference implementation for resolving configurations.
+# A more complicated plugin will need to provide its own implementation.
+def _resolve_plugin_cfg(info, options, deps, expand_location):
+    ji = java_common.merge([dep[JavaInfo] for dep in deps if JavaInfo in dep])
+    classpath = depset(ji.runtime_output_jars, transitive = [ji.transitive_runtime_jars])
+    data = None
+    data_runfiles = [d[DefaultInfo].default_runfiles for d in deps if d[DefaultInfo]]
+    if data_runfiles:
+        data = data_runfiles[0].merge_all(data_runfiles[1:])
+    return KtPluginConfiguration(
+        id = info.id,
+        options = _resolve_plugin_options(info.id, options, expand_location),
+        classpath = classpath,
+        data = data,
+    )
+
+def _merge_plugin_cfgs(info, cfgs):
+    classpath = depset(transitive = [cfg.classpath for cfg in cfgs])
+    options = [o for cfg in cfgs for o in cfg.options]
+    return KtPluginConfiguration(
+        id = info.id,
+        options = options,
+        classpath = classpath,
+        data = depset(),
+    )
+
+def _resolve_plugin_options(id, string_list_dict, expand_location):
+    """
+    Resolves plugin options from a string dict to a dict of strings.
+
+    Args:
+        id: the plugin id
+        string_list_dict: a dict of list[string].
+    Returns:
+        a dict of strings
+    """
+    options = []
+    for (k, vs) in string_list_dict.items():
+        for v in vs:
+            if "=" in k:
+                fail("kotlin compiler option keys cannot contain the = symbol")
+            value = k + "=" + expand_location(v) if v else k
+            options.append(KtCompilerPluginOption(id = id, value = value))
+    return options
+
+plugin_common = struct(
+    resolve_cfg = _resolve_plugin_cfg,
+    merge_cfgs = _merge_plugin_cfgs,
+    resolve_plugin_options = _resolve_plugin_options,
+)

src/main/starlark/core/plugin/providers.bzl

@@ -15,6 +15,7 @@ KtCompilerPluginInfo = provider(
         "options": "List of plugin options, represented as KtCompilerPluginOption, to be passed to the compiler",
         "resolve_cfg": "A Callable[[KtCompilerPluginInfo, Dict[str,str], List[Target], KtPluginConfiguration]" +
                        " that resolves an associated plugin configuration.",
+        "merge_cfgs": "A Callable[[KtCompilerPluginInfo, List[KtPluginConfiguration]]] that merge multiple plugin configurations.",
     },
 )
 
@@ -23,7 +24,7 @@ KtPluginConfiguration = provider(
         "id": "The id of the compiler plugin associated with this configuration.",
         "options": "List of plugin options, represented KtCompilerPluginOption",
         "classpath": "Depset of jars to add to the classpath when running the plugin.",
-        "data": "Depset of files to pass to the plugin as data.",
+        "data": "runfiles to pass to the plugin.",
     },
 )
 

src/test/starlark/core/plugin/test.bzl

@@ -188,6 +188,118 @@ def _test_compile_configuration(test):
         },
     )
 
+def _test_compile_multiple_configurations(test):
+    plugin_jar = test.artifact(
+        name = "plugin.jar",
+    )
+
+    plugin = test.have(
+        kt_compiler_plugin,
+        name = "plugin",
+        id = "test.stub",
+        options = {
+            "annotation": "plugin.StubForTesting",
+        },
+        deps = [
+            test.have(
+                kt_jvm_import,
+                name = "plugin_jar",
+                jars = [
+                    plugin_jar,
+                ],
+            ),
+        ],
+    )
+
+    dep_a_jar = test.artifact(
+        name = "a_dep.jar",
+    )
+    dep_b_jar = test.artifact(
+        name = "b_dep.jar",
+    )
+
+    cfg_a = test.have(
+        kt_plugin_cfg,
+        name = "cfg_a",
+        plugin = plugin,
+        options = {
+            "-Dop": ["koo"],
+        },
+        deps = [
+            test.have(
+                kt_jvm_import,
+                name = "dep_a_jar",
+                jars = [
+                    dep_a_jar,
+                ],
+            ),
+        ],
+    )
+
+    cfg_b = test.have(
+        kt_plugin_cfg,
+        name = "cfg_b",
+        plugin = plugin,
+        options = {
+            "-Dop": ["zubzub"],
+        },
+        deps = [
+            test.have(
+                kt_jvm_import,
+                name = "dep_b_jar",
+                jars = [
+                    dep_b_jar,
+                ],
+            ),
+        ],
+    )
+
+    got = test.got(
+        kt_jvm_library,
+        name = "got_library",
+        srcs = [
+            test.artifact(
+                name = "got_library.kt",
+            ),
+        ],
+        plugins = [
+            plugin,
+            cfg_a,
+            cfg_b,
+        ],
+    )
+
+    analysis_test(
+        name = test.name,
+        impl = _action_test_impl,
+        target = got,
+        attr_values = {
+            "on_action_mnemonic": "KotlinCompile",
+            "want_flags": {
+                "--compiler_plugin_options": [
+                    "test.stub:annotation=plugin.StubForTesting",
+                    "test.stub:-Dop=koo",
+                    "test.stub:-Dop=zubzub",
+                ],
+                "--stubs_plugin_options": [
+                    "test.stub:annotation=plugin.StubForTesting",
+                    "test.stub:-Dop=koo",
+                    "test.stub:-Dop=zubzub",
+                ],
+            },
+            "want_inputs": [
+                plugin_jar,
+                dep_a_jar,
+                dep_b_jar,
+            ],
+        },
+        attrs = {
+            "on_action_mnemonic": attr.string(),
+            "want_flags": attr.string_list_dict(),
+            "want_inputs": attr.label_list(providers = [DefaultInfo], allow_files = True),
+        },
+    )
+
 def _test_compile_configuration_single_phase(test):
     stub, stub_jar = plugin_for(
         test,
@@ -378,4 +490,5 @@ def test_suite(name):
         test_library_multiple_plugins_with_same_id = _test_library_multiple_plugins_with_same_id,
         test_compile_configuration_single_phase = _test_compile_configuration_single_phase,
         test_cfg_without_plugin = _test_cfg_without_plugin,
+        test_compile_multiple_configurations = _test_compile_multiple_configurations,
     )