fix: Allow passing a tuple to the `tags` attribute. (#1191)

Starlark rules allow giving the tags as a tuple. The helper function
that added the special migration tag assumed tags was always a list,
resulting in an error when it tried to concatenate a list and tuple.

To fix, check if tags is a tuple and concatenate a tuple if so. The
input type of the tags attribute is preserved so that a test verifying
tags can be passed to the underlying rule can be implemented (this test
is to verify there isn't a regression during the rewrite to Starlark).

4 files changed, 48 insertions, 3 deletions

diff --git a/docs/BUILD.bazel b/docs/BUILD.bazel
index 27af3e7..938ba85 100644
--- a/docs/BUILD.bazel
+++ b/docs/BUILD.bazel
@@ -80,6 +80,9 @@ bzl_library(
         "//python/private:stamp.bzl",
         "//python/private:util.bzl",
     ],
+    deps = [
+        "//python/private:util_bzl",
+    ],
 )
 
 # TODO: Stardoc does not guarantee consistent outputs accross platforms (Unix/Windows).
diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel
index 4068ea4..f454f42 100644
--- a/python/private/BUILD.bazel
+++ b/python/private/BUILD.bazel
@@ -44,7 +44,11 @@ bzl_library(
 bzl_library(
     name = "util_bzl",
     srcs = ["util.bzl"],
-    visibility = ["//python:__subpackages__"],
+    visibility = [
+        "//docs:__subpackages__",
+        "//python:__subpackages__",
+    ],
+    deps = ["@bazel_skylib//lib:types"],
 )
 
 # @bazel_tools can't define bzl_library itself, so we just put a wrapper around it.
diff --git a/python/private/util.bzl b/python/private/util.bzl
index 25a50aa..f0d4373 100644
--- a/python/private/util.bzl
+++ b/python/private/util.bzl
@@ -1,5 +1,7 @@
 """Functionality shared by multiple pieces of code."""
 
+load("@bazel_skylib//lib:types.bzl", "types")
+
 def copy_propagating_kwargs(from_kwargs, into_kwargs = None):
     """Copies args that must be compatible between two targets with a dependency relationship.
 
@@ -36,8 +38,23 @@ def copy_propagating_kwargs(from_kwargs, into_kwargs = None):
 _MIGRATION_TAG = "__PYTHON_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__"
 
 def add_migration_tag(attrs):
+    """Add a special tag to `attrs` to aid migration off native rles.
+
+    Args:
+        attrs: dict of keyword args. The `tags` key will be modified in-place.
+
+    Returns:
+        The same `attrs` object, but modified.
+    """
     if "tags" in attrs and attrs["tags"] != None:
-        attrs["tags"] = attrs["tags"] + [_MIGRATION_TAG]
+        tags = attrs["tags"]
+
+        # Preserve the input type: this allows a test verifying the underlying
+        # rule can accept the tuple for the tags argument.
+        if types.is_tuple(tags):
+            attrs["tags"] = tags + (_MIGRATION_TAG,)
+        else:
+            attrs["tags"] = tags + [_MIGRATION_TAG]
     else:
         attrs["tags"] = [_MIGRATION_TAG]
     return attrs
diff --git a/tools/build_defs/python/tests/base_tests.bzl b/tools/build_defs/python/tests/base_tests.bzl
index 715aea7..467611f 100644
--- a/tools/build_defs/python/tests/base_tests.bzl
+++ b/tools/build_defs/python/tests/base_tests.bzl
@@ -15,7 +15,7 @@
 
 load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
 load("@rules_testing//lib:truth.bzl", "matching")
-load("@rules_testing//lib:util.bzl", rt_util = "util")
+load("@rules_testing//lib:util.bzl", "PREVENT_IMPLICIT_BUILDING_TAGS", rt_util = "util")
 load("//python:defs.bzl", "PyInfo")
 load("//tools/build_defs/python/tests:py_info_subject.bzl", "py_info_subject")
 load("//tools/build_defs/python/tests:util.bzl", pt_util = "util")
@@ -99,5 +99,26 @@ def _test_data_sets_uses_shared_library_impl(env, target):
 
 _tests.append(_test_data_sets_uses_shared_library)
 
+def _test_tags_can_be_tuple(name, config):
+    # We don't use a helper because we want to ensure that value passed is
+    # a tuple.
+    config.base_test_rule(
+        name = name + "_subject",
+        tags = ("one", "two") + tuple(PREVENT_IMPLICIT_BUILDING_TAGS),
+    )
+    analysis_test(
+        name = name,
+        target = name + "_subject",
+        impl = _test_tags_can_be_tuple_impl,
+    )
+
+def _test_tags_can_be_tuple_impl(env, target):
+    env.expect.that_target(target).tags().contains_at_least([
+        "one",
+        "two",
+    ])
+
+_tests.append(_test_tags_can_be_tuple)
+
 def create_base_tests(config):
     return pt_util.create_tests(_tests, config = config)