Support multiple configuration files for different modules in checkstyle

[nlou9]

In some use case, different modules need different checkstyle configuration files as described in this feature request.
#454

This PR :

• upgrade the bazel version to 7.4.1 which would support data type string_keyed_label_dict
• add configs attribute which is a map of module label and configuration file.

Test:
In the test, add one checkstyle_subdir.xml file for src/subdir which only differs with different line length.

bazel lint //src/subdir:bar
INFO: Analyzed target //src/subdir:bar (0 packages loaded, 0 targets configured).
INFO: From Linting //src/subdir:bar with Checkstyle:
Checkstyle ends with 1 errors.
INFO: Found 1 target...
Aspect //tools/lint:linters.bzl%checkstyle of //src/subdir:bar up-to-date:
  bazel-bin/src/subdir/bar.AspectRulesLintCheckstyle.out
  bazel-bin/src/subdir/bar.AspectRulesLintCheckstyle.out.exit_code
INFO: Elapsed time: 0.380s, Critical Path: 0.21s
INFO: 2 processes: 1 internal, 1 darwin-sandbox.
INFO: Build completed successfully, 2 total actions
Lint results for //src/subdir:bar:

Starting audit...
[ERROR] /private/var/tmp/_bazel_louna/3cc840f4a4f44129661666d5b35a9196/sandbox/darwin-sandbox/14/execroot/_main/src/subdir/Bar.java:3: Line is longer than 84 characters (found 85). [LineLength]
Audit done.

bazel lint //src:bar
INFO: Analyzed target //src:bar (0 packages loaded, 2 targets configured).
INFO: Found 1 target...
Aspect //tools/lint:linters.bzl%checkstyle of //src:bar up-to-date:
  bazel-bin/src/bar.AspectRulesLintCheckstyle.out
  bazel-bin/src/bar.AspectRulesLintCheckstyle.out.exit_code
INFO: Elapsed time: 0.098s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
Lint results for //src:bar:

Starting audit...
[ERROR] /private/var/tmp/_bazel_louna/3cc840f4a4f44129661666d5b35a9196/sandbox/darwin-sandbox/13/execroot/_main/src/Bar.java:3: Line is longer than 20 characters (found 85). [LineLength]
[ERROR] /private/var/tmp/_bazel_louna/3cc840f4a4f44129661666d5b35a9196/sandbox/darwin-sandbox/13/execroot/_main/src/Bar.java:8: Line is longer than 20 characters (found 59). [LineLength]
[ERROR] /private/var/tmp/_bazel_louna/3cc840f4a4f44129661666d5b35a9196/sandbox/darwin-sandbox/13/execroot/_main/src/Bar.java:9: Line is longer than 20 characters (found 35). [LineLength]
Audit done.

---

Changes are visible to end-users: yes/no

• Searched for relevant documentation and updated as needed: yes/no
• Breaking change (forces users to change their own code or config): yes/no
• Suggested release notes appear below: yes/no

Test plan

• Covered by existing test cases
• New test cases added
• Manual testing; please provide instructions so we can reproduce:

[fzakaria]

do you want longest matching prefix?

[nlou9]

for the use case in the example, both //src/subdir and //src package have configuration files. I was thinking of matching the closest one.

[alexeagle]

I'd expect it to use the config from the nearest ancestor, matching most other tools

[nlou9]

The Checkstyle tool seems to retrieve the configuration file from the specified parameter. In our setup, we typically organize all Checkstyle configuration files within the checkstyle directory. It might make more sense to pass the configuration as a map instead.

[alexeagle]

How about a prefactoring where you move your checkstyle configs closer to the folder they govern? Having them all in a single directory seems like it makes it hard to reason about, even without taking Bazel into account. If someone wanted to update the checkstyle config for their code, how would they figure out which one to edit?

[nlou9]

That's a good point but in our case, every folder shares the same checkstyle.xml file, but some have different import-control.xml files. Managing them in a folder like this seems easier.

[fzakaria]

does this support allow_single_file so that it's just config.file ?
Maybe not according to bazelbuild/bazel#5355 (comment) but worth asking

[nlou9]

It doesn't seem to have allow_single_file attribute.

[alexeagle]

I think this is a workaround for a flaw in checkstyle, which I would expect to be able to resolve the right configuration for a given source file on its own. We should link to an issue there (creating it if needed) so it's clear that we're working around something, and in the event they fix it upstream we'll be able to remove our workaround.

[nlou9]

I think this is a workaround for a flaw in checkstyle, which I would expect to be able to resolve the right configuration for a given source file on its own. We should link to an issue there (creating it if needed) so it's clear that we're working around something, and in the event they fix it upstream we'll be able to remove our workaround.

open this feature request issue to checkstyle repo. checkstyle/checkstyle#16258

[alexeagle]

I'm taking another look at what apple_rules_lint did to permit users to configure linters per-directory, i.e. https://github.com/apple/apple_rules_lint/blob/main/api.md#get_lint_config allows you to set tags and https://github.com/apple/apple_rules_lint/blob/main/lint/private/package_lint_config.bzl#L14 allows the linter configuration to be selected in the same package being visited.

I'm curious, you must have considered just using apple_rules_lint instead of Aspect, since this checkstyle case is the example on their main README and I assume it motivated their design for selecting configs. Did you come to understand any tradeoffs between these approaches?