Skip to content

Commit

Permalink
Add lints for adding unsafe to methods and functions.
Browse files Browse the repository at this point in the history
Resolves #191.
  • Loading branch information
obi1kenobi committed Dec 17, 2022
1 parent 3651775 commit b306507
Show file tree
Hide file tree
Showing 13 changed files with 255 additions and 0 deletions.
49 changes: 49 additions & 0 deletions src/lints/function_unsafe_added.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
SemverQuery(
id: "function_unsafe_added",
human_readable_name: "pub fn became unsafe",
description: "A function became unsafe to call.",
required_update: Major,
reference_link: Some("https://doc.rust-lang.org/book/ch19-01-unsafe-rust.html#calling-an-unsafe-function-or-method"),
query: r#"
{
CrateDiff {
baseline {
item {
... on Function {
visibility_limit @filter(op: "=", value: ["$public"]) @output
unsafe @filter(op: "!=", value: ["$true"])
name @output @tag
importable_path {
path @output @tag
}
}
}
}
current {
item {
... on Function {
visibility_limit @filter(op: "=", value: ["$public"])
name @filter(op: "=", value: ["%name"])
unsafe @filter(op: "=", value: ["$true"])
importable_path {
path @filter(op: "=", value: ["%path"])
}
span_: span @optional {
filename @output
begin_line @output
}
}
}
}
}
}"#,
arguments: {
"public": "public",
"true": true,
},
error_message: "A publicly-visible function became `unsafe`, so calling it now requires an `unsafe` block.",
per_result_error_template: Some("function {{join \"::\" path}} in file {{span_filename}}:{{span_begin_line}}"),
)
98 changes: 98 additions & 0 deletions src/lints/inherent_method_unsafe_added.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
SemverQuery(
id: "inherent_method_unsafe_added",
human_readable_name: "pub method became unsafe",
description: "A method or associated fn became unsafe to call.",
required_update: Major,
reference_link: Some("https://doc.rust-lang.org/book/ch19-01-unsafe-rust.html#calling-an-unsafe-function-or-method"),
query: r#"
{
CrateDiff {
baseline {
item {
... on ImplOwner {
visibility_limit @filter(op: "=", value: ["$public"]) @output
name @output @tag
importable_path {
path @output @tag
}
inherent_impl {
method {
method_visibility: visibility_limit @filter(op: "=", value: ["$public"]) @output
method_name: name @output @tag
unsafe @filter(op: "!=", value: ["$true"])
span_: span @optional {
filename @output
begin_line @output
}
}
}
}
}
}
current {
item {
... on ImplOwner {
visibility_limit @filter(op: "=", value: ["$public"])
name @filter(op: "=", value: ["%name"])
importable_path {
path @filter(op: "=", value: ["%path"])
}
# We use "impl" instead of "inherent_impl" here because moving
# an inherently-implemented method to a trait is not necessarily
# a breaking change, so we don't want to report it.
#
# We look for "zero matching safe methods" rather than
# "methods that are unsafe" (incorrect!) since multiple methods with
# the same name are allowed to exist on the same type (e.g. via traits).
#
# The above by itself is still not enough: say if the method was removed,
# that still makes the "there is no method ..." statement true.
# So we add an additional clause demanding that a method by that name
# with appropriate visibility actually exists.
impl @fold @transform(op: "count") @filter(op: ">", value: ["$zero"]) {
method {
visibility_limit @filter(op: "one_of", value: ["$public_or_default"])
name @filter(op: "=", value: ["%method_name"])
}
}
impl @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
method {
visibility_limit @filter(op: "one_of", value: ["$public_or_default"])
name @filter(op: "=", value: ["%method_name"])
unsafe @filter(op: "!=", value: ["$true"])
}
}
# Get the non-matching methods by that name so we can report them
# in the lint error message.
impl @fold {
method {
visibility_limit @filter(op: "one_of", value: ["$public_or_default"])
name @filter(op: "=", value: ["%method_name"])
unsafe @filter(op: "=", value: ["$true"])
non_matching_span_: span @optional {
filename @output
begin_line @output
}
}
}
}
}
}
}
}"#,
arguments: {
"public": "public",
"public_or_default": ["public", "default"],
"zero": 0,
"true": true,
},
error_message: "A publicly-visible method or associated fn became `unsafe`, so calling it now requires an `unsafe` block.",
per_result_error_template: Some("{{name}}::{{method_name}} in {{multiple_spans non_matching_span_filename non_matching_span_begin_line}}"),
)
2 changes: 2 additions & 0 deletions src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,10 @@ add_lints!(
function_const_removed,
function_missing,
function_parameter_count_changed,
function_unsafe_added,
inherent_method_const_removed,
inherent_method_missing,
inherent_method_unsafe_added,
method_parameter_count_changed,
sized_impl_removed,
struct_marked_non_exhaustive,
Expand Down
6 changes: 6 additions & 0 deletions test_crates/function_unsafe_added/new/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "function_unsafe_added"
version = "0.1.0"
edition = "2021"

[dependencies]
3 changes: 3 additions & 0 deletions test_crates/function_unsafe_added/new/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub unsafe fn add(x: i64, y: i64) -> i64 {
x + y
}
6 changes: 6 additions & 0 deletions test_crates/function_unsafe_added/old/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "function_unsafe_added"
version = "0.1.0"
edition = "2021"

[dependencies]
3 changes: 3 additions & 0 deletions test_crates/function_unsafe_added/old/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub fn add(x: i64, y: i64) -> i64 {
x + y
}
6 changes: 6 additions & 0 deletions test_crates/inherent_method_unsafe_added/new/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "inherent_method_unsafe_added"
version = "0.1.0"
edition = "2021"

[dependencies]
11 changes: 11 additions & 0 deletions test_crates/inherent_method_unsafe_added/new/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
pub struct Foo;

impl Foo {
pub unsafe fn associated_fn(x: i64, y: i64) -> i64 {
x + y
}

pub unsafe fn method(&self, x: i64) -> i64 {
x
}
}
6 changes: 6 additions & 0 deletions test_crates/inherent_method_unsafe_added/old/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "inherent_method_unsafe_added"
version = "0.1.0"
edition = "2021"

[dependencies]
11 changes: 11 additions & 0 deletions test_crates/inherent_method_unsafe_added/old/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
pub struct Foo;

impl Foo {
pub fn associated_fn(x: i64, y: i64) -> i64 {
x + y
}

pub fn method(&self, x: i64) -> i64 {
x
}
}
14 changes: 14 additions & 0 deletions test_outputs/function_unsafe_added.output.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"./test_crates/function_unsafe_added/": [
{
"name": String("add"),
"path": List([
String("function_unsafe_added"),
String("add"),
]),
"span_begin_line": Uint64(1),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
],
}
40 changes: 40 additions & 0 deletions test_outputs/inherent_method_unsafe_added.output.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
{
"./test_crates/inherent_method_unsafe_added/": [
{
"method_name": String("associated_fn"),
"method_visibility": String("public"),
"name": String("Foo"),
"non_matching_span_begin_line": List([
Uint64(4),
]),
"non_matching_span_filename": List([
String("src/lib.rs"),
]),
"path": List([
String("inherent_method_unsafe_added"),
String("Foo"),
]),
"span_begin_line": Uint64(4),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
{
"method_name": String("method"),
"method_visibility": String("public"),
"name": String("Foo"),
"non_matching_span_begin_line": List([
Uint64(8),
]),
"non_matching_span_filename": List([
String("src/lib.rs"),
]),
"path": List([
String("inherent_method_unsafe_added"),
String("Foo"),
]),
"span_begin_line": Uint64(8),
"span_filename": String("src/lib.rs"),
"visibility_limit": String("public"),
},
],
}

0 comments on commit b306507

Please sign in to comment.