experimental: add support for running clippy#445
Open
jrobsonchase wants to merge 6 commits intonix-community:masterfrom
Open
experimental: add support for running clippy#445jrobsonchase wants to merge 6 commits intonix-community:masterfrom
jrobsonchase wants to merge 6 commits intonix-community:masterfrom
Conversation
Contributor
Author
|
Marking this ready for review while I work on tests and docs, since I think the core bits are ready for a look. |
184436e to
9340746
Compare
Contributor
Author
|
@domenkozar Got tests and docs added, so this should be ready for review! |
Contributor
Author
|
Now with dogfooding of |
ed94d04 to
595949f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #195
runClippysupportAdd support for running clippy against a crate.
The user-facing API is similar to how
runTestscurrently works. IfrunClippy = truein the call tobuildRustCrateWithFeatures, an additional derivation is built that runs clippy against the root crate and is included in the "checks" (formerly "linked") derivation for the crate.Overall approach
As suggested in #195, clippy is invoked by replacing
rustcwith (a script that runs)clippy-driver. This also has to do a bit of argument massaging to ensure that clippy is run without--cap-lints allowand to drop back to rustc if--versionis provided, since apparently some build scripts like to callrustc --versionand get mad if clippy responds.The clever bit of this is that rustc is only replaced with clippy-driver in the root crate derivation, so all of its dependencies should be built with the originally-provided rustc and thus shouldn't need to be re-built for the clippy check. This is similar to how cargo-clippy works with the
--no-depsargument.Design notes
rustcinput tobuildRustCrate.buildRustCrateWithFeatures? But then it feels "different" from howrustcandcargoare provided in thebuildRustCrateForPkgs, which doesn't feel great. I think that clippy is also tightly coupled to rustc, so if there's a version mismatch, users could have a bad time.runTests = true.I'm not entirely sure if this also implies that clippy is run against the crate's binary/library target as well, or if passingAssumptions tested - clippy does run against all targets, lib, bin, and integration test with the current setup.--testmakes clippy only run against the tests. Probably need to test some assumptions there.runClippyTestsflag so that clippy can be run both against the main crate and the test crate? This is only useful if users specifically don't want clippy to run against tests, which seems puntable until someone specifically needs it for some reason.Additional Changes
crateWithTestsinto a function that can support any number of arbitrary checks, not just the running of tests.runCrateTestsis now a standalone function that just constructs the "run the tests" derivation.buildRustCrateWithFeatures.buildTests = trueattribute down intobuiltRustCratesWithFeatureswhere it was previously overridden incrateWithTests. I believe this is semantically equivalent.TODO