-
Notifications
You must be signed in to change notification settings - Fork 534
feat: Extract rust_clippy_action to allow other rulesets to run clippy #3660
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
feat: Extract rust_clippy_action to allow other rulesets to run clippy #3660
Conversation
7a9fb72 to
b1b6e9e
Compare
| success_marker (File): A file that will be written if the clippy succeeds. | ||
| cap_at_warnings (bool): If set, it will cap all reports as warnings, allowing the build to continue even with clippy failures | ||
| extra_clippy_flags (List[str]): A list of extra options to pass to clippy | ||
| error_format (str): Which error format to use. Must be acceptable by rustc: https://doc.rust-lang.org/beta/rustc/command-line-arguments.html#--error-format-control-how-errors-are-produced |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inside rules_rust, we can use the rustc.bzl file's utility functions to get this. I tried exposing them, but it felt like a leaky enough abstraction that this solution seemed cleaner.
593f792 to
f0d657a
Compare
f0d657a to
b76a1bb
Compare
# Conflicts: # rust/private/clippy.bzl
b76a1bb to
c253b1a
Compare
|
Could I get another approval for the formatting workflow? |
UebelAndre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small request but otherwise seems fine!
UebelAndre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
|
The merge queue failure seems to be unrelated: https://buildkite.com/bazel/rules-rust-rustlang/builds/15743/steps/canvas?sid=0199c539-cfda-4661-9b9c-ebe0adfcc847#0199c54a-fed2-41f9-b51e-22dafb24d35f/10-34 |
Extract the action to run clippy into an importable struct, so that other rulesets can use it.
This is an alternative fix for #3607. Since
rules_lintdoes allow transitive semantics for linting, we can use the clippy action there, and users who want clippy to be transitive can userules_lintto run it.The tests pass, and I've also tested it manually against a branch of
rules_lint.Reviewing instructions:
rust/defs.bzl. We expose two new functions: One to get the crate info, and one to create the clippy action itself.run_clippy. I've tried to select a reasonable boundary for the arguments, but I'm definitely open to opinions. For instance, we don't ever readctx.attrsinrun_clippy, because we don't want to force users to implement the same arguments and flags, but that may be wrong.run_clippy, to see if it preserves the semantics you'd expect, particularly around capturing output.run_clippy