diff --git a/docs/BUILD.bazel b/docs/BUILD.bazel index ef22bae2..8c4cb7c5 100644 --- a/docs/BUILD.bazel +++ b/docs/BUILD.bazel @@ -52,4 +52,9 @@ stardoc_with_diff_test( bzl_library_target = "//lint:ktlint", ) +stardoc_with_diff_test( + name = "clang-tidy", + bzl_library_target = "//lint:clang_tidy", +) + update_docs(name = "update") diff --git a/docs/clang-tidy.md b/docs/clang-tidy.md new file mode 100644 index 00000000..52c74ac8 --- /dev/null +++ b/docs/clang-tidy.md @@ -0,0 +1,136 @@ + + +API for calling declaring a clang-tidy lint aspect. + +Typical usage: + +First, install clang-tidy with llvm_toolchain or as a native binary (llvm_toolchain +does not support Windows as of 06/2024, but providing a native clang-tidy.exe works) + +Next, declare a binary target for it, typically in `tools/lint/BUILD.bazel`: + +e.g. using llvm_toolchain: +```starlark +native_binary( + name = "clang_tidy", + src = "@llvm_toolchain_llvm//:bin/clang-tidy" + out = "clang_tidy", +) +``` + +e.g as native binary: +```starlark +native_binary( + name = "clang_tidy", + src = "clang-tidy.exe" + out = "clang_tidy", +) +``` + +Finally, create the linter aspect, typically in `tools/lint/linters.bzl`: + +```starlark +load("@aspect_rules_lint//lint:clang_tidy.bzl", "clang_tidy_aspect") + +clang_tidy = clang_tidy_aspect( + binary = "@@//path/to:clang-tidy", + configs = "@@//path/to:.clang-tidy", +) +``` + + + + +## clang_tidy_action + +
+clang_tidy_action(ctx, compilation_context, executable, srcs, stdout, exit_code)
+
+ +Create a Bazel Action that spawns a clang-tidy process. + +Adapter for wrapping Bazel around +https://clang.llvm.org/extra/clang-tidy/ + + +**PARAMETERS** + + +| Name | Description | Default Value | +| :------------- | :------------- | :------------- | +| ctx | an action context OR aspect context | none | +| compilation_context | from target | none | +| executable | struct with a clang-tidy field | none | +| srcs | file objects to lint | none | +| stdout | output file containing the stdout or --output-file of clang-tidy | none | +| exit_code | output file containing the exit code of clang-tidy. If None, then fail the build when clang-tidy exits non-zero. | none | + + + + +## clang_tidy_fix + +
+clang_tidy_fix(ctx, compilation_context, executable, srcs, patch, stdout, exit_code)
+
+ +Create a Bazel Action that spawns clang-tidy with --fix. + +**PARAMETERS** + + +| Name | Description | Default Value | +| :------------- | :------------- | :------------- | +| ctx | an action context OR aspect context | none | +| compilation_context | from target | none | +| executable | struct with a clang_tidy field | none | +| srcs | list of file objects to lint | none | +| patch | output file containing the applied fixes that can be applied with the patch(1) command. | none | +| stdout | output file containing the stdout or --output-file of clang-tidy | none | +| exit_code | output file containing the exit code of clang-tidy | none | + + + + +## is_parent_in_list + +
+is_parent_in_list(dir, list)
+
+ + + +**PARAMETERS** + + +| Name | Description | Default Value | +| :------------- | :------------- | :------------- | +| dir |

-

| none | +| list |

-

| none | + + + + +## lint_clang_tidy_aspect + +
+lint_clang_tidy_aspect(binary, configs, global_config, header_filter, lint_target_headers,
+                       angle_includes_are_system, verbose)
+
+ +A factory function to create a linter aspect. + +**PARAMETERS** + + +| Name | Description | Default Value | +| :------------- | :------------- | :------------- | +| binary | the clang-tidy binary, typically a rule like

