From 533cb3a982e508ea5729ce45ce2cca13bf63d5e9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 9 Sep 2023 21:48:07 -0400 Subject: [PATCH 1/3] New "module_missing" lint to detect missing, renamed, or non-visible modules (#534) * Update lockfile to latest trustfall-rustdoc-adapter(s) * Add a new "module_missing" lint Resolves #482. * module_missing: Add expected positives for trait_missing* * fixup! Add a new "module_missing" lint Tweak error message. * fixup! Add a new "module_missing" lint Adjust formatting on test_crates/module_missing * Update test_crates/module_missing/new/src/lib.rs --------- Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> --- Cargo.lock | 24 +++++------ src/lints/module_missing.ron | 46 +++++++++++++++++++++ src/query.rs | 1 + test_crates/module_missing/new/Cargo.toml | 7 ++++ test_crates/module_missing/new/src/lib.rs | 24 +++++++++++ test_crates/module_missing/old/Cargo.toml | 7 ++++ test_crates/module_missing/old/src/lib.rs | 13 ++++++ test_outputs/module_missing.output.ron | 49 +++++++++++++++++++++++ 8 files changed, 159 insertions(+), 12 deletions(-) create mode 100644 src/lints/module_missing.ron create mode 100644 test_crates/module_missing/new/Cargo.toml create mode 100644 test_crates/module_missing/new/src/lib.rs create mode 100644 test_crates/module_missing/old/Cargo.toml create mode 100644 test_crates/module_missing/old/src/lib.rs create mode 100644 test_outputs/module_missing.output.ron diff --git a/Cargo.lock b/Cargo.lock index 7e18dc05..1b019f70 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3176,9 +3176,9 @@ dependencies = [ [[package]] name = "trustfall-rustdoc-adapter" -version = "23.6.0" +version = "23.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "99f8e47989facdc13cd06bea85e0c1a9d87a6f17a05ab9d1d9d8de8add1ba8e5" +checksum = "ece63d2d0d19a07724d43f03dd9daea592ce3faed3ee12d0354c681c23a9f922" dependencies = [ "rustdoc-types 0.19.0", "trustfall", @@ -3186,9 +3186,9 @@ dependencies = [ [[package]] name = "trustfall-rustdoc-adapter" -version = "24.5.0" +version = "24.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eb2158e9583a0487c2a77e372636819bd03fee25cedb048e580b788ae054cf5f" +checksum = "8900ed1075c0fddef370e47b73920368b33562e0602019c37476f8abf0ff7a09" dependencies = [ "rustdoc-types 0.20.0", "trustfall", @@ -3196,9 +3196,9 @@ dependencies = [ [[package]] name = "trustfall-rustdoc-adapter" -version = "26.2.0" +version = "26.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2c3b046aa70f7199223e1ef4dc46b2d904ce42f4a3fb726ee712c98ea44e782e" +checksum = "f208dd20ab7220f9d54aee3c6082377376d224ae3b4d3d0b980c02f2070786e2" dependencies = [ "rustdoc-types 0.22.0", "trustfall", @@ -3206,9 +3206,9 @@ dependencies = [ [[package]] name = "trustfall-rustdoc-adapter" -version = "27.0.0" +version = "27.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9dd0afa66d619e0b540eac19b6cc6892fb4f5dd6a02e9c5171d3433e6f29cbb3" +checksum = "437e8fadfb04a4b708e96552a3bb1232861a81df5d1a14ae488c6da62764496f" dependencies = [ "rustdoc-types 0.23.0", "trustfall", @@ -3253,10 +3253,10 @@ dependencies = [ "serde", "serde_json", "trustfall", - "trustfall-rustdoc-adapter 23.6.0", - "trustfall-rustdoc-adapter 24.5.0", - "trustfall-rustdoc-adapter 26.2.0", - "trustfall-rustdoc-adapter 27.0.0", + "trustfall-rustdoc-adapter 23.6.1", + "trustfall-rustdoc-adapter 24.5.1", + "trustfall-rustdoc-adapter 26.2.1", + "trustfall-rustdoc-adapter 27.0.1", ] [[package]] diff --git a/src/lints/module_missing.ron b/src/lints/module_missing.ron new file mode 100644 index 00000000..19f44e55 --- /dev/null +++ b/src/lints/module_missing.ron @@ -0,0 +1,46 @@ +SemverQuery( + id: "module_missing", + human_readable_name: "pub module removed or renamed", + description: "A module can no longer be imported by its prior path", + required_update: Major, + reference_link: Some("https://doc.rust-lang.org/cargo/reference/semver.html#item-remove"), + query: r#" + { + CrateDiff { + baseline { + item { + ... on Module { + visibility_limit @filter(op: "=", value: ["$public"]) @output + name @output + + importable_path { + path @output @tag + } + + span_: span @optional { + filename @output + begin_line @output + } + } + } + } + current @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) { + item { + ... on Module { + visibility_limit @filter(op: "=", value: ["$public"]) + + importable_path { + path @filter(op: "=", value: ["%path"]) + } + } + } + } + } + }"#, + arguments: { + "public": "public", + "zero": 0, + }, + error_message: "A publicly-visible module cannot be imported by its prior path. A `pub use` may have been removed, or the module may have been renamed, removed, or made non-public.", + per_result_error_template: Some("{mod {{join \"::\" path}}, previously in file {{span_filename}}:{{span_begin_line}}"), +) diff --git a/src/query.rs b/src/query.rs index b06b22ff..9bce4d61 100644 --- a/src/query.rs +++ b/src/query.rs @@ -514,4 +514,5 @@ add_lints!( pub_module_level_const_missing, pub_static_missing, trait_removed_associated_type, + module_missing, ); diff --git a/test_crates/module_missing/new/Cargo.toml b/test_crates/module_missing/new/Cargo.toml new file mode 100644 index 00000000..4a00c6b2 --- /dev/null +++ b/test_crates/module_missing/new/Cargo.toml @@ -0,0 +1,7 @@ +[package] +publish = false +name = "module_missing" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/test_crates/module_missing/new/src/lib.rs b/test_crates/module_missing/new/src/lib.rs new file mode 100644 index 00000000..f03cd380 --- /dev/null +++ b/test_crates/module_missing/new/src/lib.rs @@ -0,0 +1,24 @@ +// Removing this, but no warning should happen: +// it isn't public. +// +// mod a {} + +mod b { + // Removing this, but no warning should happen: + // it isn't visible. + // pub mod b {} +} + +pub mod bb { + // Removing this should cause a warning. + // + // pub mod will_remove {} +} + +// Making this private should cause a warning +pub(crate) mod will_make_private { + mod e {} +} + +// Adding a module shouldn't cause problems. +pub mod new_module {} diff --git a/test_crates/module_missing/old/Cargo.toml b/test_crates/module_missing/old/Cargo.toml new file mode 100644 index 00000000..4a00c6b2 --- /dev/null +++ b/test_crates/module_missing/old/Cargo.toml @@ -0,0 +1,7 @@ +[package] +publish = false +name = "module_missing" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/test_crates/module_missing/old/src/lib.rs b/test_crates/module_missing/old/src/lib.rs new file mode 100644 index 00000000..15fb4e39 --- /dev/null +++ b/test_crates/module_missing/old/src/lib.rs @@ -0,0 +1,13 @@ +mod a {} + +mod b { + pub mod b {} +} + +pub mod bb { + pub mod will_remove {} +} + +pub mod will_make_private { + mod e {} +} diff --git a/test_outputs/module_missing.output.ron b/test_outputs/module_missing.output.ron new file mode 100644 index 00000000..a658bf04 --- /dev/null +++ b/test_outputs/module_missing.output.ron @@ -0,0 +1,49 @@ +{ + "./test_crates/module_missing/": [ + { + "name": String("will_remove"), + "path": List([ + String("module_missing"), + String("bb"), + String("will_remove"), + ]), + "span_begin_line": Uint64(8), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + { + "name": String("will_make_private"), + "path": List([ + String("module_missing"), + String("will_make_private"), + ]), + "span_begin_line": Uint64(11), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + ], + "./test_crates/trait_missing/": [ + { + "name": String("my_pub_mod"), + "path": List([ + String("trait_missing"), + String("my_pub_mod"), + ]), + "span_begin_line": Uint64(5), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + ], + "./test_crates/trait_missing_with_major_bump/": [ + { + "name": String("my_pub_mod"), + "path": List([ + String("trait_missing_with_major_bump"), + String("my_pub_mod"), + ]), + "span_begin_line": Uint64(5), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + ], +} From 97956ce5f62a749f5976ee11e259a8bca0128f7f Mon Sep 17 00:00:00 2001 From: samuelrayment Date: Mon, 25 Sep 2023 17:33:04 +0100 Subject: [PATCH 2/3] New lint for removed associated constant in trait (#539) * Adds lint for removed associated constant from trait * Make span `@optional` and format query. We want to make the span `@optional` because that information is only intended to help us emit a better diagnostic message. If for any reason it isn't present, we still want to flag the issue even without span data. --------- Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> --- .../trait_removed_associated_constant.ron | 54 +++++++++++++++++++ src/query.rs | 1 + .../new/Cargo.toml | 7 +++ .../new/src/lib.rs | 11 ++++ .../old/Cargo.toml | 7 +++ .../old/src/lib.rs | 23 ++++++++ test_outputs/trait_missing.output.ron | 12 +++++ ...ait_removed_associated_constant.output.ron | 26 +++++++++ 8 files changed, 141 insertions(+) create mode 100644 src/lints/trait_removed_associated_constant.ron create mode 100644 test_crates/trait_removed_associated_constant/new/Cargo.toml create mode 100644 test_crates/trait_removed_associated_constant/new/src/lib.rs create mode 100644 test_crates/trait_removed_associated_constant/old/Cargo.toml create mode 100644 test_crates/trait_removed_associated_constant/old/src/lib.rs create mode 100644 test_outputs/trait_removed_associated_constant.output.ron diff --git a/src/lints/trait_removed_associated_constant.ron b/src/lints/trait_removed_associated_constant.ron new file mode 100644 index 00000000..1a433411 --- /dev/null +++ b/src/lints/trait_removed_associated_constant.ron @@ -0,0 +1,54 @@ +SemverQuery( + id: "trait_removed_associated_constant", + human_readable_name: "trait's associated constant was removed", + description: "A trait's associated constant was removed or renamed", + required_update: Major, + reference_link: Some("https://doc.rust-lang.org/cargo/reference/semver.html#item-remove"), + query: r#" + { + CrateDiff { + baseline { + item { + ... on Trait { + trait_name: name @output + visibility_limit @filter(op: "=", value: ["$public"]) @output + + importable_path { + path @output @tag + } + + associated_constant { + associated_constant: name @output @tag + + span_: span @optional { + filename @output + begin_line @output + } + } + } + } + } + current { + item { + ... on Trait { + visibility_limit @filter(op: "=", value: ["$public"]) + + importable_path { + path @filter(op: "=", value: ["%path"]) + } + + associated_constant @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) { + name @filter(op: "=", value: ["%associated_constant"]) + } + } + } + } + } + }"#, + arguments: { + "public": "public", + "zero": 0, + }, + error_message: "A public trait's associated constant was removed or renamed.", + per_result_error_template: Some("associated constant {{trait_name}}::{{associated_constant}}, previously at {{span_filename}}:{{span_begin_line}}"), +) diff --git a/src/query.rs b/src/query.rs index 9bce4d61..6b41ab07 100644 --- a/src/query.rs +++ b/src/query.rs @@ -515,4 +515,5 @@ add_lints!( pub_static_missing, trait_removed_associated_type, module_missing, + trait_removed_associated_constant, ); diff --git a/test_crates/trait_removed_associated_constant/new/Cargo.toml b/test_crates/trait_removed_associated_constant/new/Cargo.toml new file mode 100644 index 00000000..66db2638 --- /dev/null +++ b/test_crates/trait_removed_associated_constant/new/Cargo.toml @@ -0,0 +1,7 @@ +[package] +publish = false +name = "trait_removed_associated_constant" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/test_crates/trait_removed_associated_constant/new/src/lib.rs b/test_crates/trait_removed_associated_constant/new/src/lib.rs new file mode 100644 index 00000000..d73ba2d8 --- /dev/null +++ b/test_crates/trait_removed_associated_constant/new/src/lib.rs @@ -0,0 +1,11 @@ +pub trait RemovedAssociatedConstantFromTrait { +} + +pub trait TraitWithConstant { + const APPLE: i32; +} + +pub trait RemovedAssociatedConstantDefaultFromTrait: TraitWithConstant {} + +// this trait is private therefore removing the constant is not breaking +trait PrivateTraitWithTypeRemoved {} diff --git a/test_crates/trait_removed_associated_constant/old/Cargo.toml b/test_crates/trait_removed_associated_constant/old/Cargo.toml new file mode 100644 index 00000000..66db2638 --- /dev/null +++ b/test_crates/trait_removed_associated_constant/old/Cargo.toml @@ -0,0 +1,7 @@ +[package] +publish = false +name = "trait_removed_associated_constant" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/test_crates/trait_removed_associated_constant/old/src/lib.rs b/test_crates/trait_removed_associated_constant/old/src/lib.rs new file mode 100644 index 00000000..ce4a497f --- /dev/null +++ b/test_crates/trait_removed_associated_constant/old/src/lib.rs @@ -0,0 +1,23 @@ +pub trait RemovedAssociatedConstantFromTrait { + const APPLE: i32; +} + +pub trait TraitWithConstant { + const APPLE: i32; +} + +pub trait RemovedAssociatedConstantDefaultFromTrait: TraitWithConstant { + const APPLE: i32 = 2; +} + +// this trait is private therefore removing the constant is not breaking +trait PrivateTraitWithTypeRemoved { + const APPLE: i32; +} + +/// This trait should not be reported by the `trait_removed_associated_constant` rule: +/// it will be removed altogether, so the correct rule to catch it is +/// the `trait_missing` rule and not the rule for missing associated constants. +pub trait RemovedTrait { + const APPLE: i32; +} diff --git a/test_outputs/trait_missing.output.ron b/test_outputs/trait_missing.output.ron index ca02635f..96b0ad69 100644 --- a/test_outputs/trait_missing.output.ron +++ b/test_outputs/trait_missing.output.ron @@ -109,6 +109,18 @@ "visibility_limit": String("public"), }, ], + "./test_crates/trait_removed_associated_constant/": [ + { + "name": String("RemovedTrait"), + "path": List([ + String("trait_removed_associated_constant"), + String("RemovedTrait"), + ]), + "span_begin_line": Uint64(21), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + ], "./test_crates/trait_unsafe_added/": [ { "name": String("TraitBecomesPrivateAndUnsafe"), diff --git a/test_outputs/trait_removed_associated_constant.output.ron b/test_outputs/trait_removed_associated_constant.output.ron new file mode 100644 index 00000000..54ba4cc4 --- /dev/null +++ b/test_outputs/trait_removed_associated_constant.output.ron @@ -0,0 +1,26 @@ +{ + "./test_crates/trait_removed_associated_constant/": [ + { + "associated_constant": String("APPLE"), + "path": List([ + String("trait_removed_associated_constant"), + String("RemovedAssociatedConstantFromTrait"), + ]), + "span_begin_line": Uint64(2), + "span_filename": String("src/lib.rs"), + "trait_name": String("RemovedAssociatedConstantFromTrait"), + "visibility_limit": String("public"), + }, + { + "associated_constant": String("APPLE"), + "path": List([ + String("trait_removed_associated_constant"), + String("RemovedAssociatedConstantDefaultFromTrait"), + ]), + "span_begin_line": Uint64(10), + "span_filename": String("src/lib.rs"), + "trait_name": String("RemovedAssociatedConstantDefaultFromTrait"), + "visibility_limit": String("public"), + }, + ], +} From 34216c880e035df8a4dcf292fc5b7b7ed3846ed3 Mon Sep 17 00:00:00 2001 From: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> Date: Mon, 25 Sep 2023 14:21:27 -0400 Subject: [PATCH 3/3] Use Rust 1.70 as baseline for cross-version caching test. (#540) --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bf068f30..1613dcee 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -304,7 +304,7 @@ jobs: - name: Install rust id: toolchain - uses: dtolnay/rust-toolchain@1.67 + uses: dtolnay/rust-toolchain@1.70 # Rust 1.70 uses rustdoc v24 - name: Restore rustdoc id: cache @@ -334,7 +334,7 @@ jobs: path: subject/target/semver-checks/cache - name: Install rust - uses: dtolnay/rust-toolchain@stable + uses: dtolnay/rust-toolchain@stable # Rust 1.72 uses rustdoc v26 - name: Check with older rustdoc version cached run: |