Skip to content

Conversation

blorente
Copy link

A follow up to #3607 . This PR introduces a flag, --@rules_rust//rust/settings:experimental_clippy_aspect_traverse_deps=True, that tells the clippy aspect that it should traverse the deps edges of a target and collect its Clippy output and results.

See clippy_failure_tester.sh for effects before and after.

I'm still not sure this change is wanted, but clippy is close enough to a static analyzer like asan that I think it would make sense to give folks the option to toggle this behaviour. I'm happy to close this PR, I just wanted to give an option to folks who want to use clippy like this.

>&2 echo " Output file: ${NEW_WORKSPACE}/bazel-bin/test/clippy/${2%_clippy}.clippy.out"
>&2 echo " Run \"bazel build //test/clippy:${2}\" to see the output"
>&2 echo " Run \"bazel build //test/clippy:${2} ${@:3}\" to see the output"

Copy link
Author

Choose a reason for hiding this comment

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

These two are drive-by changes, but I realized we should have the flags if we want to have an easy repro.

@blorente blorente force-pushed the blorente/no_clippy_aspect branch 2 times, most recently from 64fbead to 0b56bbd Compare September 13, 2025 14:17
@blorente blorente force-pushed the blorente/no_clippy_aspect branch from 0b56bbd to 32d58e3 Compare September 15, 2025 09:02
@illicitonion
Copy link
Collaborator

This feels quite out of character from how I generally expect linters/analysers to work... My understanding is ASAN needs to be applied to deps because they actually need to be built in that mode for ASAN to work? Whereas it feels like it's really the job of the caller to decide which targets they want to run clippy for... Like, I'd expect folks to mostly be keeping HEAD clean, and either running clippy on ... or running it only on changed targets?

We do at least set noclippy and --cap-lints=allow on cargo-bazel-generated targets, so we don't need to worry about traversing into third-party code.

@UebelAndre do you have any thoughts here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants