Skip to content
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

Support clang-tidy (early PR for review) #284

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions README-clang-tidy-pull-request.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Clang-tidy PR

(temporary file, not to be included in final merge)

I'm attempting to integrate clang-tidy with rules_lint, by integrating some of the aspect approaches from
bazel_clang_tidy into this linter framework. The idea of integrating these linters has been suggested by
maintainers of both repos (https://github.com/erenon/bazel_clang_tidy/issues/35)

## Windows setup
```
copy clang-tidy.exe into examples/tools/lint
del examples\\.bazeliskrc (aspect-cli does not support windows)
BAZEL_SH=c:\msys64\usr\bin\bash.exe # git bash doesn't seem to work
BAZEL_VC=c:\apps\MVS16\VC
```

## Example commands
Run on a binary
```
cd example
bazel build //src/cpp/main:hello-world --config=clang-tidy
```

Run with raw options, no config (this is exactly the same as above)
```
bazel build //src/cpp/main:hello-world --aspects=//tools/lint:linters.bzl%clang_tidy --output_groups=rules_lint_report
```

See clang-tidy command line for each invokation
```
bazel build //src/cpp/main:hello-world --config=clang-tidy -s
```

Run on a cc_library
```
bazel build //src/cpp/lib:hello-time --config=clang-tidy -s
```

## Questions
- clang-tidy handles only a single source file at a time. This is different to all the other linters currently supported. What is the best way to structure this code in clang_tidy.bzl? Pass one file to each invokation of clang_tidy_action? Or loop inside clang_tidy_action?
- Any other inputs?

3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ New tools are being added frequently, so check this page again!

| Language | Formatter | Linter(s) |
| ---------------------- | --------------------- | ---------------- |
| C / C++ | [clang-format] | ([#112]) |
| C / C++ | [clang-format] | ([clang-tidy]) |
| CSS, Less, Sass | [Prettier] | |
| Go | [gofmt] or [gofumpt] | |
| HCL (Hashicorp Config) | [terraform] fmt | |
Expand Down Expand Up @@ -67,6 +67,7 @@ New tools are being added frequently, so check this page again!
[shellcheck]: https://www.shellcheck.net/
[shfmt]: https://github.com/mvdan/sh
[clang-format]: https://clang.llvm.org/docs/ClangFormat.html
[clang-tidy]: https://clang.llvm.org/extra/clang-tidy/
[#112]: https://github.com/aspect-build/rules_lint/issues/112
[vale]: https://vale.sh/
[yamlfmt]: https://github.com/google/yamlfmt
Expand Down
5 changes: 5 additions & 0 deletions docs/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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")
11 changes: 11 additions & 0 deletions example/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,14 @@ build --repo_env=JAVA_HOME=../bazel_tools/jdk

common --incompatible_enable_proto_toolchain_resolution
common --@aspect_rules_ts//ts:skipLibCheck=always

# For testing only clang-tidy only
common:clang-tidy --aspects=//tools/lint:linters.bzl%clang_tidy
common:clang-tidy --output_groups=rules_lint_report --norun_validations --remote_download_regex=".*AspectRulesLint.*"
startup --windows_enable_symlinks

# 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
9 changes: 9 additions & 0 deletions example/.clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Checks: >
-*,
bugprone-*,
google-*,
misc-*,
modernize-*,
performance-*,
portability-*,
readability-*
1 change: 1 addition & 0 deletions example/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ exports_files(
".vale.ini",
".editorconfig",
"ktlint-baseline.xml",
".clang-tidy",
],
visibility = ["//visibility:public"],
)
Expand Down
1 change: 1 addition & 0 deletions example/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion example/lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ filter='.namedSetOfFiles | values | .files[] | select(.name | endswith($ext)) |

# 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 ' ' ',')"
"--aspects=$(echo //tools/lint:linters.bzl%{buf,eslint,flake8,ktlint,pmd,ruff,shellcheck,vale,clang_tidy} | tr ' ' ',')"
# Allow lints of code that fails some validation action
# See https://github.com/aspect-build/rules_ts/pull/574#issuecomment-2073632879
"--norun_validations"
Expand Down
9 changes: 9 additions & 0 deletions example/src/cpp/lib/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
load("@rules_cc//cc:defs.bzl", "cc_library")

cc_library(
name = "hello-time",
srcs = ["hello-time.cc"],
hdrs = ["hello-time.h", "xhello-time.h"],
includes = ["."],
visibility = ["//src/cpp/main:__pkg__"],
)
9 changes: 9 additions & 0 deletions example/src/cpp/lib/hello-time.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#include "hello-time.h"
#include <ctime>
#include <iostream>

void print_localtime() {
std::time_t result = std::time(nullptr);
std::string result_str = std::asctime(std::localtime(&result));
printf("%s\n", result_str.c_str());
}
16 changes: 16 additions & 0 deletions example/src/cpp/lib/hello-time.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#ifndef LIB_HELLO_TIME_H_
#define LIB_HELLO_TIME_H_

#include <string>
#include <stdio.h>
#include <xhello-time.h>

void print_localtime();

inline void print_localtime2() {
std::string a = "time";
for (int i=0; i<a.size(); ++i) {
printf("%s", a.c_str()[i]);
}
}
#endif
13 changes: 13 additions & 0 deletions example/src/cpp/lib/xhello-time.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#ifndef LIB_HELLO_TIME2_H_
#define LIB_HELLO_TIME2_H_

#include <string>
#include <stdio.h>

inline void print_localtime3() {
std::string a = "time";
for (int i=0; i<a.size(); ++i) {
printf("%s", a.c_str()[i]);
}
}
#endif
18 changes: 18 additions & 0 deletions example/src/cpp/main/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library")

cc_library(
name = "hello-greet",
srcs = ["hello-greet.cc"],
hdrs = ["hello-greet.h"],
includes = ["."],
)

cc_binary(
name = "hello-world",
srcs = ["hello-world.cc"],
deps = [
":hello-greet",
"//src/cpp/lib:hello-time",
],
includes = ["."],
)
6 changes: 6 additions & 0 deletions example/src/cpp/main/hello-greet.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include "hello-greet.h"
#include <string>

std::string get_greet(const std::string& who) {
return "Hello " + who;
}
8 changes: 8 additions & 0 deletions example/src/cpp/main/hello-greet.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#ifndef MAIN_HELLO_GREET_H_
#define MAIN_HELLO_GREET_H_

#include <string>

std::string get_greet(const std::string &thing);

#endif
14 changes: 14 additions & 0 deletions example/src/cpp/main/hello-world.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#include "hello-time.h"
#include "hello-greet.h"
#include <iostream>
#include <string>

int main(int argc, char** argv) {
std::string who = "world";
if (argc > 1) {
who = argv[1];
}
std::cout << get_greet(who) << std::endl;
print_localtime();
return 0;
}
9 changes: 9 additions & 0 deletions example/src/hello.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
#include <stdio.h>
#include <stdlib.h>

int string_to_int(const char *num) {
return atoi(num);
}

void ls() {
system("ls");
}

int main() { printf("Hello, world!\n"); }
16 changes: 16 additions & 0 deletions example/tools/lint/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}),
)

