-
-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
implement struct_now_doc_hidden lint #587
Changes from 13 commits
4810db7
56a3791
a8d279c
35897c8
b3c3e45
ded09b0
17ca637
88ced54
38115fd
21f829d
bc0de14
f605781
ed0424e
a333bae
22798c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
SemverQuery( | ||
id: "struct_now_doc_hidden", | ||
human_readable_name: "pub struct is now #[doc(hidden)]", | ||
description: "A pub struct is now marked #[doc(hidden)] and is thus no longer part of the public API.", | ||
required_update: Major, | ||
reference_link: Some("https://doc.rust-lang.org/rustdoc/write-documentation/the-doc-attribute.html#hidden"), | ||
query: r#" | ||
{ | ||
CrateDiff { | ||
baseline { | ||
item { | ||
... on Struct { | ||
visibility_limit @filter(op: "=", value: ["$public"]) | ||
|
||
importable_path { | ||
path @output @tag | ||
public_api @filter(op: "=", value: ["$true"]) | ||
} | ||
} | ||
} | ||
} | ||
current { | ||
item { | ||
... on Struct { | ||
visibility_limit @filter(op: "=", value: ["$public"]) | ||
struct_name: name @output | ||
|
||
importable_path { | ||
path @filter(op: "=", value: ["%path"]) | ||
public_api @filter(op: "!=", value: ["$true"]) | ||
} | ||
|
||
span_: span @optional { | ||
filename @output | ||
begin_line @output | ||
} | ||
} | ||
} | ||
} | ||
} | ||
}"#, | ||
arguments: { | ||
"public": "public", | ||
"true": true, | ||
}, | ||
error_message: "A pub struct is now #[doc(hidden)], removing it from the crate's public API.", | ||
per_result_error_template: Some("struct {{struct_name}} in file {{span_filename}}:{{span_begin_line}}"), | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
[package] | ||
publish = false | ||
name = "struct_now_doc_hidden" | ||
version = "0.1.0" | ||
edition = "2021" | ||
|
||
[dependencies] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
mod MyNonPublicMod { | ||
// despite adding #[doc(hidden)], this struct is in a | ||
// private mod, so it isn't part of the crate's public | ||
// api | ||
#[doc(hidden)] | ||
pub struct MyStruct; | ||
} | ||
|
||
pub mod MyPublicMod { | ||
// added #[doc(hidden)], however this struct is in a | ||
// public mod, so it is part of the crate's public api | ||
#[doc(hidden)] | ||
pub struct MyStruct; | ||
} | ||
|
||
mod MyNestedNonPublicMod { | ||
pub mod PublicInnerStruct { | ||
// despite adding #[doc(hidden)], this struct is in a | ||
// private outer mod, so it isn't part of the crate's public | ||
// api | ||
#[doc(hidden)] | ||
pub struct MyStruct; | ||
} | ||
} | ||
|
||
pub mod MyNestedPublicMod { | ||
pub mod PublicInnerStruct { | ||
// added #[doc(hidden)], however this struct is in a | ||
// public mod, so it is part of the crate's public api | ||
#[doc(hidden)] | ||
pub struct MyStruct; | ||
} | ||
} | ||
|
||
#[doc(hide)] // shouldn't flag, it should be #[doc(hidden)] not #[doc(hide)] | ||
pub struct MispelledDocHidden; | ||
|
||
#[doc(hidden)] // should flag, this is the simplest case of adding #[doc(hidden)] to a pub struct. | ||
pub struct Example; | ||
|
||
pub struct PublicStructHiddenField { | ||
// shouldn't flag `struct_now_doc_hidden` rule | ||
// as this is a field that's hidden, | ||
// not the entire struct | ||
#[doc(hidden)] | ||
pub my_field: i8, | ||
} | ||
|
||
#[doc(hidden)] | ||
struct PublicStructThatGoesPrivate; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
[package] | ||
publish = false | ||
name = "struct_now_doc_hidden" | ||
version = "0.1.0" | ||
edition = "2021" | ||
|
||
[dependencies] |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
mod MyNonPublicMod { | ||
pub struct MyStruct; | ||
} | ||
|
||
pub mod MyPublicMod { | ||
pub struct MyStruct; | ||
} | ||
|
||
pub struct MispelledDocHidden; | ||
|
||
pub struct Example; | ||
|
||
pub struct PublicStructHiddenField { | ||
pub my_field: i8, | ||
} | ||
|
||
mod MyNestedNonPublicMod { | ||
pub mod PublicInnerStruct { | ||
pub struct MyStruct; | ||
} | ||
} | ||
|
||
pub mod MyNestedPublicMod { | ||
pub mod PublicInnerStruct { | ||
pub struct MyStruct; | ||
} | ||
} | ||
|
||
pub struct PublicStructThatGoesPrivate; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
{ | ||
"./test_crates/struct_now_doc_hidden/": [ | ||
{ | ||
"path": List([ | ||
String("struct_now_doc_hidden"), | ||
String("MyPublicMod"), | ||
String("MyStruct"), | ||
]), | ||
"span_begin_line": Uint64(13), | ||
"span_filename": String("src/lib.rs"), | ||
"struct_name": String("MyStruct"), | ||
}, | ||
{ | ||
"path": List([ | ||
String("struct_now_doc_hidden"), | ||
String("MyNestedPublicMod"), | ||
String("PublicInnerStruct"), | ||
String("MyStruct"), | ||
]), | ||
"span_begin_line": Uint64(31), | ||
"span_filename": String("src/lib.rs"), | ||
"struct_name": String("MyStruct"), | ||
}, | ||
{ | ||
"path": List([ | ||
String("struct_now_doc_hidden"), | ||
String("Example"), | ||
]), | ||
"span_begin_line": Uint64(39), | ||
"span_filename": String("src/lib.rs"), | ||
"struct_name": String("Example"), | ||
}, | ||
], | ||
"./test_crates/type_hidden_from_public_api/": [ | ||
{ | ||
"path": List([ | ||
String("type_hidden_from_public_api"), | ||
String("ExamplePlainStruct"), | ||
]), | ||
"span_begin_line": Uint64(2), | ||
"span_filename": String("src/lib.rs"), | ||
"struct_name": String("ExamplePlainStruct"), | ||
}, | ||
{ | ||
"path": List([ | ||
String("type_hidden_from_public_api"), | ||
String("ExampleTupleStruct"), | ||
]), | ||
"span_begin_line": Uint64(7), | ||
"span_filename": String("src/lib.rs"), | ||
"struct_name": String("ExampleTupleStruct"), | ||
}, | ||
{ | ||
"path": List([ | ||
String("type_hidden_from_public_api"), | ||
String("ExampleUnitStruct"), | ||
]), | ||
"span_begin_line": Uint64(10), | ||
"span_filename": String("src/lib.rs"), | ||
"struct_name": String("ExampleUnitStruct"), | ||
}, | ||
], | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.htmlI'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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, I replaced it with a few valid doc attribute values.