new lint: struct_added_without_non_exhaustive

[Frank-III]

since #495 has been stale for a while.
related: #949

I don't know why the new lib.rs line: 25 would trigger the lint, I think the query should be correct?

#[non_exhaustive]
pub struct NewWithNonExhaustive {
    pub field1: i32,
    pub field2: String,
}

[obi1kenobi]

These lints might be quite noisy, and some of our users may find them annoying. I think we should consider them equivalent to clippy's pedantic level, which is opt-in and ships disabled by default. What do you think?

If that sounds right, switch the lint level to Allow from Warn. Then users can opt into this lint by configuring it from their Cargo.toml.

While lints like this are in general nice to have, they aren't necessarily the most impactful things we could add, for two reasons:

• clippy already has some similar functionality, although its workflow is different and arguably has more friction: https://rust-lang.github.io/rust-clippy/master/index.html#exhaustive_structs
• Having to make them opt-in to not annoy users means many users won't discover them. To demonstrate this: did you know about the clippy lint above? It's opt-in and disabled by default too.

I'm definitely not opposed to having this lint, and I'd happily merge it with a bit of polish. But there are also definitely more impactful lints that haven't been written yet. For example, #57 has a bunch of suggestions; I'll write up even more next week.

[obi1kenobi]

Since we don't have any prior "API evolution" advisory lints like this one (this is the first!), we haven't thought through what required_update should be here.

Right now, the only allowed options are Major and Minor. But neither seems quite right. Selecting Minor means that this lint won't run on major or minor releases, so new API additions like this will only be linted in patch releases. This doesn't seem quite right =/

[Frank-III]

hmm I see Minor is not right I believe, should we put it on-hold till we figure it out

[Frank-III]

These lints might be quite noisy, and some of our users may find them annoying. I think we should consider them equivalent to clippy's pedantic level, which is opt-in and ships disabled by default. What do you think?

If that sounds right, switch the lint level to Allow from Warn. Then users can opt into this lint by configuring it from their Cargo.toml.

Yeah make sense to me. I would do it.

Btw, if you're interested in writing more lints that will definitely be enabled by default, #57 has a bunch of suggestions and I'll write up more next week.

I would give them a try!

[obi1kenobi]

I updated my comment above with a bit more detail -- just making sure you see it since I think GitHub won't notify on edits :)

[obi1kenobi]

Yeah, possibly — depending on how much work you're open to doing here :) This is why that other PR stalled too.

…

On Fri, Jan 31, 2025, 11:23 AM frankwang ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/lints/struct_added_without_non_exhaustive.ron <#1091 (comment)> : > @@ -0,0 +1,57 @@ +SemverQuery( + id: "struct_added_without_non_exhaustive", + human_readable_name: "pub struct added without #[non_exhaustive]", + description: "A new public struct was added without #[non_exhaustive], making it harder to evolve in the future.", + required_update: Minor, hmm I see Minor is not right I believe, should we put it on-hold till we figure it out — Reply to this email directly, view it on GitHub <#1091 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAR5MSRKRUUBD7YLVJV3CR32NOPQXAVCNFSM6AAAAABWH4J4VWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKOBXGIYTQNBZGM> . You are receiving this because you commented.Message ID: ***@***.***>

[Frank-III]

Yeah, possibly — depending on how much work you're open to doing here :) This is why that other PR stalled too.
…
On Fri, Jan 31, 2025, 11:23 AM frankwang @.> wrote: @.* commented on this pull request. ------------------------------ In src/lints/struct_added_without_non_exhaustive.ron <#1091 (comment)> : > @@ -0,0 +1,57 @@ +SemverQuery( + id: "struct_added_without_non_exhaustive", + human_readable_name: "pub struct added without #[non_exhaustive]", + description: "A new public struct was added without #[non_exhaustive], making it harder to evolve in the future.", + required_update: Minor, hmm I see Minor is not right I believe, should we put it on-hold till we figure it out — Reply to this email directly, view it on GitHub <#1091 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR5MSRKRUUBD7YLVJV3CR32NOPQXAVCNFSM6AAAAABWH4J4VWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKOBXGIYTQNBZGM . You are receiving this because you commented.Message ID: @.***>

Okay, let me make it a draft then.
I'm still not very familiar with everything, but would definitely stick around for a while. The trustfall engine looks very promising! (Also the PR you wrote yesterday for the rust-doc adapter is 🤯)