Prevent ranges from getting collapsed in patterns

[ytmimi]

Fixes #6869

Expression rewriting handled this correctly, but there was distinct logic implemented for patterns. I've extracted the range formatting logic used by expressions into a common rewrite_range function and reused that when rewriting ranges in patterns and types. Was able to remove a nice amount of code after these changes were done.

PR is broken up into logical commits. Feel free to review the PR as a whole or commit by commit.

[ytmimi]

Just to be sure these changes wouldn't impact things I ran the Diff-Check --edition=2021 --style-edition=2021. Things seem to be working as expected.

[matthewhughes934]

nice refactor

View changes since this review

[matthewhughes934]

discussion/question: I'm still learning my way around Rust, is there generally a preference for the symbolic/explicit (*&) approach vs. things like Box<T, A>.as_ref?

Suggested change

	lhs.as_deref().map(|x| &(*x.value)),
	rhs.as_deref().map(|x| &(*x.value)),
	lhs.as_deref().map(|x| x.value.as_ref()),
	rhs.as_deref().map(|x| x.value.as_ref()),

[ytmimi]

&(*x) makes it explicit that you're taking a reference to an object after dereferencing a pointer. No real preference for me so I'll update this to use as_ref. Thanks for flagging.

[matthewhughes934]

for a followup, since this is all moved code, but I think the ref/de-refer-ing in this block could be simplified

Suggested change

	match (lhs.as_ref().map(|x| &**x), rhs.as_ref().map(|x| &**x)) {
	(Some(lhs), Some(rhs)) => {
	let sp_delim = if context.config.spaces_around_ranges() {
	format!(" {delim} ")
	} else {
	default_sp_delim(Some(lhs), Some(rhs))
	};
	rewrite_pair(
	&*lhs,
	&*rhs,
	PairParts::infix(&sp_delim),
	context,
	shape,
	context.config.binop_separator(),
	)
	}
	(None, Some(rhs)) => {
	let sp_delim = if context.config.spaces_around_ranges() {
	format!("{delim} ")
	} else {
	default_sp_delim(None, Some(rhs))
	};
	rewrite_unary_prefix(context, &sp_delim, &*rhs, shape)
	}
	(Some(lhs), None) => {
	let sp_delim = if context.config.spaces_around_ranges() {
	format!(" {delim}")
	} else {
	default_sp_delim(Some(lhs), None)
	};
	rewrite_unary_suffix(context, &sp_delim, &*lhs, shape)
	}
	(None, None) => Ok(delim.to_owned()),
	}
	match (lhs, rhs) {
	(Some(lhs), Some(rhs)) => {
	let sp_delim = if context.config.spaces_around_ranges() {
	format!(" {delim} ")
	} else {
	default_sp_delim(Some(lhs), Some(rhs))
	};
	rewrite_pair(
	lhs,
	rhs,
	PairParts::infix(&sp_delim),
	context,
	shape,
	context.config.binop_separator(),
	)
	}
	(None, Some(rhs)) => {
	let sp_delim = if context.config.spaces_around_ranges() {
	format!("{delim} ")
	} else {
	default_sp_delim(None, Some(&rhs))
	};
	rewrite_unary_prefix(context, &sp_delim, rhs, shape)
	}
	(Some(lhs), None) => {
	let sp_delim = if context.config.spaces_around_ranges() {
	format!(" {delim}")
	} else {
	default_sp_delim(Some(&lhs), None)
	};
	rewrite_unary_suffix(context, &sp_delim, lhs, shape)
	}
	(None, None) => Ok(delim.to_owned()),
	}

View changes since the review

[ytmimi]

Good catch. The lhs and the rhs are Option<&ast::Expr> so we can definitely simplify this. We actually don't need to specify any & or explicitly deference anything. I'll add a commit

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.