erenon / bazel_clang_tidy

Run clang-tidy on Bazel C++ targets directly, efficiently, with caching enabled
MIT License
96 stars 51 forks source link

Aspect runs on all dependencies and not only built target #46

Closed rdelfin closed 9 months ago

rdelfin commented 9 months ago

One issue I encounter in our codebase is we have a lot of generated code (e.g. protobufs) and third party code that we frankly don't want to run clang-tidy on. While the lints it catches are legitimate, we won't have time to go through all the dependencies and fix their clang-tidy bugs. We need to focus on our own codebase. However, when I build a target using the flags specified, I get errors on all dependencies, not just on the one that is being built.

Having the aspect only run on the specific target being built makes it a lot easier to be selective about what you want to run clang tidy on in CI. This is how the rust clippy aspect. Could we change the behaviour to work that way?

erenon commented 9 months ago

Thanks for the report. I think into this point:

https://github.com/erenon/bazel_clang_tidy/blob/master/clang_tidy/clang_tidy.bzl#L132

The following should be inserted:

    # Ignore external targets
    if target.label.workspace_root.startswith("external"):
        return None

https://github.com/bazelbuild/rules_rust/blob/main/rust/private/clippy.bzl#L59C1-L61C20

This would ignore the external deps. Can you make a PR?

I don't know yet how to ignore locally generated protobufs.

rdelfin commented 9 months ago

Thanks @erenon! Let me look into how to do it. The more I've looked into this, the more I realise it might also be because it's running on the generated header files. Do you have a mechanism for filtering out header files?

erenon commented 9 months ago

Should be fixed by #47, please reopen if not.