search

rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.59k stars 2.39k forks source link

Allow setting `lints.workspace` as `false` #13821

Open Kriskras99 opened 4 months ago

Kriskras99 commented 4 months ago

Problem

I expect to be able to set lints.workspace to false to explicitly opt out of the workspace lints. Currently this is not allowed.

It would also be helpful for a potential lint like https://github.com/rust-lang/rust-clippy/issues/11933.

Proposed Solution

Treat lints.workspace = false the same as not setting it at all.

Notes

No response

heisen-li commented 4 months ago

@rustbot label +A-lints-table

epage commented 4 months ago

Posted on rust-lang/rust-clippy#11933 with a framing that might not need this.

If we went down this route, we'd need to decide if we should mirror this in all of the rest of workspace inheritance.

Kriskras99 commented 4 months ago

I have actually tried setting workspace to false, before knowing about the lint idea. It just feels natural to disable the workspace lints by setting it to false. I thought it would be as simple as just allowing it and basically ignoring it, but if it also impacts other workspace stuff, then it would need some more consideration.

epage commented 4 months ago

The following statements make me suspect its best if we don't support workspace = false for now as it seems like people could do it and get the wrong impression as to what it does

From https://github.com/rust-lang/rust-clippy/issues/11933#issuecomment-1877762065

Oh my gosh, I thought/hoped that this is implicitly on without having to opt in for every workspace member crate. 😱

From https://github.com/rust-lang/cargo/issues/13821#issuecomment-2082997392

I have actually tried setting workspace to false, before knowing about the lint idea. It just feels natural to disable the workspace lints by setting it to false.

epage commented 4 months ago

I thought it would be as simple as just allowing it and basically ignoring it, but if it also impacts other workspace stuff, then it would need some more consideration.

We could allow it and it would ignore it. However, all of workspace inheritance works the same way and has the same behavior of disallowing workspace = false so we'd at least need to evaluate how important consistency is and what it would be like to support workspace = false in the other places.