From f4a560239c5590c524be8546f04d1c8b95c8da5a Mon Sep 17 00:00:00 2001 From: yottalogical Date: Tue, 19 Dec 2023 17:40:32 -0500 Subject: [PATCH 1/3] Add new lint: trait_method_unsafe_removed --- src/lints/trait_method_unsafe_removed.ron | 18 ++++++++++++++++++ src/query.rs | 1 + .../trait_method_unsafe_removed/new/Cargo.toml | 7 +++++++ .../trait_method_unsafe_removed/new/src/lib.rs | 0 .../trait_method_unsafe_removed/old/Cargo.toml | 7 +++++++ .../trait_method_unsafe_removed/old/src/lib.rs | 0 .../trait_method_unsafe_removed.output.ron | 5 +++++ 7 files changed, 38 insertions(+) create mode 100644 src/lints/trait_method_unsafe_removed.ron create mode 100644 test_crates/trait_method_unsafe_removed/new/Cargo.toml create mode 100644 test_crates/trait_method_unsafe_removed/new/src/lib.rs create mode 100644 test_crates/trait_method_unsafe_removed/old/Cargo.toml create mode 100644 test_crates/trait_method_unsafe_removed/old/src/lib.rs create mode 100644 test_outputs/trait_method_unsafe_removed.output.ron diff --git a/src/lints/trait_method_unsafe_removed.ron b/src/lints/trait_method_unsafe_removed.ron new file mode 100644 index 00000000..f0485c3c --- /dev/null +++ b/src/lints/trait_method_unsafe_removed.ron @@ -0,0 +1,18 @@ +SemverQuery( + id: "trait_method_unsafe_removed", + human_readable_name: "TODO", + description: "TODO", + required_update: Major, // TODO + reference_link: None, // TODO + query: r#" + { + CrateDiff { + # TODO + } + }"#, + arguments: { + // TODO + }, + error_message: "TODO", + per_result_error_template: Some("TODO"), +) diff --git a/src/query.rs b/src/query.rs index 2ad36cc6..37732853 100644 --- a/src/query.rs +++ b/src/query.rs @@ -522,4 +522,5 @@ add_lints!( trait_removed_associated_constant, function_changed_abi, trait_method_unsafe_added, + trait_method_unsafe_removed, ); diff --git a/test_crates/trait_method_unsafe_removed/new/Cargo.toml b/test_crates/trait_method_unsafe_removed/new/Cargo.toml new file mode 100644 index 00000000..7bf0051a --- /dev/null +++ b/test_crates/trait_method_unsafe_removed/new/Cargo.toml @@ -0,0 +1,7 @@ +[package] +publish = false +name = "trait_method_unsafe_removed" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/test_crates/trait_method_unsafe_removed/new/src/lib.rs b/test_crates/trait_method_unsafe_removed/new/src/lib.rs new file mode 100644 index 00000000..e69de29b diff --git a/test_crates/trait_method_unsafe_removed/old/Cargo.toml b/test_crates/trait_method_unsafe_removed/old/Cargo.toml new file mode 100644 index 00000000..7bf0051a --- /dev/null +++ b/test_crates/trait_method_unsafe_removed/old/Cargo.toml @@ -0,0 +1,7 @@ +[package] +publish = false +name = "trait_method_unsafe_removed" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/test_crates/trait_method_unsafe_removed/old/src/lib.rs b/test_crates/trait_method_unsafe_removed/old/src/lib.rs new file mode 100644 index 00000000..e69de29b diff --git a/test_outputs/trait_method_unsafe_removed.output.ron b/test_outputs/trait_method_unsafe_removed.output.ron new file mode 100644 index 00000000..c1d09c33 --- /dev/null +++ b/test_outputs/trait_method_unsafe_removed.output.ron @@ -0,0 +1,5 @@ +{ + "./test_crates/trait_method_unsafe_removed/": [ + // TODO + ] +} From 9e5b0291d0173d78d24cef497fedd29b60647b79 Mon Sep 17 00:00:00 2001 From: yottalogical Date: Thu, 21 Dec 2023 10:58:35 -0500 Subject: [PATCH 2/3] Create tests for lint trait_method_unsafe_removed --- .../new/src/lib.rs | 30 +++++++++++++++++++ .../old/src/lib.rs | 30 +++++++++++++++++++ .../trait_method_unsafe_removed.output.ron | 23 ++++++++++++-- test_outputs/trait_missing.output.ron | 12 ++++++++ test_outputs/trait_unsafe_removed.output.ron | 12 ++++++++ 5 files changed, 105 insertions(+), 2 deletions(-) diff --git a/test_crates/trait_method_unsafe_removed/new/src/lib.rs b/test_crates/trait_method_unsafe_removed/new/src/lib.rs index e69de29b..f2a7dfcb 100644 --- a/test_crates/trait_method_unsafe_removed/new/src/lib.rs +++ b/test_crates/trait_method_unsafe_removed/new/src/lib.rs @@ -0,0 +1,30 @@ +// Method of public trait becomes safe, should get reported. +pub trait PubTrait { + fn becomes_safe(); + fn stays_safe(); + unsafe fn stays_unsafe(); +} + +// Method of trait that was not publicly-visible becomes safe, shouldn't get +// reported. +pub trait TraitBecomesPublic { + fn becomes_safe(); +} + +// Method of trait that is publicly-visible becomes safe while trait becomes +// private. The trait becoming private should be the only violation that's +// reported. +trait TraitBecomesPrivate { + fn becomes_safe(); +} + +// Method of trait becomes safe while trait also becomes safe. Method should get +// reported along with trait. +pub trait TraitBecomesSafe { + fn becomes_safe(); +} + +// Private trait's method becomes safe, shouldn't get reported. +trait PrivateTrait { + fn becomes_safe(); +} diff --git a/test_crates/trait_method_unsafe_removed/old/src/lib.rs b/test_crates/trait_method_unsafe_removed/old/src/lib.rs index e69de29b..f800a482 100644 --- a/test_crates/trait_method_unsafe_removed/old/src/lib.rs +++ b/test_crates/trait_method_unsafe_removed/old/src/lib.rs @@ -0,0 +1,30 @@ +// Method of public trait becomes safe, should get reported. +pub trait PubTrait { + unsafe fn becomes_safe(); + fn stays_safe(); + unsafe fn stays_unsafe(); +} + +// Method of trait that was not publicly-visible becomes safe, shouldn't get +// reported. +trait TraitBecomesPublic { + unsafe fn becomes_safe(); +} + +// Method of trait that is publicly-visible becomes safe while trait becomes +// private. The trait becoming private should be the only violation that's +// reported. +pub trait TraitBecomesPrivate { + unsafe fn becomes_safe(); +} + +// Method of trait becomes safe while trait also becomes safe. Method should get +// reported along with trait. +pub unsafe trait TraitBecomesSafe { + unsafe fn becomes_safe(); +} + +// Private trait's method becomes safe, shouldn't get reported. +trait PrivateTrait { + unsafe fn becomes_safe(); +} diff --git a/test_outputs/trait_method_unsafe_removed.output.ron b/test_outputs/trait_method_unsafe_removed.output.ron index c1d09c33..41ffe657 100644 --- a/test_outputs/trait_method_unsafe_removed.output.ron +++ b/test_outputs/trait_method_unsafe_removed.output.ron @@ -1,5 +1,24 @@ { "./test_crates/trait_method_unsafe_removed/": [ - // TODO - ] + { + "method_name": String("becomes_safe"), + "path": List([ + String("trait_method_unsafe_removed"), + String("PubTrait"), + ]), + "span_begin_line": Uint64(3), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + { + "method_name": String("becomes_safe"), + "path": List([ + String("trait_method_unsafe_removed"), + String("TraitBecomesSafe"), + ]), + "span_begin_line": Uint64(24), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + } + ], } diff --git a/test_outputs/trait_missing.output.ron b/test_outputs/trait_missing.output.ron index 97667bc8..fc4d211a 100644 --- a/test_outputs/trait_missing.output.ron +++ b/test_outputs/trait_missing.output.ron @@ -47,6 +47,18 @@ "visibility_limit": String("public"), }, ], + "./test_crates/trait_method_unsafe_removed/": [ + { + "name": String("TraitBecomesPrivate"), + "path": List([ + String("trait_method_unsafe_removed"), + String("TraitBecomesPrivate"), + ]), + "span_begin_line": Uint64(17), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + ], "./test_crates/trait_missing/": [ { "name": String("RemovedTrait"), diff --git a/test_outputs/trait_unsafe_removed.output.ron b/test_outputs/trait_unsafe_removed.output.ron index 0b832f23..82c99383 100644 --- a/test_outputs/trait_unsafe_removed.output.ron +++ b/test_outputs/trait_unsafe_removed.output.ron @@ -35,4 +35,16 @@ "visibility_limit": String("public"), }, ], + "./test_crates/trait_method_unsafe_removed/": [ + { + "name": String("TraitBecomesSafe"), + "path": List([ + String("trait_method_unsafe_removed"), + String("TraitBecomesSafe"), + ]), + "span_begin_line": Uint64(23), + "span_filename": String("src/lib.rs"), + "visibility_limit": String("public"), + }, + ], } From 9eb226a361203ff060642b934d2681c99b9ec5f0 Mon Sep 17 00:00:00 2001 From: yottalogical Date: Thu, 21 Dec 2023 13:04:04 -0500 Subject: [PATCH 3/3] Implement lint trait_method_unsafe_removed --- src/lints/trait_method_unsafe_removed.ron | 57 +++++++++++++++++++---- 1 file changed, 49 insertions(+), 8 deletions(-) diff --git a/src/lints/trait_method_unsafe_removed.ron b/src/lints/trait_method_unsafe_removed.ron index f0485c3c..56a8fda7 100644 --- a/src/lints/trait_method_unsafe_removed.ron +++ b/src/lints/trait_method_unsafe_removed.ron @@ -1,18 +1,59 @@ SemverQuery( id: "trait_method_unsafe_removed", - human_readable_name: "TODO", - description: "TODO", - required_update: Major, // TODO - reference_link: None, // TODO + human_readable_name: "pub trait method became safe", + description: "A method in a public trait became safe", + required_update: Major, + reference_link: Some("https://doc.rust-lang.org/stable/reference/unsafe-keyword.html#unsafe-functions-unsafe-fn"), query: r#" { CrateDiff { - # TODO + baseline { + item { + ... on Trait { + visibility_limit @filter(op: "=", value: ["$public"]) @output + + importable_path { + path @output @tag + public_api @filter(op: "=", value: ["$true"]) + } + + method { + unsafe @filter(op: "=", value: ["$true"]) + public_api_eligible @filter(op: "=", value: ["$true"]) + method_name: name @output @tag + } + } + } + } + current { + item { + ... on Trait { + visibility_limit @filter(op: "=", value: ["$public"]) + + importable_path { + path @filter(op: "=", value: ["%path"]) + public_api @filter(op: "=", value: ["$true"]) + } + + method { + unsafe @filter(op: "!=", value: ["$true"]) + public_api_eligible @filter(op: "=", value: ["$true"]) + name @filter(op: "=", value: ["%method_name"]) + + span_: span @optional { + filename @output + begin_line @output + } + } + } + } + } } }"#, arguments: { - // TODO + "true": true, + "public": "public", }, - error_message: "TODO", - per_result_error_template: Some("TODO"), + error_message: "A publicly-visible trait method is no longer `unsafe`, so implementations must remove the `unsafe` qualifier.", + per_result_error_template: Some("trait method <{{join \"::\" path}}>::{{method_name}} in file {{span_filename}}:{{span_begin_line}}"), )