Skip to content

Commit

Permalink
Rollup merge of rust-lang#135251 - oli-obk:push-lmpyvvyrtplk, r=ytmimi
Browse files Browse the repository at this point in the history
Only treat plain literal patterns as short

See rust-lang#134228 (comment) and rust-lang#134228 (comment) for context. We never wanted to treat const blocks and paths as short, only plain literals.

I don't know how to write a test for this, it.s not clear to me how the short pattern check actually affects the formatting
  • Loading branch information
jhpratt authored Jan 10, 2025
2 parents a6d38a1 + be92ac3 commit 0dcbda8
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 12 deletions.
37 changes: 25 additions & 12 deletions src/tools/rustfmt/src/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,31 @@ use crate::utils::{format_mutability, mk_sp, mk_sp_lo_plus_one, rewrite_ident};
/// - `[small, ntp]`
/// - unary tuple constructor `([small, ntp])`
/// - `&[small]`
pub(crate) fn is_short_pattern(pat: &ast::Pat, pat_str: &str) -> bool {
pub(crate) fn is_short_pattern(
context: &RewriteContext<'_>,
pat: &ast::Pat,
pat_str: &str,
) -> bool {
// We also require that the pattern is reasonably 'small' with its literal width.
pat_str.len() <= 20 && !pat_str.contains('\n') && is_short_pattern_inner(pat)
pat_str.len() <= 20 && !pat_str.contains('\n') && is_short_pattern_inner(context, pat)
}

fn is_short_pattern_inner(pat: &ast::Pat) -> bool {
match pat.kind {
ast::PatKind::Rest
| ast::PatKind::Never
| ast::PatKind::Wild
| ast::PatKind::Err(_)
| ast::PatKind::Expr(_) => true,
fn is_short_pattern_inner(context: &RewriteContext<'_>, pat: &ast::Pat) -> bool {
match &pat.kind {
ast::PatKind::Rest | ast::PatKind::Never | ast::PatKind::Wild | ast::PatKind::Err(_) => {
true
}
ast::PatKind::Expr(expr) => match &expr.kind {
ast::ExprKind::Lit(_) => true,
ast::ExprKind::Unary(ast::UnOp::Neg, expr) => match &expr.kind {
ast::ExprKind::Lit(_) => true,
_ => unreachable!(),
},
ast::ExprKind::ConstBlock(_) | ast::ExprKind::Path(..) => {
context.config.style_edition() <= StyleEdition::Edition2024
}
_ => unreachable!(),
},
ast::PatKind::Ident(_, _, ref pat) => pat.is_none(),
ast::PatKind::Struct(..)
| ast::PatKind::MacCall(..)
Expand All @@ -57,8 +70,8 @@ fn is_short_pattern_inner(pat: &ast::Pat) -> bool {
ast::PatKind::Box(ref p)
| PatKind::Deref(ref p)
| ast::PatKind::Ref(ref p, _)
| ast::PatKind::Paren(ref p) => is_short_pattern_inner(&*p),
PatKind::Or(ref pats) => pats.iter().all(|p| is_short_pattern_inner(p)),
| ast::PatKind::Paren(ref p) => is_short_pattern_inner(context, &*p),
PatKind::Or(ref pats) => pats.iter().all(|p| is_short_pattern_inner(context, p)),
}
}

Expand Down Expand Up @@ -96,7 +109,7 @@ impl Rewrite for Pat {
let use_mixed_layout = pats
.iter()
.zip(pat_strs.iter())
.all(|(pat, pat_str)| is_short_pattern(pat, pat_str));
.all(|(pat, pat_str)| is_short_pattern(context, pat, pat_str));
let items: Vec<_> = pat_strs.into_iter().map(ListItem::from_str).collect();
let tactic = if use_mixed_layout {
DefinitiveListTactic::Mixed
Expand Down
10 changes: 10 additions & 0 deletions src/tools/rustfmt/tests/source/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,13 @@ fn issue3728() {
| c;
foo((1,));
}

fn literals() {
match 42 {
const { 1 + 2 } | 4
| 6 => {}
10 | 11 | 12
| 13 | 14 => {}
_ => {}
}
}
8 changes: 8 additions & 0 deletions src/tools/rustfmt/tests/target/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,11 @@ fn issue3728() {
let foo = |(c,)| c;
foo((1,));
}

fn literals() {
match 42 {
const { 1 + 2 } | 4 | 6 => {}
10 | 11 | 12 | 13 | 14 => {}
_ => {}
}
}

0 comments on commit 0dcbda8

Please sign in to comment.