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

rule: add union_now_doc_hidden #679

Merged
merged 4 commits into from
Mar 2, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions src/lints/union_now_doc_hidden.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
SemverQuery(
id: "union_now_doc_hidden",
human_readable_name: "pub union is now #[doc(hidden)]",
description: "A pub union 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 Union {
visibility_limit @filter(op: "=", value: ["$public"])

importable_path {
path @output @tag
public_api @filter(op: "=", value: ["$true"])
}
}
}
}
current {
item {
... on Union {
visibility_limit @filter(op: "=", value: ["$public"])
union_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 union is now #[doc(hidden)], removing it from the crate's public API.",
per_result_error_template: Some("union {{union_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 @@ -531,4 +531,5 @@ add_lints!(
repr_packed_added,
repr_packed_removed,
exported_function_changed_abi,
union_now_doc_hidden,
);
7 changes: 7 additions & 0 deletions test_crates/union_now_doc_hidden/new/cargo.toml
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've never seen cargo.toml as the filename for this, I've only ever seen Cargo.toml (note the capitalization difference).

I'm curious how you generated this file — did you use the scripts for creating new lints and test files? Are you using a filesystem that lowercases everything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason Cargo.toml is not generated after running the script but all other files do. My bad I named it wrongly.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh interesting, that sounds like a bug in the script that we should fix. Which script did you have this experience with?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running ./scripts/regenerate_test_rustdocs.sh also gives the error,
cargo-semver-checks/scripts/regenerate_test_rustdocs.sh: line 23: shopt: globstar: invalid shell option name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After commenting out line no 23 it works fine.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah can you check which shell and which version of that shell you're using? globstar was introduced in bash 4 (released 15 years ago!) but some systems still only have bash 3 by default =/

Our scripts should probably check the shell version and exit with an error if it's too old. If you're open to writing and testing that kind of PR on your machine, I'd be thrilled to merge it — I'm not able to because all my computers are updated to use bash 5.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh interesting, that sounds like a bug in the script that we should fix. Which script did you have this experience with?

make_new_lint.sh

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #680 for this, feel free to tackle it if you find it interesting! Our tooling gets better one small improvement at a time — it really adds up over the course of weeks and months.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "union_now_doc_hidden"
version = "0.1.0"
edition = "2021"

[dependencies]
5 changes: 5 additions & 0 deletions test_crates/union_now_doc_hidden/new/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#[doc(hidden)]
pub union MyUnion {
f1: u32,
f2: f32,
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: could you ensure all files have trailing newline characters? You should be able to adjust your editor to add them automatically when you save.

7 changes: 7 additions & 0 deletions test_crates/union_now_doc_hidden/old/cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "union_now_doc_hidden"
version = "0.1.0"
edition = "2021"

[dependencies]
4 changes: 4 additions & 0 deletions test_crates/union_now_doc_hidden/old/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
pub union MyUnion {
f1: u32,
f2: f32,
}
24 changes: 24 additions & 0 deletions test_outputs/union_now_doc_hidden.output.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"./test_crates/type_hidden_from_public_api/": [
{
"path": List([
String("type_hidden_from_public_api"),
String("ExampleUnion"),
]),
"span_begin_line": Uint64(19),
"span_filename": String("src/lib.rs"),
"union_name": String("ExampleUnion"),
},
],
"./test_crates/union_now_doc_hidden/": [
{
"path": List([
String("union_now_doc_hidden"),
String("MyUnion"),
]),
"span_begin_line": Uint64(2),
"span_filename": String("src/lib.rs"),
"union_name": String("MyUnion"),
},
],
}
Loading