Make layering check validate unused deps

swift/internal/compiling.bzl

@@ -719,6 +719,18 @@ def compile(
             for dep_module_context in dep_swift_info.direct_modules:
                 direct_module_names.append(dep_module_context.name)
 
+        # Excludes implicitly added deps
+        unused_check_module_name_groups = []
+        for dep_swift_info in swift_infos + private_swift_infos:
+            direct_module_names_for_dep = [
+                dep_module_context.name
+                for dep_module_context in dep_swift_info.direct_modules
+            ]
+            if direct_module_names_for_dep:
+                unused_check_module_name_groups.append(
+                    ",".join(direct_module_names_for_dep),
+                )
+
         validate_system_modules = is_feature_enabled(
             feature_configuration = feature_configuration,
             feature_name = SWIFT_FEATURE_USE_C_MODULES,
@@ -750,6 +762,7 @@ def compile(
             deps_modules_file = deps_modules_file,
             direct_module_names = direct_module_names,
             transitive_module_names = transitive_module_names,
+            unused_check_module_name_groups = unused_check_module_name_groups,
         )
     else:
         deps_modules_file = None
@@ -1932,7 +1945,8 @@ def _write_deps_modules_file(
         actions,
         deps_modules_file,
         direct_module_names,
-        transitive_module_names):
+        transitive_module_names,
+        unused_check_module_name_groups):
     """Writes a file containing the module names of direct dependencies.
 
     This file is used by the Swift worker process to perform layering checks.
@@ -1949,11 +1963,15 @@ def _write_deps_modules_file(
             dependencies of the code being compiled.
         transitive_module_names: The list of names of modules in the target's
             transitive dependency graph.
+        unused_check_module_name_groups: A list of comma-separated module name
+            groups. Each group represents the direct modules provided by one
+            user-declared dependency and is used to detect unused dependencies.
     """
     deps_mapping = actions.args()
     deps_mapping.set_param_file_format("multiline")
     deps_mapping.add_all(direct_module_names, format_each = "direct:%s")
     deps_mapping.add_all(transitive_module_names, format_each = "transitive:%s")
+    deps_mapping.add_all(unused_check_module_name_groups, format_each = "unused-check:%s")
 
     actions.write(
         content = deps_mapping,

swift/internal/feature_names.bzl

@@ -128,7 +128,8 @@ SWIFT_FEATURE_MODULAR_INDEXING = "swift.modular_indexing"
 SWIFT_FEATURE_LAYERING_CHECK_FOR_C_DEPS = "swift.layering_check_for_c_deps"
 
 # If enabled, an error will be emitted when compiling Swift code if it imports
-# any module that is not listed among the direct dependencies of the target.
+# any module that is not listed among the direct dependencies of the target, or
+# if it declares a direct dependency that is never imported.
 SWIFT_FEATURE_LAYERING_CHECK_SWIFT = "swift.layering_check_swift"
 
 # If enabled with `swift.layering_check_swift`, Swift layering checks will also

test/fixtures/layering_check/BUILD

@@ -103,6 +103,14 @@ swift_library(
     deps = [":DirectDependency"],
 )
 
+swift_library(
+    name = "unused_dependency",
+    srcs = ["UnusedDependency.swift"],
+    module_name = "UnusedDependency",
+    tags = FIXTURE_TAGS,
+    deps = [":DirectDependency"],
+)
+
 transition_binary(
     name = "layering_violation_explicit_modules",
     tags = FIXTURE_TAGS,

test/fixtures/layering_check/UnusedDependency.swift

@@ -0,0 +1,3 @@
+public func unusedDependencyValue() -> String {
+  return "unused"
+}

test/fixtures/layering_check/layering_check_failure_test.sh

@@ -30,6 +30,29 @@ check_failure() {
   done
 }
 
+check_unused_failure() {
+  local target="$1"
+  local expected_module="${2:-DirectDependency}"
+
+  if "$bazel" build "$target" &>"$log"; then
+    cat "$log"
+    echo "Expected $target to fail to build" >&2
+    exit 1
+  fi
+
+  for expected in \
+    "Unused dependencies in" \
+    "$target" \
+    "$expected_module" \
+    "Please remove the unused 'deps'"; do
+    if ! grep -Fq "$expected" "$log"; then
+      cat "$log"
+      echo "Expected failure log for $target to contain: $expected" >&2
+      exit 1
+    fi
+  done
+}
+
 check_failure \
   "//test/fixtures/layering_check:foundation_consumer_violation_precompiled_modules" \
   "//test/fixtures/layering_check:foundation_consumer" \
@@ -41,3 +64,4 @@ check_failure \
 check_failure \
   "//test/fixtures/layering_check:layering_violation_explicit_modules_default_precompiled_modules" \
   "//test/fixtures/layering_check:layering_violation"
+check_unused_failure "//test/fixtures/layering_check:unused_dependency"

tools/worker/swift_runner.cc

@@ -20,10 +20,12 @@
 #include <optional>
 #include <sstream>
 #include <utility>
+#include <vector>
 
 #include "absl/container/btree_set.h"
 #include "absl/container/flat_hash_set.h"
 #include "absl/strings/match.h"
+#include "absl/strings/str_split.h"
 #include "absl/strings/string_view.h"
 #include "absl/strings/strip.h"
 #include "absl/strings/substitute.h"
@@ -305,6 +307,7 @@ void PrintVerboseInvocation(const std::vector<std::string>& args,
 
 struct LayeringCheckModules {
   absl::btree_set<std::string> direct_modules;
+  std::vector<absl::btree_set<std::string>> direct_module_groups_to_check;
   absl::btree_set<std::string> transitive_modules;
 };
 
@@ -321,6 +324,17 @@ LayeringCheckModules ReadLayeringCheckModules(absl::string_view path) {
       modules.transitive_modules.insert(std::string(value));
     } else if (absl::ConsumePrefix(&value, "transitive:")) {
       modules.transitive_modules.insert(std::string(value));
+    } else if (absl::ConsumePrefix(&value, "unused-check:")) {
+      absl::btree_set<std::string> direct_module_group;
+      for (absl::string_view module_name : absl::StrSplit(value, ',')) {
+        if (!module_name.empty()) {
+          direct_module_group.insert(std::string(module_name));
+        }
+      }
+      if (!direct_module_group.empty()) {
+        modules.direct_module_groups_to_check.push_back(
+            std::move(direct_module_group));
+      }
     } else {
       // Compatibility with the original file format, which contained one
       // direct module name per line.
@@ -407,8 +421,10 @@ bool SkipLayeringCheckIncompatibleArgs(std::vector<std::string>::iterator& it) {
 // modules.
 static const absl::flat_hash_set<absl::string_view>
     kModulesIgnorableForLayeringCheck = {
-        "Builtin",      "Swift",        "SwiftOnoneSupport",
-        "_Backtracing", "_Concurrency", "_StringProcessing",
+        "Builtin",           "Swift",
+        "SwiftOnoneSupport", "SwiftShims",
+        "_Backtracing",      "_Concurrency",
+        "_StringProcessing", "_SwiftConcurrencyShims",
 };
 
 // Returns true if the module can be ignored for the purposes of layering check
@@ -894,26 +910,56 @@ int SwiftRunner::PerformLayeringCheck(std::ostream& stderr_stream,
   // Use a `btree_set` so that the output is automatically sorted
   // lexicographically.
   absl::btree_set<std::string> missing_deps;
+  absl::btree_set<std::string> imported_modules;
   std::ifstream imported_modules_stream(imported_modules_path);
   std::string module_name;
+  auto diagnostic_module_name = [this](const std::string& module_name) {
+    // Swift's `-emit-imported-modules` output reports resolved aliased module
+    // names. Map them back to the names users write in source before reporting
+    // diagnostics.
+    if (auto alias_and_source_name = alias_to_source_mapping_.find(module_name);
+        alias_and_source_name != alias_to_source_mapping_.end()) {
+      return alias_and_source_name->second;
+    }
+    return module_name;
+  };
+
   while (std::getline(imported_modules_stream, module_name)) {
+    imported_modules.insert(module_name);
+
     // A module can import itself when the Swift module has an underlying Clang
     // module, such as with `@_exported import X` in a Swift overlay for X.
     if (module_name != module_name_ &&
         !IsModuleIgnorableForLayeringCheck(module_name) &&
         layering_check_modules.transitive_modules.contains(module_name) &&
         !layering_check_modules.direct_modules.contains(module_name)) {
-      // Swift's `-emit-imported-modules` output reports resolved aliased module
-      // names. Map them back to the names users write in source before
-      // reporting missing deps.
-      if (auto alias_and_source_name =
-              alias_to_source_mapping_.find(module_name);
-          alias_and_source_name != alias_to_source_mapping_.end()) {
-        missing_deps.insert(alias_and_source_name->second);
-      } else {
-        missing_deps.insert(module_name);
+      missing_deps.insert(diagnostic_module_name(module_name));
+    }
+  }
+
+  absl::btree_set<std::string> unused_deps;
+  for (const auto& direct_module_group :
+       layering_check_modules.direct_module_groups_to_check) {
+    bool has_checkable_module = false;
+    bool has_imported_module = false;
+    absl::btree_set<std::string> unused_modules_in_group;
+    for (const std::string& module_name : direct_module_group) {
+      if (module_name == module_name_ ||
+          IsModuleIgnorableForLayeringCheck(module_name)) {
+        continue;
+      }
+
+      has_checkable_module = true;
+      unused_modules_in_group.insert(diagnostic_module_name(module_name));
+      if (imported_modules.contains(module_name)) {
+        has_imported_module = true;
       }
     }
+
+    if (has_checkable_module && !has_imported_module) {
+      unused_deps.insert(unused_modules_in_group.begin(),
+                         unused_modules_in_group.end());
+    }
   }
 
   if (!missing_deps.empty()) {
@@ -934,6 +980,29 @@ int SwiftRunner::PerformLayeringCheck(std::ostream& stderr_stream,
     WithColor(stderr_stream, Color::kBold)
         << "Please add the correct 'deps' to " << target_label_
         << " to import those modules." << std::endl;
+  }
+
+  if (!unused_deps.empty()) {
+    stderr_stream << std::endl;
+    WithColor(stderr_stream, Color::kBoldRed) << "error: ";
+    WithColor(stderr_stream, Color::kBold) << "Unused dependencies in ";
+    WithColor(stderr_stream, Color::kBoldGreen) << target_label_ << std::endl;
+    stderr_stream
+        << "The following modules are provided by direct dependencies, but "
+        << "they were not imported:" << std::endl
+        << std::endl;
+
+    for (const std::string& module_name : unused_deps) {
+      stderr_stream << "    " << module_name << std::endl;
+    }
+    stderr_stream << std::endl;
+
+    WithColor(stderr_stream, Color::kBold)
+        << "Please remove the unused 'deps' from " << target_label_ << "."
+        << std::endl;
+  }
+
+  if (!missing_deps.empty() || !unused_deps.empty()) {
     return 1;
   }