Allow skipping external targets

README.md

@@ -154,7 +154,16 @@ You can also configure the aspect to skip targets based on a custom list of tags
 your_aspect = dwyu_aspect_factory(skipped_tags = ["tag1_marking_skipping", "tag2_marking_skipping"])
 ```
 
-This is demonstrated in the [skipping_targets example](/examples/skipping_targets).
+Another possibility is skipping all targets from external workspaces.
+Often external dependencies are not our under control and thus analyzing them is of little value.
+This option is mostly useful in combination with the recursive analysis.
+You configure it like this:
+
+```starlark
+your_aspect = dwyu_aspect_factory(skip_external_targets = True)
+```
+
+Both options are demonstrated in the [skipping_targets example](/examples/skipping_targets).
 
 ## Recursion
 

examples/MODULE.bazel

@@ -20,3 +20,13 @@ python = use_extension(
     dev_dependency = True,
 )
 python.toolchain(python_version = "3.8")
+
+#
+# Support to make the examples work
+#
+
+bazel_dep(name = "external_targets", dev_dependency = True)
+local_path_override(
+    module_name = "external_targets",
+    path = "support/external_targets",
+)

examples/WORKSPACE

@@ -36,3 +36,11 @@ python_register_toolchains(
     name = "python",
     python_version = "3.8",
 )
+
+#
+# Support to make the examples work
+#
+
+load("//:support/external_targets.bzl", "load_external_targets")
+
+load_external_targets()

examples/aspect.bzl

@@ -3,6 +3,7 @@ load("@depend_on_what_you_use//:defs.bzl", "dwyu_aspect_factory")
 dwyu = dwyu_aspect_factory(use_implementation_deps = True)
 
 dwyu_recursive = dwyu_aspect_factory(recursive = True)
+dwyu_recursive_skip_external = dwyu_aspect_factory(recursive = True, skip_external_targets = True)
 dwyu_custom_skipping = dwyu_aspect_factory(skipped_tags = ["my_tag"])
 
 # We need to explicitly pass labels as passing strings does not work with a bzlmod setup.

examples/skipping_targets/BUILD

@@ -28,3 +28,9 @@ cc_library(
     name = "lib_b",
     hdrs = ["lib_b.h"],
 )
+
+cc_library(
+    name = "use_broken_external_dependency",
+    hdrs = ["use_broken_external_dependency.h"],
+    deps = ["@external_targets//:has_unused_dep"],
+)

examples/skipping_targets/README.md

@@ -13,3 +13,7 @@ succeeds due to the target being tagged with `no-dwyu`.
 Executing <br>
 `bazel build --aspects=//:aspect.bzl%dwyu_custom_skipping --output_groups=dwyu //skipping_targets:bad_target_custom_skip` <br>
 showcases the same skipping behavior, but for the tag `mmy_tag` which we configured in [aspect.bzl](../aspect.bzl).
+
+Executing <br>
+`bazel build --aspects=//:aspect.bzl%dwyu_recursive_skip_external --output_groups=dwyu //skipping_targets:use_broken_external_dependency` <br>
+showcases how we can ignore targets from external workspaces.

examples/skipping_targets/use_broken_external_dependency.h

@@ -0,0 +1 @@
+#include "has_unused_dep.h"

examples/support/external_targets.bzl

@@ -0,0 +1,6 @@
+# buildifier: disable=unnamed-macro
+def load_external_targets():
+    native.local_repository(
+        name = "external_targets",
+        path = "support/external_targets",
+    )

examples/support/external_targets/BUILD

@@ -0,0 +1,11 @@
+cc_library(
+    name = "has_unused_dep",
+    hdrs = ["has_unused_dep.h"],
+    visibility = ["//visibility:public"],
+    deps = [":foo"],
+)
+
+cc_library(
+    name = "foo",
+    hdrs = ["foo.h"],
+)

examples/support/external_targets/MODULE.bazel

@@ -0,0 +1 @@
+module(name = "external_targets")

examples/test.py

@@ -57,6 +57,10 @@ class Example:
         build_cmd="--aspects=//:aspect.bzl%dwyu_custom_skipping --output_groups=dwyu //skipping_targets:bad_target_custom_skip",
         expected_success=True,
     ),