Expand Down Expand Up @@ -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",
Expand All @@ -68,3 +70,17 @@ 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",
"@bazel_tools//src/conditions:windows_x64": "clang-tidy.exe",
},
),
out = "clang_tidy",
)
10 changes: 10 additions & 0 deletions example/tools/lint/linters.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ load("@aspect_rules_lint//lint:pmd.bzl", "lint_pmd_aspect")
load("@aspect_rules_lint//lint:ruff.bzl", "lint_ruff_aspect")
load("@aspect_rules_lint//lint:shellcheck.bzl", "lint_shellcheck_aspect")
load("@aspect_rules_lint//lint:vale.bzl", "lint_vale_aspect")
load("@aspect_rules_lint//lint:clang_tidy.bzl", "lint_clang_tidy_aspect")

buf = lint_buf_aspect(
config = "@@//:buf.yaml",
Expand Down Expand Up @@ -71,3 +72,12 @@ ktlint = lint_ktlint_aspect(
)

ktlint_test = lint_test(aspect = ktlint)

clang_tidy = lint_clang_tidy_aspect(
binary = "@@//tools/lint:clang_tidy",
config = "@@//:.clang-tidy",
lint_matching_header = True,
angle_includes_are_system = False,
)

clang_tidy_test = lint_test(aspect = clang_tidy)
5 changes: 5 additions & 0 deletions example/workaround_windows.bat
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
:: aspect-cli doesn't support windows, so disable it
del %~dp0.bazeliskrc
:: ../.bazelversion syntax not supported by windows bazelisk
del %~dp0.bazelversion
copy %~dp0\..\.bazelversion %~dp0
6 changes: 6 additions & 0 deletions lint/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,9 @@ bzl_library(
"@bazel_skylib//lib:dicts",
],
)

bzl_library(
name = "clang-tidy",
srcs = ["clang-tidy.bzl"],
visibility = ["//visibility:public"],
)
Loading