Skip to content

Add suggestion to remove derive() if invoked macro is non-derive #109638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bors merged 9 commits into from
Apr 11, 2023
Merged

Add suggestion to remove derive() if invoked macro is non-derive #109638

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
42f2be8
Add suggestion to remove derive() if invoked macro is non-derive
NotStirred Mar 26, 2023
10c3644
Update non-derive macro error message to match suggestion
NotStirred Mar 30, 2023
ed84d1d
Fix typo
NotStirred Mar 30, 2023
86230dc
Bless
NotStirred Apr 1, 2023
56207b8
Skip help messages if macro span is from macro
NotStirred Apr 2, 2023
be44169
Collapse if statements, change message to lowercase
NotStirred Apr 6, 2023
5c5c3c9
Add more complex test cases for new suggestion
NotStirred Apr 7, 2023
18388c9
Rewrite added diagnostics as translatable
NotStirred Apr 7, 2023
668a629
Change type and field name to be clearer
NotStirred Apr 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions compiler/rustc_resolve/src/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,11 +544,19 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
if let Some((article, expected)) = unexpected_res {
let path_str = pprust::path_to_string(path);
let msg = format!("expected {}, found {} `{}`", expected, res.descr(), path_str);
self.tcx
.sess
.struct_span_err(path.span, &msg)
.span_label(path.span, format!("not {} {}", article, expected))
.emit();
let mut err = self.tcx.sess.struct_span_err(path.span, &msg);

err.span_label(path.span, format!("not {} {}", article, expected));

if !path.span.from_expansion() {
// Suggest moving the macro out of the derive() as the macro isn't Derive
if kind == MacroKind::Derive && ext.macro_kind() != MacroKind::Derive {
err.span_help(path.span, "Remove from the surrounding `derive()`");
err.help(format!("Add as non-Derive macro\n`#[{}]`", path_str));
}
}

err.emit();
return Ok((self.dummy_ext(kind), Res::Err));
}

Expand Down
8 changes: 8 additions & 0 deletions tests/ui/macros/macro-path-prelude-fail-4.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ error: expected derive macro, found built-in attribute `inline`
|
LL | #[derive(inline)]
| ^^^^^^ not a derive macro
|
help: Remove from the surrounding `derive()`
Copy link
Member

@davidtwco davidtwco Apr 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a new test case that has some of the trickier cases for this change? e.g. two derives, macro (to confirm it doesn't trigger), etc.

Copy link
Contributor Author

@NotStirred NotStirred Apr 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell, this isn't valid. Was there something else you meant with macro?

macro_rules! foo {
    () => {
        Debug
    };
}

#[derive(foo!())]
struct V;

Copy link
Member

@davidtwco davidtwco Apr 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

macro_rules! foo {
    ($foo:expr) => {
		#[derive($foo)]
		struct V;
    };
}

Something more like this?

--> $DIR/macro-path-prelude-fail-4.rs:1:10
|
LL | #[derive(inline)]
| ^^^^^^
= help: Add as non-Derive macro
`#[inline]`

error: aborting due to previous error

16 changes: 16 additions & 0 deletions tests/ui/proc-macro/macro-namespace-reserved-2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ error: expected derive macro, found attribute macro `my_macro_attr`
|
LL | #[derive(my_macro_attr)]
| ^^^^^^^^^^^^^ not a derive macro
|
help: Remove from the surrounding `derive()`
--> $DIR/macro-namespace-reserved-2.rs:53:10
|
LL | #[derive(my_macro_attr)]
| ^^^^^^^^^^^^^
= help: Add as non-Derive macro
`#[my_macro_attr]`

error: can't use a procedural macro from the same crate that defines it
--> $DIR/macro-namespace-reserved-2.rs:56:10
Expand Down Expand Up @@ -87,6 +95,14 @@ error: expected derive macro, found macro `crate::my_macro`
|
LL | #[derive(crate::my_macro)]
| ^^^^^^^^^^^^^^^ not a derive macro
|
help: Remove from the surrounding `derive()`
--> $DIR/macro-namespace-reserved-2.rs:50:10
|
LL | #[derive(crate::my_macro)]
| ^^^^^^^^^^^^^^^
= help: Add as non-Derive macro
`#[crate::my_macro]`

error: cannot find macro `my_macro_attr` in this scope
--> $DIR/macro-namespace-reserved-2.rs:28:5
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/tool-attributes/tool-attributes-misplaced-2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ error: expected derive macro, found tool attribute `rustfmt::skip`
|
LL | #[derive(rustfmt::skip)]
| ^^^^^^^^^^^^^ not a derive macro
|
help: Remove from the surrounding `derive()`
--> $DIR/tool-attributes-misplaced-2.rs:1:10
|
LL | #[derive(rustfmt::skip)]
| ^^^^^^^^^^^^^
= help: Add as non-Derive macro
`#[rustfmt::skip]`

error: expected macro, found tool attribute `rustfmt::skip`
--> $DIR/tool-attributes-misplaced-2.rs:5:5
Expand Down