-
Notifications
You must be signed in to change notification settings - Fork 0
Add clang-tidy rule [BUILD-304] #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| Checks: "-*, | ||
| cert*, | ||
| clang-analyzer*, | ||
| cppcoreguidelines*, | ||
| google*, | ||
| misc*, | ||
| modernize*, | ||
| performance*, | ||
| readability*, | ||
| -clang-analyzer-osx*, | ||
| -clang-analyzer-optin.osx.*, | ||
| -clang-analyzer-apiModeling.google.GTest, | ||
| -clang-analyzer-llvm.Conventions, | ||
| -readability-function-size, | ||
| -clang-analyzer-optin.mpi*, | ||
| -google-objc*, | ||
| -readability-misleading-indentation, | ||
| -readability-identifier-naming, | ||
| -misc-unused-parameters, | ||
| -clang-analyzer-security.insecureAPI*, | ||
| -cert-dcl03-c, | ||
| -cert-dcl21-cpp, | ||
| -cert-err34-c, | ||
| -cert-err58-cpp, | ||
| -clang-analyzer-alpha*, | ||
| -clang-analyzer-core.CallAndMessage, | ||
| -clang-analyzer-core.UndefinedBinaryOperatorResult, | ||
| -clang-analyzer-core.uninitialized.Assign, | ||
| -clang-analyzer-core.uninitialized.UndefReturn, | ||
| -clang-analyzer-optin.cplusplus.VirtualCall, | ||
| -clang-analyzer-optin.performance.Padding, | ||
| -cppcoreguidelines-owning-memory, | ||
| -cppcoreguidelines-pro-bounds-array-to-pointer-decay, | ||
| -cppcoreguidelines-pro-bounds-constant-array-index, | ||
| -cppcoreguidelines-pro-bounds-pointer-arithmetic, | ||
| -cppcoreguidelines-pro-type-member-init, | ||
| -cppcoreguidelines-pro-type-static-cast-downcast, | ||
| -cppcoreguidelines-pro-type-union-access, | ||
| -cppcoreguidelines-pro-type-vararg, | ||
| -cppcoreguidelines-special-member-functions, | ||
| -google-runtime-references, | ||
| -misc-static-assert, | ||
| -modernize-deprecated-headers, | ||
| -modernize-pass-by-value, | ||
| -modernize-redundant-void-arg, | ||
| -modernize-return-braced-init-list, | ||
| -modernize-use-auto, | ||
| -modernize-use-bool-literals, | ||
| -modernize-use-default-member-init, | ||
| -modernize-use-emplace, | ||
| -modernize-use-equals-default, | ||
| -modernize-use-equals-delete, | ||
| -modernize-use-using, | ||
| -performance-unnecessary-value-param, | ||
| -readability-avoid-const-params-in-decls, | ||
| -readability-non-const-parameter, | ||
| -readability-redundant-declaration, | ||
| -readability-redundant-member-init, | ||
| -clang-analyzer-core.StackAddressEscape, | ||
| -clang-analyzer-core.VLASize, | ||
| -clang-analyzer-cplusplus.NewDeleteLeaks, | ||
| -clang-analyzer-deadcode.DeadStores, | ||
| -clang-analyzer-optin.cplusplus.UninitializedObject, | ||
| -cert-err33-c, | ||
| -cert-exp42-c, | ||
| -cert-dcl37-c, | ||
| -cert-dcl51-cpp, | ||
| -cert-flp37-c, | ||
| -cert-oop54-cpp, | ||
| -cert-str34-c, | ||
| -cppcoreguidelines-avoid-c-arrays, | ||
| -cppcoreguidelines-avoid-goto, | ||
| -cppcoreguidelines-avoid-magic-numbers, | ||
| -cppcoreguidelines-avoid-non-const-global-variables, | ||
| -cppcoreguidelines-init-variables, | ||
| -cppcoreguidelines-macro-usage, | ||
| -cppcoreguidelines-narrowing-conversions, | ||
| -cppcoreguidelines-non-private-member-variables-in-classes, | ||
| -cppcoreguidelines-prefer-member-initializer, | ||
| -cppcoreguidelines-virtual-class-destructor, | ||
| -misc-non-private-member-variables-in-classes, | ||
| -misc-no-recursion, | ||
| -modernize-avoid-c-arrays, | ||
| -modernize-use-trailing-return-type, | ||
| -performance-no-int-to-ptr, | ||
| -readability-const-return-type, | ||
| -readability-container-data-pointer, | ||
| -readability-convert-member-functions-to-static, | ||
| -readability-duplicate-include, | ||
| -readability-function-cognitive-complexity, | ||
| -readability-identifier-length, | ||
| -readability-isolate-declaration, | ||
| -readability-make-member-function-const, | ||
| -readability-magic-numbers, | ||
| -readability-qualified-auto, | ||
| -readability-redundant-access-specifiers, | ||
| -readability-suspicious-call-argument, | ||
| -readability-uppercase-literal-suffix, | ||
| -readability-use-anyofallof" | ||
| HeaderFilterRegex: '.*' | ||
| AnalyzeTemporaryDtors: true |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| load(":choose_clang_tidy.bzl", "choose_clang_tidy") | ||
|
|
||
| choose_clang_tidy( | ||
| name = "clang_tidy_bin", | ||
| visibility = ["//visibility:public"], | ||
| ) | ||
|
|
||
| sh_binary( | ||
| name = "clang_tidy", | ||
| srcs = ["run_clang_tidy.sh"], | ||
| data = [":clang_tidy_config"], | ||
| visibility = ["//visibility:public"], | ||
| ) | ||
|
|
||
| filegroup( | ||
| name = "clang_tidy_config_default", | ||
| srcs = [".clang-tidy"], | ||
| ) | ||
|
|
||
| label_flag( | ||
| name = "clang_tidy_config", | ||
| build_setting_default = ":clang_tidy_config_default", | ||
| visibility = ["//visibility:public"], | ||
| ) | ||
|
|
||
| filegroup( | ||
| name = "clang_tidy_executable_default", | ||
| srcs = [":clang_tidy_bin"], | ||
| ) | ||
|
|
||
| label_flag( | ||
| name = "clang_tidy_executable", | ||
| build_setting_default = ":clang_tidy_executable_default", | ||
| visibility = ["//visibility:public"], | ||
| ) | ||
|
|
||
| filegroup( | ||
| name = "clang_tidy_additional_deps_default", | ||
| srcs = [], | ||
| ) | ||
|
|
||
| label_flag( | ||
| name = "clang_tidy_additional_deps", | ||
| build_setting_default = ":clang_tidy_additional_deps_default", | ||
| visibility = ["//visibility:public"], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| def _choose_clang_tidy(ctx): | ||
| out = ctx.actions.declare_file("clang_tidy_bin.sh") | ||
|
|
||
| ctx.actions.run_shell( | ||
| outputs = [out], | ||
| command = """ | ||
| if command -v clang-tidy-14 &> /dev/null | ||
| then | ||
| echo clang-tidy-14 \\"\\$@\\" > {0} | ||
| elif command -v clang-tidy &> /dev/null | ||
| then | ||
| echo clang-tidy \\"\\$@\\" > {0} | ||
| else | ||
| err_msg='clang-tidy-14 / clang-tidy: command not found' | ||
| echo $err_msg | ||
| echo "echo "$err_msg">&2" >> {0} | ||
| echo "exit 1" >> {0} | ||
| fi | ||
| """.format(out.path), | ||
| ) | ||
|
|
||
| return [DefaultInfo(files = depset([out]))] | ||
|
|
||
| choose_clang_tidy = rule( | ||
| implementation = _choose_clang_tidy, | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain") | ||
| load("//cc:defs.bzl", "BINARY", "LIBRARY") | ||
|
|
||
| def _flatten(input_list): | ||
| return [item for sublist in input_list for item in sublist] | ||
|
|
||
| def _run_tidy(ctx, wrapper, exe, additional_deps, config, flags, compilation_contexts, infile, discriminator): | ||
| inputs = depset(direct = [infile, config] + additional_deps.files.to_list() + ([exe.files_to_run.executable] if exe.files_to_run.executable else []), transitive = [compilation_context.headers for compilation_context in compilation_contexts]) | ||
|
|
||
| args = ctx.actions.args() | ||
|
|
||
| # specify the output file - twice | ||
| outfile = ctx.actions.declare_file( | ||
| "bazel_clang_tidy_" + infile.path + "." + discriminator + ".clang-tidy.yaml", | ||
| ) | ||
|
|
||
| # this is consumed by the wrapper script | ||
| if len(exe.files.to_list()) == 0: | ||
| args.add("clang-tidy") | ||
| else: | ||
| args.add(exe.files_to_run.executable) | ||
|
|
||
| args.add(config.path) | ||
|
|
||
| args.add(outfile.path) # this is consumed by the wrapper script | ||
| args.add("--export-fixes", outfile.path) | ||
|
|
||
| # add source to check | ||
| args.add(infile.path) | ||
|
|
||
| # start args passed to the compiler | ||
| args.add("--") | ||
|
|
||
| # add args specified by the toolchain, on the command line and rule copts | ||
| args.add_all(flags) | ||
|
|
||
| # add defines | ||
| for define in _flatten([compilation_context.defines.to_list() for compilation_context in compilation_contexts]): | ||
| args.add("-D" + define) | ||
|
|
||
| for define in _flatten([compilation_context.local_defines.to_list() for compilation_context in compilation_contexts]): | ||
| args.add("-D" + define) | ||
|
|
||
| # add includes | ||
| for i in _flatten([compilation_context.framework_includes.to_list() for compilation_context in compilation_contexts]): | ||
| args.add("-F" + i) | ||
|
|
||
| for i in _flatten([compilation_context.includes.to_list() for compilation_context in compilation_contexts]): | ||
| if "gflags" in i: | ||
| args.add("-isystem" + i) | ||
| else: | ||
| args.add("-I" + i) | ||
|
|
||
| args.add_all(_flatten([compilation_context.quote_includes.to_list() for compilation_context in compilation_contexts]), before_each = "-iquote") | ||
|
|
||
| args.add_all(_flatten([compilation_context.system_includes.to_list() for compilation_context in compilation_contexts]), before_each = "-isystem") | ||
|
|
||
| ctx.actions.run( | ||
| inputs = inputs, | ||
| outputs = [outfile], | ||
| executable = wrapper, | ||
| arguments = [args], | ||
| mnemonic = "ClangTidy", | ||
| use_default_shell_env = True, | ||
| progress_message = "Run clang-tidy on {}".format(infile.short_path), | ||
| ) | ||
| return outfile | ||
|
|
||
| def _rule_sources(ctx): | ||
| srcs = [] | ||
| if hasattr(ctx.rule.attr, "srcs"): | ||
| for src in ctx.rule.attr.srcs: | ||
| srcs += [src for src in src.files.to_list() if src.is_source] | ||
| return srcs | ||
|
|
||
| def _toolchain_flags(ctx): | ||
| 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 = "c++-compile", # tools/build_defs/cc/action_names.bzl CPP_COMPILE_ACTION_NAME | ||
| variables = compile_variables, | ||
| ) | ||
| return flags | ||
|
|
||
| def _safe_flags(flags): | ||
| # Some flags might be used by GCC, but not understood by Clang. | ||
| # 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) | ||
| unsupported_flags = [ | ||
| "-fno-canonical-system-headers", | ||
| "-fstack-usage", | ||
| "-Wno-free-nonheap-object", | ||
| "-Wunused-but-set-parameter", | ||
| ] | ||
|
|
||
| return [flag for flag in flags if flag not in unsupported_flags and not flag.startswith("--sysroot") and not "-std=" in flag] | ||
|
|
||
| def _replace_gendir(flags, ctx): | ||
| return [flag.replace("$(GENDIR)", ctx.genfiles_dir.path) for flag in flags] | ||
|
||
|
|
||
| # since implementation_deps is currently an experimental feature we have to add compilation context from implementation_deps manually | ||
| def _get_compilation_contexts(target, ctx): | ||
|
||
| compilation_contexts = [target[CcInfo].compilation_context] | ||
|
|
||
| implementation_deps = getattr(ctx.rule.attr, "implementation_deps", []) | ||
| for implementation_dep in implementation_deps: | ||
| compilation_contexts.append(implementation_dep[CcInfo].compilation_context) | ||
|
|
||
| return compilation_contexts | ||
|
|
||
| def _clang_tidy_aspect_impl(target, ctx): | ||
| # if not a C/C++ target, we are not interested | ||
| if not CcInfo in target: | ||
| return [] | ||
|
|
||
| tags = getattr(ctx.rule.attr, "tags", []) | ||
| if not LIBRARY in tags and not BINARY in tags: | ||
| return [] | ||
|
|
||
| wrapper = ctx.attr._clang_tidy_wrapper.files_to_run | ||
| exe = ctx.attr._clang_tidy_executable | ||
| additional_deps = ctx.attr._clang_tidy_additional_deps | ||
| config = ctx.attr._clang_tidy_config.files.to_list()[0] | ||
| toolchain_flags = _toolchain_flags(ctx) | ||
| rule_flags = ctx.rule.attr.copts if hasattr(ctx.rule.attr, "copts") else [] | ||
| safe_flags = _safe_flags(toolchain_flags + rule_flags) | ||
| final_flags = _replace_gendir(safe_flags, ctx) | ||
| compilation_contexts = _get_compilation_contexts(target, ctx) | ||
|
|
||
| srcs = _rule_sources(ctx) | ||
| outputs = [_run_tidy(ctx, wrapper, exe, additional_deps, config, final_flags, compilation_contexts, src, target.label.name) for src in srcs] | ||
| return [ | ||
| OutputGroupInfo(report = depset(direct = outputs)), | ||
| ] | ||
|
|
||
| clang_tidy_aspect = aspect( | ||
| implementation = _clang_tidy_aspect_impl, | ||
| fragments = ["cpp"], | ||
| attrs = { | ||
| "_cc_toolchain": attr.label(default = Label("@bazel_tools//tools/cpp:current_cc_toolchain")), | ||
| "_clang_tidy_wrapper": attr.label(default = Label("//clang_tidy:clang_tidy")), | ||
| "_clang_tidy_executable": attr.label(default = Label("//clang_tidy:clang_tidy_executable")), | ||
| "_clang_tidy_additional_deps": attr.label(default = Label("//clang_tidy:clang_tidy_additional_deps")), | ||
| "_clang_tidy_config": attr.label(default = Label("//clang_tidy:clang_tidy_config")), | ||
| }, | ||
| toolchains = ["@bazel_tools//tools/cpp:toolchain_type"], | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| #! /bin/bash | ||
| # Usage: run_clang_tidy <CONFIG> <OUTPUT> [ARGS...] | ||
| set -ue | ||
|
|
||
| CLANG_TIDY_BIN=$1 | ||
| shift | ||
|
|
||
| CONFIG=$1 | ||
| shift | ||
|
|
||
| OUTPUT=$1 | ||
| shift | ||
|
|
||
| # .clang-tidy config file has to be placed in the current working directory | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might be the piece that was missing in the |
||
| if [ ! -f ".clang-tidy" ]; then | ||
| ln -s $CONFIG .clang-tidy | ||
| fi | ||
|
|
||
| # clang-tidy doesn't create a patchfile if there are no errors. | ||
| # make sure the output exists, and empty if there are no errors, | ||
| # so the build system will not be confused. | ||
| touch $OUTPUT | ||
| truncate -s 0 $OUTPUT | ||
|
|
||
| "${CLANG_TIDY_BIN}" "$@" | ||
|
|
||
| test ! -s $OUTPUT | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Workaround for gflag that I described in the ticket.