Explain flags missing in cargo check in --help

[nfejzic]

This commit closes #7389. As stated in the issue, cargo clippy --help
provides explanation for some flags and states that the rest are same
as in cargo check --help, even though some clippy specific flags
exist.

This commit extends the cargo clippy --help with two additional flags,

• cargo clippy --fix
• cargo clippy --no-deps

If there are more flags which are not present in cargo check --help
please bring these to my attention, I will include these aswell.
For now, I noticed only the two flags mentioned above.

changelog: cargo clippy --help now explains additional flags missing in cargo check --help.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @camsteffen (or someone else) soon.

Please see the contribution instructions for more information.

[camsteffen]

To be honest I don't fully understand the implications of --no-deps. Is it somehow specific to workspaces? Does it exclude path dependencies or all dependencies? Is it the same concept as cargo doc --no-deps? Why does it come after --, and how should we document that? Finally, I think we should indicate that --fix implies --no-deps.

Sorry to leave more questions than answers. @rust-lang/clippy hopefully someone knows more than me.

[flip1995]

--no-deps exists, because of --fix and its interaction with workspaces. The problem was, that Clippy lints all packages in a workspace at once by default. But IIRC this didn't play well with --fix, since --fix can only fix the primary package and would then error out, if there are other lint warnings in other packages (or something like that, I can't really remember anymore, but it was definitely because of --fix).

--no-deps only applies to packages in a workspace. When this flag is passed only the primary package is linted. I.e. if you cd into the dir of a sub package of the workspace and run cargo clippy -- --no-deps only lints for that sub package are emitted.

It comes after the -- since that's how cargo works. All tool specific flags (aka non-cargo flags) come after the --.

[flip1995]

Why does it come after --, and how should we document that?

Maybe add an additional section to the help message

Additional options after `--`:

    --fix ...
    --no-deps ...

[Manishearth]

I like @flip1995's proposed approach.

Perhaps we should also just support --fix and --no-deps directly and pass them down ourselves? (and document them in flags)

[camsteffen]

It comes after the -- since that's how cargo works. All tool specific flags (aka non-cargo flags) come after the --.

But isn't --fix tool-specific and yet it comes before --? It seems inconsistent. I thought the -- is to separate rustc args?

Perhaps we should also just support --fix and --no-deps directly and pass them down ourselves? (and document them in flags)

Yeah that makes sense to me.

[nfejzic]

It comes after the -- since that's how cargo works. All tool specific flags (aka non-cargo flags) come after the --.

But isn't --fix tool-specific and yet it comes before --? It seems inconsistent. I thought the -- is to separate rustc args?

I wanted to ask the same thing, this doesn't seem very consistent. I feel this would be much better if either all flags must come after --, or the -- is not needed at all.

[flip1995]

Oh, for some reason I was under the impression, that the --fix argument for Clippy is handled by cargo. But that isn't the case. So yeah, we can just handle the --no-deps argument the same as --fix. To do this, we have to adapt the code here:

rust-clippy/src/main.rs

Lines 75 to 80 in fd2b43d

	for arg in old_args.by_ref() {
	match arg.as_str() {
	"--fix" => {
	cargo_subcommand = "fix";
	continue;
	},

And our tests here:

rust-clippy/tests/dogfood.rs

Lines 71 to 72 in fd2b43d

	// `path_dep` is a path dependency of `subcrate` that would trigger a denied lint.
	// Make sure that with the `--no-deps` argument Clippy does not run on `path_dep`.

@nfejzic are you comfortable making those changes? Otherwise I can do that.

[nfejzic]

@nfejzic are you comfortable making those changes? Otherwise I can do that.

I pushed a commit which should now handle the --no-deps with or without the --. As far as I can tell, whether it should be accepted after the -- or not is handled somewhere else, so I didn't change that. This means it works both ways now:
cargo clippy -- --no-deps should be equivalent to cargo clippy --no-deps.

For the test, I just changed the order in the existing test, so it now tests the cargo clippy --no-deps case.
Should I write another test for the cargo clippy -- --no-deps so that both cases are tested?

[flip1995]

For the test, I just changed the order in the existing test, so it now tests the cargo clippy --no-deps case.
Should I write another test for the cargo clippy -- --no-deps so that both cases are tested?

I don't think this is necessary.

What happens with cargo clippy --no-deps -- --no-deps or cargo clippy --fix -- --no-deps?

[nfejzic]

What happens with cargo clippy --no-deps -- --no-deps or cargo clippy --fix -- --no-deps?

I just tested with the following cases:

1. cargo clippy --no-deps -- --no-deps
2. cargo clippy --fix -- --no-deps
3. cargo clippy --no-deps --fix
4. cargo clippy --no-deps --fix -- --no-desp

All of them finish with the following output:

    Compiling clippy v0.1.55 (/Users/nfejzic/Develop/oss/rust-clippy)
    Finished dev [unoptimized + debuginfo] target(s) in 2.74s

[flip1995]

LGTM, but would like a second pair of eyes on those changes since this code is quite finicky.

[Manishearth]

@bors r+

[bors]

📌 Commit f7af8bf has been approved by Manishearth

[bors]

⌛ Testing commit f7af8bf with merge ac0fd99...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Manishearth
Pushing ac0fd99 to master...

[camsteffen]

Are we going to accept -- --no-deps indefinitely? We should at least add a comment explaining why.