Add canonical lint set #68
Conversation
content/wiki/canonical_lints.md
Outdated
| # This catches duplicated dependencies in the tree, which we don't have much control over | ||
| # We should use cargo deny for this, anyway | ||
| clippy.cargo = { level = "warn", priority = -1 } |
There was a problem hiding this comment.
I copied this from Peniko, but I have no idea what priority does, and I think the comment is a little too vague.
This part should probably be fixed up.
Apply lint set defined in linebender/linebender.github.io#68
|
I've applied the lint set to peniko in linebender/peniko#56 as an example. It seems to compile without issue, so I'd say this PR is ready to merge. |
content/wiki/canonical_lints.md
Outdated
| @@ -9,7 +9,14 @@ All Linebender projects should include the following set of lints in their `Carg | |||
| # This one may vary depending on the project. | |||
| rust.unsafe_code = "forbid" | |||
|
|
|||
| # These two should be added to projects with an MSRV of 1.81 or higher | |||
There was a problem hiding this comment.
Why special case these? The lints themselves already check the msrv to see if they should run
There was a problem hiding this comment.
Because actually addressing these lints requires features (expect annotations and allow reasons) that came out in 1.81.
There was a problem hiding this comment.
I'm not sure I follow. These lints only trigger if your package's MSRV is above 1.81 anyway, so why does it matter if you need features from Rust 1.81 to address them? If you're getting this error, surely you have those features?
There was a problem hiding this comment.
So the lints are basically no-ops if rust-version = "1.80", even if the user has a Rust 1.81 compiler?
content/wiki/canonical_lints.md
Outdated
| clippy.allow_attributes = "warn" | ||
| # Combined with allow_attributes, this is expected to trigger on module-wide or crate-wide `#![allow(...)]` attributes. | ||
| clippy.allow_attributes_without_reason = "warn" | ||
|
|
||
| # LINEBENDER LINT SET |
There was a problem hiding this comment.
Can we have versioning here? Keeping up with updates otherwise isn't ideal
content/wiki/canonical_lints.md
Outdated
| And in their `lib.rs`: | ||
|
|
||
| ```rust | ||
| // These two aren't included in Cargo.toml because they |
There was a problem hiding this comment.
I'd probably lean towards having them in a "versioned"/tagged block as well, with a similar link as in Cargo.toml.
I might also want to have them in a single warn, but I can see the stylistic reason not to do so.
Also, there are three lints here. I'd just remove the word "two" here to make that slightly less confused
content/wiki/canonical_lints.md
Outdated
|
|
||
| To keep this process simple, avoid modifying this list in individual projects. | ||
| If you want to add other per-project lints, add them besides the list. | ||
| If you want to remove a lint, `#![allow]` or `#![expect]` it at the project root. |
There was a problem hiding this comment.
How would this work for clippy::multiple_crate_versions. I don't think setting this in lib.rs does anything, and most of our projects are likely to fail this.
content/wiki/canonical_lints.md
Outdated
| If you think a new lint should be added to Linebender projects, add it to this file in alphabetical order, then copy-paste the list across projects. | ||
|
|
||
| To keep this process simple, avoid modifying this list in individual projects. | ||
| If you want to add other per-project lints, add them besides the list. |
There was a problem hiding this comment.
| If you want to add other per-project lints, add them besides the list. | |
| If you want to add other per-project lints, add them above the list. |
content/wiki/canonical_lints.md
Outdated
|
|
||
| To keep this process simple, avoid modifying this list in individual projects. | ||
| If you want to add other per-project lints, add them besides the list. | ||
| If you want to remove a lint, `#![allow]` or `#![expect]` it at the project root. |
There was a problem hiding this comment.
The "project root" as you use it here doesn't exist for multi-crate projects (which is any with even an example, or a workspace). Can we say "crate root" here instead please?
|
I removed |
| // shouldn't apply to examples and tests | ||
| #![warn(unused_crate_dependencies)] | ||
| #![warn(clippy::print_stdout, clippy::print_stderr)] | ||
| ``` |
There was a problem hiding this comment.
| ``` | |
| // END LINEBENDER LINT SET | |
| // (crate specific exceptions go here, as needed) | |
| ``` |
There was a problem hiding this comment.
The problem with the // END comment is that rustfmt really wants line comments to be next to the following items.
There was a problem hiding this comment.
// (crate specific exceptions go here, as needed)
I think that's too much detail.
Apply lint set defined in linebender/linebender.github.io#68 --------- Co-authored-by: Daniel McNab <[email protected]>
No description provided.