From 21b435b59999513d43e27facbbd767144c4dc77c Mon Sep 17 00:00:00 2001
From: Nicholas Levin <nglevin@google.com>
Date: Tue, 12 Dec 2023 13:43:27 -0800
Subject: [PATCH] Only allow one module to define App Intents, as that is all
 that Apple's app intents metadata processor tool allows for.

PiperOrigin-RevId: 590322476
---
 apple/internal/aspects/BUILD                  |  2 ++
 apple/internal/aspects/app_intents_aspect.bzl | 15 ++++++++---
 .../internal/resource_actions/app_intents.bzl | 25 ++++++++++++-------
 test/starlark_tests/ios_application_tests.bzl | 16 ++++++------
 4 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/apple/internal/aspects/BUILD b/apple/internal/aspects/BUILD
index 9f689c4554..def6568814 100644
--- a/apple/internal/aspects/BUILD
+++ b/apple/internal/aspects/BUILD
@@ -16,7 +16,9 @@ bzl_library(
     deps = [
         "//apple/internal:cc_info_support",
         "//apple/internal/providers:app_intents_info",
+        "@bazel_skylib//lib:collections",
         "@build_bazel_apple_support//lib:apple_support",
+        "@build_bazel_rules_swift//swift:module_name",
         "@build_bazel_rules_swift//swift:providers",
     ],
 )
diff --git a/apple/internal/aspects/app_intents_aspect.bzl b/apple/internal/aspects/app_intents_aspect.bzl
index 9d70372558..e077ac626c 100644
--- a/apple/internal/aspects/app_intents_aspect.bzl
+++ b/apple/internal/aspects/app_intents_aspect.bzl
@@ -14,6 +14,10 @@
 
 """Implementation of the aspect that propagates AppIntentsInfo providers."""
 
+load(
+    "@bazel_skylib//lib:collections.bzl",
+    "collections",
+)
 load(
     "@build_bazel_apple_support//lib:apple_support.bzl",
     "apple_support",
@@ -23,6 +27,10 @@ load(
     "@build_bazel_rules_apple//apple/internal/providers:app_intents_info.bzl",
     "AppIntentsInfo",
 )
+load(
+    "@build_bazel_rules_swift//swift:module_name.bzl",
+    "derive_swift_module_name",
+)
 load(
     "@build_bazel_rules_swift//swift:providers.bzl",
     "SwiftInfo",
@@ -43,12 +51,11 @@ def _app_intents_aspect_impl(target, ctx):
         )
 
     swiftconstvalues_files = []
-    module_names = []
+    module_names = collections.uniq([x.name for x in target[SwiftInfo].direct_modules if x.swift])
+    if not module_names:
+        module_names = [derive_swift_module_name(ctx.label)]
     xcode_version_config = ctx.attr._xcode_config[apple_common.XcodeVersionConfig]
     if xcode_version_config.xcode_version() >= apple_common.dotted_version("15.0"):
-        # TODO(b/315847370): See if these names need to be deduplicated, if duplicate entries are at
-        # all possible.
-        module_names = [x.name for x in target[SwiftInfo].direct_modules if x.swift]
         swiftconstvalues_files = target[OutputGroupInfo]["const_values"].to_list()
 
     return [
diff --git a/apple/internal/resource_actions/app_intents.bzl b/apple/internal/resource_actions/app_intents.bzl
index cb965ff5f6..ad6dafc857 100644
--- a/apple/internal/resource_actions/app_intents.bzl
+++ b/apple/internal/resource_actions/app_intents.bzl
@@ -59,15 +59,22 @@ def generate_app_intents_metadata_bundle(
     args.add("appintentsmetadataprocessor")
 
     args.add("--binary-file", bundle_binary)
-    if len(intents_module_names) == 0:
-        # TODO(b/315847370): See why this works for Xcode 14.x, and if it should be an actual module
-        # name instead.
-        args.add("--module-name", label.name)
-    else:
-        args.add_all(
-            intents_module_names,
-            before_each = "--module-name",
-        )
+
+    if len(intents_module_names) > 1:
+        fail("""
+Found the following module names in the top level target {label} for app_intents: {intents_module_names}
+
+App Intents must have only one module name for metadata generation to work correctly.
+""".format(
+            intents_module_names = ", ".join(intents_module_names),
+            label = str(label),
+        ))
+    elif len(intents_module_names) == 0:
+        fail("""
+Could not find a module name for app_intents. One is required for App Intents metadata generation.
+""")
+
+    args.add("--module-name", intents_module_names[0])
     args.add("--output", output.dirname)
     args.add_all(
         source_files,
diff --git a/test/starlark_tests/ios_application_tests.bzl b/test/starlark_tests/ios_application_tests.bzl
index 2a7187673e..201b13e951 100644
--- a/test/starlark_tests/ios_application_tests.bzl
+++ b/test/starlark_tests/ios_application_tests.bzl
@@ -593,15 +593,15 @@ def ios_application_test_suite(name):
         ],
     )
 
-    # Test app with a Widget Configuration Intent with a computed property generates and bundles Metadata.appintents bundle.
-    archive_contents_test(
-        name = "{}_with_app_intent_and_widget_configuration_intent_contains_app_intents_metadata_bundle_test".format(name),
-        build_type = "simulator",
+    # Test app that has two Intents defined as top level modules generates an error message.
+    analysis_failure_message_test(
+        name = "{}_with_two_app_intents_and_two_modules_fails".format(name),
         target_under_test = "//test/starlark_tests/targets_under_test/ios:app_with_app_intent_and_widget_configuration_intent",
-        contains = [
-            "$BUNDLE_ROOT/Metadata.appintents/extract.actionsdata",
-            "$BUNDLE_ROOT/Metadata.appintents/version.json",
-        ],
+        expected_error = (
+            "App Intents must have only one module name for metadata generation to work correctly."
+        ).format(
+            package = "//test/starlark_tests/targets_under_test/ios",
+        ),
         tags = [
             name,
         ],