starlark native_binary(     name = "clang_tidy",     src = "clang-tidy.exe"     out = "clang_tidy", )
| none | +| configs | labels of the .clang-tidy files to make available to clang-tidy's config search. These may be in subdirectories and clang-tidy will apply them if appropriate. This may also include .clang-format files which may be used for formatting fixes. | [] | +| global_config | label of a single global .clang-tidy file to pass to clang-tidy on the command line. This will cause clang-tidy to ignore any other config files in the source directories. | [] | +| header_filter | optional, set to a posix regex to supply to clang-tidy with the -header-filter option | "" | +| lint_target_headers | optional, set to True to pass a pattern that includes all headers with the target's directory prefix. This crude control may include headers from the linted target in the results. If supplied, overrides the header_filter option. | False | +| angle_includes_are_system | controls how angle includes are passed to clang-tidy. By default, Bazel passes these as -isystem. Change this to False to pass these as -I, which allows clang-tidy to regard them as regular header files. | True | +| verbose | print debug messages including clang-tidy command lines being invoked. | False | + + diff --git a/example/.bazelrc b/example/.bazelrc index a3898fb8..1913390b 100644 --- a/example/.bazelrc +++ b/example/.bazelrc @@ -1,6 +1,22 @@ +# Automatically apply --config=linux, --config=windows etc +common --enable_platform_specific_config + # Don't depend on a JAVA_HOME pointing at a system JDK # see https://github.com/bazelbuild/rules_jvm_external/issues/445 build --repo_env=JAVA_HOME=../bazel_tools/jdk common --incompatible_enable_proto_toolchain_resolution common --@aspect_rules_ts//ts:skipLibCheck=always + +# enable symlinks on windows +startup --windows_enable_symlinks + +# c++ options +common:linux --action_env=BAZEL_CXXOPTS="-std=c++20" +common:windows --action_env=BAZEL_CXXOPTS="/std:c++20" + +# clang-tidy needs the INCLUDE variable set to the system value. +# Would prefer to do this inside the aspect, not sure if it is possible. +common --action_env=INCLUDE +# ensure that minimal other envvars are passed by clang-tidy run_shell +common --incompatible_strict_action_env diff --git a/example/.clang-tidy b/example/.clang-tidy new file mode 100644 index 00000000..b2b110c0 --- /dev/null +++ b/example/.clang-tidy @@ -0,0 +1,19 @@ +# https://clang.llvm.org/extra/clang-tidy/ +# config-file to provoke some checks in example +--- +Checks: "*, + -abseil-*, + -altera-*, + -android-*, + -fuchsia-*, + -google-*, + -llvm*, + -modernize-use-trailing-return-type, + -zircon-*, + -readability-else-after-return, + -readability-static-accessed-through-instance, + -readability-avoid-const-params-in-decls, + -cppcoreguidelines-non-private-member-variables-in-classes, + -misc-non-private-member-variables-in-classes," +# promote a random warning to error to test integration. Do not copy this line. +WarningsAsErrors: "readability-inconsistent-declaration-parameter-name" diff --git a/example/BUILD.bazel b/example/BUILD.bazel index 1fd5ed82..46fcf6c7 100644 --- a/example/BUILD.bazel +++ b/example/BUILD.bazel @@ -22,6 +22,7 @@ exports_files( ".vale.ini", ".editorconfig", "ktlint-baseline.xml", + ".clang-tidy", ], visibility = ["//visibility:public"], ) diff --git a/example/MODULE.bazel b/example/MODULE.bazel index 9ca16283..1a6c9461 100644 --- a/example/MODULE.bazel +++ b/example/MODULE.bazel @@ -17,6 +17,7 @@ bazel_dep(name = "rules_rust", version = "0.45.1") bazel_dep(name = "buildifier_prebuilt", version = "6.3.3") bazel_dep(name = "platforms", version = "0.0.8") bazel_dep(name = "rules_kotlin", version = "1.9.0") +bazel_dep(name = "rules_cc", version = "0.0.9") local_path_override( module_name = "aspect_rules_lint", diff --git a/example/lint.sh b/example/lint.sh index ebbb4bbf..f4e6c199 100755 --- a/example/lint.sh +++ b/example/lint.sh @@ -22,9 +22,26 @@ fix="" buildevents=$(mktemp) filter='.namedSetOfFiles | values | .files[] | select(.name | endswith($ext)) | ((.pathPrefix | join("/")) + "/" + .name)' +unameOut="$(uname -s)" +case "${unameOut}" in + Linux*) machine=Linux;; + Darwin*) machine=Mac;; + CYGWIN*) machine=Windows;; + MINGW*) machine=Windows;; + MSYS_NT*) machine=Windows;; + *) machine="UNKNOWN:${unameOut}" +esac + +args=() +if [ $machine == "Windows" ]; then + # avoid missing linters on windows platform + args=("--aspects=$(echo //tools/lint:linters.bzl%{flake8,pmd,ruff,vale,clang_tidy} | tr ' ' ',')") +else + args=("--aspects=$(echo //tools/lint:linters.bzl%{buf,eslint,flake8,ktlint,pmd,ruff,shellcheck,vale,clang_tidy} | tr ' ' ',')") +fi + # NB: perhaps --remote_download_toplevel is needed as well with remote execution? -args=( - "--aspects=$(echo //tools/lint:linters.bzl%{buf,eslint,flake8,ktlint,pmd,ruff,shellcheck,vale} | tr ' ' ',')" +args+=( # Allow lints of code that fails some validation action # See https://github.com/aspect-build/rules_ts/pull/574#issuecomment-2073632879 "--norun_validations" @@ -59,7 +76,12 @@ fi bazel build ${args[@]} $@ # TODO: Maybe this could be hermetic with bazel run @aspect_bazel_lib//tools:jq or sth -valid_reports=$(jq --arg ext report --raw-output "$filter" "$buildevents") +if [ $machine == "Windows" ]; then + # jq on windows outputs CRLF which breaks this script. https://github.com/jqlang/jq/issues/92 + valid_reports=$(jq --arg ext report --raw-output "$filter" "$buildevents" | tr -d '\r') +else + valid_reports=$(jq --arg ext report --raw-output "$filter" "$buildevents") +fi # Show the results. while IFS= read -r report; do @@ -77,7 +99,7 @@ done <<<"$valid_reports" # [*] 1 fixable with the `--fix` option. # so that the naive thing of pasting that flag to lint.sh will do what the user expects. if [ -n "$fix" ]; then - valid_patches=$(jq --arg ext patch --raw-output "$filter" "$buildevents") + valid_patches=$valid_reports while IFS= read -r patch; do # Exclude coverage reports, and check if the report is empty. if [[ "$patch" == *coverage.dat ]] || [[ ! -s "$patch" ]]; then diff --git a/example/src/cpp/lib/BUILD b/example/src/cpp/lib/BUILD new file mode 100644 index 00000000..b49df3b1 --- /dev/null +++ b/example/src/cpp/lib/BUILD @@ -0,0 +1,18 @@ +load("@rules_cc//cc:defs.bzl", "cc_library") + +exports_files(["get/.clang-tidy"]) + +cc_library( + name = "hello-time", + srcs = [ + "get/get-time.cc", + "get/get-time.h", + "hello-time.cc", + ], + hdrs = [ + "hello-time.h", + "xhello-time.h", + ], + includes = ["."], + visibility = ["//src/cpp/main:__pkg__"], +) diff --git a/example/src/cpp/lib/get/.clang-tidy b/example/src/cpp/lib/get/.clang-tidy new file mode 100644 index 00000000..2826f037 --- /dev/null +++ b/example/src/cpp/lib/get/.clang-tidy @@ -0,0 +1,7 @@ +# https://clang.llvm.org/extra/clang-tidy/ +# this is an example of a .clang-tidy file in a subdirectory that overrides +# the configuration for files in this directory and below. It must be made +# visibile through an export_files declaration in this package and added +# to the 'configs' attribute in clang_tidy aspect. +--- +Checks: "-*, misc-const-correctness" diff --git a/example/src/cpp/lib/get/get-time.cc b/example/src/cpp/lib/get/get-time.cc new file mode 100644 index 00000000..864174fa --- /dev/null +++ b/example/src/cpp/lib/get/get-time.cc @@ -0,0 +1,13 @@ +#include "get-time.h" +#include +#include + +// Some deliberately bad code +const char* get_localtime_impl() { + // warning: variable 'result' of type 'std::time_t' (aka 'long long') can be declared 'const' [misc-const-correctness] + std::time_t result = std::time(nullptr); + // warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead [bugprone-unsafe-functions,cert-msc24-c,cert-msc33-c] + std::string result_str = std::asctime(std::localtime(&result)); + // rules_lint author: note the memory leak is not detected + return strdup(result_str.c_str()); +} diff --git a/example/src/cpp/lib/get/get-time.h b/example/src/cpp/lib/get/get-time.h new file mode 100644 index 00000000..5408f708 --- /dev/null +++ b/example/src/cpp/lib/get/get-time.h @@ -0,0 +1,17 @@ +#ifndef LIB_GET_TIME_H_ +#define LIB_GET_TIME_H_ + +#include + +const char* get_localtime_impl(); + +// Deliberatly bad code +inline char* get_localtime() { + const char* time = get_localtime_impl(); + char timebuf[20]; + strcpy(timebuf, time); + // warning: Address of stack memory associated with local variable 'timebuf' returned to caller [clang-analyzer-core.StackAddressEscape] + return timebuf; +} + +#endif diff --git a/example/src/cpp/lib/hello-time.cc b/example/src/cpp/lib/hello-time.cc new file mode 100644 index 00000000..c029fdc0 --- /dev/null +++ b/example/src/cpp/lib/hello-time.cc @@ -0,0 +1,12 @@ +#include "hello-time.h" +// warning: included header ctime is not used directly [misc-include-cleaner] +#include +#include +#include + +// Deliberately bad code +void print_localtime() { + char* localtime = get_localtime(); + // warning: do not call c-style vararg functions [cppcoreguidelines-pro-type-vararg,hicpp-vararg] + printf("%s\n", localtime); +} diff --git a/example/src/cpp/lib/hello-time.h b/example/src/cpp/lib/hello-time.h new file mode 100644 index 00000000..ee01af1e --- /dev/null +++ b/example/src/cpp/lib/hello-time.h @@ -0,0 +1,16 @@ +#ifndef LIB_HELLO_TIME_H_ +#define LIB_HELLO_TIME_H_ + +#include +#include +#include + +void print_localtime(); + +inline void print_localtime2() { + std::string a = "time"; + for (int i=0; i +#include + +inline void print_localtime3() { + std::string a = "time"; + for (int i=0; i to provoke error +std::string get_greet(const std::string &thing); + +#endif diff --git a/example/src/cpp/main/hello-greet.cc b/example/src/cpp/main/hello-greet.cc new file mode 100644 index 00000000..10efa2b9 --- /dev/null +++ b/example/src/cpp/main/hello-greet.cc @@ -0,0 +1,6 @@ +#include "hello-greet.h" +#include + +std::string get_greet(const std::string& who) { + return "Hello " + who; +} diff --git a/example/src/cpp/main/hello-greet.h b/example/src/cpp/main/hello-greet.h new file mode 100644 index 00000000..2543b46b --- /dev/null +++ b/example/src/cpp/main/hello-greet.h @@ -0,0 +1,8 @@ +#ifndef MAIN_HELLO_GREET_H_ +#define MAIN_HELLO_GREET_H_ + +#include + +std::string get_greet(const std::string &thing); + +#endif diff --git a/example/src/cpp/main/hello-world.cc b/example/src/cpp/main/hello-world.cc new file mode 100644 index 00000000..b1d1e8ff --- /dev/null +++ b/example/src/cpp/main/hello-world.cc @@ -0,0 +1,15 @@ +#include "hello-time.h" +#include "hello-greet.h" +#include +#include + +int main(int argc, char** argv) { + std::string who = "world"; + if (argc > 1) { + who = argv[1]; + } + // warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl] + std::cout << get_greet(who) << std::endl; + print_localtime(); + return 0; +} diff --git a/example/src/hello.cpp b/example/src/hello.cpp index af2f6a9e..126b7f8d 100644 --- a/example/src/hello.cpp +++ b/example/src/hello.cpp @@ -1,3 +1,51 @@ #include +#include +#include +#include +#include +#include -int main() { printf("Hello, world!\n"); } +// deliberately bad code to trigger clang-tidy warning +int string_to_int(const char *num) { + return atoi(num); +} + +// deliberately insecure code to trigger clang-tidy warning +void ls() { + system("ls"); +} + +// Code with a fixable issue +void remove_from_vector() { + std::vector xs = {1,2,3,4,5,6}; + std::remove(xs.begin(), xs.end(), 4); +} + +// Code with a fixable issue +static auto stringCpy(const std::string &str) -> char * { + char *result = reinterpret_cast(malloc(str.size())); + strcpy(result, str.data()); + return result; +} + +class dummy { +public: + dummy() {}; +private: + int x; +}; + +static int compare(int x, int y) { + if (x < y); + { + x++; + } + return x; +} + +int main() { + printf("Hello, world!\n"); + compare(3, 4); + char* a = NULL; + char* b = 0; +} diff --git a/example/src/subdir/gitlab-generated.js b/example/src/subdir/gitlab-generated.js index 5dba6bb1..bdb0ac5a 100644 --- a/example/src/subdir/gitlab-generated.js +++ b/example/src/subdir/gitlab-generated.js @@ -1,3 +1,2 @@ // Ignored by https://github.com/aspect-build/rules_lint/blob/example/.gitattributes - export var x = "white space issue and no semi colon" - \ No newline at end of file +export var x = "white space issue and no semi colon"; diff --git a/example/src/subdir/linguist-generated.js b/example/src/subdir/linguist-generated.js index 5dba6bb1..bdb0ac5a 100644 --- a/example/src/subdir/linguist-generated.js +++ b/example/src/subdir/linguist-generated.js @@ -1,3 +1,2 @@ // Ignored by https://github.com/aspect-build/rules_lint/blob/example/.gitattributes - export var x = "white space issue and no semi colon" - \ No newline at end of file +export var x = "white space issue and no semi colon"; diff --git a/example/src/subdir/rules-lint-ignored.js b/example/src/subdir/rules-lint-ignored.js index 5dba6bb1..bdb0ac5a 100644 --- a/example/src/subdir/rules-lint-ignored.js +++ b/example/src/subdir/rules-lint-ignored.js @@ -1,3 +1,2 @@ // Ignored by https://github.com/aspect-build/rules_lint/blob/example/.gitattributes - export var x = "white space issue and no semi colon" - \ No newline at end of file +export var x = "white space issue and no semi colon"; diff --git a/example/tools/lint/BUILD.bazel b/example/tools/lint/BUILD.bazel index 655662ac..9d0aa219 100644 --- a/example/tools/lint/BUILD.bazel +++ b/example/tools/lint/BUILD.bazel @@ -20,6 +20,7 @@ alias( "@bazel_tools//src/conditions:linux_aarch64": "@ruff_aarch64-unknown-linux-gnu//:ruff", "@bazel_tools//src/conditions:darwin_arm64": "@ruff_aarch64-apple-darwin//:ruff", "@bazel_tools//src/conditions:darwin_x86_64": "@ruff_x86_64-apple-darwin//:ruff", + "@bazel_tools//src/conditions:windows_x64": "@ruff_x86_64-pc-windows-msvc//:ruff.exe", }), ) @@ -51,6 +52,7 @@ native_binary( "@bazel_tools//src/conditions:linux_aarch64": "@vale_Linux_arm64//:vale", "@bazel_tools//src/conditions:darwin_x86_64": "@vale_macOS_64-bit//:vale", "@bazel_tools//src/conditions:darwin_arm64": "@vale_macOS_arm64//:vale", + "@bazel_tools//src/conditions:windows_x64": "@vale_Windows_64-bit//:vale.exe", }, ), out = "vale", @@ -68,3 +70,20 @@ copy_to_directory( ], include_external_repositories = ["vale_*"], ) + +native_binary( + name = "clang_tidy", + src = select( + { + "@bazel_tools//src/conditions:linux_x86_64": "@llvm_toolchain_llvm//:bin/clang-tidy", + "@bazel_tools//src/conditions:linux_aarch64": "@llvm_toolchain_llvm//:bin/clang-tidy", + "@bazel_tools//src/conditions:darwin_x86_64": "@llvm_toolchain_llvm//:bin/clang-tidy", + "@bazel_tools//src/conditions:darwin_arm64": "@llvm_toolchain_llvm//:bin/clang-tidy", + # llvm_toolchain doesn't support windows: https://github.com/bazel-contrib/toolchains_llvm/issues/4 + # as a workaround, you can download exes from + # https://github.com/llvm/llvm-project/releases/tag/llvmorg-18.1.6 and make available locally. + "@bazel_tools//src/conditions:windows_x64": "clang-tidy.exe", + }, + ), + out = "clang_tidy", +) diff --git a/example/tools/lint/linters.bzl b/example/tools/lint/linters.bzl index 0c53222f..4af3c91d 100644 --- a/example/tools/lint/linters.bzl +++ b/example/tools/lint/linters.bzl @@ -1,6 +1,7 @@ "Define linter aspects" load("@aspect_rules_lint//lint:buf.bzl", "lint_buf_aspect") +load("@aspect_rules_lint//lint:clang_tidy.bzl", "lint_clang_tidy_aspect") load("@aspect_rules_lint//lint:eslint.bzl", "lint_eslint_aspect") load("@aspect_rules_lint//lint:flake8.bzl", "lint_flake8_aspect") load("@aspect_rules_lint//lint:ktlint.bzl", "lint_ktlint_aspect") @@ -71,3 +72,26 @@ ktlint = lint_ktlint_aspect( ) ktlint_test = lint_test(aspect = ktlint) + +clang_tidy = lint_clang_tidy_aspect( + binary = "@@//tools/lint:clang_tidy", + configs = [ + "@@//:.clang-tidy", + "@@//src/cpp/lib:get/.clang-tidy", + ], + lint_target_headers = True, + angle_includes_are_system = False, + verbose = False, +) + +clang_tidy_test = lint_test(aspect = clang_tidy) + +# an example of setting up a different clang-tidy aspect with different +# options. This one uses a single global clang-tidy file +clang_tidy_global_config = lint_clang_tidy_aspect( + binary = "@@//tools/lint:clang_tidy", + global_config = "@@//:.clang-tidy", + lint_target_headers = True, + angle_includes_are_system = False, + verbose = False, +) diff --git a/format/test/format_test.bats b/format/test/format_test.bats index e36c0a7b..7598e43e 100644 --- a/format/test/format_test.bats +++ b/format/test/format_test.bats @@ -116,7 +116,7 @@ bats_load_library "bats-assert" run bazel run //format/test:format_C++_with_clang-format assert_success - assert_output --partial "+ clang-format -style=file --fallback-style=none -i example/src/hello.cpp" + assert_output --partial "+ clang-format -style=file --fallback-style=none -i example/src/cpp/lib/get/get-time.cc" } @test "should run clang-format on Cuda" { diff --git a/lint/BUILD.bazel b/lint/BUILD.bazel index e279c7be..16543295 100644 --- a/lint/BUILD.bazel +++ b/lint/BUILD.bazel @@ -197,3 +197,21 @@ bzl_library( "@bazel_skylib//lib:dicts", ], ) + +bzl_library( + name = "clang_tidy", + srcs = ["clang_tidy.bzl"], + visibility = ["//visibility:public"], + deps = _BAZEL_TOOLS + [ + "//lint/private:lint_aspect", + "@bazel_skylib//lib:dicts", + "@bazel_tools//tools/build_defs/cc:action_names.bzl", + "@bazel_tools//tools/cpp:toolchain_utils.bzl", + ], +) + +sh_binary( + name = "clang_tidy_wrapper", + srcs = ["clang_tidy_wrapper.bash"], + visibility = ["//visibility:public"], +) diff --git a/lint/clang_tidy.bzl b/lint/clang_tidy.bzl new file mode 100644 index 00000000..a885d69f --- /dev/null +++ b/lint/clang_tidy.bzl @@ -0,0 +1,405 @@ +"""API for calling declaring a clang-tidy lint aspect. + +Typical usage: + +First, install clang-tidy with llvm_toolchain or as a native binary (llvm_toolchain +does not support Windows as of 06/2024, but providing a native clang-tidy.exe works) + +Next, declare a binary target for it, typically in `tools/lint/BUILD.bazel`: + +e.g. using llvm_toolchain: +```starlark +native_binary( + name = "clang_tidy", + src = "@llvm_toolchain_llvm//:bin/clang-tidy" + out = "clang_tidy", +) +``` + +e.g as native binary: +```starlark +native_binary( + name = "clang_tidy", + src = "clang-tidy.exe" + out = "clang_tidy", +) +``` + +Finally, create the linter aspect, typically in `tools/lint/linters.bzl`: + +```starlark +load("@aspect_rules_lint//lint:clang_tidy.bzl", "clang_tidy_aspect") + +clang_tidy = clang_tidy_aspect( + binary = "@@//path/to:clang-tidy", + configs = "@@//path/to:.clang-tidy", +) +``` +""" + +load("@bazel_tools//tools/build_defs/cc:action_names.bzl", "ACTION_NAMES") +load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain") +load("//lint/private:lint_aspect.bzl", "LintOptionsInfo", "dummy_successful_lint_action", "patch_and_report_files", "report_files") + +_MNEMONIC = "AspectRulesLintClangTidy" + +def _gather_inputs(ctx, compilation_context, srcs): + inputs = srcs + ctx.files._configs + compilation_context.headers.to_list() + if (any(ctx.files._global_config)): + inputs.append(ctx.files._global_config[0]) + return inputs + +def _toolchain_flags(ctx, action_name = ACTION_NAMES.cpp_compile): + cc_toolchain = find_cpp_toolchain(ctx) + feature_configuration = cc_common.configure_features( + ctx = ctx, + cc_toolchain = cc_toolchain, + ) + compile_variables = cc_common.create_compile_variables( + feature_configuration = feature_configuration, + cc_toolchain = cc_toolchain, + user_compile_flags = ctx.fragments.cpp.cxxopts + ctx.fragments.cpp.copts, + ) + flags = cc_common.get_memory_inefficient_command_line( + feature_configuration = feature_configuration, + action_name = action_name, + variables = compile_variables, + ) + return flags + +def _update_flag(flag): + # update from MSVC C++ standard to clang C++ standard + unsupported_flags = [ + "-fno-canonical-system-headers", + "-fstack-usage", + "/nologo", + "/COMPILER_MSVC", + "/showIncludes", + ] + if (flag in unsupported_flags): + return None + + # omit warning flags + if (flag.startswith("/wd") or flag.startswith("-W")): + return None + + # remap c++ standard to clang + if (flag.startswith("/std:")): + flag = "-std=" + flag.removeprefix("/std:") + + # remap defines + if (flag.startswith("/D")): + flag = "-" + flag[1:] + if (flag.startswith("/FI")): + flag = "-include=" + flag.removeprefix("/FI") + + # skip other msvc options + if (flag.startswith("/")): + return None + return flag + +def _safe_flags(ctx, flags): + # Some flags might be used by GCC/MSVC, but not understood by Clang. + # Remap or remove them here, to allow users to run clang-tidy, without having + # a clang toolchain configured (that would produce a good command line with --compiler clang) + flags = [] + skipped_flags = [] + for flag in flags: + flag = _update_flag(flag) + if (flag): + flags.append(flag) + elif (ctx.attr._verbose): + skipped_flags.append(flag) + if (ctx.attr._verbose and any(skipped_flags)): + # buildifier: disable=print + print("skipped flags: " + " ".join(skipped_flags)) + return flags + +def _prefixed(list, prefix): + array = [] + for arg in list: + array.append(prefix) + array.append(arg) + return array + +def _angle_includes_option(ctx): + if (ctx.attr._angle_includes_are_system): + return "-isystem" + return "-I" + +def _is_cxx(file): + if file.extension == "c": + return False + return True + +def _is_source(file): + permitted_source_types = [ + "c", + "cc", + "cpp", + "cxx", + "c++", + "C", + ] + return (file.is_source and file.extension in permitted_source_types) + +# modification of filter_srcs in lint_aspect.bzl that filters out header files +def _filter_srcs(rule): + if "lint-genfiles" in rule.attr.tags: + return rule.files.srcs + else: + return [s for s in rule.files.srcs if _is_source(s)] + +def is_parent_in_list(dir, list): + for item in list: + if (dir != item and dir.startswith(item)): + return True + return False + +def _common_prefixes(headers): + # crude code to work out a common directory prefix for all headers + # is there a canonical way to do this in starlark? + dirs = [] + for h in headers: + dir = h.dirname + if dir not in dirs: + dirs.append(dir) + dirs2 = [] + for dir in dirs: + if (not is_parent_in_list(dir, dirs)): + dirs2.append(dir) + return dirs2 + +def _aggregate_regex(compilation_context): + dirs = _common_prefixes(compilation_context.direct_headers) + if not any(dirs): + regex = None + elif len(dirs) == 1: + regex = ".*" + dirs[0] + "/.*" + else: + regex = ".*" + return regex + +def _quoted_arg(arg): + return "\"" + arg + "\"" + +def _get_args(ctx, compilation_context, srcs): + args = [] + if (any(ctx.files._global_config)): + args.append("--config-file=" + ctx.files._global_config[0].short_path) + if (ctx.attr._lint_target_headers): + regex = _aggregate_regex(compilation_context) + if (regex): + args.append(_quoted_arg("-header-filter=" + regex)) + elif (ctx.attr._header_filter): + regex = ctx.attr._header_filter + args.append(_quoted_arg("-header-filter=" + regex)) + args.extend([src.short_path for src in srcs]) + + args.append("--") + + # add args specified by the toolchain, on the command line and rule copts + rule_flags = ctx.rule.attr.copts if hasattr(ctx.rule.attr, "copts") else [] + sources_are_cxx = _is_cxx(srcs[0]) + if (sources_are_cxx): + args.extend(_safe_flags(ctx, _toolchain_flags(ctx, ACTION_NAMES.cpp_compile) + rule_flags) + ["-xc++"]) + else: + args.extend(_safe_flags(ctx, _toolchain_flags(ctx, ACTION_NAMES.c_compile) + rule_flags) + ["-xc"]) + + # add defines + for define in compilation_context.defines.to_list(): + args.append("-D" + define) + for define in compilation_context.local_defines.to_list(): + args.append("-D" + define) + + # add includes + args.extend(_prefixed(compilation_context.framework_includes.to_list(), "-F")) + args.extend(_prefixed(compilation_context.includes.to_list(), "-I")) + args.extend(_prefixed(compilation_context.quote_includes.to_list(), "-iquote")) + args.extend(_prefixed(compilation_context.system_includes.to_list(), _angle_includes_option(ctx))) + args.extend(_prefixed(compilation_context.external_includes.to_list(), "-isystem")) + + return args + +def clang_tidy_action(ctx, compilation_context, executable, srcs, stdout, exit_code): + """Create a Bazel Action that spawns a clang-tidy process. + + Adapter for wrapping Bazel around + https://clang.llvm.org/extra/clang-tidy/ + + Args: + ctx: an action context OR aspect context + compilation_context: from target + executable: struct with a clang-tidy field + srcs: file objects to lint + stdout: output file containing the stdout or --output-file of clang-tidy + exit_code: output file containing the exit code of clang-tidy. + If None, then fail the build when clang-tidy exits non-zero. + """ + + outputs = [stdout] + env = {} + env["CLANG_TIDY__STDOUT_STDERR_OUTPUT_FILE"] = stdout.path + if exit_code: + env["CLANG_TIDY__EXIT_CODE_OUTPUT_FILE"] = exit_code.path + outputs.append(exit_code) + if (ctx.attr._verbose): + env["CLANG_TIDY__VERBOSE"] = "1" + + ctx.actions.run_shell( + inputs = _gather_inputs(ctx, compilation_context, srcs), + outputs = outputs, + tools = [executable._clang_tidy_wrapper, executable._clang_tidy], + command = executable._clang_tidy_wrapper.path + " $@", + arguments = [executable._clang_tidy.path] + _get_args(ctx, compilation_context, srcs), + use_default_shell_env = True, + env = env, + mnemonic = _MNEMONIC, + progress_message = "Linting %{label} with clang-tidy", + ) + +def clang_tidy_fix(ctx, compilation_context, executable, srcs, patch, stdout, exit_code): + """Create a Bazel Action that spawns clang-tidy with --fix. + + Args: + ctx: an action context OR aspect context + compilation_context: from target + executable: struct with a clang_tidy field + srcs: list of file objects to lint + patch: output file containing the applied fixes that can be applied with the patch(1) command. + stdout: output file containing the stdout or --output-file of clang-tidy + exit_code: output file containing the exit code of clang-tidy + """ + patch_cfg = ctx.actions.declare_file("_{}.patch_cfg".format(ctx.label.name)) + + env = {} + if (ctx.attr._verbose): + env["CLANG_TIDY__VERBOSE"] = "1" + + ctx.actions.write( + output = patch_cfg, + content = json.encode({ + "linter": executable._clang_tidy_wrapper.path, + "args": [executable._clang_tidy.path, "--fix"] + _get_args(ctx, compilation_context, srcs), + "env": env, + "files_to_diff": [src.path for src in srcs], + "output": patch.path, + }), + ) + + ctx.actions.run( + inputs = _gather_inputs(ctx, compilation_context, srcs) + [patch_cfg], + outputs = [patch, stdout, exit_code], + executable = executable._patcher, + arguments = [patch_cfg.path], + env = { + "BAZEL_BINDIR": ".", + "JS_BINARY__EXIT_CODE_OUTPUT_FILE": exit_code.path, + "JS_BINARY__STDOUT_OUTPUT_FILE": stdout.path, + "JS_BINARY__SILENT_ON_SUCCESS": "1", + }, + tools = [executable._clang_tidy_wrapper, executable._clang_tidy], + mnemonic = _MNEMONIC, + progress_message = "Linting %{label} with clang-tidy", + ) + +# buildifier: disable=function-docstring +def _clang_tidy_aspect_impl(target, ctx): + if not CcInfo in target: + return [] + + files_to_lint = _filter_srcs(ctx.rule) + compilation_context = target[CcInfo].compilation_context + + if ctx.attr._options[LintOptionsInfo].fix: + patch, report, exit_code, info = patch_and_report_files(_MNEMONIC, target, ctx) + if len(files_to_lint) == 0: + dummy_successful_lint_action(ctx, report, exit_code, patch) + else: + clang_tidy_fix(ctx, compilation_context, ctx.executable, files_to_lint, patch, report, exit_code) + else: + report, exit_code, info = report_files(_MNEMONIC, target, ctx) + if len(files_to_lint) == 0: + dummy_successful_lint_action(ctx, report, exit_code) + else: + clang_tidy_action(ctx, compilation_context, ctx.executable, files_to_lint, report, exit_code) + return [info] + +def lint_clang_tidy_aspect(binary, configs = [], global_config = [], header_filter = "", lint_target_headers = False, angle_includes_are_system = True, verbose = False): + """A factory function to create a linter aspect. + + Args: + binary: the clang-tidy binary, typically a rule like + + ```starlark + native_binary( + name = "clang_tidy", + src = "clang-tidy.exe" + out = "clang_tidy", + ) + ``` + configs: labels of the .clang-tidy files to make available to clang-tidy's config search. These may be + in subdirectories and clang-tidy will apply them if appropriate. This may also include .clang-format + files which may be used for formatting fixes. + global_config: label of a single global .clang-tidy file to pass to clang-tidy on the command line. This + will cause clang-tidy to ignore any other config files in the source directories. + header_filter: optional, set to a posix regex to supply to clang-tidy with the -header-filter option + lint_target_headers: optional, set to True to pass a pattern that includes all headers with the target's + directory prefix. This crude control may include headers from the linted target in the results. If + supplied, overrides the header_filter option. + angle_includes_are_system: controls how angle includes are passed to clang-tidy. By default, Bazel + passes these as -isystem. Change this to False to pass these as -I, which allows clang-tidy to regard + them as regular header files. + verbose: print debug messages including clang-tidy command lines being invoked. + """ + + if type(global_config) == "string": + global_config = [global_config] + + return aspect( + implementation = _clang_tidy_aspect_impl, + attrs = { + "_options": attr.label( + default = "//lint:options", + providers = [LintOptionsInfo], + ), + "_configs": attr.label_list( + default = configs, + allow_files = True, + ), + "_global_config": attr.label_list( + default = global_config, + allow_files = True, + ), + "_lint_target_headers": attr.bool( + default = lint_target_headers, + ), + "_header_filter": attr.string( + default = header_filter, + ), + "_angle_includes_are_system": attr.bool( + default = angle_includes_are_system, + ), + "_verbose": attr.bool( + default = verbose, + ), + "_clang_tidy": attr.label( + default = binary, + executable = True, + cfg = "exec", + ), + "_clang_tidy_wrapper": attr.label( + default = Label("@aspect_rules_lint//lint:clang_tidy_wrapper"), + executable = True, + cfg = "exec", + ), + "_patcher": attr.label( + default = "@aspect_rules_lint//lint/private:patcher", + executable = True, + cfg = "exec", + ), + "_cc_toolchain": attr.label(default = Label("@bazel_tools//tools/cpp:current_cc_toolchain")), + }, + toolchains = ["@bazel_tools//tools/cpp:toolchain_type"], + fragments = ["cpp"], + ) diff --git a/lint/clang_tidy_wrapper.bash b/lint/clang_tidy_wrapper.bash new file mode 100755 index 00000000..e94d28f5 --- /dev/null +++ b/lint/clang_tidy_wrapper.bash @@ -0,0 +1,84 @@ +#!/usr/bin/bash +# This is a wrapper for clang-tidy which gives us control over error handling +# Usage: clang_tidy_wrapper.bash ... -- +# +# Controls: +# - CLANG_TIDY__VERBOSE: If set, be verbose +# - CLANG_TIDY__STDOUT_STDERR_OUTPUT_FILE: If set, write stdout and stderr to this file +# - CLANG_TIDY__EXIT_CODE_OUTPUT_FILE: If set, write the highest exit code +# to this file and return success + +# First arg is clang-tidy path +clang_tidy=$1 +shift + +if [[ -n $CLANG_TIDY__STDOUT_STDERR_OUTPUT_FILE ]]; then + # Create the file if it doesn't exist + touch $CLANG_TIDY__STDOUT_STDERR_OUTPUT_FILE + # Clear the file if it does exist + > $CLANG_TIDY__STDOUT_STDERR_OUTPUT_FILE + if [[ -n $CLANG_TIDY__VERBOSE ]]; then + echo "Output > ${CLANG_TIDY__STDOUT_STDERR_OUTPUT_FILE}" + fi +fi +if [[ -n $CLANG_TIDY__EXIT_CODE_OUTPUT_FILE ]]; then + if [[ -n $CLANG_TIDY__VERBOSE ]]; then + echo "Exit Code -> ${CLANG_TIDY__EXIT_CODE_OUTPUT_FILE}" + fi +fi + +if [[ -n $CLANG_TIDY__STDOUT_STDERR_OUTPUT_FILE ]]; then + out_file=$CLANG_TIDY__STDOUT_STDERR_OUTPUT_FILE +else + out_file=$(mktemp) +fi +# include stderr in output file; it contains some of the diagnostics +command="$clang_tidy $@ $file > $out_file 2>&1" +if [[ -n $CLANG_TIDY__VERBOSE ]]; then + echo "$@" + echo "cwd: " `pwd` + echo $command +fi +eval $command +exit_code=$? +if [[ -z $CLANG_TIDY__STDOUT_STDERR_OUTPUT_FILE ]]; then + cat $out_file +fi +# distinguish between compile (fatal) errors and warnings-as-errors errors +fatal_error=0 +if [ $exit_code -ne 0 ] && [ -s $out_file ]; then + while read line + do + if [[ $line == *"clang-diagnostic-error"* ]]; then + fatal_error=1 + break + fi + done < "$out_file" +fi +if [ $fatal_error -ne 0 ]; then + cat $out_file + rm $out_file + if [[ -n $CLANG_TIDY__VERBOSE ]]; then + echo "found clang-diagnostic-error (regarding as fatal)" + echo "exit $exit_code" + fi + exit $exit_code +fi +if [[ -z $CLANG_TIDY__STDOUT_STDERR_OUTPUT_FILE ]]; then + rm $out_file +fi + +# if CLANG_TIDY__EXIT_CODE_FILE is set, write the max exit code to that file and return success +if [[ -n $CLANG_TIDY__EXIT_CODE_OUTPUT_FILE ]]; then + if [[ -n $CLANG_TIDY__VERBOSE ]]; then + echo "echo $exit_code > $CLANG_TIDY__EXIT_CODE_OUTPUT_FILE" + echo "exit 0" + fi + echo $exit_code > $CLANG_TIDY__EXIT_CODE_OUTPUT_FILE + exit 0 +fi + +if [[ -n $CLANG_TIDY__VERBOSE ]]; then + echo exit $exit_code +fi +exit $exit_code