From b41a0061317f28b901d526e99b069fddb843b3f3 Mon Sep 17 00:00:00 2001 From: Martin Medler Date: Mon, 7 Mar 2022 23:15:02 +0100 Subject: [PATCH] Start of public history --- .bazelrc | 14 + .github/CODEOWNERS | 1 + .gitignore | 14 + BUILD | 0 CONTRIBUTING.md | 20 ++ LICENSE | 21 ++ README.md | 191 ++++++++++- WORKSPACE | 40 +++ aspect/BUILD | 1 + aspect/dwyu.bzl | 169 ++++++++++ aspect/factory.bzl | 58 ++++ aspect/private/debugging.bzl | 13 + aspect/private/empty_default_config.json | 0 defs.bzl | 3 + docs/todos.md | 61 ++++ pyproject.toml | 2 + scripts/extract_std_headers.py | 15 + src/BUILD | 22 ++ src/evaluate_includes.py | 197 +++++++++++ src/get_dependencies.py | 83 +++++ src/main.py | 119 +++++++ src/parse_config.py | 11 + src/parse_source.py | 84 +++++ src/std_header.py | 165 +++++++++ src/test/BUILD | 40 +++ src/test/data/.clang-format | 4 + src/test/data/another_header.h | 1 + .../data/commented_includes/block_comments.h | 18 + .../data/commented_includes/mixed_style.h | 2 + .../commented_includes/single_line_comments.h | 6 + src/test/data/config.json | 10 + src/test/data/config_empty.json | 4 + src/test/data/deps_info_empty.json | 8 + src/test/data/deps_info_full.json | 41 +++ src/test/data/some_header.h | 9 + src/test/evaluate_includes_test.py | 314 ++++++++++++++++++ src/test/get_dependencies_test.py | 101 ++++++ src/test/parse_config_test.py | 26 ++ src/test/parse_source_test.py | 101 ++++++ test/BUILD | 9 + test/README.md | 11 + test/aspect.bzl | 3 + test/complex_includes/BUILD | 23 ++ test/complex_includes/README.md | 1 + test/complex_includes/complex_includes.cpp | 12 + test/complex_includes/ext_repo.bzl | 5 + test/complex_includes/ext_repo/BUILD | 19 ++ test/complex_includes/ext_repo/WORKSPACE | 0 .../ext_repo/complex_includes.cpp | 12 + .../ext_repo/some/dir/complex_include_a.h | 6 + .../ext_repo/some/dir/complex_include_b.h | 6 + .../ext_repo/use_complex_includes.cpp | 7 + .../some/dir/complex_include_a.h | 6 + .../some/dir/complex_include_b.h | 6 + .../complex_includes/use_complex_includes.cpp | 7 + .../use_complex_includes_from_extern.cpp | 0 test/custom_config/BUILD | 11 + test/custom_config/README.md | 1 + test/custom_config/aspect.bzl | 3 + test/custom_config/custom_config.json | 6 + test/custom_config/foo.h | 1 + test/dependency_utilization/BUILD | 20 ++ test/dependency_utilization/README.md | 2 + test/dependency_utilization/aspect.bzl | 3 + test/dependency_utilization/lib_part_a.h | 0 test/dependency_utilization/lib_part_b.h | 0 test/dependency_utilization/lib_part_c.h | 0 .../utilization_high.cpp | 7 + .../utilization_low.cpp | 6 + test/execute_tests.py | 165 +++++++++ test/execute_tests_impl.py | 220 ++++++++++++ test/execute_tests_utest.py | 185 +++++++++++ test/external_repo/BUILD | 8 + test/external_repo/README.md | 1 + test/external_repo/repo.bzl | 5 + test/external_repo/repo/BUILD | 11 + test/external_repo/repo/WORKSPACE | 0 test/external_repo/repo/foo.h | 9 + test/external_repo/repo/some/dir/bar.h | 9 + test/external_repo/use_external_libs.h | 12 + test/generated_code/BUILD | 16 + test/generated_code/REAME.md | 1 + test/generated_code/foo.cpp | 6 + test/implementation_deps/BUILD | 27 ++ test/implementation_deps/README.md | 1 + test/implementation_deps/aspect.bzl | 3 + test/implementation_deps/bar.h | 9 + test/implementation_deps/foo.h | 9 + test/implementation_deps/use_libs.cpp | 6 + test/implementation_deps/use_libs.h | 13 + test/includes/BUILD | 15 + test/includes/README.md | 1 + test/includes/includes.cpp | 8 + test/includes/some/sub/dir/includes.h | 6 + test/includes/use_includes.cpp | 8 + test/recursion/BUILD | 53 +++ test/recursion/README.md | 3 + test/recursion/a.h | 6 + test/recursion/aspect.bzl | 3 + test/recursion/b.h | 6 + test/recursion/c.h | 6 + test/recursion/d.h | 4 + test/recursion/e.h | 4 + test/recursion/main.cpp | 7 + test/recursion/rule.bzl | 21 ++ test/test.bzl | 6 + test/unused_dep/BUILD | 19 ++ test/unused_dep/README.md | 1 + test/unused_dep/bar.h | 9 + test/unused_dep/foo.h | 9 + test/unused_dep/main.cpp | 6 + test/using_transitive_dep/BUILD | 16 + test/using_transitive_dep/README.md | 1 + test/using_transitive_dep/bar.h | 10 + test/using_transitive_dep/foo.h | 9 + test/using_transitive_dep/main.cpp | 10 + test/valid/BUILD | 19 ++ test/valid/README.md | 1 + test/valid/bar/bar.cc | 10 + test/valid/bar/bar.h | 8 + test/valid/bar/private_a.h | 9 + test/valid/bar/sub/dir/private_b.h | 9 + test/valid/foo/a.h | 9 + test/valid/foo/b.h | 9 + test/valid/foo/textual.cc | 0 test/virtual_includes/BUILD | 25 ++ test/virtual_includes/README.md | 1 + test/virtual_includes/prefixed.cpp | 6 + test/virtual_includes/prefixed.h | 6 + test/virtual_includes/some/sub/dir/stripped.h | 6 + test/virtual_includes/stripped.cpp | 6 + test/virtual_includes/use_prefixed.cpp | 6 + test/virtual_includes/use_stripped.cpp | 6 + 133 files changed, 3280 insertions(+), 1 deletion(-) create mode 100644 .bazelrc create mode 100644 .github/CODEOWNERS create mode 100644 .gitignore create mode 100644 BUILD create mode 100644 CONTRIBUTING.md create mode 100644 LICENSE create mode 100644 WORKSPACE create mode 100644 aspect/BUILD create mode 100644 aspect/dwyu.bzl create mode 100644 aspect/factory.bzl create mode 100644 aspect/private/debugging.bzl create mode 100644 aspect/private/empty_default_config.json create mode 100644 defs.bzl create mode 100644 docs/todos.md create mode 100644 pyproject.toml create mode 100755 scripts/extract_std_headers.py create mode 100644 src/BUILD create mode 100644 src/evaluate_includes.py create mode 100644 src/get_dependencies.py create mode 100644 src/main.py create mode 100644 src/parse_config.py create mode 100644 src/parse_source.py create mode 100644 src/std_header.py create mode 100644 src/test/BUILD create mode 100644 src/test/data/.clang-format create mode 100644 src/test/data/another_header.h create mode 100644 src/test/data/commented_includes/block_comments.h create mode 100644 src/test/data/commented_includes/mixed_style.h create mode 100644 src/test/data/commented_includes/single_line_comments.h create mode 100644 src/test/data/config.json create mode 100644 src/test/data/config_empty.json create mode 100644 src/test/data/deps_info_empty.json create mode 100644 src/test/data/deps_info_full.json create mode 100644 src/test/data/some_header.h create mode 100644 src/test/evaluate_includes_test.py create mode 100644 src/test/get_dependencies_test.py create mode 100644 src/test/parse_config_test.py create mode 100644 src/test/parse_source_test.py create mode 100644 test/BUILD create mode 100644 test/README.md create mode 100644 test/aspect.bzl create mode 100644 test/complex_includes/BUILD create mode 100644 test/complex_includes/README.md create mode 100644 test/complex_includes/complex_includes.cpp create mode 100644 test/complex_includes/ext_repo.bzl create mode 100644 test/complex_includes/ext_repo/BUILD create mode 100644 test/complex_includes/ext_repo/WORKSPACE create mode 100644 test/complex_includes/ext_repo/complex_includes.cpp create mode 100644 test/complex_includes/ext_repo/some/dir/complex_include_a.h create mode 100644 test/complex_includes/ext_repo/some/dir/complex_include_b.h create mode 100644 test/complex_includes/ext_repo/use_complex_includes.cpp create mode 100644 test/complex_includes/some/dir/complex_include_a.h create mode 100644 test/complex_includes/some/dir/complex_include_b.h create mode 100644 test/complex_includes/use_complex_includes.cpp create mode 100644 test/complex_includes/use_complex_includes_from_extern.cpp create mode 100644 test/custom_config/BUILD create mode 100644 test/custom_config/README.md create mode 100644 test/custom_config/aspect.bzl create mode 100644 test/custom_config/custom_config.json create mode 100644 test/custom_config/foo.h create mode 100644 test/dependency_utilization/BUILD create mode 100644 test/dependency_utilization/README.md create mode 100644 test/dependency_utilization/aspect.bzl create mode 100644 test/dependency_utilization/lib_part_a.h create mode 100644 test/dependency_utilization/lib_part_b.h create mode 100644 test/dependency_utilization/lib_part_c.h create mode 100644 test/dependency_utilization/utilization_high.cpp create mode 100644 test/dependency_utilization/utilization_low.cpp create mode 100755 test/execute_tests.py create mode 100644 test/execute_tests_impl.py create mode 100644 test/execute_tests_utest.py create mode 100644 test/external_repo/BUILD create mode 100644 test/external_repo/README.md create mode 100644 test/external_repo/repo.bzl create mode 100644 test/external_repo/repo/BUILD create mode 100644 test/external_repo/repo/WORKSPACE create mode 100644 test/external_repo/repo/foo.h create mode 100644 test/external_repo/repo/some/dir/bar.h create mode 100644 test/external_repo/use_external_libs.h create mode 100644 test/generated_code/BUILD create mode 100644 test/generated_code/REAME.md create mode 100644 test/generated_code/foo.cpp create mode 100644 test/implementation_deps/BUILD create mode 100644 test/implementation_deps/README.md create mode 100644 test/implementation_deps/aspect.bzl create mode 100644 test/implementation_deps/bar.h create mode 100644 test/implementation_deps/foo.h create mode 100644 test/implementation_deps/use_libs.cpp create mode 100644 test/implementation_deps/use_libs.h create mode 100644 test/includes/BUILD create mode 100644 test/includes/README.md create mode 100644 test/includes/includes.cpp create mode 100644 test/includes/some/sub/dir/includes.h create mode 100644 test/includes/use_includes.cpp create mode 100644 test/recursion/BUILD create mode 100644 test/recursion/README.md create mode 100644 test/recursion/a.h create mode 100644 test/recursion/aspect.bzl create mode 100644 test/recursion/b.h create mode 100644 test/recursion/c.h create mode 100644 test/recursion/d.h create mode 100644 test/recursion/e.h create mode 100644 test/recursion/main.cpp create mode 100644 test/recursion/rule.bzl create mode 100644 test/test.bzl create mode 100644 test/unused_dep/BUILD create mode 100644 test/unused_dep/README.md create mode 100644 test/unused_dep/bar.h create mode 100644 test/unused_dep/foo.h create mode 100644 test/unused_dep/main.cpp create mode 100644 test/using_transitive_dep/BUILD create mode 100644 test/using_transitive_dep/README.md create mode 100644 test/using_transitive_dep/bar.h create mode 100644 test/using_transitive_dep/foo.h create mode 100644 test/using_transitive_dep/main.cpp create mode 100644 test/valid/BUILD create mode 100644 test/valid/README.md create mode 100644 test/valid/bar/bar.cc create mode 100644 test/valid/bar/bar.h create mode 100644 test/valid/bar/private_a.h create mode 100644 test/valid/bar/sub/dir/private_b.h create mode 100644 test/valid/foo/a.h create mode 100644 test/valid/foo/b.h create mode 100644 test/valid/foo/textual.cc create mode 100644 test/virtual_includes/BUILD create mode 100644 test/virtual_includes/README.md create mode 100644 test/virtual_includes/prefixed.cpp create mode 100644 test/virtual_includes/prefixed.h create mode 100644 test/virtual_includes/some/sub/dir/stripped.h create mode 100644 test/virtual_includes/stripped.cpp create mode 100644 test/virtual_includes/use_prefixed.cpp create mode 100644 test/virtual_includes/use_stripped.cpp diff --git a/.bazelrc b/.bazelrc new file mode 100644 index 00000000..22903791 --- /dev/null +++ b/.bazelrc @@ -0,0 +1,14 @@ +# Make sure not to use legacy features and be prepared for future Bazel versions +# These flags have to be supported by all Bazel versions supported by DWYU +# Flags only available for specific Bazel versions are managed directly in the acceptance tests +common --incompatible_disable_target_provider_fields +common --incompatible_disallow_empty_glob +common --incompatible_no_implicit_file_export +common --incompatible_struct_has_no_methods +common --incompatible_use_cc_configure_from_rules_cc +common --incompatible_use_platforms_repo_for_constraints +common --incompatible_visibility_private_attributes_at_definition +build --incompatible_default_to_explicit_init_py + +# Allow users to provide their own workspace settings +try-import %workspace%/.bazelrc.user diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 00000000..c3668ef6 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1 @@ +* @martis42 diff --git a/.gitignore b/.gitignore new file mode 100644 index 00000000..dc69f1b7 --- /dev/null +++ b/.gitignore @@ -0,0 +1,14 @@ +*~ + +# Bazel +bazel-* +.bazelrc.user + +# Python +__pycache__ +*.pyc + +# IDEs +.clwb +.idea +.vscode diff --git a/BUILD b/BUILD new file mode 100644 index 00000000..e69de29b diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..8ab66e15 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,20 @@ +# Code reviews + +All contributions require a review from the [code owners](.github/CODEOWNERS). + +# Bug reports + +Please make sure you are aware of the [early project stage](https://github.com/martis42/depend_on_what_you_use#word-of-warning) and are accustomed with the [known limitations](https://github.com/martis42/depend_on_what_you_use#known-limitations) + +Please introduce a minimal example for reproducing the bug. +Ideally as a test case, but any minimal example helps + +# Feature changes/requests + +* Please create an [issue](https://github.com/martis42/depend_on_what_you_use/issues) before changing features or introducing new features. + Discussing your idea first makes sure it is in the interest of the project and your work will not be in vain due to following an undesired direction. +* No feature shall be changed or introduced without adapting or adding tests accordingly. + +# Trivial changes + +Feel free to directly create pull requests without creating an issue for small improvements like fixing typos. diff --git a/LICENSE b/LICENSE new file mode 100644 index 00000000..2f8eab1b --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2022 Martin Medler + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md index 81e2663e..a2a37251 100644 --- a/README.md +++ b/README.md @@ -1 +1,190 @@ -# depend_on_what_you_use \ No newline at end of file +# Depend on what you use (DWYU) + +DWYU is a Bazel aspect for C++ projects to make sure the headers your targets are using are aligned with the targets dependency list. +It applies the principals established by [Include What You Use](https://github.com/include-what-you-use/include-what-you-use) +to the dependencies of your `cc_*` targets. + +The main features are: +* Finding include statements which are not available through a direct dependency, aka **preventing to rely on transitive dependencies** +* Finding unused dependencies +* Given one uses `implementation_deps`, finding dependencies which are used only in private code and thus should be an implementation dependency + +# Word of Warning + +This project is still in a prototyping phase. +It has been made public to ease sharing with other developers for gathering feedback on the concept. +Do not expect stability of +* the git history +* the project structure +* the feature set +* the tools chosen to implement DWYU + +# Getting started + +## Get a release + +Choose a release from the [release page](https://github.com/martis42/depend_on_what_you_use/releases) and follow the instructions. + +## Get a specific commit + +Put the following into your `WORKSPACE` file: +```sh +load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") + +dwyu_version = "" + +http_archive( + name = "depend_on_what_you_use", + sha256 = "", + strip_prefix = "depend_on_what_you_use-{}".format(dwyu_version), + url = "https://github.com/bazelbuild/rules_python/archive/{}.zip".format(dwyu_version), +) +``` + +## Use DWYU + +### Configure the aspect + +Configure an aspect with the desired behavior. +The features which can be configured trough the the aspect factory attributes are documented at [Features](#features). +Put the following inside a `aspect.bzl` file (file name is exemplary): +```sh +load("@depend_on_what_you_use//:defs.bzl", "dwyu_aspect_factory") + +# Provide no arguments for the default behavior +# Or set a custom value for the various attributes +your_dywu_aspect = dwyu_aspect_factory() +``` + +### Use the aspect + +Invoke the aspect through the command line on a target: \ +`bazel build --aspects=//:aspect.bzl%your_dywu_aspect --output_groups=cc_dwyu_output` + +If no problem is found, the command will exit with `INFO: Build completed successfully`. \ +If a problem is detected, the build command will fail with an error and a description of the problem will be printed in the terminal. +For example: +``` +================================================================================ +DWYU: Failure +Unused dependencies (none of their headers are referenced): + Dependency='//some/target/with_an_unused:dependency' +=============================================================================== +``` + +### Create a rule invoking the aspect. + +You can invoke the aspect from within a rule. +This can be useful to make the execution part of a bazel build (e.g. `bazel build //...`) without having to execute the longish manual aspect build command. + +The Bazel documentation for invoking an aspect from within a rule can be found [here](https://bazel.build/rules/aspects#invoking_the_aspect_from_a_rule). +A concrete example for doing so for a DWYU aspect can be [found in the recursion test cases](test/recursion/rule.bzl). + +## Features + +### Custom header ignore list + +By default DWYU ignores all header from the standard library when comparing include statements to the dependencies. + +You can exclude a custom set of header files by providing a config file in json format to the aspect. +An example for this and the correct format can be seen at [test/custom_config](../test/custom_config). + +### Recursion + +By default DWYU analyzes only the target it is being applied to. + +You can activate recursive analysis, meaning the aspect analyzes recursively all dependencies of the target it is being +applied to. Activate this behavior via: +``` +your_aspect = dwyu_aspect_factory(recursive = True) +``` + +This can be used to create a rule invoking DWYU on a target and all its dependencies as part of a normal build command. +Also it can be a convenience to analyze specific fraction of your stack without utilizing bazel (c)query. + +Examples for this can be seen at [test/recursion](../test/recursion). + +### Measure dependency utilization + +If a library provides many headers but typically only a fraction of them are used at the call sites, this can be a hint +that the library should be split into smaller parts. DWYU allows you to find cases where a percentage less than +a provided threshold of headers from a dependency is used by a call site. + +This feature is intended to analyze your stack and search for suboptimal dependencies. It is not recommended to enforce +unconditionally a minimum dependency header utilization on your whole stack. \ +Using `include_prefix`, `strip_include_prefix` or `includes` on `cc_` targets tampers with the reliability of this measurement. +Those attributes make multiple include paths available for a single header file. +These, paths appear as multiple files to the DWYU implementation. +Consequently, the amount of headers provided by a dependency increases virtually and thus utilization drops. + +Activate this behavior via: +``` +your_aspect = dwyu_aspect_factory(min_utilization = [0..100]) +``` + +Examples for this can be seen at [test/dependency_utilization](../test/dependency_utilization). + +### Implementation_deps + +Bazel 5.0.0 introduces the experimental feature [`implementation_deps`](https://docs.bazel.build/versions/main/be/c-cpp.html#cc_library.implementation_deps) +for `cc_library`. In short, this enables you to specify dependencies which are only relevant for your `srcs` files and +are not made available to users of the library. + +DWYU can report dependencies which are only used in private sources and should be moved from `deps` to `implementation_deps`. + +Activate this behavior via: +``` +your_aspect = dwyu_aspect_factory(use_implementation_deps = True) +``` + +Examples for this can be seen at [test/implementation_deps](../test/implementation_deps). + +## Supported Platforms + +### Bazel + +Minimum required Bazel version is **4.0.0**. +* Before 3.3.0 CcInfo compilation_context has a structure which is not supported by the aspect +* Before 4.0.0 the global json module is not available in Starlark + +### Python + +Requires Python 3. Code is only tested with Python 3.8, but should work with most 3.X versions. + +## Operating system + +DWYU is not designed for a specific platform. +Ubuntu 20.04 is however the only OS DWYU is currently being tested on. + +## Known limitations + +* If includes are added through a macro, this is invisible to DWYU. +* Defines are ignored. + No matter if they are defined directly inside the header under inspection, headers from the dependencies or injected through the `defines = [...]` attribute of the `cc_` rules. +* Include statements utilizing `..` to go up the directory tree are not resolved. + +# Project Roadmap + +## Overview of current state + +The focus until now has been to come up with a prototype for the idea behind DWYU and to be able to gather first user feedback. + +[Test cases](test) have been established to make sure the concept works with a wide range of use cases and DWYU can evolve without introducing regressions. + +The Bazel aspect implementation is for sure not yet ideal, but is considered it a solid starting point. + +The Python implementation to analyze the include statements in C++ code is a minimal implementation with [several restrictions](#Known-limitations). +If it will be expanded or replaced is currently unclear. + +## TODOs + +See [docs/todos](docs/todos.md). + +# Contributing + +See [Contributing](CONTRIBUTING.md). + +# License + +Copyright © 2022-present, [Martin Medler](https://github.com/martis42). \ +This project licensed under the [MIT](https://opensource.org/licenses/MIT) license. diff --git a/WORKSPACE b/WORKSPACE new file mode 100644 index 00000000..82399160 --- /dev/null +++ b/WORKSPACE @@ -0,0 +1,40 @@ +workspace(name = "depend_on_what_you_use") + +load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") + +# +# External dependencies +# + +http_archive( + name = "rules_python", + sha256 = "a30abdfc7126d497a7698c29c46ea9901c6392d6ed315171a6df5ce433aa4502", + strip_prefix = "rules_python-0.6.0", + urls = ["https://github.com/bazelbuild/rules_python/archive/0.6.0.tar.gz"], +) + +http_archive( + name = "bazel_skylib", + sha256 = "c6966ec828da198c5d9adbaa94c05e3a1c7f21bd012a0b29ba8ddbccb2c93b0d", + urls = ["https://github.com/bazelbuild/bazel-skylib/releases/download/1.1.1/bazel-skylib-1.1.1.tar.gz"], +) + +load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace") + +bazel_skylib_workspace() + +# +# Project +# + +load("@bazel_skylib//lib:versions.bzl", "versions") + +versions.check(minimum_bazel_version = "4.0.0") + +# +# Testing +# + +load("//test:test.bzl", "test_setup") + +test_setup() diff --git a/aspect/BUILD b/aspect/BUILD new file mode 100644 index 00000000..a02873b9 --- /dev/null +++ b/aspect/BUILD @@ -0,0 +1 @@ +exports_files(["private/empty_default_config.json"]) diff --git a/aspect/dwyu.bzl b/aspect/dwyu.bzl new file mode 100644 index 00000000..879650ad --- /dev/null +++ b/aspect/dwyu.bzl @@ -0,0 +1,169 @@ +def _parse_sources_impl(sources, out_files): + for src in sources: + file = src.files.to_list()[0] + out_files.append(file) + +def _parse_sources(attr): + """Split source files into public and private ones""" + public_files = [] + private_files = [] + + if hasattr(attr, "srcs"): + _parse_sources_impl(sources = attr.srcs, out_files = private_files) + if hasattr(attr, "hdrs"): + _parse_sources_impl(sources = attr.hdrs, out_files = public_files) + if hasattr(attr, "textual_hdrs"): + _parse_sources_impl(sources = attr.textual_hdrs, out_files = public_files) + + return public_files, private_files + +def _make_args(ctx, public_files, private_files, report_file, headers_info_file, ensure_private_deps): + args = ctx.actions.args() + + args.add_all("--public-files", [pf.path for pf in public_files]) + args.add_all("--private-files", [pf.path for pf in private_files]) + args.add("--headers-info", headers_info_file) + args.add("--report", report_file) + + if ctx.file._config.path != "aspect/private/empty_default_config.json": + args.add("--config", ctx.file._config) + + if ensure_private_deps: + args.add("--implementation-deps-available") + + if ctx.attr._min_utilization > 0: + args.add("--min-dependency-utilization", ctx.attr._min_utilization) + + return args + +def _get_available_include_paths(label, system_includes, header_file): + """ + Get all paths at which a header file is available to code using it. + + Args: + label: Label of the target providing the header file + system_includes: system_include paths of the target providing the header file + header_file: Header file + """ + + # Paths at which headers are available from targets which utilize "include_prefix" or "strip_include_prefix" + if "_virtual_includes" in header_file.path: + return [header_file.path.partition("_virtual_includes" + "/" + label.name + "/")[2]] + + # Paths at which headers are available from targets which utilize "includes = [...]" + includes = [] + for si in system_includes.to_list(): + si_path = si + "/" + if header_file.path.startswith(si_path): + includes.append(header_file.path.partition(si_path)[2]) + if includes: + return includes + + # Paths for headers from external repos are prefixed with the external repo root. But the headers are + # included relative to the external workspace rootbs users + if header_file.owner.workspace_root != "": + return [header_file.path.replace(header_file.owner.workspace_root + "/", "")] + + # Default case for single header in workspace target without any special attributes + return [header_file.short_path] + +def _make_target_info(target): + includes = [] + for hdr in target[CcInfo].compilation_context.direct_headers: + inc = _get_available_include_paths( + label = target.label, + system_includes = target[CcInfo].compilation_context.system_includes, + header_file = hdr, + ) + includes.extend(inc) + + return struct(target = str(target.label), headers = [inc for inc in includes]) + +def _make_dep_info(dep): + includes = [] + for hdr in dep[CcInfo].compilation_context.direct_public_headers: + inc = _get_available_include_paths( + label = dep.label, + system_includes = dep[CcInfo].compilation_context.system_includes, + header_file = hdr, + ) + includes.extend(inc) + + for hdr in dep[CcInfo].compilation_context.direct_textual_headers: + inc = _get_available_include_paths( + label = dep.label, + system_includes = dep[CcInfo].compilation_context.system_includes, + header_file = hdr, + ) + includes.extend(inc) + + return struct(target = str(dep.label), headers = [inc for inc in includes]) + +def _make_headers_info(target, deps, impl_deps): + """ + Create a struct summarizing all information about the target and the dependency headers required for DWYU. + + Args: + target: Current target under inspection + deps: Direct dependencies of target under inspection + impl_deps: Implementation dependencies of target under inspection + """ + return struct( + self = _make_target_info(target), + public_deps = [_make_dep_info(dep) for dep in deps], + private_deps = [_make_dep_info(dep) for dep in impl_deps], + ) + +def _label_to_name(label): + return str(label).replace("//", "").replace("/", "_").replace(":", "_") + +def dwyu_aspect_impl(target, ctx): + """ + Implementation for the "Depend on What You Use" (DWYU) aspect. + + Args: + target: Target under inspection. Aspect will only do work for specific cc_* rules + ctx: Context + + Returns: + OutputGroup containing the generated report file + """ + if not ctx.rule.kind in ["cc_binary", "cc_library", "cc_test"]: + return [] + + implementation_deps = [] + ensure_private_deps = False + if ctx.attr._use_implementation_deps and hasattr(ctx.rule.attr, "implementation_deps"): + implementation_deps = ctx.rule.attr.implementation_deps + ensure_private_deps = True + + public_files, private_files = _parse_sources(ctx.rule.attr) + report_file = ctx.actions.declare_file("{}_report.txt".format(_label_to_name(target.label))) + headers_info_file = ctx.actions.declare_file("{}_deps_info.json".format(_label_to_name(target.label))) + headers_info = _make_headers_info(target = target, deps = ctx.rule.attr.deps, impl_deps = implementation_deps) + ctx.actions.write(headers_info_file, json.encode_indent(headers_info)) + + args = _make_args( + ctx = ctx, + public_files = public_files, + private_files = private_files, + report_file = report_file, + headers_info_file = headers_info_file, + ensure_private_deps = ensure_private_deps, + ) + ctx.actions.run( + executable = ctx.executable._dwyu_binary, + inputs = [headers_info_file, ctx.file._config] + public_files + private_files, + outputs = [report_file], + mnemonic = "CompareIncludesToDependencies", + progress_message = "Analyze dependencies of {}".format(target.label), + arguments = [args], + ) + + if ctx.attr._recursive: + transitive_reports = [dep[OutputGroupInfo].cc_dwyu_output for dep in ctx.rule.attr.deps] + else: + transitive_reports = [] + accumulated_reports = depset(direct = [report_file], transitive = transitive_reports) + + return [OutputGroupInfo(cc_dwyu_output = accumulated_reports)] diff --git a/aspect/factory.bzl b/aspect/factory.bzl new file mode 100644 index 00000000..268be20d --- /dev/null +++ b/aspect/factory.bzl @@ -0,0 +1,58 @@ +load(":dwyu.bzl", "dwyu_aspect_impl") + +def dwyu_aspect_factory( + config = Label("@//aspect:private/empty_default_config.json"), + recursive = False, + use_implementation_deps = False, + min_utilization = 0): + """ + Create a "Depend on What You Use" (DWYU) aspect. + + An aspect can only have default values and cannot be configured on the command line. Use this factory to create + an aspect with the desired behavior and then use it on the command line or in rules. + + Args: + config: Configuration file for the tool comparing the include statements to the dependencies. + recursive: If true, execute the aspect on all trannsitive dependencies. + If false, analyze only the target the aspect is being executed on. + use_implementation_deps: If true, ensure cc_library dependencies which are used only on private files are + listed in implementation_deps. Only available for Bazel >= 5.0.0 and if flag + '--experimental_cc_implementation_deps' is provided. + min_utilization: [Percent] Analyze how many headers from a dependency are used by the target. Fail, if a + smaller percentage than specified of the dependency headers is utilized. + CAUTION: Virtual include paths virtally increase the amount of headers available from a + dependency and thus utilization is lower than one might expect. + Returns: + Configured DWYU aspect + """ + if min_utilization < 0 or min_utilization > 100: + fail("min_utilization has to be an integer in the range [0, 100]") + + attr_aspects = ["deps"] if recursive else [] + return aspect( + implementation = dwyu_aspect_impl, + attr_aspects = attr_aspects, + attrs = { + "_dwyu_binary": attr.label( + default = Label("@//src:analyze_includes"), + allow_files = True, + executable = True, + cfg = "exec", + doc = "Tool Analyzing the include statement in the source code under inspection" + + " and comparing them to the available dependencies.", + ), + "_config": attr.label( + default = config, + allow_single_file = [".json"], + ), + "_recursive": attr.bool( + default = recursive, + ), + "_use_implementation_deps": attr.bool( + default = use_implementation_deps, + ), + "_min_utilization": attr.int( + default = min_utilization, + ), + }, + ) diff --git a/aspect/private/debugging.bzl b/aspect/private/debugging.bzl new file mode 100644 index 00000000..bd219236 --- /dev/null +++ b/aspect/private/debugging.bzl @@ -0,0 +1,13 @@ +def print_cc_info(cc_info): + print("compilation_context") + print(" defines : {}".format(cc_info.compilation_context.defines)) + print(" direct_headers : {}".format(cc_info.compilation_context.direct_headers)) + print(" direct_private_headers : {}".format(cc_info.compilation_context.direct_private_headers)) + print(" direct_public_headers : {}".format(cc_info.compilation_context.direct_public_headers)) + print(" direct_textual_headers : {}".format(cc_info.compilation_context.direct_textual_headers)) + print(" framework_includes : {}".format(cc_info.compilation_context.framework_includes)) + print(" headers : {}".format(cc_info.compilation_context.headers)) + print(" includes : {}".format(cc_info.compilation_context.includes)) + print(" local_defines : {}".format(cc_info.compilation_context.local_defines)) + print(" quote_includes : {}".format(cc_info.compilation_context.quote_includes)) + print(" system_includes : {}".format(cc_info.compilation_context.system_includes)) diff --git a/aspect/private/empty_default_config.json b/aspect/private/empty_default_config.json new file mode 100644 index 00000000..e69de29b diff --git a/defs.bzl b/defs.bzl new file mode 100644 index 00000000..38f252de --- /dev/null +++ b/defs.bzl @@ -0,0 +1,3 @@ +load("//aspect:factory.bzl", _dwyu_aspect_factory = "dwyu_aspect_factory") + +dwyu_aspect_factory = _dwyu_aspect_factory diff --git a/docs/todos.md b/docs/todos.md new file mode 100644 index 00000000..d1dd007a --- /dev/null +++ b/docs/todos.md @@ -0,0 +1,61 @@ +# New Feature: Setup CI with GitHub actions + +Currently a Linux development PC is the only machine executing the tests. +A CI system would provide a neutral source for test results and enables to provide automated feedback to pull request authors. + +Further reading on Bazel and GitHub actions: https://dev.to/davidb31/experimentations-on-bazel-github-action-5639 + +# Evaluation: Dependency utilization + +Is the feature to analyze the dependency utilization indeed useful? + +It is not part of the core DWYU concept to make sure only header from direct dependencies are used and no superfluous dependencies exist. +While the implementation of the feature is not complex it is still code which has to be tested, documented and maintained. +On top, the feature is not reliable when virtual includes are involved. +Furthermore, most likely an extension will be required to allow users to specify an allow list of dependencies where low utilizations are accepted for whatever reason. + +Then again, having small targets allows building slim and efficient dependency trees and this feature can help with that. +Moving this into an own project seems wasteful, since the implementation would be mostly identical to DWYU. + +Current ideas: +* Remove it, if user feedback for this is not positive. It is not part to invest much here, since it is not a core DWYU feature. +* If user feedback is positive, move it into an own tool/aspect but reuse the DWYU implementation. + +# New Feature: Automatic fixes + +A separate tool could read all the report files generated by DWYU and then invoke [buildozer](https://github.com/bazelbuild/buildtools/blob/master/buildozer/README.md) to apply the fixes to the workspace. +This would allow automatically removing unused dependencies and moving public dependencies to `implementation_deps`. + +# Improvement: Support more include patterns + +The [known limitations](#Known-limitations) are caused by the complexity of all possible ways to include a header in C++. +The complexity is caused mainly by the preprocessor which allows dynamically adding include statements through macros and to enable/disable includes based on defines. + +We could reimplement the preprocessor logic inside DWYU to support more complex include patterns. +The benefit would be that DYWU remains self-contained without external dependencies. +However, this would also be reinventing the wheel. +The C preprocessor is an old problem which has been solved many times until now. +Trying to implement and test a C preprocessor solely for the sake of prevent third party dependencies is wasteful. +The world does not need yet another C Preprocessor. + +No design how to proceed has been made up yet. + +The following tools could be part of the solution if we want to keep Python as implementation language: +* https://pypi.org/project/pcpp/ +* https://github.com/paulross/cpip +* https://github.com/eliben/pycparser +* https://eli.thegreenplace.net/2011/07/03/parsing-c-in-python-with-clang + +We could also move the implementation to C++. +The LLVM toolchain is a quite powerful toolset and allows interaction with the preprocessor. +See for example https://clang.llvm.org/doxygen/classclang_1_1Preprocessor.html#details. +LLVM is however not a lightweight dependency and having to compile DWYU will reduce its portability. + +# New Feature: Use bazelmod for third_party + +Bazel 5.0.0 introduced a new approach to manage external dependencies. +See [bazelmod](https://docs.bazel.build/versions/5.0.0/bzlmod.html). + +# Improvement: Utilize GitHub projects + +Utilize GitHub issues and project boards to organized the roadmap TODOs. diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 00000000..e34796ec --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,2 @@ +[tool.black] +line-length = 120 \ No newline at end of file diff --git a/scripts/extract_std_headers.py b/scripts/extract_std_headers.py new file mode 100755 index 00000000..69331531 --- /dev/null +++ b/scripts/extract_std_headers.py @@ -0,0 +1,15 @@ +#!/usr/bin/env python3 + +""" +Given a file containing the text of https://en.cppreference.com/w/cpp/header create a list of std headers +""" + +import re + +INPUT_FILE = "content_of_cppreference.txt" + +with open(INPUT_FILE, mode="r", encoding="utf-8") as fin: + for line in fin.readlines(): + include = re.findall("^<(.+)>$", line) + if len(include) == 1: + print(f'"{include[0]}",') diff --git a/src/BUILD b/src/BUILD new file mode 100644 index 00000000..abe89851 --- /dev/null +++ b/src/BUILD @@ -0,0 +1,22 @@ +load("@rules_python//python:defs.bzl", "py_binary", "py_library") + +py_library( + name = "lib", + srcs = [ + "evaluate_includes.py", + "get_dependencies.py", + "parse_config.py", + "parse_source.py", + "std_header.py", + ], + visibility = [":__subpackages__"], +) + +py_binary( + name = "analyze_includes", + srcs = ["main.py"], + legacy_create_init = False, + main = "main.py", + visibility = ["//visibility:public"], + deps = [":lib"], +) diff --git a/src/evaluate_includes.py b/src/evaluate_includes.py new file mode 100644 index 00000000..0e7c7984 --- /dev/null +++ b/src/evaluate_includes.py @@ -0,0 +1,197 @@ +from dataclasses import dataclass, field +from pathlib import Path +from typing import List + +from src.get_dependencies import AvailableDependencies, AvailableDependency, IncludeUsage +from src.parse_source import Include + + +@dataclass +class DependencyUtilization: + """How many percent of headers from a dependency are used in the target under inspection""" + + name: str + utilization: int + + +@dataclass +class Result: + invalid_includes: List[Include] = field(default_factory=list) + unused_deps: List[str] = field(default_factory=list) + deps_which_should_be_private: List[str] = field(default_factory=list) + deps_with_low_utilization: List[DependencyUtilization] = field(default_factory=list) + + def is_ok(self) -> bool: + return ( + len(self.invalid_includes) == 0 + and len(self.unused_deps) == 0 + and len(self.deps_which_should_be_private) == 0 + and len(self.deps_with_low_utilization) == 0 + ) + + def to_str(self) -> str: + if self.is_ok(): + return self._framed_msg("DWYU: Success") + else: + msg = "DWYU: Failure" + if self.invalid_includes: + msg += "\nIncludes which are not available from the direct dependencies:\n" + msg += "\n".join(f" {inc}" for inc in self.invalid_includes) + if self.unused_deps: + msg += "\nUnused dependencies (none of their headers are referenced):\n" + msg += "\n".join(f" Dependency='{dep}'" for dep in self.unused_deps) + if self.deps_which_should_be_private: + msg += ( + "\nPublic dependencies which are only used in private code, move them to 'implementation_deps':\n" + ) + msg += "\n".join(f" Dependency='{dep}'" for dep in self.deps_which_should_be_private) + if self.deps_with_low_utilization: + msg += "\nDependencies with utilization below the threshold:\n" + msg += "\n".join( + f" Dependency='{dep.name}', utilization='{dep.utilization}'" + for dep in self.deps_with_low_utilization + ) + return self._framed_msg(msg) + + @staticmethod + def _framed_msg(msg: str) -> str: + """Put a msg vertically between 2 borders""" + border = 80 * "=" + return border + "\n" + msg + "\n" + border + + +def _check_for_invalid_includes_impl( + includes: List[Include], + self: AvailableDependency, + dependencies: List[AvailableDependency], + usage: IncludeUsage, +) -> List[str]: + invalid_includes = [] + for inc in includes: + legal = False + for dep in dependencies: + for dep_hdr in dep.hdrs: + if inc.include == dep_hdr.hdr: + legal = True + dep_hdr.update_usage(usage) + break + if not legal: + # Might be a file from the target under inspection + legal = any(inc.include == sh.hdr for sh in self.hdrs) + if not legal: + # Might be a file from the target under inspection with a relative include + curr_dir = inc.file.parent + for source in self.hdrs: + try: + rel_path = Path(source.hdr).relative_to(curr_dir) + if rel_path == Path(inc.include): + legal = True + break + except ValueError: + pass + if not legal: + invalid_includes.append(inc) + return invalid_includes + + +def _check_for_invalid_includes( + public_includes: List[Include], private_includes: List[Include], dependencies: AvailableDependencies +) -> List[Include]: + """Iterate through all include statements and determine if they correspond to an available dependency""" + + invalid_includes = _check_for_invalid_includes_impl( + includes=public_includes, + self=dependencies.self, + dependencies=dependencies.public, + usage=IncludeUsage.PUBLIC, + ) + invalid_includes.extend( + _check_for_invalid_includes_impl( + includes=private_includes, + self=dependencies.self, + dependencies=dependencies.public + dependencies.private, + usage=IncludeUsage.PRIVATE, + ) + ) + return invalid_includes + + +def _check_for_unused_dependencies_impl(dependencies: List[AvailableDependency]) -> List[str]: + return [dep.name for dep in dependencies if not any((hdr.used != IncludeUsage.NONE for hdr in dep.hdrs))] + + +def _check_for_unused_dependencies(dependencies: AvailableDependencies) -> List[str]: + unused_public_deps = _check_for_unused_dependencies_impl(dependencies.public) + unused_private_deps = _check_for_unused_dependencies_impl(dependencies.private) + return unused_public_deps + unused_private_deps + + +def _check_for_public_deps_which_should_be_private(dependencies: AvailableDependencies) -> List[str]: + should_be_private = [] + for dep in dependencies.public: + if all((hdr.used in (IncludeUsage.NONE, IncludeUsage.PRIVATE) for hdr in dep.hdrs)) and any( + hdr.used != IncludeUsage.NONE for hdr in dep.hdrs + ): + should_be_private.append(dep.name) + return should_be_private + + +def _check_for_deps_whith_low_utilization_impl( + dependencies: List[AvailableDependency], min_utilization: int +) -> List[DependencyUtilization]: + lowly_utilized_deps = [] + for dep in dependencies: + used = sum(hdr.used != IncludeUsage.NONE for hdr in dep.hdrs) + utilization = int(100 * used / len(dep.hdrs)) + if utilization < min_utilization: + lowly_utilized_deps.append(DependencyUtilization(name=dep.name, utilization=utilization)) + return lowly_utilized_deps + + +def _check_for_deps_whith_low_utilization( + dependencies: AvailableDependencies, min_utilization: int +) -> List[DependencyUtilization]: + lowly_utilized_deps = _check_for_deps_whith_low_utilization_impl( + dependencies=dependencies.public, min_utilization=min_utilization + ) + lowly_utilized_deps.extend( + _check_for_deps_whith_low_utilization_impl(dependencies=dependencies.private, min_utilization=min_utilization) + ) + return lowly_utilized_deps + + +def _filter_empty_dependencies(deps: AvailableDependencies) -> AvailableDependencies: + """ + Some dependencies contain no headers and provide only libraries to link against. Since our analysis is based on + includes we are not interested in those and throw them away to prevent them raising findings regarding unused + dependencies or header utilization. + """ + return AvailableDependencies( + self=deps.self, + public=[pub for pub in deps.public if pub.hdrs], + private=[pri for pri in deps.private if pri.hdrs], + ) + + +def evaluate_includes( + public_includes: List[Include], + private_includes: List[Include], + dependencies: AvailableDependencies, + ensure_private_deps: bool, + min_dependency_utilization: int, +) -> Result: + result = Result() + dependencies = _filter_empty_dependencies(dependencies) + + result.invalid_includes = _check_for_invalid_includes( + public_includes=public_includes, private_includes=private_includes, dependencies=dependencies + ) + result.unused_deps = _check_for_unused_dependencies(dependencies) + if ensure_private_deps: + result.deps_which_should_be_private = _check_for_public_deps_which_should_be_private(dependencies) + if min_dependency_utilization > 0: + result.deps_with_low_utilization = _check_for_deps_whith_low_utilization( + dependencies=dependencies, min_utilization=min_dependency_utilization + ) + + return result diff --git a/src/get_dependencies.py b/src/get_dependencies.py new file mode 100644 index 00000000..5cf0fd24 --- /dev/null +++ b/src/get_dependencies.py @@ -0,0 +1,83 @@ +import json +from dataclasses import dataclass +from enum import Enum, auto +from pathlib import Path +from typing import Dict, List + + +class IncludeUsage(Enum): + """Classification whether a header was used in public or private files""" + + NONE = auto() + PUBLIC = auto() + PRIVATE = auto() + PUBLIC_AND_PRIVATE = auto() + + +@dataclass +class AvailableInclude: + """Include path provided by a dependency""" + + hdr: str + used: IncludeUsage = IncludeUsage.NONE + + def update_usage(self, usage: IncludeUsage) -> None: + if usage == IncludeUsage.NONE: + raise Exception("Resetting the include usage is not supported") + + if self.used == IncludeUsage.PUBLIC_AND_PRIVATE: + return + + if usage == IncludeUsage.PUBLIC_AND_PRIVATE: + self.used = IncludeUsage.PUBLIC_AND_PRIVATE + elif self.used == IncludeUsage.NONE: + self.used = usage + elif (self.used == IncludeUsage.PUBLIC and usage == IncludeUsage.PRIVATE) or ( + self.used == IncludeUsage.PRIVATE and usage == IncludeUsage.PUBLIC + ): + self.used = IncludeUsage.PUBLIC_AND_PRIVATE + + +@dataclass +class AvailableDependency: + """A dependency and the header files it provides""" + + name: str + hdrs: List[AvailableInclude] + + +@dataclass +class AvailableDependencies: + """All sources for headers the target under inspection can include""" + + # header files from target under inspection + self: AvailableDependency + # corresponds to targets from 'deps' + public: List[AvailableDependency] + # corresponds to targets from 'implementation_deps' + private: List[AvailableDependency] + + +def _make_available_dependency(info: Dict) -> AvailableDependency: + dep = AvailableDependency(name=info["target"], hdrs=[]) + for hdr in info["headers"]: + dep.hdrs.append(AvailableInclude(hdr)) + return dep + + +def _get_available_dependencies_impl(dependencies) -> List[AvailableDependency]: + includes = [] + for dep in dependencies: + avail = _make_available_dependency(dep) + includes.append(avail) + return includes + + +def get_available_dependencies(allowed_includes_file: Path) -> AvailableDependencies: + with open(allowed_includes_file, mode="r", encoding="utf-8") as fin: + loaded = json.load(fin) + return AvailableDependencies( + self=_make_available_dependency(loaded["self"]), + public=_get_available_dependencies_impl(loaded["public_deps"]), + private=_get_available_dependencies_impl(loaded["private_deps"]), + ) diff --git a/src/main.py b/src/main.py new file mode 100644 index 00000000..fba9b3bd --- /dev/null +++ b/src/main.py @@ -0,0 +1,119 @@ +import sys +from argparse import ArgumentParser +from pathlib import Path +from typing import Any + +from src.evaluate_includes import evaluate_includes +from src.get_dependencies import get_available_dependencies +from src.parse_config import load_config +from src.parse_source import get_relevant_includes_from_files +from src.std_header import STD_HEADER + + +def cli(): + parser = ArgumentParser() + parser.add_argument( + "--public-files", metavar="PATH", nargs="+", help="All public files of the target under inspection." + ) + parser.add_argument( + "--private-files", metavar="PATH", nargs="+", help="All private files of the target under inspection." + ) + parser.add_argument( + "--headers-info", metavar="PATH", help="Json file containing informations about all relevant header files." + ) + parser.add_argument("--report", metavar="FILE", help="Report result into this file.") + parser.add_argument( + "--config", + metavar="FILE", + help="Config file in Json fomrat overriting the attributes '--ignore-include-paths' and '--extra-ignore-include-paths'.", + ) + parser.add_argument( + "--implementation-deps-available", + action="store_true", + help="If this Bazel 5.0 feature is vailable, then check if some dependencies could be private instead of public." + " Meaning headers from them are only used in the private files.", + ) + parser.add_argument( + "--min-dependency-utilization", + metavar="PERCENT", + type=int, + choices=range(0, 101), + help="Report an error if a smaller fraction of the public headers of any dependency is included.", + ) + parser.add_argument( + "--ignore-include-paths", + metavar="PATH", + nargs="*", + help="By default all headers of the standard library are ignored. " + "You can however also provide your own list of include paths to be ignored.", + ) + parser.add_argument( + "--extra-ignore-include-paths", + metavar="PATH", + nargs="+", + help="By default all headers of the standard library are ignored. " + "If you want to ignore further include paths, you can specify them here. " + "If you provided a custom ignore list, the include paths here are added to it.", + ) + + args = parser.parse_args() + if not args.public_files and not args.private_files: + print("You have to provide at least one of the arguments '--public-files' and '--private-files'") + sys.exit(1) + + if args.config and (args.ignore_include_paths or args.extra_ignore_include_paths): + print("'--config' overwrites 'ignore-include-paths' and 'extra-ignore-include-paths'. Don't combine them") + sys.exit(1) + + return args + + +def _get_ignored_includes(args: Any) -> set: + if args.config: + ignored_includes, extra_ignored_includes = load_config(Path(args.config)) + ignored_includes += extra_ignored_includes + return set(ignored_includes) + + ignored_includes = STD_HEADER if not args.ignore_include_paths else {args.ignore_include_paths} + if args.extra_ignore_include_paths: + ignored_includes += {args.extra_ignore_include_paths} + + return ignored_includes + + +def main(args: Any) -> int: + ignored_includes = _get_ignored_includes(args) + + all_includes_from_public = get_relevant_includes_from_files( + files=args.public_files, ignored_includes=ignored_includes + ) + all_includes_from_private = get_relevant_includes_from_files( + files=args.private_files, ignored_includes=ignored_includes + ) + + allowed_includes = get_available_dependencies(args.headers_info) + + min_dependency_utilization = args.min_dependency_utilization if args.min_dependency_utilization else 0 + result = evaluate_includes( + public_includes=all_includes_from_public, + private_includes=all_includes_from_private, + dependencies=allowed_includes, + ensure_private_deps=args.implementation_deps_available, + min_dependency_utilization=min_dependency_utilization, + ) + + out = Path(args.report) + out.parent.mkdir(parents=True, exist_ok=True) + result_msg = result.to_str() + with open(out, mode="w", encoding="utf-8") as fout: + fout.write(result_msg) + if result.is_ok(): + return 0 + else: + print(result_msg) + return 1 + + +if __name__ == "__main__": + cli_args = cli() + sys.exit(main(cli_args)) diff --git a/src/parse_config.py b/src/parse_config.py new file mode 100644 index 00000000..3a597a1d --- /dev/null +++ b/src/parse_config.py @@ -0,0 +1,11 @@ +import json +from pathlib import Path +from typing import List, Tuple + + +def load_config(config: Path) -> Tuple[List[str], List[str]]: + with open(config, mode="r", encoding="utf-8") as fin: + loaded = json.load(fin) + ignored_includes = loaded["ignore_include_paths"] + extra_ignored_includes = loaded["extra_ignore_include_paths"] + return ignored_includes, extra_ignored_includes diff --git a/src/parse_source.py b/src/parse_source.py new file mode 100644 index 00000000..9b8987b4 --- /dev/null +++ b/src/parse_source.py @@ -0,0 +1,84 @@ +import re +from dataclasses import dataclass +from enum import Enum, auto +from pathlib import Path +from typing import List, Union + + +@dataclass +class Include: + """Single include statement in a specific file""" + + file: Path + include: str + + def __hash__(self) -> int: + return hash(str(self.file) + self.include) + + def __str__(self) -> str: + return f"File='{self.file}', include='{self.include}'" + + +def get_includes_from_file(file: Path) -> List[Include]: + """ + Parse a C/C++ file and extract include statements which are neither commented nor disabled through a define. + + Constraints on include statements which are used to simplify the logic: + - Only a single include statement can exist per line. If multiple include statement exist, the compiler ignores all + after the first one. + - Only whitespace and commented code shall exist between the line start and the include statement. + - One can concatinate multiple comment block openings "/*", but one cannot concatinate comment block ends "*/". + An appearing comment block end closes all existing comment block openings. + + Known limitations: + - Include statements which are added through a macro are not detected. + - Defines are inored. Thus, a superset of all mentioned headers is analyzed, even if normally a define would make + sure only a subset of headers is used for compilation. + - Include paths utilizing '../' are not resolved. + """ + includes = [] + with open(file, mode="r", encoding="utf-8") as fin: + inside_comment_block = False + for line in fin.readlines(): + if not inside_comment_block and "/*" in line: + if not "*/" in line: + inside_comment_block = True + continue + else: + while "*/" in line: + line = line.partition("*/")[2] + if "/*" in line: + inside_comment_block = True + continue + if inside_comment_block and not "*/" in line: + continue + if inside_comment_block and "*/" in line: + inside_comment_block = False + line = line.partition("*/")[2] + + if line.strip().startswith("#include"): + include = re.findall('#include\s*["<](.+)[">]', line) + if not include: + raise Exception(f"Did not find any include path in file '{file}' in line '{line}'") + if len(include) > 1: + raise Exception(f"Found unexpectedly multiple include paths in file '{file}' in line '{line}'") + includes.append(Include(file=file, include=include[0])) + return includes + + +def filter_includes(includes: List[Include], ignored_includes: set) -> List[Include]: + """ + - deduplicate list entries + - throw away unintersting includes (e.g. from standard library) + """ + unique_includes = set(includes) + return [ui for ui in unique_includes if not ui.include in ignored_includes] + + +def get_relevant_includes_from_files(files: Union[List[str], None], ignored_includes: set) -> List[Include]: + all_includes = [] + if files: + for f in files: + includes = get_includes_from_file(Path(f)) + all_includes.extend(includes) + return filter_includes(includes=all_includes, ignored_includes=ignored_includes) diff --git a/src/std_header.py b/src/std_header.py new file mode 100644 index 00000000..1ca608c6 --- /dev/null +++ b/src/std_header.py @@ -0,0 +1,165 @@ +""" +List is based on https://en.cppreference.com/w/cpp/header. +The content of the website has been copied into a file (plain ctrl + c) and then extract_std_headers.py has been executed on the file. + +The list is a superset of all headers, no matter in which standard they have been introduced or if they are already removed. +If you require a list tailored to a specific standard, you have to define it yourself and provide it to the tool through the CLI. +""" + +# extracted 2022.01.03 +STD_HEADER = { + # concepts + "concepts", + # coroutines + "coroutine", + # utilities - generic + "any", + "bitset", + "chrono", + "compare", + "csetjmp", + "csignal", + "cstdarg", + "cstddef", + "cstdlib", + "ctime", + "functional", + "initializer_list", + "optional", + "source_location", + "stacktrace", + "tuple", + "type_traits", + "typeindex", + "typeinfo", + "utility", + "variant", + "version", + # utilities - dynamic memory management + "memory", + "memory_resource", + "new", + "scoped_allocator", + # utilities - numeric limits + "cfloat", + "cinttypes", + "climits", + "cstdint", + "limits", + # utilities - error handling + "cassert", + "cerrno", + "exception", + "stdexcept", + "system_error", + # strings + "cctype", + "charconv", + "cstring", + "cuchar", + "cwchar", + "cwctype", + "format", + "string", + "string_view", + # containers + "array", + "deque", + "forward_list", + "list", + "map", + "queue", + "set", + "span", + "stack", + "unordered_map", + "unordered_set", + "vector", + # iterators + "iterator", + # ranges + "ranges", + # algorithms + "algorithm", + "execution", + # numerics + "bit", + "cfenv", + "cmath", + "complex", + "numbers", + "numeric", + "random", + "ratio", + "valarray", + # localization + "clocale", + "codecvt", + "locale", + # input/output + "cstdio", + "fstream", + "iomanip", + "ios", + "iosfwd", + "iostream", + "istream", + "ostream", + "spanstream", + "sstream", + "streambuf", + "strstream", + "syncstream", + # filesystem + "filesystem", + # regular expression + "regex", + # atomic operations + "atomic", + # thread support + "barrier", + "condition_variable", + "future", + "latch", + "mutex", + "semaphore", + "shared_mutex", + "stop_token", + "thread", + # C compatibility headers + "assert.h", + "ctype.h", + "errno.h", + "fenv.h", + "float.h", + "inttypes.h", + "limits.h", + "locale.h", + "math.h", + "setjmp.h", + "signal.h", + "stdarg.h", + "stddef.h", + "stdint.h", + "stdio.h", + "stdlib.h", + "string.h", + "time.h", + "uchar.h", + "wchar.h", + "wctype.h", + # special C compatibility headers + "stdatomic.h", + # Empty C headers + "ccomplex", + "complex.h", + "ctgmath", + "tgmath.h", + # Meaningless C headers + "ciso646", + "cstdalign", + "cstdbool", + "iso646.h", + "stdalign.h", + "stdbool.h", +} diff --git a/src/test/BUILD b/src/test/BUILD new file mode 100644 index 00000000..710bed08 --- /dev/null +++ b/src/test/BUILD @@ -0,0 +1,40 @@ +load("@rules_python//python:defs.bzl", "py_test") + +py_test( + name = "evaluate_includes_test", + srcs = ["evaluate_includes_test.py"], + deps = ["//src:lib"], +) + +py_test( + name = "get_dependencies_test", + srcs = ["get_dependencies_test.py"], + data = [ + "data/deps_info_empty.json", + "data/deps_info_full.json", + ], + deps = ["//src:lib"], +) + +py_test( + name = "parse_config_test", + srcs = ["parse_config_test.py"], + data = [ + "data/config.json", + "data/config_empty.json", + ], + deps = ["//src:lib"], +) + +py_test( + name = "parse_source_test", + srcs = ["parse_source_test.py"], + data = [ + "data/another_header.h", + "data/commented_includes/block_comments.h", + "data/commented_includes/mixed_style.h", + "data/commented_includes/single_line_comments.h", + "data/some_header.h", + ], + deps = ["//src:lib"], +) diff --git a/src/test/data/.clang-format b/src/test/data/.clang-format new file mode 100644 index 00000000..f1a20aaa --- /dev/null +++ b/src/test/data/.clang-format @@ -0,0 +1,4 @@ +{ + "DisableFormat": true, + "SortIncludes": "false" // with clang-format version < 13 use `false` here. For newer use `Never` +} \ No newline at end of file diff --git a/src/test/data/another_header.h b/src/test/data/another_header.h new file mode 100644 index 00000000..7cd03a17 --- /dev/null +++ b/src/test/data/another_header.h @@ -0,0 +1 @@ +#include "foo/bar.h" \ No newline at end of file diff --git a/src/test/data/commented_includes/block_comments.h b/src/test/data/commented_includes/block_comments.h new file mode 100644 index 00000000..e66297b5 --- /dev/null +++ b/src/test/data/commented_includes/block_comments.h @@ -0,0 +1,18 @@ +/* #include "commented.h" */ + +/* +#include "commented.h" +#include "commented.h" +*/ #include "active_a.h" + +/* some text */ #include "active_b.h" + +/* #include "commented.h" */ /**/ /**/ #include "active_c.h" + +/*/* #include "commented.h" */ #include "active_d.h" + +/* +/* +/* +*/ +#include "active_e.h" diff --git a/src/test/data/commented_includes/mixed_style.h b/src/test/data/commented_includes/mixed_style.h new file mode 100644 index 00000000..ae57264c --- /dev/null +++ b/src/test/data/commented_includes/mixed_style.h @@ -0,0 +1,2 @@ +/* */ // #include "commented.h" +#include "active.h" diff --git a/src/test/data/commented_includes/single_line_comments.h b/src/test/data/commented_includes/single_line_comments.h new file mode 100644 index 00000000..67cb9a5a --- /dev/null +++ b/src/test/data/commented_includes/single_line_comments.h @@ -0,0 +1,6 @@ +//#include "commented.h" +// #include + +#include "active.h" + +void Foo(); // #include "commented.h" diff --git a/src/test/data/config.json b/src/test/data/config.json new file mode 100644 index 00000000..fdcf0ee4 --- /dev/null +++ b/src/test/data/config.json @@ -0,0 +1,10 @@ +{ + "ignore_include_paths": [ + "foo", + "bar" + ], + "extra_ignore_include_paths": [ + "wup", + "wupwup" + ] +} diff --git a/src/test/data/config_empty.json b/src/test/data/config_empty.json new file mode 100644 index 00000000..fd6e0b7b --- /dev/null +++ b/src/test/data/config_empty.json @@ -0,0 +1,4 @@ +{ + "ignore_include_paths": [], + "extra_ignore_include_paths": [] +} diff --git a/src/test/data/deps_info_empty.json b/src/test/data/deps_info_empty.json new file mode 100644 index 00000000..737f1230 --- /dev/null +++ b/src/test/data/deps_info_empty.json @@ -0,0 +1,8 @@ +{ + "private_deps": [], + "public_deps": [], + "self": { + "headers": [], + "target": "//:foo" + } +} diff --git a/src/test/data/deps_info_full.json b/src/test/data/deps_info_full.json new file mode 100644 index 00000000..815dc58d --- /dev/null +++ b/src/test/data/deps_info_full.json @@ -0,0 +1,41 @@ +{ + "private_deps": [ + { + "headers": [ + "private/dep/foo_a.h", + "private/dep/foo_b.h" + ], + "target": "//private/dep:foo" + }, + { + "headers": [ + "private/dep/bar_a.h", + "private/dep/bar_b.h" + ], + "target": "//private/dep:bar" + } + ], + "public_deps": [ + { + "headers": [ + "public/dep/foo_a.h", + "public/dep/foo_b.h" + ], + "target": "//public/dep:foo" + }, + { + "headers": [ + "public/dep/bar_a.h", + "public/dep/bar_b.h" + ], + "target": "//public/dep:bar" + } + ], + "self": { + "headers": [ + "self/a.h", + "self/b.h" + ], + "target": "//:baz" + } +} diff --git a/src/test/data/some_header.h b/src/test/data/some_header.h new file mode 100644 index 00000000..6077c9ef --- /dev/null +++ b/src/test/data/some_header.h @@ -0,0 +1,9 @@ +#ifndef SOME_HEADER_H +#define SOME_HEADER_H + +#include "bar.h" + #include "foo/bar/baz.h" +#include +#include + +#endif \ No newline at end of file diff --git a/src/test/evaluate_includes_test.py b/src/test/evaluate_includes_test.py new file mode 100644 index 00000000..88b3c01a --- /dev/null +++ b/src/test/evaluate_includes_test.py @@ -0,0 +1,314 @@ +import unittest +from pathlib import Path + +from src.evaluate_includes import DependencyUtilization, Result, evaluate_includes +from src.get_dependencies import AvailableDependencies, AvailableDependency, AvailableInclude +from src.parse_source import Include + + +class TestResult(unittest.TestCase): + @staticmethod + def _expected_msg(msg): + border = 80 * "=" + return border + "\n" + msg + "\n" + border + + def test_is_ok(self): + unit = Result() + + self.assertTrue(unit.is_ok()) + self.assertEqual(unit.to_str(), self._expected_msg("DWYU: Success")) + + def test_is_ok_fails_due_to_invalid_includes(self): + unit = Result(invalid_includes=[Include(file=Path("foo"), include="bar")]) + + self.assertFalse(unit.is_ok()) + self.assertEqual( + unit.to_str(), + self._expected_msg( + "DWYU: Failure\n" + "Includes which are not available from the direct dependencies:\n" + " File='foo', include='bar'" + ), + ) + + def test_is_ok_fails_due_to_unused_deps(self): + unit = Result(unused_deps=["foo"]) + + self.assertFalse(unit.is_ok()) + self.assertEqual( + unit.to_str(), + self._expected_msg( + "DWYU: Failure\nUnused dependencies (none of their headers are referenced):\n Dependency='foo'" + ), + ) + + def test_is_ok_fails_due_to_deps_which_should_be_private(self): + unit = Result(deps_which_should_be_private=["foo"]) + + self.assertFalse(unit.is_ok()) + self.assertEqual( + unit.to_str(), + self._expected_msg( + "DWYU: Failure\n" + "Public dependencies which are only used in private code, move them to 'implementation_deps':\n" + " Dependency='foo'" + ), + ) + + def test_is_ok_fails_due_to_lowly_utilized_deps(self): + unit = Result(deps_with_low_utilization=[DependencyUtilization(name="foo", utilization=42)]) + + self.assertFalse(unit.is_ok()) + self.assertEqual( + unit.to_str(), + self._expected_msg( + "DWYU: Failure\n" + "Dependencies with utilization below the threshold:\n" + " Dependency='foo', utilization='42'" + ), + ) + + +class TestEvaluateIncludes(unittest.TestCase): + def test_success_for_valid_external_dependencies(self): + result = evaluate_includes( + public_includes=[ + Include(file=Path("file1"), include="foo.h"), + Include(file=Path("file2"), include="foo/bar.h"), + ], + private_includes=[ + Include(file=Path("file3"), include="baz.h"), + Include(file=Path("file4"), include="self/own_header.h"), + ], + dependencies=AvailableDependencies( + self=AvailableDependency(name="self", hdrs=[AvailableInclude("self/own_header.h")]), + public=[ + AvailableDependency( + name="foo_pkg", hdrs=[AvailableInclude("foo.h"), AvailableInclude("foo/bar.h")] + ), + AvailableDependency(name="lib_without_hdrs_purely_for_linking", hdrs=[]), + ], + private=[AvailableDependency(name="baz_pkg", hdrs=[AvailableInclude("baz.h")])], + ), + ensure_private_deps=True, + min_dependency_utilization=0, + ) + + self.assertTrue(result.is_ok()) + + def test_success_for_target_internal_includes_with_flat_structure(self): + result = evaluate_includes( + public_includes=[Include(file=Path("foo.h"), include="bar.h")], + private_includes=[], + dependencies=AvailableDependencies( + self=AvailableDependency(name="", hdrs=[AvailableInclude("foo.h"), AvailableInclude("bar.h")]), + public=[], + private=[], + ), + ensure_private_deps=True, + min_dependency_utilization=0, + ) + + self.assertTrue(result.is_ok()) + + def test_success_for_target_internal_includes_with_nested_structure(self): + result = evaluate_includes( + public_includes=[ + Include(file=Path("nested/dir/foo.h"), include="bar.h"), + Include(file=Path("nested/dir/foo.h"), include="sub/baz.h"), + ], + private_includes=[], + dependencies=AvailableDependencies( + self=AvailableDependency( + name="self", + hdrs=[ + AvailableInclude("nested/dir/foo.h"), + AvailableInclude("nested/dir/bar.h"), + AvailableInclude("nested/dir/sub/baz.h"), + ], + ), + public=[], + private=[], + ), + ensure_private_deps=True, + min_dependency_utilization=0, + ) + + self.assertTrue(result.is_ok()) + + def test_invalid_includes_missing_internal_include(self): + result = evaluate_includes( + public_includes=[Include(file=Path("nested/dir/foo.h"), include="bar.h")], + private_includes=[], + dependencies=AvailableDependencies( + self=AvailableDependency( + name="self", hdrs=[AvailableInclude("nested/dir/foo.h"), AvailableInclude("some/other/dir/bar.h")] + ), + public=[], + private=[], + ), + ensure_private_deps=True, + min_dependency_utilization=0, + ) + + self.assertFalse(result.is_ok()) + self.assertEqual(result.unused_deps, []) + self.assertEqual(result.deps_which_should_be_private, []) + self.assertEqual(result.deps_with_low_utilization, []) + self.assertEqual(result.invalid_includes, [Include(file=Path("nested/dir/foo.h"), include="bar.h")]) + + def test_missing_includes_from_dependencies(self): + result = evaluate_includes( + public_includes=[ + Include(file=Path("public_file"), include="foo.h"), + Include(file=Path("public_file"), include="foo/foo.h"), + Include(file=Path("public_file"), include="foo/bar.h"), + ], + private_includes=[ + Include(file=Path("private_file"), include="bar.h"), + Include(file=Path("private_file"), include="bar/foo.h"), + Include(file=Path("private_file"), include="bar/bar.h"), + ], + dependencies=AvailableDependencies( + self=AvailableDependency(name="", hdrs=[]), + public=[AvailableDependency(name="foo", hdrs=[AvailableInclude("foo.h")])], + private=[AvailableDependency(name="bar", hdrs=[AvailableInclude("bar.h")])], + ), + ensure_private_deps=True, + min_dependency_utilization=0, + ) + + self.assertFalse(result.is_ok()) + self.assertEqual(result.unused_deps, []) + self.assertEqual(result.deps_which_should_be_private, []) + self.assertEqual(result.deps_with_low_utilization, []) + self.assertEqual(len(result.invalid_includes), 4) + self.assertTrue(Include(file=Path("public_file"), include="foo/foo.h") in result.invalid_includes) + self.assertTrue(Include(file=Path("public_file"), include="foo/bar.h") in result.invalid_includes) + self.assertTrue(Include(file=Path("private_file"), include="bar/foo.h") in result.invalid_includes) + self.assertTrue(Include(file=Path("private_file"), include="bar/bar.h") in result.invalid_includes) + + def test_unused_dependencies(self): + result = evaluate_includes( + public_includes=[Include(file=Path("public_file"), include="foobar.h")], + private_includes=[Include(file=Path("private_file"), include="impl_dep.h")], + dependencies=AvailableDependencies( + self=AvailableDependency(name="", hdrs=[]), + public=[ + AvailableDependency(name="foobar", hdrs=[AvailableInclude("foobar.h")]), + AvailableDependency(name="foo", hdrs=[AvailableInclude("foo.h")]), + AvailableDependency(name="bar", hdrs=[AvailableInclude("bar.h")]), + ], + private=[ + AvailableDependency(name="impl_dep", hdrs=[AvailableInclude("impl_dep.h")]), + AvailableDependency(name="impl_foo", hdrs=[AvailableInclude("impl_dep_foo.h")]), + AvailableDependency(name="impl_bar", hdrs=[AvailableInclude("impl_dep_bar.h")]), + ], + ), + ensure_private_deps=True, + min_dependency_utilization=0, + ) + + self.assertFalse(result.is_ok()) + self.assertEqual(result.invalid_includes, []) + self.assertEqual(result.deps_which_should_be_private, []) + self.assertEqual(result.deps_with_low_utilization, []) + self.assertEqual(len(result.unused_deps), 4) + self.assertTrue("foo" in result.unused_deps) + self.assertTrue("bar" in result.unused_deps) + self.assertTrue("impl_foo" in result.unused_deps) + self.assertTrue("impl_bar" in result.unused_deps) + + def test_public_dependencies_which_should_be_private(self): + result = evaluate_includes( + public_includes=[Include(file=Path("public_file"), include="foobar.h")], + private_includes=[ + Include(file=Path("private_file"), include="impl_dep_foo.h"), + Include(file=Path("private_file"), include="impl_dep_bar.h"), + ], + dependencies=AvailableDependencies( + self=AvailableDependency(name="", hdrs=[]), + public=[ + AvailableDependency(name="foobar", hdrs=[AvailableInclude("foobar.h")]), + AvailableDependency(name="foo", hdrs=[AvailableInclude("impl_dep_foo.h")]), + AvailableDependency(name="bar", hdrs=[AvailableInclude("impl_dep_bar.h")]), + ], + private=[], + ), + ensure_private_deps=True, + min_dependency_utilization=0, + ) + + self.assertFalse(result.is_ok()) + self.assertEqual(result.invalid_includes, []) + self.assertEqual(result.unused_deps, []) + self.assertEqual(result.deps_with_low_utilization, []) + self.assertEqual(len(result.deps_which_should_be_private), 2) + self.assertTrue("foo" in result.deps_which_should_be_private) + self.assertTrue("bar" in result.deps_which_should_be_private) + + def test_public_dependencies_which_should_be_private_disabled(self): + result = evaluate_includes( + public_includes=[Include(file=Path("public_file"), include="foobar.h")], + private_includes=[ + Include(file=Path("private_file"), include="impl_dep_foo.h"), + Include(file=Path("private_file"), include="impl_dep_bar.h"), + ], + dependencies=AvailableDependencies( + self=AvailableDependency(name="", hdrs=[]), + public=[ + AvailableDependency(name="foobar", hdrs=[AvailableInclude("foobar.h")]), + AvailableDependency(name="foo", hdrs=[AvailableInclude("impl_dep_foo.h")]), + AvailableDependency(name="bar", hdrs=[AvailableInclude("impl_dep_bar.h")]), + ], + private=[], + ), + ensure_private_deps=False, + min_dependency_utilization=0, + ) + + self.assertTrue(result.is_ok()) + + def test_min_dependecy_utilization_infringed(self): + result = evaluate_includes( + public_includes=[Include(file=Path("public_file"), include="foo1.h")], + private_includes=[Include(file=Path("private_file"), include="bar1.h")], + dependencies=AvailableDependencies( + self=AvailableDependency(name="", hdrs=[]), + public=[ + AvailableDependency( + name="foo", + hdrs=[ + AvailableInclude("foo1.h"), + AvailableInclude("foo2.h"), + AvailableInclude("foo3.h"), + AvailableInclude("foo4.h"), + ], + ), + ], + private=[ + AvailableDependency( + name="bar", + hdrs=[ + AvailableInclude("bar1.h"), + AvailableInclude("bar2.h"), + AvailableInclude("bar3.h"), + ], + ), + ], + ), + ensure_private_deps=True, + min_dependency_utilization=50, + ) + + self.assertFalse(result.is_ok()) + self.assertEqual(result.invalid_includes, []) + self.assertEqual(result.unused_deps, []) + self.assertEqual(result.deps_which_should_be_private, []) + self.assertEqual(len(result.deps_with_low_utilization), 2) + self.assertTrue(DependencyUtilization(name="foo", utilization=25) in result.deps_with_low_utilization) + self.assertTrue(DependencyUtilization(name="bar", utilization=33) in result.deps_with_low_utilization) + + +if __name__ == "__main__": + unittest.main() diff --git a/src/test/get_dependencies_test.py b/src/test/get_dependencies_test.py new file mode 100644 index 00000000..0e94242a --- /dev/null +++ b/src/test/get_dependencies_test.py @@ -0,0 +1,101 @@ +import unittest +from pathlib import Path + +from src.get_dependencies import AvailableInclude, IncludeUsage, get_available_dependencies + + +class TestAvailableInclude(unittest.TestCase): + def test_default_ctor(self): + unit = AvailableInclude(hdr="hdr") + + self.assertEqual(unit.used, IncludeUsage.NONE) + + def test_make_public(self): + unit = AvailableInclude(hdr="hdr", used=IncludeUsage.NONE) + unit.update_usage(IncludeUsage.PUBLIC) + + self.assertEqual(unit.used, IncludeUsage.PUBLIC) + + def test_make_private(self): + unit = AvailableInclude(hdr="hdr", used=IncludeUsage.NONE) + unit.update_usage(IncludeUsage.PRIVATE) + + self.assertEqual(unit.used, IncludeUsage.PRIVATE) + + def test_make_public_and_private(self): + unit = AvailableInclude(hdr="hdr", used=IncludeUsage.NONE) + unit.update_usage(IncludeUsage.PUBLIC_AND_PRIVATE) + + self.assertEqual(unit.used, IncludeUsage.PUBLIC_AND_PRIVATE) + + def test_add_private_to_public(self): + unit = AvailableInclude(hdr="hdr", used=IncludeUsage.PUBLIC) + unit.update_usage(IncludeUsage.PRIVATE) + + self.assertEqual(unit.used, IncludeUsage.PUBLIC_AND_PRIVATE) + + def test_add_public_to_private(self): + unit = AvailableInclude(hdr="hdr", used=IncludeUsage.PRIVATE) + unit.update_usage(IncludeUsage.PUBLIC) + + self.assertEqual(unit.used, IncludeUsage.PUBLIC_AND_PRIVATE) + + def test_public_and_private_state_is_stable(self): + unit = AvailableInclude(hdr="hdr", used=IncludeUsage.PUBLIC_AND_PRIVATE) + + unit.update_usage(IncludeUsage.PUBLIC) + self.assertEqual(unit.used, IncludeUsage.PUBLIC_AND_PRIVATE) + + unit.update_usage(IncludeUsage.PRIVATE) + self.assertEqual(unit.used, IncludeUsage.PUBLIC_AND_PRIVATE) + + unit.update_usage(IncludeUsage.PUBLIC_AND_PRIVATE) + self.assertEqual(unit.used, IncludeUsage.PUBLIC_AND_PRIVATE) + + def test_no_usage_resetting(self): + unit = AvailableInclude(hdr="hdr") + with self.assertRaises(Exception): + unit.update_usage(IncludeUsage.NONE) + + +class TestGetAvailableDependencies(unittest.TestCase): + def test_load_full_file(self): + deps = get_available_dependencies(Path("src/test/data/deps_info_full.json")) + + self.assertEqual(len(deps.private), 2) + self.assertEqual(len(deps.public), 2) + + self.assertEqual(deps.private[0].name, "//private/dep:foo") + self.assertEqual( + deps.private[0].hdrs, [AvailableInclude("private/dep/foo_a.h"), AvailableInclude("private/dep/foo_b.h")] + ) + self.assertEqual(deps.private[1].name, "//private/dep:bar") + self.assertEqual( + deps.private[1].hdrs, [AvailableInclude("private/dep/bar_a.h"), AvailableInclude("private/dep/bar_b.h")] + ) + + self.assertEqual(deps.public[0].name, "//public/dep:foo") + self.assertEqual( + deps.public[0].hdrs, [AvailableInclude("public/dep/foo_a.h"), AvailableInclude("public/dep/foo_b.h")] + ) + self.assertEqual(deps.public[1].name, "//public/dep:bar") + self.assertEqual( + deps.public[1].hdrs, [AvailableInclude("public/dep/bar_a.h"), AvailableInclude("public/dep/bar_b.h")] + ) + + self.assertEqual(deps.self.name, "//:baz") + self.assertEqual(len(deps.self.hdrs), 2) + self.assertEqual(deps.self.hdrs, [AvailableInclude("self/a.h"), AvailableInclude("self/b.h")]) + + def test_load_empty_file(self): + deps = get_available_dependencies(Path("src/test/data/deps_info_empty.json")) + + self.assertEqual(len(deps.private), 0) + self.assertEqual(len(deps.public), 0) + + self.assertEqual(deps.self.name, "//:foo") + self.assertEqual(len(deps.self.hdrs), 0) + + +if __name__ == "__main__": + unittest.main() diff --git a/src/test/parse_config_test.py b/src/test/parse_config_test.py new file mode 100644 index 00000000..f3745d8e --- /dev/null +++ b/src/test/parse_config_test.py @@ -0,0 +1,26 @@ +import unittest +from pathlib import Path + +from src.parse_config import load_config + + +class TestLoadConfig(unittest.TestCase): + def test_load_empty(self): + ignored_inclues, extra_ignoredf_includes = load_config(Path("src/test/data/config_empty.json")) + + self.assertEqual(ignored_inclues, []) + self.assertEqual(extra_ignoredf_includes, []) + + def test_load_config(self): + ignored_includes, extra_ignored_includes = load_config(Path("src/test/data/config.json")) + + self.assertEqual(len(ignored_includes), 2) + self.assertTrue("foo" in ignored_includes) + self.assertTrue("bar" in ignored_includes) + self.assertEqual(len(extra_ignored_includes), 2) + self.assertTrue("wup" in extra_ignored_includes) + self.assertTrue("wupwup" in extra_ignored_includes) + + +if __name__ == "__main__": + unittest.main() diff --git a/src/test/parse_source_test.py b/src/test/parse_source_test.py new file mode 100644 index 00000000..046866b3 --- /dev/null +++ b/src/test/parse_source_test.py @@ -0,0 +1,101 @@ +import unittest +from pathlib import Path + +from src.parse_source import Include, filter_includes, get_includes_from_file, get_relevant_includes_from_files + + +class TestInclude(unittest.TestCase): + def test_in(self): + unit = [Include(file=Path("foo"), include="foo.h"), Include(file=Path("bar"), include="bar.h")] + + self.assertTrue(Include(file=Path("foo"), include="foo.h") in unit) + self.assertTrue(Include(file=Path("bar"), include="bar.h") in unit) + + def test_hash(self): + foo = Include(file=Path("foo"), include="bar") + unit = {foo, foo, Include(file=Path("foo"), include="bar"), Include(file=Path("bar"), include="foo")} + + self.assertEqual(len(unit), 2) + self.assertTrue(Include(file=Path("foo"), include="bar") in unit) + self.assertTrue(Include(file=Path("bar"), include="foo") in unit) + + def test_str(self): + unit = Include(file=Path("foo"), include="bar") + + self.assertEqual(str(unit), "File='foo', include='bar'") + + +class TestFilterIncludes(unittest.TestCase): + def test_filter_includes(self): + result = filter_includes( + includes=[ + Include(file=Path("file1"), include="hiho.h"), + Include(file=Path("file2"), include="foo"), + Include(file=Path("file3"), include="some/deep/path.h"), + Include(file=Path("file4"), include="bar/baz.h"), + ], + ignored_includes={"foo", "bar/baz.h"}, + ) + + self.assertEqual(len(result), 2) + self.assertTrue(Include(file=Path("file1"), include="hiho.h") in result) + self.assertTrue(Include(file=Path("file3"), include="some/deep/path.h") in result) + + +class TestGetIncludesFromFile(unittest.TestCase): + def test_single_include(self): + test_file = Path("src/test/data/another_header.h") + result = get_includes_from_file(test_file) + + self.assertEqual(result, [Include(file=test_file, include="foo/bar.h")]) + + def test_multiple_includes(self): + test_file = Path("src/test/data/some_header.h") + result = get_includes_from_file(test_file) + + self.assertEqual(len(result), 4) + self.assertTrue(Include(file=test_file, include="bar.h") in result) + self.assertTrue(Include(file=test_file, include="foo/bar/baz.h") in result) + self.assertTrue(Include(file=test_file, include="some/path/to_a/system_header.h") in result) + + def test_commented_includes_single_line_comments(self): + test_file = Path("src/test/data/commented_includes/single_line_comments.h") + result = get_includes_from_file(test_file) + + self.assertEqual(result, [Include(file=test_file, include="active.h")]) + + def test_commented_includes_block_comments(self): + test_file = Path("src/test/data/commented_includes/block_comments.h") + result = get_includes_from_file(test_file) + + self.assertEqual(len(result), 5) + self.assertTrue(Include(file=test_file, include="active_a.h") in result) + self.assertTrue(Include(file=test_file, include="active_b.h") in result) + self.assertTrue(Include(file=test_file, include="active_c.h") in result) + self.assertTrue(Include(file=test_file, include="active_d.h") in result) + self.assertTrue(Include(file=test_file, include="active_e.h") in result) + + def test_commented_includes_mixed_style(self): + test_file = Path("src/test/data/commented_includes/mixed_style.h") + result = get_includes_from_file(test_file) + + self.assertEqual(result, [Include(file=test_file, include="active.h")]) + + +class TestGetRelevantIncludesFromFiles(unittest.TestCase): + def test_get_relevant_includes_from_files(self): + result = get_relevant_includes_from_files( + files=["src/test/data/some_header.h", "src/test/data/another_header.h"], ignored_includes={"vector"} + ) + + self.assertEqual(len(result), 4) + self.assertTrue(Include(file=Path("src/test/data/some_header.h"), include="bar.h") in result) + self.assertTrue(Include(file=Path("src/test/data/some_header.h"), include="foo/bar/baz.h") in result) + self.assertTrue( + Include(file=Path("src/test/data/some_header.h"), include="some/path/to_a/system_header.h") in result + ) + self.assertTrue(Include(file=Path("src/test/data/another_header.h"), include="foo/bar.h") in result) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/BUILD b/test/BUILD new file mode 100644 index 00000000..899f47e3 --- /dev/null +++ b/test/BUILD @@ -0,0 +1,9 @@ +load("@rules_python//python:defs.bzl", "py_test") + +py_test( + name = "execute_tests_utest", + srcs = [ + "execute_tests_impl.py", + "execute_tests_utest.py", + ], +) diff --git a/test/README.md b/test/README.md new file mode 100644 index 00000000..9f02ae24 --- /dev/null +++ b/test/README.md @@ -0,0 +1,11 @@ +This directory contains the acceptance tests. +They are executed with various supported Bazel versions. +[bazelisk](https://github.com/bazelbuild/bazelisk) has to be available on `PATH`. + +The test cases and their expected behavior are defined in [execute_tests.py](execute_tests.py). + +Execute all tests with various Bazel versions: \ +`./execute_tests.py` + +You can execute specific test cases or use specific Bazel versions. For details see the help: \ +`./execute_tests.py --help` diff --git a/test/aspect.bzl b/test/aspect.bzl new file mode 100644 index 00000000..aaad17ce --- /dev/null +++ b/test/aspect.bzl @@ -0,0 +1,3 @@ +load("//:defs.bzl", "dwyu_aspect_factory") + +dwyu_default_aspect = dwyu_aspect_factory() diff --git a/test/complex_includes/BUILD b/test/complex_includes/BUILD new file mode 100644 index 00000000..99f8a9c7 --- /dev/null +++ b/test/complex_includes/BUILD @@ -0,0 +1,23 @@ +cc_library( + name = "complex_includes", + srcs = ["complex_includes.cpp"], + hdrs = [ + "some/dir/complex_include_a.h", + "some/dir/complex_include_b.h", + ], + include_prefix = "virtual/prefix", + includes = ["some"], + strip_include_prefix = "some", +) + +cc_library( + name = "use_complex_includes", + srcs = ["use_complex_includes.cpp"], + deps = [":complex_includes"], +) + +cc_library( + name = "use_complex_includes_from_extern", + srcs = ["use_complex_includes_from_extern.cpp"], + deps = ["@complex_includes_repo//:complex_includes"], +) diff --git a/test/complex_includes/README.md b/test/complex_includes/README.md new file mode 100644 index 00000000..2ac21328 --- /dev/null +++ b/test/complex_includes/README.md @@ -0,0 +1 @@ +These tests show that complex combinations of include path modifications can be handled. diff --git a/test/complex_includes/complex_includes.cpp b/test/complex_includes/complex_includes.cpp new file mode 100644 index 00000000..bf57b22f --- /dev/null +++ b/test/complex_includes/complex_includes.cpp @@ -0,0 +1,12 @@ +#include +#include "virtual/prefix/dir/complex_include_b.h" + +int doComplexIncludeA() +{ + return 42; +} + +int doComplexIncludeB() +{ + return 24; +} diff --git a/test/complex_includes/ext_repo.bzl b/test/complex_includes/ext_repo.bzl new file mode 100644 index 00000000..4cf19c46 --- /dev/null +++ b/test/complex_includes/ext_repo.bzl @@ -0,0 +1,5 @@ +def load_complex_includes_repo(): + native.local_repository( + name = "complex_includes_repo", + path = "test/complex_includes/ext_repo", + ) diff --git a/test/complex_includes/ext_repo/BUILD b/test/complex_includes/ext_repo/BUILD new file mode 100644 index 00000000..0e9efddb --- /dev/null +++ b/test/complex_includes/ext_repo/BUILD @@ -0,0 +1,19 @@ +cc_library( + name = "complex_includes", + srcs = ["complex_includes.cpp"], + hdrs = [ + "some/dir/complex_include_a.h", + "some/dir/complex_include_b.h", + ], + include_prefix = "virtual/prefix", + includes = ["some"], + strip_include_prefix = "some", + visibility = ["//visibility:public"], +) + +cc_library( + name = "use_complex_includes", + srcs = ["use_complex_includes.cpp"], + visibility = ["//visibility:public"], + deps = [":complex_includes"], +) diff --git a/test/complex_includes/ext_repo/WORKSPACE b/test/complex_includes/ext_repo/WORKSPACE new file mode 100644 index 00000000..e69de29b diff --git a/test/complex_includes/ext_repo/complex_includes.cpp b/test/complex_includes/ext_repo/complex_includes.cpp new file mode 100644 index 00000000..bf57b22f --- /dev/null +++ b/test/complex_includes/ext_repo/complex_includes.cpp @@ -0,0 +1,12 @@ +#include +#include "virtual/prefix/dir/complex_include_b.h" + +int doComplexIncludeA() +{ + return 42; +} + +int doComplexIncludeB() +{ + return 24; +} diff --git a/test/complex_includes/ext_repo/some/dir/complex_include_a.h b/test/complex_includes/ext_repo/some/dir/complex_include_a.h new file mode 100644 index 00000000..3c4fa5c3 --- /dev/null +++ b/test/complex_includes/ext_repo/some/dir/complex_include_a.h @@ -0,0 +1,6 @@ +#ifndef COMPLEX_INCLUDE_A_H +#define COMPLEX_INCLUDE_A_H + +int doComplexIncludeA(); + +#endif diff --git a/test/complex_includes/ext_repo/some/dir/complex_include_b.h b/test/complex_includes/ext_repo/some/dir/complex_include_b.h new file mode 100644 index 00000000..b0459f29 --- /dev/null +++ b/test/complex_includes/ext_repo/some/dir/complex_include_b.h @@ -0,0 +1,6 @@ +#ifndef COMPLEX_INCLUDE_B_H +#define COMPLEX_INCLUDE_B_H + +int doComplexIncludeB(); + +#endif diff --git a/test/complex_includes/ext_repo/use_complex_includes.cpp b/test/complex_includes/ext_repo/use_complex_includes.cpp new file mode 100644 index 00000000..d0872c1d --- /dev/null +++ b/test/complex_includes/ext_repo/use_complex_includes.cpp @@ -0,0 +1,7 @@ +#include +#include "virtual/prefix/dir/complex_include_b.h" + +int useComplexIncludes() +{ + return doComplexIncludeA() + doComplexIncludeB(); +} diff --git a/test/complex_includes/some/dir/complex_include_a.h b/test/complex_includes/some/dir/complex_include_a.h new file mode 100644 index 00000000..3c4fa5c3 --- /dev/null +++ b/test/complex_includes/some/dir/complex_include_a.h @@ -0,0 +1,6 @@ +#ifndef COMPLEX_INCLUDE_A_H +#define COMPLEX_INCLUDE_A_H + +int doComplexIncludeA(); + +#endif diff --git a/test/complex_includes/some/dir/complex_include_b.h b/test/complex_includes/some/dir/complex_include_b.h new file mode 100644 index 00000000..b0459f29 --- /dev/null +++ b/test/complex_includes/some/dir/complex_include_b.h @@ -0,0 +1,6 @@ +#ifndef COMPLEX_INCLUDE_B_H +#define COMPLEX_INCLUDE_B_H + +int doComplexIncludeB(); + +#endif diff --git a/test/complex_includes/use_complex_includes.cpp b/test/complex_includes/use_complex_includes.cpp new file mode 100644 index 00000000..d0872c1d --- /dev/null +++ b/test/complex_includes/use_complex_includes.cpp @@ -0,0 +1,7 @@ +#include +#include "virtual/prefix/dir/complex_include_b.h" + +int useComplexIncludes() +{ + return doComplexIncludeA() + doComplexIncludeB(); +} diff --git a/test/complex_includes/use_complex_includes_from_extern.cpp b/test/complex_includes/use_complex_includes_from_extern.cpp new file mode 100644 index 00000000..e69de29b diff --git a/test/custom_config/BUILD b/test/custom_config/BUILD new file mode 100644 index 00000000..9ac084c6 --- /dev/null +++ b/test/custom_config/BUILD @@ -0,0 +1,11 @@ +filegroup( + name = "custom_config", + srcs = ["custom_config.json"], + visibility = ["//visibility:public"], +) + +# Has an invalid include, which does however not cause an error due to the ognore list in the config +cc_library( + name = "foo", + hdrs = ["foo.h"], +) diff --git a/test/custom_config/README.md b/test/custom_config/README.md new file mode 100644 index 00000000..64d3dfeb --- /dev/null +++ b/test/custom_config/README.md @@ -0,0 +1 @@ +Users can provide their own config for headers which shall be ignored. diff --git a/test/custom_config/aspect.bzl b/test/custom_config/aspect.bzl new file mode 100644 index 00000000..c8baa2fc --- /dev/null +++ b/test/custom_config/aspect.bzl @@ -0,0 +1,3 @@ +load("//:defs.bzl", "dwyu_aspect_factory") + +custom_config_aspect = dwyu_aspect_factory(config = "//test/custom_config") diff --git a/test/custom_config/custom_config.json b/test/custom_config/custom_config.json new file mode 100644 index 00000000..2078269e --- /dev/null +++ b/test/custom_config/custom_config.json @@ -0,0 +1,6 @@ +{ + "ignore_include_paths": [ + "some_arcane_header.h" + ], + "extra_ignore_include_paths": [] +} diff --git a/test/custom_config/foo.h b/test/custom_config/foo.h new file mode 100644 index 00000000..1bc61692 --- /dev/null +++ b/test/custom_config/foo.h @@ -0,0 +1 @@ +#include "some_arcane_header.h" diff --git a/test/dependency_utilization/BUILD b/test/dependency_utilization/BUILD new file mode 100644 index 00000000..12456a28 --- /dev/null +++ b/test/dependency_utilization/BUILD @@ -0,0 +1,20 @@ +cc_library( + name = "lib", + hdrs = [ + "lib_part_a.h", + "lib_part_b.h", + "lib_part_c.h", + ], +) + +cc_binary( + name = "utilization_low", + srcs = ["utilization_low.cpp"], + deps = [":lib"], +) + +cc_binary( + name = "utilization_high", + srcs = ["utilization_high.cpp"], + deps = [":lib"], +) diff --git a/test/dependency_utilization/README.md b/test/dependency_utilization/README.md new file mode 100644 index 00000000..7662b1a5 --- /dev/null +++ b/test/dependency_utilization/README.md @@ -0,0 +1,2 @@ +Libraries from which only a small fraction of headers is used are a hint that those targets can be split more fine grained. +The DWYU aspect allows finding such cases. diff --git a/test/dependency_utilization/aspect.bzl b/test/dependency_utilization/aspect.bzl new file mode 100644 index 00000000..00f07682 --- /dev/null +++ b/test/dependency_utilization/aspect.bzl @@ -0,0 +1,3 @@ +load("//:defs.bzl", "dwyu_aspect_factory") + +utilization_aspect = dwyu_aspect_factory(min_utilization = 50) diff --git a/test/dependency_utilization/lib_part_a.h b/test/dependency_utilization/lib_part_a.h new file mode 100644 index 00000000..e69de29b diff --git a/test/dependency_utilization/lib_part_b.h b/test/dependency_utilization/lib_part_b.h new file mode 100644 index 00000000..e69de29b diff --git a/test/dependency_utilization/lib_part_c.h b/test/dependency_utilization/lib_part_c.h new file mode 100644 index 00000000..e69de29b diff --git a/test/dependency_utilization/utilization_high.cpp b/test/dependency_utilization/utilization_high.cpp new file mode 100644 index 00000000..f89babac --- /dev/null +++ b/test/dependency_utilization/utilization_high.cpp @@ -0,0 +1,7 @@ +#include "test/dependency_utilization/lib_part_a.h" +#include "test/dependency_utilization/lib_part_b.h" + +int main() +{ + return 0; +} diff --git a/test/dependency_utilization/utilization_low.cpp b/test/dependency_utilization/utilization_low.cpp new file mode 100644 index 00000000..8062ca4b --- /dev/null +++ b/test/dependency_utilization/utilization_low.cpp @@ -0,0 +1,6 @@ +#include "test/dependency_utilization/lib_part_a.h" + +int main() +{ + return 0; +} diff --git a/test/execute_tests.py b/test/execute_tests.py new file mode 100755 index 00000000..78006e81 --- /dev/null +++ b/test/execute_tests.py @@ -0,0 +1,165 @@ +#!/usr/bin/env python3 +import sys +from execute_tests_impl import ExpectedResult, TestCase, TestCmd, cli, main + +BAZEL_VERSIONS = [ + "4.0.0", + "4.2.2", + "5.0.0", + "6.0.0-pre.20220127.1", +] + +VERSION_SPECIFIC_ARGS = { + "5.0.0": [ + "--incompatible_enforce_config_setting_visibility", + "--incompatible_config_setting_private_default_visibility", + ] +} + +DEFAULT_ASPECT = "//test:aspect.bzl%dwyu_default_aspect" + +TESTS = [ + TestCase( + name="custom_config", + cmd=TestCmd(target="//test/custom_config:foo", aspect="//test/custom_config:aspect.bzl%custom_config_aspect"), + expected=ExpectedResult(success=True), + ), + TestCase( + name="unused_dep", + cmd=TestCmd(target="//test/unused_dep:main", aspect=DEFAULT_ASPECT), + expected=ExpectedResult(success=False, unused_deps=["//test/unused_dep:foo"]), + ), + TestCase( + name="use_transitive_deps", + cmd=TestCmd(target="//test/using_transitive_dep:main", aspect=DEFAULT_ASPECT), + expected=ExpectedResult( + success=False, + invalid_includes=["File='test/using_transitive_dep/main.cpp', include='test/using_transitive_dep/foo.h'"], + ), + ), + TestCase( + name="low_dependecy_utilization", + cmd=TestCmd( + target="//test/dependency_utilization:utilization_low", + aspect="//test/dependency_utilization:aspect.bzl%utilization_aspect", + ), + expected=ExpectedResult( + success=False, + deps_with_low_utilization=["Dependency='//test/dependency_utilization:lib', utilization='33"], + ), + ), + TestCase( + name="high_dependecy_utilization", + cmd=TestCmd( + target="//test/dependency_utilization:utilization_high", + aspect="//test/dependency_utilization:aspect.bzl%utilization_aspect", + ), + expected=ExpectedResult(success=True), + ), + TestCase( + name="use_external_dependencies", + cmd=TestCmd(target="//test/external_repo:use_external_libs", aspect=DEFAULT_ASPECT), + expected=ExpectedResult(success=True), + ), + TestCase( + name="proper implementation_deps", + min_version="5.0.0", + cmd=TestCmd( + target="//test/implementation_deps:proper_private_deps", + aspect="//test/implementation_deps:aspect.bzl%implementation_deps_aspect", + extra_args=["--experimental_cc_implementation_deps"], + ), + expected=ExpectedResult(success=True), + ), + TestCase( + name="superfluous public_dep", + min_version="5.0.0", + cmd=TestCmd( + target="//test/implementation_deps:superfluous_public_dep", + aspect="//test/implementation_deps:aspect.bzl%implementation_deps_aspect", + extra_args=["--experimental_cc_implementation_deps"], + ), + expected=ExpectedResult(success=False, deps_which_should_be_private=["//test/implementation_deps:foo"]), + ), + TestCase( + name="generated_code", + cmd=TestCmd(target="//test/generated_code:foo", aspect=DEFAULT_ASPECT), + expected=ExpectedResult(success=True), + ), + # Make sure headers from own lib are discovered correctly + TestCase( + name="system_includes_lib", + cmd=TestCmd(target="//test/includes:includes", aspect=DEFAULT_ASPECT), + expected=ExpectedResult(success=True), + ), + TestCase( + name="system_includes_use_lib", + cmd=TestCmd(target="//test/includes:use_includes", aspect=DEFAULT_ASPECT), + expected=ExpectedResult(success=True), + ), + # Make sure headers from own lib are discovered correctly + TestCase( + name="virtual_includes_add_prefix_lib", + cmd=TestCmd(target="//test/virtual_includes:prefixed", aspect=DEFAULT_ASPECT), + expected=ExpectedResult(success=True), + ), + TestCase( + name="virtual_includes_use_add_prefix_lib", + cmd=TestCmd(target="//test/virtual_includes:use_prefixed", aspect=DEFAULT_ASPECT), + expected=ExpectedResult(success=True), + ), + # Make sure headers from own lib are discovered correctly + TestCase( + name="virtual_includes_strip_prefix_lib", + cmd=TestCmd(target="//test/virtual_includes:stripped", aspect=DEFAULT_ASPECT), + expected=ExpectedResult(success=True), + ), + TestCase( + name="virtual_includes_use_strip_prefix_lib", + cmd=TestCmd(target="//test/virtual_includes:use_stripped", aspect=DEFAULT_ASPECT), + expected=ExpectedResult(success=True), + ), + TestCase( + name="valid", + cmd=TestCmd(target="//test/valid:bar", aspect=DEFAULT_ASPECT), + expected=ExpectedResult(success=True), + ), + TestCase( + name="recursive_disabled", + cmd=TestCmd(target="//test/recursion:main", aspect=DEFAULT_ASPECT), + expected=ExpectedResult(success=True), + ), + TestCase( + name="recursive_fail_transitively", + cmd=TestCmd(target="//test/recursion:main", aspect="//test/recursion:aspect.bzl%recursive_aspect"), + expected=ExpectedResult(success=False, unused_deps=["//test/recursion:e"]), + ), + TestCase( + name="rule_direct_success", + cmd=TestCmd(target="//test/recursion:dwyu_direct_main"), + expected=ExpectedResult(success=True), + ), + TestCase( + name="rule_recursive_failure", + cmd=TestCmd(target="//test/recursion:dwyu_recursive_main"), + expected=ExpectedResult(success=False, unused_deps=["//test/recursion:e"]), + ), + TestCase( + name="complex_includes", + min_version="4.1.0", # Does not even compile with 4.0.0 + cmd=TestCmd(target="//test/complex_includes:all"), + expected=ExpectedResult(success=True), + ), + TestCase( + name="complex_includes_in_ext_repo", + min_version="4.1.0", # Does not even compile with 4.0.0 + cmd=TestCmd(target="@complex_includes_repo//..."), + expected=ExpectedResult(success=True), + ), +] + +if __name__ == "__main__": + args = cli() + sys.exit( + main(args=args, test_cases=TESTS, test_versions=BAZEL_VERSIONS, version_specific_args=VERSION_SPECIFIC_ARGS) + ) diff --git a/test/execute_tests_impl.py b/test/execute_tests_impl.py new file mode 100644 index 00000000..215423f4 --- /dev/null +++ b/test/execute_tests_impl.py @@ -0,0 +1,220 @@ +import argparse +import os +import subprocess as sb +from dataclasses import dataclass, field +from typing import Dict, List, Union + +# Each line in the output corresponding to an error is expexted to start with this +ERRORS_PREFIX = " " * 2 + +# DWYU terminal output key fragments +DWYU_FAILURE = "DWYU: Failure" +CATEGORY_INVALID_INCLUDES = "Includes which are not available from the direct dependencies" +CATEGORY_NON_PRIVATE_DEPS = "Public dependencies which are only used in private code" +CATEGORY_UNUSED_DEPS = "Unused dependencies (none of their headers are referenced)" +CATEGORY_UTILIZATION = "Dependencies with utilization below the threshold" + + +@dataclass +class TestCmd: + target: str + aspect: str = "" + extra_args: List[str] = field(default_factory=list) + + +@dataclass +class ExpectedResult: + """ + TODO + """ + + success: bool + invalid_includes: List[str] = field(default_factory=list) + unused_deps: List[str] = field(default_factory=list) + deps_which_should_be_private: List[str] = field(default_factory=list) + deps_with_low_utilization: List[str] = field(default_factory=list) + + def matches_expectation(self, return_code: int, dwyu_output: str) -> bool: + if not self._has_correct_status(return_code=return_code, output=dwyu_output): + return False + + output_lines = dwyu_output.split("\n") + if not ExpectedResult._has_expected_errors( + expected_errors=self.invalid_includes, + error_category=CATEGORY_INVALID_INCLUDES, + output=output_lines, + ): + return False + if not ExpectedResult._has_expected_errors( + expected_errors=self.unused_deps, + error_category=CATEGORY_UNUSED_DEPS, + output=output_lines, + ): + return False + if not ExpectedResult._has_expected_errors( + expected_errors=self.deps_which_should_be_private, + error_category=CATEGORY_NON_PRIVATE_DEPS, + output=output_lines, + ): + return False + if not ExpectedResult._has_expected_errors( + expected_errors=self.deps_with_low_utilization, + error_category=CATEGORY_UTILIZATION, + output=output_lines, + ): + return False + + return True + + def _has_correct_status(self, return_code: int, output: str) -> bool: + if self.success and return_code == 0: + return True + if not self.success and return_code != 0 and DWYU_FAILURE in output: + return True + return False + + @staticmethod + def _has_expected_errors(expected_errors: List[str], error_category: str, output: List[str]) -> bool: + idx_category = ExpectedResult._find_line_with(lines=output, val=error_category) + if expected_errors and idx_category is None: + return False + if expected_errors: + # todo idx magic into helper func + errors_begin = idx_category + 1 + errors_end = 0 + for i in range(errors_begin, len(output)): + if output[i].startswith(ERRORS_PREFIX): + errors_end = i + 1 + else: + break + if not ExpectedResult._has_errors( + error_lines=output[errors_begin:errors_end], expected_errors=expected_errors + ): + return False + if not expected_errors and not idx_category is None: + return False + + return True + + @staticmethod + def _find_line_with(lines: List[str], val: str) -> Union[int, None]: + for idx, line in enumerate(lines): + if val in line: + return idx + return None + + @staticmethod + def _has_errors(error_lines: List[str], expected_errors: List[str]) -> bool: + if len(error_lines) != len(expected_errors): + return False + for error in expected_errors: + if not any(error in line for line in error_lines): + return False + return True + + +@dataclass +class TestCase: + name: str + cmd: TestCmd + expected: ExpectedResult + min_version: str = "" + + +@dataclass +class FailedTest: + name: str + version: str + + +def verify_test(test: TestCase, process: sb.CompletedProcess, verbose: bool) -> bool: + if test.expected.matches_expectation(return_code=process.returncode, dwyu_output=process.stdout): + if verbose: + print("\n" + process.stdout + "\n") + ok_verb = "succeeded" if test.expected.success else "failed" + print(f"<<< OK '{test.name}' {ok_verb} as expected") + return True + else: + print("\n" + process.stdout + "\n") + print(f"<<< ERROR '{test.name}' did not behave as expected") + return False + + +def make_cmd(test_cmd: TestCmd, extra_args: List[str]) -> List[str]: + cmd = ["bazelisk", "build", "--noshow_progress"] + if test_cmd.aspect: + cmd.extend([f"--aspects={test_cmd.aspect}", "--output_groups=cc_dwyu_output"]) + cmd.extend(extra_args) + cmd.extend(test_cmd.extra_args) + cmd.append("--") + cmd.append(test_cmd.target) + return cmd + + +def execute_tests( + versions: List[str], tests: List[TestCase], version_specific_args: Dict, verbose: bool = False +) -> List[FailedTest]: + failed_tests = [] + + test_env = {} + test_env["HOME"] = os.getenv("HOME") # Required by Bazel to determine bazel-out + test_env["PATH"] = os.getenv("PATH") # Access to bazelisk + + for version in versions: + test_env["USE_BAZEL_VERSION"] = version + + extra_args = [] + for args_version, args in version_specific_args.items(): + if args_version <= version: + extra_args.extend(args) + + for test in tests: + if test.min_version and test.min_version > version: + print(f"\n--- Skip '{test.name}' due to incompatible Bazel '{version}'") + continue + else: + print(f"\n>>> Execute '{test.name}' with Bazel '{version}'") + + process = sb.run( + make_cmd(test_cmd=test.cmd, extra_args=extra_args), + env=test_env, + text=True, + stdout=sb.PIPE, + stderr=sb.STDOUT, + ) + + if not verify_test(test=test, process=process, verbose=verbose): + failed_tests.append(FailedTest(name=test.name, version=version)) + + return failed_tests + + +def cli(): + parser = argparse.ArgumentParser() + parser.add_argument("--verbose", "-v", action="store_true", help="Show output of test runs.") + parser.add_argument("--bazel", "-b", metavar="VERSION", help="Run tests with the specified Bazel version.") + parser.add_argument("--test", "-t", nargs="+", help="Run the specified test cases.") + args = parser.parse_args() + return args + + +def main(args: argparse.Namespace, test_cases: List[TestCase], test_versions: List[str], version_specific_args: Dict): + bazel_versions = [args.bazel] if args.bazel else test_versions + if args.test: + active_tests = [tc for tc in test_cases if tc.name in args.test] + else: + active_tests = test_cases + + failed_tests = execute_tests( + versions=bazel_versions, tests=active_tests, version_specific_args=version_specific_args, verbose=args.verbose + ) + + print("\n" + 30 * "#" + " SUMMARY " + 30 * "#" + "\n") + if failed_tests: + print("FAILURE\n") + print("These tests failed:") + print("\n".join(f"- '{t.name} - for Bazel version '{t.version}'" for t in failed_tests)) + return 1 + else: + print("SUCCESS\n") + return 0 diff --git a/test/execute_tests_utest.py b/test/execute_tests_utest.py new file mode 100644 index 00000000..5f79b117 --- /dev/null +++ b/test/execute_tests_utest.py @@ -0,0 +1,185 @@ +import unittest + +from execute_tests_impl import ( + CATEGORY_INVALID_INCLUDES, + CATEGORY_NON_PRIVATE_DEPS, + CATEGORY_UNUSED_DEPS, + CATEGORY_UTILIZATION, + DWYU_FAILURE, + ERRORS_PREFIX, + ExpectedResult, + TestCmd, + make_cmd, +) + + +class TestExpectedResult(unittest.TestCase): + @staticmethod + def _make_error_output(category, errors): + msg = DWYU_FAILURE + "\n" + msg += category + "\n" + msg += "\n".join(f"{ERRORS_PREFIX}{err}" for err in errors) + return msg + + def test_expected_success_ok(self): + unit = ExpectedResult(success=True) + self.assertTrue(unit.matches_expectation(return_code=0, dwyu_output="")) + + def test_expected_success_error(self): + unit = ExpectedResult(success=True) + self.assertFalse(unit.matches_expectation(return_code=1, dwyu_output="")) + + def test_expected_failure_ok(self): + unit = ExpectedResult(success=False) + self.assertTrue(unit.matches_expectation(return_code=1, dwyu_output=DWYU_FAILURE)) + + def test_expected_failure_error(self): + unit = ExpectedResult(success=False) + self.assertFalse(unit.matches_expectation(return_code=0, dwyu_output="")) + + def test_expected_failure_error_due_to_missing_output(self): + unit = ExpectedResult(success=False) + self.assertFalse(unit.matches_expectation(return_code=1, dwyu_output="")) + + def test_expected_fail_due_to_invalid_includes(self): + unit = ExpectedResult(success=False, invalid_includes=["foo/bar.cpp", "bar/foo.h"]) + self.assertTrue( + unit.matches_expectation( + return_code=1, + dwyu_output=self._make_error_output( + category=CATEGORY_INVALID_INCLUDES, errors=["foo/bar.cpp", "bar/foo.h"] + ), + ) + ) + + def test_expected_fail_due_to_invalid_includes_fails(self): + unit = ExpectedResult(success=False, invalid_includes=["foo/bar.cpp", "bar/foo.h"]) + self.assertFalse( + unit.matches_expectation( + return_code=1, + dwyu_output=self._make_error_output(category=CATEGORY_INVALID_INCLUDES, errors=["foo/bar.cpp"]), + ) + ) + + def test_expected_fail_due_to_invalid_includes_fails_on_other_error(self): + unit = ExpectedResult(success=False, invalid_includes=["foo/bar.cpp", "bar/foo.h"]) + for cat in [CATEGORY_NON_PRIVATE_DEPS, CATEGORY_UNUSED_DEPS, CATEGORY_UTILIZATION]: + self.assertFalse( + unit.matches_expectation( + return_code=1, + dwyu_output=self._make_error_output(category=cat, errors=["foo/bar.cpp", "bar/foo.h"]), + ) + ) + + def test_expected_fail_due_to_unused_deps(self): + unit = ExpectedResult(success=False, unused_deps=["//foo:bar", "//bar:foo"]) + self.assertTrue( + unit.matches_expectation( + return_code=1, + dwyu_output=self._make_error_output(category=CATEGORY_UNUSED_DEPS, errors=["//foo:bar", "//bar:foo"]), + ) + ) + + def test_expected_fail_due_to_unused_deps_fails(self): + unit = ExpectedResult(success=False, unused_deps=["//foo:bar", "//bar:foo"]) + self.assertFalse( + unit.matches_expectation( + return_code=1, + dwyu_output=self._make_error_output(category=CATEGORY_UNUSED_DEPS, errors=["//foo:bar"]), + ) + ) + + def test_expected_fail_due_to_unused_deps_fails_on_other_error(self): + unit = ExpectedResult(success=False, unused_deps=["//foo:bar", "//bar:foo"]) + for cat in [CATEGORY_INVALID_INCLUDES, CATEGORY_NON_PRIVATE_DEPS, CATEGORY_UTILIZATION]: + self.assertFalse( + unit.matches_expectation( + return_code=1, + dwyu_output=self._make_error_output(category=cat, errors=["//foo:bar", "//bar:foo"]), + ) + ) + + def test_expected_fail_due_to_non_private_deps(self): + unit = ExpectedResult(success=False, deps_which_should_be_private=["//foo:bar", "//bar:foo"]) + self.assertTrue( + unit.matches_expectation( + return_code=1, + dwyu_output=self._make_error_output( + category=CATEGORY_NON_PRIVATE_DEPS, errors=["//foo:bar", "//bar:foo"] + ), + ) + ) + + def test_expected_fail_due_to_non_private_deps_fails(self): + unit = ExpectedResult(success=False, deps_which_should_be_private=["//foo:bar", "//bar:foo"]) + self.assertFalse( + unit.matches_expectation( + return_code=1, + dwyu_output=self._make_error_output(category=CATEGORY_NON_PRIVATE_DEPS, errors=["//foo:bar"]), + ) + ) + + def test_expected_fail_due_to_non_private_deps_fails_on_other_error(self): + unit = ExpectedResult(success=False, deps_which_should_be_private=["//foo:bar", "//bar:foo"]) + for cat in [CATEGORY_INVALID_INCLUDES, CATEGORY_UNUSED_DEPS, CATEGORY_UTILIZATION]: + self.assertFalse( + unit.matches_expectation( + return_code=1, + dwyu_output=self._make_error_output(category=cat, errors=["//foo:bar", "//bar:foo"]), + ) + ) + + def test_expected_fail_due_to_low_utilization(self): + unit = ExpectedResult(success=False, deps_with_low_utilization=["//foo:bar", "//bar:foo"]) + self.assertTrue( + unit.matches_expectation( + return_code=1, + dwyu_output=self._make_error_output(category=CATEGORY_UTILIZATION, errors=["//foo:bar", "//bar:foo"]), + ) + ) + + def test_expected_fail_due_to_low_utilization_fails(self): + unit = ExpectedResult(success=False, deps_with_low_utilization=["//foo:bar", "//bar:foo"]) + self.assertFalse( + unit.matches_expectation( + return_code=1, + dwyu_output=self._make_error_output(category=CATEGORY_UTILIZATION, errors=["//foo:bar"]), + ) + ) + + def test_expected_fail_due_to_low_utilization_on_other_error(self): + unit = ExpectedResult(success=False, deps_with_low_utilization=["//foo:bar", "//bar:foo"]) + for cat in [CATEGORY_INVALID_INCLUDES, CATEGORY_NON_PRIVATE_DEPS, CATEGORY_UNUSED_DEPS]: + self.assertFalse( + unit.matches_expectation( + return_code=1, + dwyu_output=self._make_error_output(category=cat, errors=["//foo:bar", "//bar:foo"]), + ) + ) + + +class TestMakeCmd(unittest.TestCase): + @staticmethod + def _base_cmd(): + return ["bazelisk", "build", "--noshow_progress"] + + def test_with_aspect(self): + cmd = make_cmd(test_cmd=TestCmd(target="//foo:bar"), extra_args=[]) + self.assertEqual(cmd, self._base_cmd() + ["--", "//foo:bar"]) + + def test_with_aspect(self): + cmd = make_cmd(test_cmd=TestCmd(target="//foo:bar", aspect="//some/aspect.bzl"), extra_args=[]) + self.assertEqual( + cmd, + self._base_cmd() + ["--aspects=//some/aspect.bzl", "--output_groups=cc_dwyu_output", "--", "//foo:bar"], + ) + + def test_with_extra_args(self): + cmd = make_cmd( + test_cmd=TestCmd(target="//foo:bar", extra_args=["--abc", "--cba"]), extra_args=["--some_bazel_extra_arg"] + ) + self.assertEqual(cmd, self._base_cmd() + ["--some_bazel_extra_arg", "--abc", "--cba", "--", "//foo:bar"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/external_repo/BUILD b/test/external_repo/BUILD new file mode 100644 index 00000000..861c6cd4 --- /dev/null +++ b/test/external_repo/BUILD @@ -0,0 +1,8 @@ +cc_library( + name = "use_external_libs", + hdrs = ["use_external_libs.h"], + deps = [ + "@ext_repo//:ext_bar", + "@ext_repo//:ext_foo", + ], +) diff --git a/test/external_repo/README.md b/test/external_repo/README.md new file mode 100644 index 00000000..05a74b5d --- /dev/null +++ b/test/external_repo/README.md @@ -0,0 +1 @@ +Headers from external dependencies shall be mapped to their include statements. diff --git a/test/external_repo/repo.bzl b/test/external_repo/repo.bzl new file mode 100644 index 00000000..773e5f37 --- /dev/null +++ b/test/external_repo/repo.bzl @@ -0,0 +1,5 @@ +def load_external_repo(): + native.local_repository( + name = "ext_repo", + path = "test/external_repo/repo", + ) diff --git a/test/external_repo/repo/BUILD b/test/external_repo/repo/BUILD new file mode 100644 index 00000000..58a502c4 --- /dev/null +++ b/test/external_repo/repo/BUILD @@ -0,0 +1,11 @@ +cc_library( + name = "ext_bar", + hdrs = ["some/dir/bar.h"], + visibility = ["//visibility:public"], +) + +cc_library( + name = "ext_foo", + hdrs = ["foo.h"], + visibility = ["//visibility:public"], +) diff --git a/test/external_repo/repo/WORKSPACE b/test/external_repo/repo/WORKSPACE new file mode 100644 index 00000000..e69de29b diff --git a/test/external_repo/repo/foo.h b/test/external_repo/repo/foo.h new file mode 100644 index 00000000..27a7cb33 --- /dev/null +++ b/test/external_repo/repo/foo.h @@ -0,0 +1,9 @@ +#ifndef FOO_H +#define FOO_H + +int doFoo() +{ + return 42; +} + +#endif diff --git a/test/external_repo/repo/some/dir/bar.h b/test/external_repo/repo/some/dir/bar.h new file mode 100644 index 00000000..ca2c6627 --- /dev/null +++ b/test/external_repo/repo/some/dir/bar.h @@ -0,0 +1,9 @@ +#ifndef BAR_H +#define BAR_H + +int doBar() +{ + return 13; +} + +#endif diff --git a/test/external_repo/use_external_libs.h b/test/external_repo/use_external_libs.h new file mode 100644 index 00000000..5cd0eab2 --- /dev/null +++ b/test/external_repo/use_external_libs.h @@ -0,0 +1,12 @@ +#ifndef USE_EXTERNAL_LIBS_H +#define USE_EXTERNAL_LIBS_H + +#include "foo.h" +#include "some/dir/bar.h" + +int useLibs() +{ + return doFoo() + doBar(); +} + +#endif diff --git a/test/generated_code/BUILD b/test/generated_code/BUILD new file mode 100644 index 00000000..9fc5399a --- /dev/null +++ b/test/generated_code/BUILD @@ -0,0 +1,16 @@ +genrule( + name = "bar_h", + outs = ["bar.h"], + cmd = "echo '// Header content' > \"$@\"", +) + +cc_library( + name = "bar", + hdrs = ["bar.h"], +) + +cc_binary( + name = "foo", + srcs = ["foo.cpp"], + deps = [":bar"], +) diff --git a/test/generated_code/REAME.md b/test/generated_code/REAME.md new file mode 100644 index 00000000..e882e1de --- /dev/null +++ b/test/generated_code/REAME.md @@ -0,0 +1 @@ +Headers generated into the output tree shall be mapped to their include statements. diff --git a/test/generated_code/foo.cpp b/test/generated_code/foo.cpp new file mode 100644 index 00000000..0f54ca99 --- /dev/null +++ b/test/generated_code/foo.cpp @@ -0,0 +1,6 @@ +#include "test/generated_code/bar.h" + +int main() +{ + return 0; +} diff --git a/test/implementation_deps/BUILD b/test/implementation_deps/BUILD new file mode 100644 index 00000000..1b2a8d95 --- /dev/null +++ b/test/implementation_deps/BUILD @@ -0,0 +1,27 @@ +cc_library( + name = "foo", + hdrs = ["foo.h"], +) + +cc_library( + name = "bar", + hdrs = ["bar.h"], +) + +cc_library( + name = "proper_private_deps", + srcs = ["use_libs.cpp"], + hdrs = ["use_libs.h"], + implementation_deps = [":foo"], + deps = [":bar"], +) + +cc_library( + name = "superfluous_public_dep", + srcs = ["use_libs.cpp"], + hdrs = ["use_libs.h"], + deps = [ + ":bar", + ":foo", + ], +) diff --git a/test/implementation_deps/README.md b/test/implementation_deps/README.md new file mode 100644 index 00000000..91ed4f6a --- /dev/null +++ b/test/implementation_deps/README.md @@ -0,0 +1 @@ +If `implementation_deps` are available, all dependencies which are solely used in private files shall be listed in `implementation_deps`. diff --git a/test/implementation_deps/aspect.bzl b/test/implementation_deps/aspect.bzl new file mode 100644 index 00000000..46686e05 --- /dev/null +++ b/test/implementation_deps/aspect.bzl @@ -0,0 +1,3 @@ +load("//:defs.bzl", "dwyu_aspect_factory") + +implementation_deps_aspect = dwyu_aspect_factory(use_implementation_deps = True) diff --git a/test/implementation_deps/bar.h b/test/implementation_deps/bar.h new file mode 100644 index 00000000..ca2c6627 --- /dev/null +++ b/test/implementation_deps/bar.h @@ -0,0 +1,9 @@ +#ifndef BAR_H +#define BAR_H + +int doBar() +{ + return 13; +} + +#endif diff --git a/test/implementation_deps/foo.h b/test/implementation_deps/foo.h new file mode 100644 index 00000000..27a7cb33 --- /dev/null +++ b/test/implementation_deps/foo.h @@ -0,0 +1,9 @@ +#ifndef FOO_H +#define FOO_H + +int doFoo() +{ + return 42; +} + +#endif diff --git a/test/implementation_deps/use_libs.cpp b/test/implementation_deps/use_libs.cpp new file mode 100644 index 00000000..13e4d109 --- /dev/null +++ b/test/implementation_deps/use_libs.cpp @@ -0,0 +1,6 @@ +#include "test/implementation_deps/foo.h" + +int useLibs() +{ + return doFoo() + 11; +} diff --git a/test/implementation_deps/use_libs.h b/test/implementation_deps/use_libs.h new file mode 100644 index 00000000..ea9eac4e --- /dev/null +++ b/test/implementation_deps/use_libs.h @@ -0,0 +1,13 @@ +#ifndef USE_LIBS_H +#define USE_LIBS_H + +#include "test/implementation_deps/bar.h" + +int wrapBar() +{ + return doBar() * 2; +} + +int useLibs(); + +#endif diff --git a/test/includes/BUILD b/test/includes/BUILD new file mode 100644 index 00000000..ca4d6032 --- /dev/null +++ b/test/includes/BUILD @@ -0,0 +1,15 @@ +cc_library( + name = "includes", + srcs = ["includes.cpp"], + hdrs = ["some/sub/dir/includes.h"], + includes = [ + "some", + "some/sub", + ], +) + +cc_binary( + name = "use_includes", + srcs = ["use_includes.cpp"], + deps = [":includes"], +) diff --git a/test/includes/README.md b/test/includes/README.md new file mode 100644 index 00000000..fd332c8a --- /dev/null +++ b/test/includes/README.md @@ -0,0 +1 @@ +Include paths created by the `cc_*` rule attribute `includes` shall not prevent mapping headers to their include statements. diff --git a/test/includes/includes.cpp b/test/includes/includes.cpp new file mode 100644 index 00000000..e0a44136 --- /dev/null +++ b/test/includes/includes.cpp @@ -0,0 +1,8 @@ +// Header is available via multiple paths +#include +#include + +int doIncludes() +{ + return 42; +} diff --git a/test/includes/some/sub/dir/includes.h b/test/includes/some/sub/dir/includes.h new file mode 100644 index 00000000..474bde33 --- /dev/null +++ b/test/includes/some/sub/dir/includes.h @@ -0,0 +1,6 @@ +#ifndef INCLUDES_H +#define INCLUDES_H + +int doIncludes(); + +#endif diff --git a/test/includes/use_includes.cpp b/test/includes/use_includes.cpp new file mode 100644 index 00000000..6177fe6a --- /dev/null +++ b/test/includes/use_includes.cpp @@ -0,0 +1,8 @@ +// Header is available via multiple paths +#include +#include + +int main() +{ + return doIncludes(); +} diff --git a/test/recursion/BUILD b/test/recursion/BUILD new file mode 100644 index 00000000..8d3aa315 --- /dev/null +++ b/test/recursion/BUILD @@ -0,0 +1,53 @@ +load("//test/recursion:rule.bzl", "dwyu_rule_direct", "dwyu_rule_recursive") + +# Has a diamond dependency towards C through A and B +# main specifies its dependencies correctly, but C has an error +cc_binary( + name = "main", + srcs = ["main.cpp"], + deps = [ + ":a", + ":b", + ], +) + +dwyu_rule_direct( + name = "dwyu_direct_main", + deps = [":main"], +) + +dwyu_rule_recursive( + name = "dwyu_recursive_main", + deps = [":main"], +) + +cc_library( + name = "a", + hdrs = ["a.h"], + deps = [":c"], +) + +cc_library( + name = "b", + hdrs = ["b.h"], + deps = [":c"], +) + +cc_library( + name = "c", + hdrs = ["c.h"], + deps = [ + ":d", + ":e", # unused dependency + ], +) + +cc_library( + name = "d", + hdrs = ["d.h"], +) + +cc_library( + name = "e", + hdrs = ["e.h"], +) diff --git a/test/recursion/README.md b/test/recursion/README.md new file mode 100644 index 00000000..420cab80 --- /dev/null +++ b/test/recursion/README.md @@ -0,0 +1,3 @@ +The aspect can analyze either solely the targets is is being executed on or it can can analyze the whole dependency tree. + +One can use a rule to perform the aspect analysis as part of a normal build. diff --git a/test/recursion/a.h b/test/recursion/a.h new file mode 100644 index 00000000..bfce8b0e --- /dev/null +++ b/test/recursion/a.h @@ -0,0 +1,6 @@ +#ifndef A_H +#define A_H + +#include "test/recursion/c.h" + +#endif diff --git a/test/recursion/aspect.bzl b/test/recursion/aspect.bzl new file mode 100644 index 00000000..2b742bfe --- /dev/null +++ b/test/recursion/aspect.bzl @@ -0,0 +1,3 @@ +load("//:defs.bzl", "dwyu_aspect_factory") + +recursive_aspect = dwyu_aspect_factory(recursive = True) diff --git a/test/recursion/b.h b/test/recursion/b.h new file mode 100644 index 00000000..5ec73be2 --- /dev/null +++ b/test/recursion/b.h @@ -0,0 +1,6 @@ +#ifndef B_H +#define B_H + +#include "test/recursion/c.h" + +#endif diff --git a/test/recursion/c.h b/test/recursion/c.h new file mode 100644 index 00000000..7f6b301f --- /dev/null +++ b/test/recursion/c.h @@ -0,0 +1,6 @@ +#ifndef C_H +#define C_H + +#include "test/recursion/d.h" + +#endif diff --git a/test/recursion/d.h b/test/recursion/d.h new file mode 100644 index 00000000..f9938f5d --- /dev/null +++ b/test/recursion/d.h @@ -0,0 +1,4 @@ +#ifndef D_H +#define D_H + +#endif diff --git a/test/recursion/e.h b/test/recursion/e.h new file mode 100644 index 00000000..d49613aa --- /dev/null +++ b/test/recursion/e.h @@ -0,0 +1,4 @@ +#ifndef E_H +#define E_H + +#endif diff --git a/test/recursion/main.cpp b/test/recursion/main.cpp new file mode 100644 index 00000000..e80f90ad --- /dev/null +++ b/test/recursion/main.cpp @@ -0,0 +1,7 @@ +#include "test/recursion/a.h" +#include "test/recursion/b.h" + +int main() +{ + return 0; +} diff --git a/test/recursion/rule.bzl b/test/recursion/rule.bzl new file mode 100644 index 00000000..16d3df1d --- /dev/null +++ b/test/recursion/rule.bzl @@ -0,0 +1,21 @@ +load("//test:aspect.bzl", "dwyu_default_aspect") +load("//test/recursion:aspect.bzl", "recursive_aspect") + +def _dwyu_rule_impl(ctx): + # gather artifacts to make sure the aspect is executed + aspect_artifacts = depset(transitive = [dep[OutputGroupInfo].cc_dwyu_output for dep in ctx.attr.deps]) + return [DefaultInfo(files = aspect_artifacts)] + +dwyu_rule_direct = rule( + implementation = _dwyu_rule_impl, + attrs = { + "deps": attr.label_list(aspects = [dwyu_default_aspect]), + }, +) + +dwyu_rule_recursive = rule( + implementation = _dwyu_rule_impl, + attrs = { + "deps": attr.label_list(aspects = [recursive_aspect]), + }, +) diff --git a/test/test.bzl b/test/test.bzl new file mode 100644 index 00000000..6ce482fe --- /dev/null +++ b/test/test.bzl @@ -0,0 +1,6 @@ +load("//test/external_repo:repo.bzl", "load_external_repo") +load("//test/complex_includes:ext_repo.bzl", "load_complex_includes_repo") + +def test_setup(): + load_external_repo() + load_complex_includes_repo() diff --git a/test/unused_dep/BUILD b/test/unused_dep/BUILD new file mode 100644 index 00000000..f5dea5e5 --- /dev/null +++ b/test/unused_dep/BUILD @@ -0,0 +1,19 @@ +cc_library( + name = "foo", + hdrs = ["foo.h"], +) + +cc_library( + name = "bar", + hdrs = ["bar.h"], +) + +# ERROR: main uses only bar, but also depends on foo +cc_binary( + name = "main", + srcs = ["main.cpp"], + deps = [ + ":bar", + ":foo", + ], +) diff --git a/test/unused_dep/README.md b/test/unused_dep/README.md new file mode 100644 index 00000000..38920df3 --- /dev/null +++ b/test/unused_dep/README.md @@ -0,0 +1 @@ +Dependencies from which not a single header is referenced shall cause a failure. diff --git a/test/unused_dep/bar.h b/test/unused_dep/bar.h new file mode 100644 index 00000000..5775ec8a --- /dev/null +++ b/test/unused_dep/bar.h @@ -0,0 +1,9 @@ +#ifndef BAR_H +#define BAR_H + +int doStuff() +{ + return 1; +} + +#endif diff --git a/test/unused_dep/foo.h b/test/unused_dep/foo.h new file mode 100644 index 00000000..e732c1b6 --- /dev/null +++ b/test/unused_dep/foo.h @@ -0,0 +1,9 @@ +#ifndef FOO_H +#define FOO_H + +int theAnswer() +{ + return 42; +} + +#endif diff --git a/test/unused_dep/main.cpp b/test/unused_dep/main.cpp new file mode 100644 index 00000000..b3e7e930 --- /dev/null +++ b/test/unused_dep/main.cpp @@ -0,0 +1,6 @@ +#include "test/unused_dep/bar.h" + +int main() +{ + return doStuff(); +} diff --git a/test/using_transitive_dep/BUILD b/test/using_transitive_dep/BUILD new file mode 100644 index 00000000..956302ec --- /dev/null +++ b/test/using_transitive_dep/BUILD @@ -0,0 +1,16 @@ +cc_library( + name = "foo", + hdrs = ["foo.h"], +) + +cc_library( + name = "bar", + hdrs = ["bar.h"], + deps = [":foo"], +) + +cc_binary( + name = "main", + srcs = ["main.cpp"], + deps = [":bar"], +) diff --git a/test/using_transitive_dep/README.md b/test/using_transitive_dep/README.md new file mode 100644 index 00000000..9e2af0fa --- /dev/null +++ b/test/using_transitive_dep/README.md @@ -0,0 +1 @@ +Using headers from transitive dependencies without directly depending on them shall cause an error. diff --git a/test/using_transitive_dep/bar.h b/test/using_transitive_dep/bar.h new file mode 100644 index 00000000..49cbff21 --- /dev/null +++ b/test/using_transitive_dep/bar.h @@ -0,0 +1,10 @@ +#ifndef BAR_H +#define BAR_H +#include "test/using_transitive_dep/foo.h" + +int doStuff() +{ + return theAnswer() + 1; +} + +#endif diff --git a/test/using_transitive_dep/foo.h b/test/using_transitive_dep/foo.h new file mode 100644 index 00000000..e732c1b6 --- /dev/null +++ b/test/using_transitive_dep/foo.h @@ -0,0 +1,9 @@ +#ifndef FOO_H +#define FOO_H + +int theAnswer() +{ + return 42; +} + +#endif diff --git a/test/using_transitive_dep/main.cpp b/test/using_transitive_dep/main.cpp new file mode 100644 index 00000000..e27ca54f --- /dev/null +++ b/test/using_transitive_dep/main.cpp @@ -0,0 +1,10 @@ +#include "test/using_transitive_dep/foo.h" +#include "test/using_transitive_dep/bar.h" + +int main() +{ + // ERROR: Using a function from library foo but depending only on library bar + const int answer = theAnswer(); + const int stuff = doStuff(); + return answer != stuff; +} diff --git a/test/valid/BUILD b/test/valid/BUILD new file mode 100644 index 00000000..2802c748 --- /dev/null +++ b/test/valid/BUILD @@ -0,0 +1,19 @@ +cc_library( + name = "foo", + hdrs = [ + "foo/a.h", + "foo/b.h", + ], + textual_hdrs = ["foo/textual.cc"], +) + +cc_library( + name = "bar", + srcs = [ + "bar/bar.cc", + "bar/private_a.h", + "bar/sub/dir/private_b.h", + ], + hdrs = ["bar/bar.h"], + deps = [":foo"], +) diff --git a/test/valid/README.md b/test/valid/README.md new file mode 100644 index 00000000..4736efd3 --- /dev/null +++ b/test/valid/README.md @@ -0,0 +1 @@ +Targets with valid dependency usage which shall not cause failures. diff --git a/test/valid/bar/bar.cc b/test/valid/bar/bar.cc new file mode 100644 index 00000000..0d593ecc --- /dev/null +++ b/test/valid/bar/bar.cc @@ -0,0 +1,10 @@ +#include "private_a.h" +#include "sub/dir/private_b.h" +#include "test/valid/bar/bar.h" +#include "test/valid/foo/b.h" +#include "test/valid/foo/textual.cc" + +int doBar() +{ + return doB() + doPrivateA() + doPrivateB(); +} diff --git a/test/valid/bar/bar.h b/test/valid/bar/bar.h new file mode 100644 index 00000000..678d95ab --- /dev/null +++ b/test/valid/bar/bar.h @@ -0,0 +1,8 @@ +#ifndef BAR_H +#define BAR_H + +#include "test/valid/foo/a.h" + +int doBar(); + +#endif diff --git a/test/valid/bar/private_a.h b/test/valid/bar/private_a.h new file mode 100644 index 00000000..c23797f6 --- /dev/null +++ b/test/valid/bar/private_a.h @@ -0,0 +1,9 @@ +#ifndef PRIVATE_A_H +#define PRIVATE_A_H + +int doPrivateA() +{ + return 13; +} + +#endif diff --git a/test/valid/bar/sub/dir/private_b.h b/test/valid/bar/sub/dir/private_b.h new file mode 100644 index 00000000..ee819241 --- /dev/null +++ b/test/valid/bar/sub/dir/private_b.h @@ -0,0 +1,9 @@ +#ifndef PRIVATE_B_H +#define PRIVATE_B_H + +int doPrivateB() +{ + return 5; +} + +#endif diff --git a/test/valid/foo/a.h b/test/valid/foo/a.h new file mode 100644 index 00000000..716c1ce2 --- /dev/null +++ b/test/valid/foo/a.h @@ -0,0 +1,9 @@ +#ifndef A_H +#define A_H + +int doA() +{ + return 42; +} + +#endif diff --git a/test/valid/foo/b.h b/test/valid/foo/b.h new file mode 100644 index 00000000..2b9ebe80 --- /dev/null +++ b/test/valid/foo/b.h @@ -0,0 +1,9 @@ +#ifndef B_H +#define B_H + +int doB() +{ + return 1; +} + +#endif diff --git a/test/valid/foo/textual.cc b/test/valid/foo/textual.cc new file mode 100644 index 00000000..e69de29b diff --git a/test/virtual_includes/BUILD b/test/virtual_includes/BUILD new file mode 100644 index 00000000..f6e4658d --- /dev/null +++ b/test/virtual_includes/BUILD @@ -0,0 +1,25 @@ +cc_library( + name = "prefixed", + srcs = ["prefixed.cpp"], + hdrs = ["prefixed.h"], + include_prefix = "virtual/prefix", +) + +cc_library( + name = "stripped", + srcs = ["stripped.cpp"], + hdrs = ["some/sub/dir/stripped.h"], + strip_include_prefix = "some", +) + +cc_binary( + name = "use_prefixed", + srcs = ["use_prefixed.cpp"], + deps = [":prefixed"], +) + +cc_binary( + name = "use_stripped", + srcs = ["use_stripped.cpp"], + deps = [":stripped"], +) diff --git a/test/virtual_includes/README.md b/test/virtual_includes/README.md new file mode 100644 index 00000000..9229781e --- /dev/null +++ b/test/virtual_includes/README.md @@ -0,0 +1 @@ +Virtual include paths created by the `cc_*` rule attributes `include_prefix` and `strip_include_prefix` shall not prevent mapping headers to their include statements. diff --git a/test/virtual_includes/prefixed.cpp b/test/virtual_includes/prefixed.cpp new file mode 100644 index 00000000..c5449a9b --- /dev/null +++ b/test/virtual_includes/prefixed.cpp @@ -0,0 +1,6 @@ +#include "virtual/prefix/prefixed.h" + +int doPrefixed() +{ + return 42; +} diff --git a/test/virtual_includes/prefixed.h b/test/virtual_includes/prefixed.h new file mode 100644 index 00000000..474d8725 --- /dev/null +++ b/test/virtual_includes/prefixed.h @@ -0,0 +1,6 @@ +#ifndef PREFIXED_H +#define PREFIXED_H + +int doPrefixed(); + +#endif diff --git a/test/virtual_includes/some/sub/dir/stripped.h b/test/virtual_includes/some/sub/dir/stripped.h new file mode 100644 index 00000000..a84096b1 --- /dev/null +++ b/test/virtual_includes/some/sub/dir/stripped.h @@ -0,0 +1,6 @@ +#ifndef STRIPPED_H +#define STRIPPED_H + +int doStripped(); + +#endif diff --git a/test/virtual_includes/stripped.cpp b/test/virtual_includes/stripped.cpp new file mode 100644 index 00000000..4fcf349a --- /dev/null +++ b/test/virtual_includes/stripped.cpp @@ -0,0 +1,6 @@ +#include "sub/dir/stripped.h" + +int doStripped() +{ + return 42; +} diff --git a/test/virtual_includes/use_prefixed.cpp b/test/virtual_includes/use_prefixed.cpp new file mode 100644 index 00000000..ef62bef6 --- /dev/null +++ b/test/virtual_includes/use_prefixed.cpp @@ -0,0 +1,6 @@ +#include "virtual/prefix/prefixed.h" + +int main() +{ + return doPrefixed(); +} diff --git a/test/virtual_includes/use_stripped.cpp b/test/virtual_includes/use_stripped.cpp new file mode 100644 index 00000000..dbbd63aa --- /dev/null +++ b/test/virtual_includes/use_stripped.cpp @@ -0,0 +1,6 @@ +#include "sub/dir/stripped.h" + +int main() +{ + return doStripped(); +}