Skip to content
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

Merged
merged 15 commits into from
Nov 28, 2023
Merged
48 changes: 48 additions & 0 deletions src/lints/struct_now_doc_hidden.ron
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}}"),
)
1 change: 1 addition & 0 deletions src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ add_lints!(
struct_marked_non_exhaustive,
struct_missing,
struct_must_use_added,
struct_now_doc_hidden,
struct_pub_field_missing,
struct_repr_c_removed,
struct_repr_transparent_removed,
Expand Down
7 changes: 7 additions & 0 deletions test_crates/struct_now_doc_hidden/new/Cargo.toml
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]
50 changes: 50 additions & 0 deletions test_crates/struct_now_doc_hidden/new/src/lib.rs
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)]
Copy link
Owner

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.html

I'd probably pick one of the other existing doc commands instead of using one that doesn't exist.

Copy link
Contributor Author

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.

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;
7 changes: 7 additions & 0 deletions test_crates/struct_now_doc_hidden/old/Cargo.toml
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]
29 changes: 29 additions & 0 deletions test_crates/struct_now_doc_hidden/old/src/lib.rs
Copy link
Owner

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;
13 changes: 13 additions & 0 deletions test_outputs/struct_missing.output.ron
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,19 @@
"visibility_limit": String("public"),
},
],
"./test_crates/struct_now_doc_hidden/": [
{
"name": String("PublicStructThatGoesPrivate"),
"path": List([
String("struct_now_doc_hidden"),
String("PublicStructThatGoesPrivate"),
]),
"span_begin_line": Uint64(29),
"span_filename": String("src/lib.rs"),
"struct_type": String("unit"),
"visibility_limit": String("public"),
},
],
"./test_crates/struct_pub_field_missing/": [
{
"name": String("StructRemoved"),
Expand Down
63 changes: 63 additions & 0 deletions test_outputs/struct_now_doc_hidden.output.ron
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"),
},
],
}
Loading