From 3c3904bec058d59e0b5bf0fdae1eaccaf7473f49 Mon Sep 17 00:00:00 2001 From: Predrag Gruevski Date: Fri, 13 Dec 2024 22:32:33 +0000 Subject: [PATCH] Fix false-positive "macro no longer exported" report. 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. --- src/lints/declarative_macro_missing.ron | 2 +- src/lints/macro_no_longer_exported.ron | 21 ++++++++++-- .../macro_no_longer_exported/new/src/lib.rs | 17 ++++++++++ .../macro_no_longer_exported/old/src/lib.rs | 16 +++++++++ .../macro_no_longer_exported.snap | 33 ++++++++++++++++++- 5 files changed, 85 insertions(+), 4 deletions(-) diff --git a/src/lints/declarative_macro_missing.ron b/src/lints/declarative_macro_missing.ron index 8c9efe35..4cdfdf5c 100644 --- a/src/lints/declarative_macro_missing.ron +++ b/src/lints/declarative_macro_missing.ron @@ -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}}!(...);"# diff --git a/src/lints/macro_no_longer_exported.ron b/src/lints/macro_no_longer_exported.ron index 3a23b557..1047295c 100644 --- a/src/lints/macro_no_longer_exported.ron +++ b/src/lints/macro_no_longer_exported.ron @@ -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 } @@ -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}}"), diff --git a/test_crates/macro_no_longer_exported/new/src/lib.rs b/test_crates/macro_no_longer_exported/new/src/lib.rs index 1e27c54f..73e1b8bd 100644 --- a/test_crates/macro_no_longer_exported/new/src/lib.rs +++ b/test_crates/macro_no_longer_exported/new/src/lib.rs @@ -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 { + () => {} + } +} diff --git a/test_crates/macro_no_longer_exported/old/src/lib.rs b/test_crates/macro_no_longer_exported/old/src/lib.rs index 37e278ff..03965feb 100644 --- a/test_crates/macro_no_longer_exported/old/src/lib.rs +++ b/test_crates/macro_no_longer_exported/old/src/lib.rs @@ -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 { + () => {} + } +} diff --git a/test_outputs/query_execution/macro_no_longer_exported.snap b/test_outputs/query_execution/macro_no_longer_exported.snap index 5969578d..d4d71ed8 100644 --- a/test_outputs/query_execution/macro_no_longer_exported.snap +++ b/test_outputs/query_execution/macro_no_longer_exported.snap @@ -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"), },