Skip to content

Commit

Permalink
add function_export_name_changed lint (#728)
Browse files Browse the repository at this point in the history
* add function_export_name_changed lint

* consolidate export_name_changed and export_name_missing into one lint

* Apply suggestions from code review (pt1)

Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>

* apply feedback from code review (pt2)

- remove old comments
- add output from new `new_export_name` field

---------

Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>
  • Loading branch information
suaviloquence and obi1kenobi authored Apr 1, 2024
1 parent dd63a1c commit 1a48764
Show file tree
Hide file tree
Showing 7 changed files with 231 additions and 0 deletions.
47 changes: 47 additions & 0 deletions src/lints/function_export_name_changed.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
SemverQuery(
id: "function_export_name_changed",
human_readable_name: "function's export name has changed or been removed",
description: "A function's ABI name with #[no_mangle] or #[export_name = \"name\"] has changed or been removed",
required_update: Major,
reference_link: Some("https://doc.rust-lang.org/reference/abi.html#the-no_mangle-attribute"),
query: r#"
{
CrateDiff {
baseline {
item {
... on Function {
importable_path {
path @output @tag
}
export_name @output @tag @filter(op: "is_not_null")
}
}
}
current {
item {
... on Function {
name @output
new_export_name: export_name @filter(op: "!=", value: ["%export_name"]) @output
importable_path {
path @filter(op: "=", value: ["%path"])
}
span_: span @optional {
filename @output
begin_line @output
}
}
}
}
}
}"#,
arguments: {
},
error_message: "The ABI export name of a function has been changed or removed through the #[no_mangle] or #[export_name] attributes.",
per_result_error_template: Some("function {{join \"::\" path}} no longer has ABI name {{export_name}} in {{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 @@ -476,6 +476,7 @@ macro_rules! add_lints {
}

add_lints!(
function_export_name_changed,
union_field_missing,
pub_static_now_doc_hidden,
function_abi_no_longer_unwind,
Expand Down
7 changes: 7 additions & 0 deletions test_crates/function_export_name_changed/new/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "function_export_name_changed"
version = "0.1.0"
edition = "2021"

[dependencies]
43 changes: 43 additions & 0 deletions test_crates/function_export_name_changed/new/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/// positive test - a function changes export_name
#[export_name = "export_name_changed_new"]
pub fn export_name_changed() {}

/// positive test - a function changes no_mangle to a different export_name
#[export_name = "no_mangle_changed_to_other_export_name_new"]
pub fn no_mangle_changed_to_other_export_name() {}

/// positive test - a function changes export_name to a different no_mangle
#[no_mangle]
pub fn export_name_changed_to_other_no_mangle() {}

/// negative test - a function changes no_mangle to an equivalent export_name
#[export_name = "no_mangle_changed_to_same_export_name"]
pub fn no_mangle_changed_to_same_export_name() {}

/// negative test - a function changes export_name to an equivalent no_mangle
#[no_mangle]
pub fn export_name_changed_to_same_no_mangle() {}

/// positive test - a function's no_mangle is removed
pub fn no_mangle_removed() {}

/// positive test - a function's export_name is removed
pub fn export_name_removed() {}

/// positive test - a non-public function changes export name
#[export_name = "private_export_name_changed_new"]
pub fn private_export_name_changed() {}

/// positive test - export name on one function gets moved to the other
/// this is still a breaking change because a call to `export_name_moved_1` in the API
/// is no longer equivalent to a call to `export_name_moved` in the ABI
pub mod export_name_moved {
pub fn export_name_moved_1() {}

#[export_name = "export_name_moved"]
pub fn export_name_moved_2() {}
}

/// negative test - a function's export name is added
#[export_name = "export_name_added"]
pub fn export_name_added() {}
7 changes: 7 additions & 0 deletions test_crates/function_export_name_changed/old/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
publish = false
name = "function_export_name_changed"
version = "0.1.0"
edition = "2021"

[dependencies]
44 changes: 44 additions & 0 deletions test_crates/function_export_name_changed/old/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/// positive test - a function changes export_name
#[export_name = "export_name_changed_old"]
pub fn export_name_changed() {}

/// positive test - a function changes no_mangle to a different export_name
#[no_mangle]
pub fn no_mangle_changed_to_other_export_name() {}

/// positive test - a function changes export_name to a different no_mangle
#[export_name = "export_name_changed_to_other_no_mangle_old"]
pub fn export_name_changed_to_other_no_mangle() {}

/// negative test - a function changes no_mangle to an equivalent export_name
#[no_mangle]
pub fn no_mangle_changed_to_same_export_name() {}

/// negative test - a function changes export_name to an equivalent no_mangle
#[export_name = "export_name_changed_to_same_no_mangle"]
pub fn export_name_changed_to_same_no_mangle() {}

/// positive test - a function's no_mangle is removed
#[no_mangle]
pub fn no_mangle_removed() {}

/// positive test - a function's export_name is removed
#[export_name = "export_name_removed"]
pub fn export_name_removed() {}

/// positive test - a non-public function changes export name
#[export_name = "private_export_name_changed_old"]
pub fn private_export_name_changed() {}

/// positive test - export name on one function gets moved to the other
/// this is still a breaking change because a call to `export_name_moved_1` in the API
/// is no longer equivalent to a call to `export_name_moved` in the ABI
pub mod export_name_moved {
#[export_name = "export_name_moved"]
pub fn export_name_moved_1() {}

pub fn export_name_moved_2() {}
}

/// negative test - a function's export name is added
pub fn export_name_added() {}
82 changes: 82 additions & 0 deletions test_outputs/function_export_name_changed.output.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
{
"./test_crates/function_export_name_changed/": [
{
"export_name": String("export_name_changed_old"),
"name": String("export_name_changed"),
"new_export_name": String("export_name_changed_new"),
"path": List([
String("function_export_name_changed"),
String("export_name_changed"),
]),
"span_begin_line": Uint64(3),
"span_filename": String("src/lib.rs"),
},
{
"export_name": String("no_mangle_changed_to_other_export_name"),
"name": String("no_mangle_changed_to_other_export_name"),
"new_export_name": String("no_mangle_changed_to_other_export_name_new"),
"path": List([
String("function_export_name_changed"),
String("no_mangle_changed_to_other_export_name"),
]),
"span_begin_line": Uint64(7),
"span_filename": String("src/lib.rs"),
},
{
"export_name": String("export_name_changed_to_other_no_mangle_old"),
"name": String("export_name_changed_to_other_no_mangle"),
"new_export_name": String("export_name_changed_to_other_no_mangle"),
"path": List([
String("function_export_name_changed"),
String("export_name_changed_to_other_no_mangle"),
]),
"span_begin_line": Uint64(11),
"span_filename": String("src/lib.rs"),
},
{
"export_name": String("no_mangle_removed"),
"name": String("no_mangle_removed"),
"new_export_name": Null,
"path": List([
String("function_export_name_changed"),
String("no_mangle_removed"),
]),
"span_begin_line": Uint64(22),
"span_filename": String("src/lib.rs"),
},
{
"export_name": String("export_name_removed"),
"name": String("export_name_removed"),
"new_export_name": Null,
"path": List([
String("function_export_name_changed"),
String("export_name_removed"),
]),
"span_begin_line": Uint64(25),
"span_filename": String("src/lib.rs"),
},
{
"export_name": String("private_export_name_changed_old"),
"name": String("private_export_name_changed"),
"new_export_name": String("private_export_name_changed_new"),
"path": List([
String("function_export_name_changed"),
String("private_export_name_changed"),
]),
"span_begin_line": Uint64(29),
"span_filename": String("src/lib.rs"),
},
{
"export_name": String("export_name_moved"),
"name": String("export_name_moved_1"),
"new_export_name": Null,
"path": List([
String("function_export_name_changed"),
String("export_name_moved"),
String("export_name_moved_1"),
]),
"span_begin_line": Uint64(35),
"span_filename": String("src/lib.rs"),
},
]
}

0 comments on commit 1a48764

Please sign in to comment.