Added lint public_module_level_constant_now_doc_hidden #690
Conversation
There was a problem hiding this comment.
The lint query looks good! Some of the surrounding metadata could use another polishing pass, and I made a few specific suggestions for that. Looking forward to merging this soon!
As a small tangent: I highly recommend reading your PR in the GitHub PR review interface before making the PR ready for review. A lot of the suggestions I made here are much easier to see in the PR review interface than in your own editor, so I bet you would have spotted them yourself if you did a self-review like that. In turn, that would have saved us both some time because it's always faster to merge a PR than to have a back-and-forth around it.
Anyway — excited to merge this soon and include it in the new release!
There was a problem hiding this comment.
Please make sure the name of the file matches the value of the id field in the lint's metadata.
| human_readable_name: "a public module level constant is marked #[doc(hidden)]", | ||
| description: "a public module level constant is now marked #[doc(hidden)] and is thus not a part of public API", |
There was a problem hiding this comment.
Please try to match the style of the other lints for these metadata fields.
For example, compare the contents of the pub_module_level_const_missing lint for these fields:
human_readable_name: "pub module-level const is missing",
description: "A pub const is missing, renamed, or changed to static.",
| error_message: "A public module level constant is now marked #[doc(hidden)], removing it from the crate's public API.", | ||
| per_result_error_template: Some("constant {{constant_name}} in file {{span_filename}}:{{span_begin_line}}"), |
There was a problem hiding this comment.
Similarly, to match the output and style of pub_module_level_const_missing lint:
| error_message: "A public module level constant is now marked #[doc(hidden)], removing it from the crate's public API.", | |
| per_result_error_template: Some("constant {{constant_name}} in file {{span_filename}}:{{span_begin_line}}"), | |
| error_message: "A public const is now marked #[doc(hidden)], removing it from the crate's public API.", | |
| per_result_error_template: Some("{{constant_name}} in file {{span_filename}}:{{span_begin_line}}"), |
| @@ -0,0 +1,7 @@ | |||
| [package] | |||
| publish = false | |||
| name = "public_module_level_constant_now_doc_hidden" | |||
There was a problem hiding this comment.
Please make sure to rename all the test crates directories and names to match the lint's id and filename. This seems like you might have accidentally created the lint with the name public_module_level_constant_now_doc_hidden and then only partially renamed it and its metadata.
There was a problem hiding this comment.
Nice! I'm going to merge this.
It seems that this lint name is inconsistent with the other lints that target pub constants / pub statics (pub_module_level_const_missing / pub_static_missing), and this consistency is important since it'll aid users when they need to refer to lints by name. For example, when opting into or out of lints via package.metadata configuration like in #58.
I'll open a PR to rename the lint and its test outputs to pub_module_level_const_now_doc_hidden.
|
Renamed in #703. |
Lint for when a public module level constant is marked as #[doc(hidden)]
Issue mention in #576
Caught a lint in test crate for the lint function_now_doc_hidden, which is mentioned in the test output for this lint