Implement new_struct_added lint #495
Conversation
u9g
changed the title
There was a problem hiding this comment.
Sorry, like I mentioned over Discord, this is blocked on having a more granular way to control the lint level (allow / warn / error / forbid): #58
Otherwise we'll end up having a bunch of upset users when we ship this.
| // TODO: Change the reference link to point to the cargo semver reference | ||
| // once it has a section on attribute #[must_use]. | ||
| reference_link: Some("https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute"), |
There was a problem hiding this comment.
This seems like copy-pasta, change it to None for now.
| item { | ||
| ... on Struct { | ||
| name @tag(name: "new_crate_name") @output(name: "new_struct_added_name") | ||
| visibility_limit @filter(op: "=", value: ["$public"]) | ||
|
|
||
| span_: span { | ||
| filename @output | ||
| begin_line @output | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| baseline { | ||
| item @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) { | ||
| ... on Struct { | ||
| name @filter(op: "=", value: ["%new_crate_name"]) | ||
| visibility_limit @filter(op: "=", value: ["$public"]) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This will actually also fire on structs that are just moved, renamed, and re-exported in their original place under their new name, which is a false-positive unless there's a new name in the public API.
You'll need to use importable_path to avoid that.
| "span_filename": String("src/structs.rs"), | ||
| }, | ||
| ], | ||
| } |
There was a problem hiding this comment.
This is lacking a trailing newline, please configure your editor to add them automatically.
| id: "new_struct_added", | ||
| human_readable_name: "new struct added", | ||
| description: "A new public struct has been exposed.", | ||
| required_update: Minor, |
There was a problem hiding this comment.
| required_update: Minor, | |
| required_update: Minor, | |
| lint_level: Allow, |
or
| required_update: Minor, | |
| required_update: Minor, | |
| lint_level: Warn, |
With the new version of cargo-semver-checks, it is now possible to have allow-by-default and warn-by-default checks.
There was a problem hiding this comment.
I'd probably make this Allow by default. I'm sure many users will want to opt-in, but it might be a bit noisy to enable for everyone.
|
Hey @u9g, just checking if you might still be interested in driving this PR forward to merge, now that No worries if you're too busy with other stuff, and no pressure! It's totally fine if you don't have the time or interest in driving this forward yourself right now — I'm sure we can get it merged either way. I just wanted to check in and get your thoughts on it. |
No description provided.