implement struct_now_doc_hidden lint #587
Conversation
u9g
force-pushed
the
struct-now-hidden-lint
branch
from
b7d1ec5 to
16b9a81
Compare
u9g
force-pushed
the
struct-now-hidden-lint
branch
from
16b9a81 to
4810db7
Compare
There was a problem hiding this comment.
As always, good work! Will be happy to merge it.
I made a few minor suggestions and pointed out a few opportunities for improvement, if you're interested in making the code even better. Better tests don't seem that valuable when you're writing them, but they pay off massively by speeding up future development.
There was a problem hiding this comment.
These test cases are fine, but please do try to think of some more edge cases to test for. Simple lints like this are great for practicing those skills!
One trick to help you think of test cases that is called "mutation testing." Take the correct lint you've implemented, and intentionally introduce a bug into it: for example, delete a filter clause, change a filter operator, or add an unnecessary filter or edge traversal somewhere. Then run the tests and see if they catch the bug.
You know you have good test coverage when no matter what bug you introduce, the tests always catch it.
After some practice, you'll be able to run this process "in your head" without actually modifying the lint, and you'll be able to directly write sufficient test cases to catch most reasonable bugs. At the moment, I think there are several plausible ways to accidentally mis-implement this lint that I don't think our test suite as a whole will catch.
There was a problem hiding this comment.
Really good point, I went ahead and tried to get rid of some lines in the lint which actually ended up being optional entirely :)
In terms of tests, I added a bunch more tests that should make it a lot harder to mis-implement this rule.
Co-authored-by: Predrag Gruevski <[email protected]>
Co-authored-by: Predrag Gruevski <[email protected]>
Co-authored-by: Predrag Gruevski <[email protected]>
u9g
force-pushed
the
struct-now-hidden-lint
branch
from
541896c to
ed0424e
Compare
| } | ||
| } | ||
|
|
||
| #[doc(hide)] // shouldn't flag, it should be #[doc(hidden)] not #[doc(hide)] |
There was a problem hiding this comment.
I don't think this is a legal attribute, and it might break in the future. Here's the list of things the doc attribute supports and we probably should stick to it: https://doc.rust-lang.org/rustdoc/write-documentation/the-doc-attribute.html
I'd probably pick one of the other existing doc commands instead of using one that doesn't exist.
There was a problem hiding this comment.
good point, I replaced it with a few valid doc attribute values.
Implements a lint to catch structs that are public and have just had
#[doc(hidden)]added to them.checks the first checkmark on #578 (comment)