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.

Sign up for GitHub

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

fix: Add option to skip target without srcs #447

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Add option to skip target without srcs #447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion docs/clang-tidy.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 10 additions & 3 deletions lint/clang_tidy.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,12 @@ def _is_source(file):
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):
def _filter_srcs(rule, skip_if_no_srcs = False):
if "lint-genfiles" in rule.attr.tags:
return rule.files.srcs
else:
if skip_if_no_srcs and not hasattr(rule.files, "srcs"):
return []
return [s for s in rule.files.srcs if _is_source(s)]

def is_parent_in_list(dir, list):
Expand Down Expand Up @@ -369,7 +371,7 @@ def _clang_tidy_aspect_impl(target, ctx):
if not CcInfo in target:
return []

files_to_lint = _filter_srcs(ctx.rule)
files_to_lint = _filter_srcs(ctx.rule, ctx.attr._skip_if_no_srcs)
compilation_context = target[CcInfo].compilation_context
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I look at it - compilation_context could be None - see https://bazel.build/rules/lib/providers/CcInfo#CcInfo; I suspect that this will happen precisely when there are no srcs present and no deps? And if so we might need to protect other places further down where we access compilation_context?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johan-vcc does it fix the issue for you if you check for this being None ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried it out, then that check would need to be before it tries to access rule.files.srcs in _filter_srcs().
However, in my case the targets have compilation_context so that would not avoid the need to check srcs as well.

if hasattr(ctx.rule.attr, "implementation_deps"):
compilation_context = cc_common.merge_compilation_contexts(
Expand All @@ -394,7 +396,7 @@ def _clang_tidy_aspect_impl(target, ctx):
clang_tidy_action(ctx, compilation_context, ctx.executable, files_to_lint, outputs.machine.out, outputs.machine.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):
def lint_clang_tidy_aspect(binary, configs = [], global_config = [], header_filter = "", lint_target_headers = False, angle_includes_are_system = True, skip_if_no_srcs = False, verbose = False):
"""A factory function to create a linter aspect.

Args:
Expand All @@ -419,6 +421,8 @@ def lint_clang_tidy_aspect(binary, configs = [], global_config = [], header_filt
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.
skip_if_no_srcs: optional, set to True to skip target if it provides CcInfo but has no source files. By
default this will generate an error.
verbose: print debug messages including clang-tidy command lines being invoked.
"""

Expand Down Expand Up @@ -449,6 +453,9 @@ def lint_clang_tidy_aspect(binary, configs = [], global_config = [], header_filt
"_angle_includes_are_system": attr.bool(
default = angle_includes_are_system,
),
"_skip_if_no_srcs": attr.bool(
default = skip_if_no_srcs,
),
"_verbose": attr.bool(
default = verbose,
),
Expand Down
Loading