Add lint pub_static_mut_now_immutable #768
Conversation
| SemverQuery( | ||
| id: "pub_static_mut_now_immutable", | ||
| human_readable_name: "pub static mut is now immutable", | ||
| description: "A mutable static became immutable and thus cannot be imported", |
There was a problem hiding this comment.
Is imported here intentional? I admit I haven't looked too closely at what the exact breakage is when a mutable static becomes immutable. I just want to make sure that it is indeed the use import statement that will trigger a compilation error if a static becomes immutable, since that seems a bit surprising to me.
There was a problem hiding this comment.
I am not sure either. I just read the discussion on the issue and figured the user would not be able to use the static since they depended on it being mutable.
There was a problem hiding this comment.
Ah okay. Let's step back for a sec, since I think the bigger picture is more important right now.
cargo-semver-checks exists as a tool because the rules for SemVer in Rust are very complex. We humans have a hard time learning them and applying them consistently, so we delegate some of that responsibility to this tool: it can be more consistent, and it also holds the distilled expertise of dozens of humans. Each person there dug into some possible SemVer problem, understood it in detail, figured out how to prevent it from ever happening again, and then wrote a lint that both catches and describes the problem to the end user.
If some of the distilled expertise is wrong, that violates users' trust. Why should they delegate trust and responsibility to a tool that violates their trust? They will stop using it.
This is an existential threat to cargo-semver-checks. This is not exaggeration. Several prior attempts at a SemVer linter for Rust have been abandoned due to reporting incorrect information, such as false-positive lints. We may very well increment that number by one more if we aren't careful.
The real value you are contributing in PRs that add lints is not the lint queries, the test cases, etc. It is your expertise on that particular facet of the SemVer rules. It happens to be distilled into a lint file with its query and error message etc. but that's just the packaging.
The value is realized when users can say "ah, I would have made a terrible mistake here, but cargo-semver-checks caught it and explained it to me in a way that I understand the problem and its consequences." This doesn't happen if the lint query is wrong. It also doesn't happen if the error message or description. Or if any other metadata in the lint is wrong, like if we mis-categorized a lint as Minor instead of Major.
So please dig into the problem space, win that expertise, and contribute it as part of the lint as a whole. Otherwise, our users don't get any value — in fact, if we teach them something plainly incorrect, they may even be worse off overall!
There was a problem hiding this comment.
Coming back to this specific instance, the user would not be able to use the static in the same way, since it isn't mutable. But I wouldn't expect the manner of "use" they can't do is "import it."
Obtaining the necessary expertise here is figuring out what manner of "use" is denied to the user that depended on a mutable static that became no longer mutable. You'll know you've gained that expertise when you can write a concrete snippet of Rust that compiles with a mutable static, but has a compile error when the static is no longer mutable.
There was a problem hiding this comment.
I looked into the compile error and it seems compile error is only caused by assigning to the immutable static. So, I updated the error message and description accordingly.
P.S: I was looking through issue #767 and wanted to ask if is there any documentation on lint optimization I should go through for future lints? Its nothing urgent and I doubt I would be able to go through them this week.
There was a problem hiding this comment.
Great!
For lint optimization, there's not much one can do within the lints themselves. There's room for minor improvements like "order edges such that filtering happens as early as possible" but that won't make a big difference for cases like #767. Don't worry, you didn't cause that issue and neither did anyone else writing lints. It'll get resolved, and it won't require any changes to the lints themselves to do so.
The cause of that issue is a missing optimization in the Trustfall query engine adapter for rustdoc. If you are curious, you can read more about how I resolved a previous instance of the same issue here: https://predr.ag/blog/speeding-up-rust-semver-checking-by-over-2000x/
| "public": "public", | ||
| "true": true, | ||
| }, | ||
| error_message: "A mutable static is now immutable", |
There was a problem hiding this comment.
We may want to expand this error message a bit.
If you just ran cargo-semver-checks and it reported this problem, what information would you like to see printed as part of the error log? You'd like to know what happened, why it's a problem, and where to look for it -- in as few characters as possible. This is tough, but it's a skill like any other and practicing it will help.
Lint for when a mutable static becomes immutable static
Issue mentioned in #366 (comment)
Resolves #366.