fix: Add option to skip target without srcs

[johan-vcc]

Description

Add option 'skip_if_no_srcs' to lint_clang_tidy aspect.
Set to True to skip target if it provides CcInfo but has no source files. By default this will generate an error.

See issue #406

Adding an option to skip these targets enables us to run ./lint.sh //package//... even if some target has no source files, e.g. code generation targets.

Changes are visible to end-users: yes

• Searched for relevant documentation and updated as needed: yes
• Breaking change (forces users to change their own code or config): no
• Suggested release notes appear below: yes
  Add option 'skip_if_no_srcs' to lint_clang_tidy aspect.

Test plan

• Manual testing; please provide instructions so we can reproduce:
  bazel build with lint_clang_tidy_aspect for a target that provides 'CcInfo' but does not have 'srcs' attribute.
  With skip_if_no_srcs set to True:
  Build completes with empty target.AspectRulesLintClangTidy.out file.
  Without new option / new option set to False:

File ".../aspect_rules_lint~/lint/clang_tidy.bzl", line 186, column 38, in _filter_srcs
                return [s for s in rule.files.srcs if _is_source(s)]
Error: No attribute 'srcs' in files. Make sure there is a label or label_list type attribute with this name

[CLAassistant]

All committers have signed the CLA.

[alexeagle]

Thanks - is there ever a reason users ought to set skip_if_no_srcs=False ? If not, can we make a simpler change to always exclude rather than introduce a new configuration option?

[johan-vcc]

I am unsure of other use-cases. That is why I added the option in case the old behavior is needed.
If you think a simpler always-exclude is good, that would work for my use-case.

[alexeagle]

@jsharpe can you think of any reason the aspect might need to visit a library with empty srcs? Or is there a better attribute on CcInfo that tells us it's a no-op?

[jsharpe]

I don't think it would need to visit the target if it has an empty srcs since this is where we construct the list of files to apply clang-tidy to anyway so if that was empty there is nothing to do. IMO we can always skip if srcs is empty.

[jsharpe]

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?

[alexeagle]

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

[johan-vcc]

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.