Do not suggest adding semicolon/changing delimiters for macros in item position that originates in macros

[ChayimFriedman2]

Fixes #91800.

[rust-highfive]

r? @michaelwoerister

(rust-highfive has picked a reviewer for you, use r? to override)

[michaelwoerister]

Thanks for the PR, @ChayimFriedman2! That FIXME suggests that there's a diagnostics design decision to be made here.
r? rust-lang/diagnostics

[estebank]

What happens if you instead use the following? Is the FIXME still applicable then? I suspect it might not be.

Suggested change

	if !span.from_expansion() {
	if !span.can_be_used_for_suggestions() {

[ChayimFriedman2]

That doesn't work. can_be_used_for_suggestions() only checks for derive macros where the span they emit is different from the span of themselves. This will both provide incorrect suggestions for those macros and still fail as with the FIXME.

rust/compiler/rustc_span/src/lib.rs

Lines 575 to 583 in 49c82f3

	/// Gate suggestions that would not be appropriate in a context the user didn't write.
	pub fn can_be_used_for_suggestions(self) -> bool {
	!self.from_expansion()
	// FIXME: If this span comes from a `derive` macro but it points at code the user wrote,
	// the callsite span and the span will be pointing at different places. It also means that
	// we can safely provide suggestions on this span.
	|| (matches!(self.ctxt().outer_expn_data().kind, ExpnKind::Macro(MacroKind::Derive, _))
	&& self.parent_callsite().map(|p| (p.lo(), p.hi())) != Some((self.lo(), self.hi())))
	}

[estebank]

What about using in_derive_expansion and only gate on those, then?

[ChayimFriedman2]

I've thought about that, but attribute macros should not suggest help either; and probably neither should declarative macros from separate crates.

[estebank]

Could you then checks for !matches!(kind, ExprKind::Macro(MacroKind::Derive | MacroKind::Attr, _)) and using tcx.sess().source_map().lookup_char_pos(span.lo()).file on both the suggestion span and the callsite span to see if they belong to the same file?

Also, an additional test for those cases so that we see when the suggestion should appear in the same file might be useful. That might be easy to do for a macro by example. You might need to expand the test to also have an "external crate" example to check for that case too.

Would you have time to get these things done in this PR?

[ChayimFriedman2]

Soon. But should it be the same file or same crate?

[ChayimFriedman2]

And what about function-like proc macros? How can I differentiate those?

[estebank]

Ah, you're right. @petrochenkov what would the best way of checking that a span doesn't correspond to a proc-macro of any type nor a macro by example from a foreign crate, while parsing an item?

[estebank]

@petrochenkov friendly ping, if you have advice re: ^

I'm ok with landing with if !span.from_expansion() {, at least for now. Would you mind rebasing @ChayimFriedman2?

[ChayimFriedman2]

@estebank ^^^ Done.

[bors]

☔ The latest upstream changes (presumably #98066) made this pull request unmergeable. Please resolve the merge conflicts.

[estebank]

@bors r+

[bors]

📌 Commit 0ef4098 has been approved by estebank