Allow macro expansions into RestPat in tuple args work as ellipsis like plain RestPat

[ShoyuVanilla]

Fixes #17292

Currently, Rust Analyzer lowers ast::Pat::RestPat into Pat::Missing in general cases on the following lines;

rust-analyzer/crates/hir-def/src/body/lower.rs

Lines 1359 to 1367 in ffbc5ad

	ast::Pat::RestPat(_) => {
	// `RestPat` requires special handling and should not be mapped
	// to a Pat. Here we are using `Pat::Missing` as a fallback for
	// when `RestPat` is mapped to `Pat`, which can easily happen
	// when the source code being analyzed has a malformed pattern
	// which includes `..` in a place where it isn't valid.
	Pat::Missing
	}

And in some proper positions such as TupleStruct(..), it is specially handed on the following lines;

rust-analyzer/crates/hir-def/src/body/lower.rs

Lines 1429 to 1437 in ffbc5ad

	fn collect_tuple_pat(
	&mut self,
	args: AstChildren<ast::Pat>,
	has_leading_comma: bool,
	binding_list: &mut BindingList,
	) -> (Box<[PatId]>, Option<usize>) {
	// Find the location of the `..`, if there is one. Note that we do not
	// consider the possibility of there being multiple `..` here.
	let ellipsis = args.clone().position(|p| matches!(p, ast::Pat::RestPat(_)));

This behavior is reasonable because rustc does similar things in
https://github.com/rust-lang/rust/blob/62c068feeafd1f4abbf87243d69cf8862e4dd277/compiler/rustc_ast_lowering/src/pat.rs#L108-L111
and
https://github.com/rust-lang/rust/blob/62c068feeafd1f4abbf87243d69cf8862e4dd277/compiler/rustc_ast_lowering/src/pat.rs#L123-L142

But this sometimes works differently because Rust Analyzer expands macros while ast lowering;

rust-analyzer/crates/hir-def/src/body/lower.rs

Lines 1386 to 1398 in ffbc5ad

	ast::Pat::MacroPat(mac) => match mac.macro_call() {
	Some(call) => {
	let macro_ptr = AstPtr::new(&call);
	let src = self.expander.in_file(AstPtr::new(&pat));
	let pat =
	self.collect_macro_call(call, macro_ptr, true, |this, expanded_pat| {
	this.collect_pat_opt(expanded_pat, binding_list)
	});
	self.source_map.pat_map.insert(src, pat);
	return pat;
	}
	None => Pat::Missing,
	},

rust-analyzer/crates/hir-def/src/body/lower.rs

Lines 941 to 963 in ffbc5ad

	fn collect_macro_call<T, U>(
	&mut self,
	mcall: ast::MacroCall,
	syntax_ptr: AstPtr<ast::MacroCall>,
	record_diagnostics: bool,
	collector: impl FnOnce(&mut Self, Option<T>) -> U,
	) -> U
	where
	T: ast::AstNode,
	{
	// File containing the macro call. Expansion errors will be attached here.
	let outer_file = self.expander.current_file_id();
	let macro_call_ptr = self.expander.in_file(syntax_ptr);
	let module = self.expander.module.local_id;
	let res = match self.def_map.modules[module]
	.scope
	.macro_invoc(InFile::new(outer_file, self.ast_id_map.ast_id_for_ptr(syntax_ptr)))
	{
	// fast path, macro call is in a block module
	Some(call) => Ok(self.expander.enter_expand_id(self.db, call)),
	None => self.expander.enter_expand(self.db, mcall, |path| {

but rustc uses expanded ast in the corresponding tuple-handling process, so it does not have macro patterns there.
https://github.com/rust-lang/rust/blob/62c068feeafd1f4abbf87243d69cf8862e4dd277/compiler/rustc_ast_lowering/src/pat.rs#L114

So, if a macro expansion in a tuple arg results in .., rustc permits it like plain .. pattern, but Rust Analyzer rejects it.
This is the root cause of #17292 and this PR allows macros expanded into .. in a tuple arg position work as ellipsis like that.

[Veykril]

I think something like

let ellipsis = None;
let mut args: Vec<_> = args.enumerate().filter(|(idx, p)| {
    	if matches!(p, ast::Pat::RestPath(_) {
    	    ellipsis = ellipsis.or(Some(idx));
    		return false
	    }
		true
	})
	.map(|(_, p)| self.collect_pat(p, binding_list))
    .collect();

should also work? Then we skip the extra allocation and duplication of the collect function

[ShoyuVanilla]

Sadly, this makes the test fail;

running 1 test
test handlers::mismatched_arg_count::tests::rest_pat_in_macro_expansion ... FAILED

failures:

---- handlers::mismatched_arg_count::tests::rest_pat_in_macro_expansion stdout ----
Code in range 700..788 = enum_str! {
    TupleVariant1(i32),
    TupleVariant2(),
    TupleVariant3(i8,u8,i128)
}
Code in range 700..788 = enum_str! {
    TupleVariant1(i32),
    TupleVariant2(),
    TupleVariant3(i8,u8,i128)
}
thread 'handlers::mismatched_arg_count::tests::rest_pat_in_macro_expansion' panicked at crates/ide-diagnostics/src/handlers/mismatched_arg_count.rs:261:9:
Diagnostic test failed.
False negatives: []
False positives: [(LineCol { line: 38, col: 0 }, 700..788, "error: this pattern has 1 field, but the corresponding tuple struct has 0 fields"), (LineCol { line: 38, col: 0 }, 700..788, "error: this pattern has 1 field, but
the corresponding tuple struct has 3 fields")]

because of the commented lines below;

    fn collect_tuple_pat(
        &mut self,
        args: AstChildren<ast::Pat>,
        has_leading_comma: bool,
        binding_list: &mut BindingList,
    ) -> (Box<[PatId]>, Option<usize>) {
        let mut ellipsis = None;
        let mut args: Vec<_> = args
            .enumerate()
            .filter(|(idx, p)| {
                if matches!(p, ast::Pat::RestPat(_)) {
                    ellipsis = ellipsis.or(Some(*idx));
                    return false;
                }
                // If the `p` is `ast::Pat::MacroPat` that will be expanded into ast::Pat::RestPat is not filtered here because it is not expanded yet
                true
            })
            .map(|(_, p)| self.collect_pat(p, binding_list))
            // It is expanded here in `collect_pat`,
            // but then it is lowered into `Pat::Missing` right after expansion
            // before it gets into our hands 😢 
            .collect();
        if has_leading_comma {
            args.insert(0, self.missing_pat());
        }

        (args.into_boxed_slice(), ellipsis)
    }

I'm not happy with my code for duplicates in collect_pat_possibly_rest too, but to avoid it I think that I should modify current collect_pat functions' signatures a lot and perhaps add a mutable boolean field inside Ctx to mark whether the RestPat is allowed or not in current collecting process (And I also think that it would worth to do so if that case is more desirable, of course)

[Veykril]

I'd definitely like if we could clean that up somehow though I am unsure how to as well, so I will merge this for now

[Veykril]

@bors r+

[bors]

📌 Commit b238855 has been approved by Veykril

It is now in the queue for this repository.

[bors]

⌛ Testing commit b238855 with merge 6f3030f...

[bors]

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 6f3030f to master...