Skip to content

Commit

Permalink
Fix false-positive "macro no longer exported" report.
Browse files Browse the repository at this point in the history
The old implementation checked that there's a macro with a matching name that isn't exported in the linted code, but didn't check that there is no *exported* macro by that name in the code. This was the cause of issue #1042.

Fixes #1042.
  • Loading branch information
obi1kenobi committed Dec 13, 2024
1 parent b9709e6 commit 9310f80
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/lints/declarative_macro_missing.ron
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ SemverQuery(
"zero": 0,
"true": true,
},
error_message: "A `macro_rules` declarative macro cannot be imported by its prior name. The macro may have been renamed or removed entirely.",
error_message: "A `macro_rules` declarative macro cannot be invoked by its prior name. The macro may have been renamed or removed entirely.",
per_result_error_template: Some("macro_rules! {{name}}, previously in file {{span_filename}}:{{span_begin_line}}"),
witness: (
hint_template: r#"{{name}}!(...);"#
Expand Down
21 changes: 19 additions & 2 deletions src/lints/macro_no_longer_exported.ron
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,35 @@ SemverQuery(
... on Macro {
name @output @tag
public_api_eligible @filter(op: "=", value: ["$true"])
span_: span @optional {
filename @output
begin_line @output
}
}
}
}
current {
item {
# There's no exported macro under the name we wanted.
item @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
... on Macro {
name @filter(op: "=", value: ["%name"])
public_api_eligible @filter(op: "=", value: ["$true"])
}
}
# There is at least one macro under the same name that is *not* exported.
# Perhaps the macro is not deleted, just no longer exported.
item @fold @transform(op: "count") @filter(op: ">", value: ["$zero"]) {
... on Macro {
name @filter(op: "=", value: ["%name"])
# Check the macro still exists but is no longer public API
# and isn't doc(hidden) (which would be caught by another lint)
public_api_eligible @filter(op: "!=", value: ["$true"])
doc_hidden @filter(op: "!=", value: ["$true"])
span_: span @optional {
non_exported_span_: span @optional {
filename @output
begin_line @output
}
Expand All @@ -36,6 +52,7 @@ SemverQuery(
}"#,
arguments: {
"true": true,
"zero": 0,
},
error_message: "A macro that was previously exported with #[macro_export] is no longer exported. This breaks downstream code that used the macro.",
per_result_error_template: Some("macro {{name}} in {{span_filename}}:{{span_begin_line}}"),
Expand Down
17 changes: 17 additions & 0 deletions test_crates/macro_no_longer_exported/new/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,20 @@ macro_rules! internal_macro {
println!("Internal macro");
};
}

pub mod foo {
// Private macro, which is not exported despite being in a public module.
// Macros require `#[macro_export]` or they aren't visible outside their crate.
//
// This is a breaking change.
macro_rules! some_macro {
() => {}
}
}

mod bar {
// Private macro by the same name.
macro_rules! some_macro {
() => {}
}
}
16 changes: 16 additions & 0 deletions test_crates/macro_no_longer_exported/old/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,19 @@ macro_rules! internal_macro {
println!("Internal macro");
};
}

mod foo {
// Public macro. Exported even though it's in a private module,
// because of the `#[macro_export]`.
#[macro_export]
macro_rules! some_macro {
() => {}
}
}

mod bar {
// Private macro by the same name.
macro_rules! some_macro {
() => {}
}
}
33 changes: 32 additions & 1 deletion test_outputs/query_execution/macro_no_longer_exported.snap
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,51 @@ snapshot_kind: text
"./test_crates/declarative_macro_missing/": [
{
"name": String("will_no_longer_be_exported"),
"span_begin_line": Uint64(1),
"non_exported_span_begin_line": List([
Uint64(1),
]),
"non_exported_span_filename": List([
String("src/lib.rs"),
]),
"span_begin_line": Uint64(7),
"span_filename": String("src/lib.rs"),
},
],
"./test_crates/macro_no_longer_exported/": [
{
"name": String("example_macro"),
"non_exported_span_begin_line": List([
Uint64(2),
]),
"non_exported_span_filename": List([
String("src/lib.rs"),
]),
"span_begin_line": Uint64(2),
"span_filename": String("src/lib.rs"),
},
{
"name": String("some_macro"),
"non_exported_span_begin_line": List([
Uint64(36),
Uint64(29),
]),
"non_exported_span_filename": List([
String("src/lib.rs"),
String("src/lib.rs"),
]),
"span_begin_line": Uint64(26),
"span_filename": String("src/lib.rs"),
},
],
"./test_crates/macro_now_doc_hidden/": [
{
"name": String("becomes_non_exported"),
"non_exported_span_begin_line": List([
Uint64(36),
]),
"non_exported_span_filename": List([
String("src/lib.rs"),
]),
"span_begin_line": Uint64(36),
"span_filename": String("src/lib.rs"),
},
Expand Down
7 changes: 6 additions & 1 deletion test_outputs/witnesses/macro_no_longer_exported.snap
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,19 @@ snapshot_kind: text
---
[["./test_crates/declarative_macro_missing/"]]
filename = 'src/lib.rs'
begin_line = 1
begin_line = 7
hint = 'will_no_longer_be_exported!(...);'

[["./test_crates/macro_no_longer_exported/"]]
filename = 'src/lib.rs'
begin_line = 2
hint = 'example_macro!(...);'

[["./test_crates/macro_no_longer_exported/"]]
filename = 'src/lib.rs'
begin_line = 26
hint = 'some_macro!(...);'

[["./test_crates/macro_now_doc_hidden/"]]
filename = 'src/lib.rs'
begin_line = 36
Expand Down

0 comments on commit 9310f80

Please sign in to comment.