+    Example(
+        build_cmd="--aspects=//:aspect.bzl%dwyu_recursive_skip_external --output_groups=dwyu //skipping_targets:use_broken_external_dependency",
+        expected_success=True,
+    ),
     Example(
         build_cmd="--aspects=//:aspect.bzl%dwyu_map_specific_deps --output_groups=dwyu //target_mapping:use_lib_b",
         expected_success=True,

src/aspect/dwyu.bzl

@@ -3,6 +3,9 @@ load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")
 load("@depend_on_what_you_use//src/cc_info_mapping:cc_info_mapping.bzl", "DwyuCcInfoRemappingsInfo")
 load("@rules_cc//cc:defs.bzl", "CcInfo", "cc_common")
 
+def _is_external(ctx):
+    return ctx.label.workspace_root.startswith("external")
+
 def _get_target_sources(rule):
     public_files = []
     private_files = []
@@ -154,6 +157,12 @@ def _do_ensure_private_deps(ctx):
     """
     return ctx.rule.kind == "cc_library" and ctx.attr._use_implementation_deps
 
+def _dywu_results_from_deps(deps):
+    """
+    We cannot rely on DWYU being executed on the dependencies as analysis of the dependency might have been skipped.
+    """
+    return [dep[OutputGroupInfo].dwyu for dep in deps if hasattr(dep[OutputGroupInfo], "dwyu")]
+
 def _gather_transitive_reports(ctx):
     """
     In recursive operation mode we have to propagate the DWYU report files of all transitive dependencies to ensure
@@ -162,9 +171,9 @@ def _gather_transitive_reports(ctx):
     """
     reports = []
     if ctx.attr._recursive:
-        reports.extend([dep[OutputGroupInfo].dwyu for dep in ctx.rule.attr.deps])
+        reports.extend(_dywu_results_from_deps(ctx.rule.attr.deps))
         if hasattr(ctx.rule.attr, "implementation_deps"):
-            reports.extend([dep[OutputGroupInfo].dwyu for dep in ctx.rule.attr.implementation_deps])
+            reports.extend(_dywu_results_from_deps(ctx.rule.attr.implementation_deps))
     return reports
 
 def dwyu_aspect_impl(target, ctx):
@@ -189,6 +198,10 @@ def dwyu_aspect_impl(target, ctx):
     if not ctx.rule.kind in ["cc_binary", "cc_library", "cc_test"]:
         return []
 
+    # If configured, skip external targets
+    if ctx.attr._skip_external_targets and _is_external(ctx):
+        return []
+
     # Skip targets which explicitly opt-out
     if any([tag in ctx.attr._skipped_tags for tag in ctx.rule.attr.tags]):
         return []

src/aspect/factory.bzl

@@ -4,6 +4,7 @@ load(":dwyu.bzl", "dwyu_aspect_impl")
 def dwyu_aspect_factory(
         ignored_includes = None,
         recursive = False,
+        skip_external_targets = False,
         skipped_tags = None,
         target_mapping = None,
         use_implementation_deps = False,
@@ -16,6 +17,7 @@ def dwyu_aspect_factory(
                           nothing is specified, the standard library headers are ignored by default.
         recursive: If true, execute the aspect on all transitive dependencies.
                    If false, analyze only the target the aspect is being executed on.
+        skip_external_targets: If a target is from an external workspace DWYU skips analyzing it.
         skipped_tags: Do not execute the aspect on targets with at least one of those tags. By default skips the
                       analysis for targets tagged with 'no-dwyu'.
         target_mapping: A target providing a map of target labels to alternative CcInfo provider objects for those
@@ -67,6 +69,9 @@ def dwyu_aspect_factory(
             "_recursive": attr.bool(
                 default = recursive,
             ),
+            "_skip_external_targets": attr.bool(
+                default = skip_external_targets,
+            ),
             "_skipped_tags": attr.string_list(
                 default = aspect_skipped_tags,
             ),

test/aspect/MODULE.bazel

@@ -55,6 +55,12 @@ local_path_override(
     path = "complex_includes/ext_repo",
 )
 
+bazel_dep(name = "skip_external_deps_test_repo", dev_dependency = True)
+local_path_override(
+    module_name = "skip_external_deps_test_repo",
+    path = "skip_external_targets/external_dep",
+)
+
 ##
 ## The Migration phase using WORKSPACE.bzlmod and MODULE.bazel together does not support properly loading the implicit
 ## Bazel dependencies. Thus, we need to load some basic things directly. This should become superfluous when we are

test/aspect/WORKSPACE

@@ -49,7 +49,10 @@ python_register_multi_toolchains(
 
 load("//complex_includes:ext_repo.bzl", "load_complex_includes_repo")
 load("//external_repo:repo.bzl", "load_external_repo")
+load("//skip_external_targets:external_dep.bzl", "load_external_dep")
 
 load_external_repo()
 
 load_complex_includes_repo()
+
+load_external_dep()

test/aspect/recursion/BUILD

@@ -45,7 +45,16 @@ cc_library(
     hdrs = ["d.h"],
 )
 
+# Skipping DWYU on this target demonstrates that the recursive analysis and DWYU report aggregation work, even if DWYU
+# is not executed on some targets
 cc_library(
     name = "e",
     hdrs = ["e.h"],
+    tags = ["no-dwyu"],
+    deps = [":f"],  # unused dependency
+)
+
+cc_library(
+    name = "f",
+    hdrs = ["f.h"],
 )

test/aspect/recursion/f.h

@@ -0,0 +1,4 @@
+#ifndef F_H
+#define F_H
+
+#endif

test/aspect/skip_external_targets/aspect.bzl

@@ -0,0 +1,3 @@
+load("@depend_on_what_you_use//:defs.bzl", "dwyu_aspect_factory")
+
+dwyu_skip_external = dwyu_aspect_factory(skip_external_targets = True)

test/aspect/skip_external_targets/external_dep.bzl

@@ -0,0 +1,6 @@
+# buildifier: disable=unnamed-macro
+def load_external_dep():
+    native.local_repository(
+        name = "skip_external_deps_test_repo",
+        path = "skip_external_targets/external_dep",
+    )

test/aspect/skip_external_targets/external_dep/BUILD

@@ -0,0 +1,16 @@
+cc_library(
+    name = "broken_dep",
+    hdrs = ["broken_dep.h"],
+    deps = [":a"],  # Missing dependency to //lib:b
+)
+
+cc_library(
+    name = "a",
+    hdrs = ["lib/a.h"],
+    deps = [":b"],
+)
+
+cc_library(
+    name = "b",
+    hdrs = ["lib/b.h"],
+)

test/aspect/skip_external_targets/external_dep/MODULE.bazel

@@ -0,0 +1 @@
+module(name = "skip_external_deps_test_repo")

test/aspect/skip_external_targets/external_dep/broken_dep.h

@@ -0,0 +1,6 @@
+#include "lib/a.h"
+#include "lib/b.h"
+
+int useLibWithoutDirectDep() {
+    return doLibA() + doLibB();
+}

test/aspect/skip_external_targets/external_dep/lib/a.h

@@ -0,0 +1,5 @@
+#include "lib/lib_b.h"
+
+int doLibA() {
+    return doLibB() + 42;
+}

test/aspect/skip_external_targets/external_dep/lib/b.h

@@ -0,0 +1,3 @@
+int doLibB() {
+    return 1337;
+}

test/aspect/skip_external_targets/test_skip_broken_target.py

@@ -0,0 +1,14 @@
+from result import ExpectedResult, Result
+from test_case import TestCaseBase
+
+
+class TestCase(TestCaseBase):
+    def execute_test_logic(self) -> Result:
+        # If we would not skip al external targets the analysis would find an issue with the broken dependency
+        expected = ExpectedResult(success=True)
+        actual = self._run_dwyu(
+            target="@skip_external_deps_test_repo//:broken_dep",
+            aspect="//skip_external_targets:aspect.bzl%dwyu_skip_external",
+        )
+
+        return self._check_result(actual=actual, expected=expected)