Skip to content

Commit

Permalink
Change Pattern to preserve user intent of ParensAround
Browse files Browse the repository at this point in the history
  • Loading branch information
@joshuawarner32
joshuawarner32 committed Jan 30, 2025
1 parent 689c58f commit e50fe80
Show file tree
Hide file tree
Showing 31 changed files with 351 additions and 253 deletions.
1 change: 1 addition & 0 deletions crates/check/can_solo/src/desugar.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1464,6 +1464,7 @@ fn desugar_pattern<'a>(
| MalformedIdent(_, _)
| MalformedExpr(_)
| QualifiedIdentifier { .. } => pattern,
ParensAround(inner) => desugar_pattern(env, scope, *inner),

Apply(tag, arg_patterns) => {
// Skip desugaring the tag, it should either be a Tag or OpaqueRef
Expand Down
2 changes: 1 addition & 1 deletion crates/compiler/can/src/pattern.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ pub fn canonicalize_pattern<'a>(
}
}

SpaceBefore(sub_pattern, _) | SpaceAfter(sub_pattern, _) => {
ParensAround(sub_pattern) | SpaceBefore(sub_pattern, _) | SpaceAfter(sub_pattern, _) => {
return canonicalize_pattern(
env,
var_store,
Expand Down
7 changes: 6 additions & 1 deletion crates/compiler/fmt/src/pattern.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ impl<'a> Formattable for PatternAs<'a> {

impl<'a> Formattable for Pattern<'a> {
fn is_multiline(&self) -> bool {
// Theory: a pattern should only be multiline when it contains a comment
match self {
Pattern::SpaceBefore(pattern, spaces) | Pattern::SpaceAfter(pattern, spaces) => {
debug_assert!(
Expand All @@ -58,6 +57,7 @@ impl<'a> Formattable for Pattern<'a> {

spaces.iter().any(|s| s.is_comment()) || pattern.is_multiline()
}
Pattern::ParensAround(inner) => inner.is_multiline(),

Pattern::RecordDestructure(fields) => fields.iter().any(|f| f.is_multiline()),
Pattern::RequiredField(_, subpattern) => subpattern.is_multiline(),
Expand Down Expand Up @@ -168,6 +168,9 @@ fn fmt_pattern_only(
buf.indent(indent);
buf.push_str(name);
}
Pattern::ParensAround(inner) => {
fmt_pattern(buf, inner, indent, parens);
}
Pattern::PncApply(loc_pattern, loc_arg_patterns) => {
pattern_fmt_apply(
buf,
Expand Down Expand Up @@ -664,6 +667,7 @@ fn pattern_prec(pat: Pattern<'_>) -> Prec {
| Pattern::Tuple(..)
| Pattern::List(..)
| Pattern::ListRest(_)
| Pattern::ParensAround(_)
| Pattern::PncApply(_, _) => Prec::Term,
Pattern::Apply(_, _) | Pattern::As(_, _) => Prec::Apply,
Pattern::SpaceBefore(inner, _) | Pattern::SpaceAfter(inner, _) => pattern_prec(*inner),
Expand Down Expand Up @@ -764,6 +768,7 @@ pub fn pattern_lift_spaces<'a, 'b: 'a>(
inner.after = merge_spaces(arena, inner.after, spaces);
inner
}
Pattern::ParensAround(expr) => pattern_lift_spaces(arena, expr),
_ => Spaces {
before: &[],
item: *pat,
Expand Down
8 changes: 5 additions & 3 deletions crates/compiler/parse/src/ast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1656,6 +1656,7 @@ pub enum Pattern<'a> {
// Space
SpaceBefore(&'a Pattern<'a>, &'a [CommentOrNewline<'a>]),
SpaceAfter(&'a Pattern<'a>, &'a [CommentOrNewline<'a>]),
ParensAround(&'a Pattern<'a>),

// Malformed
Malformed(&'a str),
Expand Down Expand Up @@ -1804,8 +1805,8 @@ impl<'a> Pattern<'a> {
false
}
}
SpaceBefore(x, _) | SpaceAfter(x, _) => match other {
SpaceBefore(y, _) | SpaceAfter(y, _) => x.equivalent(y),
SpaceBefore(x, _) | SpaceAfter(x, _) | ParensAround(x) => match other {
SpaceBefore(y, _) | SpaceAfter(y, _) | ParensAround(y) => x.equivalent(y),
y => x.equivalent(y),
},
Malformed(x) => {
Expand Down Expand Up @@ -2581,7 +2582,8 @@ impl<'a> Malformed for Pattern<'a> {
ListRest(_) =>false,
As(pat, _) => pat.is_malformed(),
SpaceBefore(pat, _) |
SpaceAfter(pat, _) => pat.is_malformed(),
SpaceAfter(pat, _) |
ParensAround(pat) => pat.is_malformed(),

Malformed(_) |
MalformedIdent(_, _) |
Expand Down
25 changes: 12 additions & 13 deletions crates/compiler/parse/src/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,14 @@ fn loc_expr_in_parens_help<'a>() -> impl Parser<'a, Loc<Expr<'a>>, EInParens<'a>
} else if elements.is_empty() {
Err((NoProgress, EInParens::Empty(state.pos())))
} else {
// TODO: don't discard comments before/after
// (stored in the Collection)
let inner = elements.items[0]
.value
.maybe_after(arena, elements.final_comments());
Ok((
MadeProgress,
Loc::at(
elements.items[0].region,
Expr::ParensAround(&elements.items[0].value),
Expr::ParensAround(arena.alloc(inner)),
),
state,
))
Expand Down Expand Up @@ -2083,15 +2084,7 @@ pub fn merge_spaces<'a>(
/// If the given Expr would parse the same way as a valid Pattern, convert it.
/// Example: (foo) could be either an Expr::Var("foo") or Pattern::Identifier("foo")
fn expr_to_pattern_help<'a>(arena: &'a Bump, expr: &Expr<'a>) -> Result<Pattern<'a>, ()> {
let mut expr = expr.extract_spaces();

while let Expr::ParensAround(loc_expr) = &expr.item {
let expr_inner = loc_expr.extract_spaces();

expr.before = merge_spaces(arena, expr.before, expr_inner.before);
expr.after = merge_spaces(arena, expr_inner.after, expr.after);
expr.item = expr_inner.item;
}
let expr = expr.extract_spaces();

let mut pat = match expr.item {
Expr::Var { module_name, ident } => {
Expand Down Expand Up @@ -2137,7 +2130,11 @@ fn expr_to_pattern_help<'a>(arena: &'a Bump, expr: &Expr<'a>) -> Result<Pattern<

Expr::Try => Pattern::Identifier { ident: "try" },

Expr::SpaceBefore(..) | Expr::SpaceAfter(..) | Expr::ParensAround(..) => unreachable!(),
Expr::ParensAround(inner) => {
Pattern::ParensAround(arena.alloc(expr_to_pattern_help(arena, inner)?))
}

Expr::SpaceBefore(..) | Expr::SpaceAfter(..) => unreachable!(),

Expr::Record(fields) => {
let patterns = fields.map_items_result(arena, |loc_assigned_field| {
Expand Down Expand Up @@ -3385,6 +3382,8 @@ fn starts_with_spaces_conservative(value: &Pattern<'_>) -> bool {
Pattern::RequiredField(_, _) | Pattern::OptionalField(_, _) => false,
Pattern::SpaceBefore(_, _) => true,
Pattern::SpaceAfter(inner, _) => starts_with_spaces_conservative(inner),
// The parens may disappear during formatting, so we conservatively assume they might have
Pattern::ParensAround(inner) => starts_with_spaces_conservative(inner),
Pattern::Malformed(_) | Pattern::MalformedIdent(_, _) | Pattern::MalformedExpr(_) => true,
}
}
Expand Down
8 changes: 7 additions & 1 deletion crates/compiler/parse/src/normalize.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,8 @@ impl<'a> Normalize<'a> for Expr<'a> {
},
Expr::When(a, b) => Expr::When(arena.alloc(a.normalize(arena)), b.normalize(arena)),
Expr::ParensAround(a) => {
// The formatter can remove redundant parentheses, so also remove these when normalizing for comparison.
// The formatter can remove redundant parentheses,
// so also remove these when normalizing for comparison.
a.normalize(arena)
}
Expr::MalformedIdent(a, b) => Expr::MalformedIdent(a, remove_spaces_bad_ident(b)),
Expand Down Expand Up @@ -937,6 +938,11 @@ impl<'a> Normalize<'a> for Pattern<'a> {
}
Pattern::SpaceBefore(a, _) => a.normalize(arena),
Pattern::SpaceAfter(a, _) => a.normalize(arena),
Pattern::ParensAround(a) => {
// The formatter can remove redundant parentheses,
// so also remove these when normalizing for comparison.
a.normalize(arena)
}
Pattern::SingleQuote(a) => Pattern::SingleQuote(a),
Pattern::List(pats) => Pattern::List(pats.normalize(arena)),
Pattern::Tuple(pats) => Pattern::Tuple(pats.normalize(arena)),
Expand Down
17 changes: 12 additions & 5 deletions crates/compiler/parse/src/pattern.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ fn loc_pattern_in_parens_help<'a>() -> impl Parser<'a, Loc<Pattern<'a>>, PInPare
byte(b')', PInParens::End),
Pattern::SpaceBefore,
)),
move |_arena, state, _, loc_elements| {
move |arena, state, _, loc_elements| {
let elements = loc_elements.value;
let region = loc_elements.region;

Expand All @@ -213,10 +213,17 @@ fn loc_pattern_in_parens_help<'a>() -> impl Parser<'a, Loc<Pattern<'a>>, PInPare
} else if elements.is_empty() {
Err((NoProgress, PInParens::Empty(state.pos())))
} else {
// TODO: don't discard comments before/after
// (stored in the Collection)
// TODO: add Pattern::ParensAround to faithfully represent the input
Ok((MadeProgress, elements.items[0], state))
let inner = elements.items[0]
.value
.maybe_after(arena, elements.final_comments());
Ok((
MadeProgress,
Loc::at(
elements.items[0].region,
Pattern::ParensAround(arena.alloc(inner)),
),
state,
))
}
},
)
Expand Down
20 changes: 11 additions & 9 deletions ...compiler/test_syntax/tests/snapshots/pass/ann_pattern_comment_before_body.expr.result-ast
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,17 @@
],
value_defs: [
Body(
@7-8 SpaceBefore(
Identifier {
ident: "s",
},
[
LineComment(
"",
),
],
@7-8 ParensAround(
SpaceBefore(
Identifier {
ident: "s",
},
[
LineComment(
"",
),
],
),
),
@10-11 Var {
module_name: "",
Expand Down
58 changes: 31 additions & 27 deletions crates/compiler/test_syntax/tests/snapshots/pass/arg_pattern_as.expr.result-ast
View file
Original file line number Diff line number Diff line change
@@ -1,37 +1,41 @@
@0-64 SpaceAfter(
Closure(
[
@2-19 As(
@2-10 RecordDestructure(
[
@4-5 Identifier {
ident: "x",
},
@7-8 Identifier {
ident: "y",
},
],
@2-19 ParensAround(
As(
@2-10 RecordDestructure(
[
@4-5 Identifier {
ident: "x",
},
@7-8 Identifier {
ident: "y",
},
],
),
PatternAs {
spaces_before: [],
identifier: @14-19 "point",
},
),
PatternAs {
spaces_before: [],
identifier: @14-19 "point",
},
),
@23-47 As(
@23-38 Apply(
@23-32 OpaqueRef(
"@Location",
@23-47 ParensAround(
As(
@23-38 Apply(
@23-32 OpaqueRef(
"@Location",
),
[
@33-38 Identifier {
ident: "inner",
},
],
),
[
@33-38 Identifier {
ident: "inner",
},
],
PatternAs {
spaces_before: [],
identifier: @42-47 "outer",
},
),
PatternAs {
spaces_before: [],
identifier: @42-47 "outer",
},
),
],
@56-64 SpaceBefore(
Expand Down
6 changes: 4 additions & 2 deletions ...compiler/test_syntax/tests/snapshots/pass/closure_arg_parens_then_comment.expr.result-ast
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
@0-7 SpaceAfter(
Closure(
[
@2-3 NumLiteral(
"8",
@2-3 ParensAround(
NumLiteral(
"8",
),
),
],
@6-7 Tag(
Expand Down
16 changes: 9 additions & 7 deletions .../compiler/test_syntax/tests/snapshots/pass/closure_parens_double_newlines.expr.result-ast
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@
),
[
@4-5 SpaceAfter(
SpaceAfter(
Identifier {
ident: "z",
},
[
Newline,
],
ParensAround(
SpaceAfter(
Identifier {
ident: "z",
},
[
Newline,
],
),
),
[
Newline,
Expand Down
20 changes: 11 additions & 9 deletions ...es/compiler/test_syntax/tests/snapshots/pass/comment_before_pat_in_parens.expr.result-ast
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,18 @@
type_defs: [],
value_defs: [
Annotation(
@4-5 SpaceBefore(
NumLiteral(
"6",
),
[
Newline,
LineComment(
"",
@4-5 ParensAround(
SpaceBefore(
NumLiteral(
"6",
),
],
[
Newline,
LineComment(
"",
),
],
),
),
@7-8 BoundVariable(
"s",
Expand Down
16 changes: 11 additions & 5 deletions crates/compiler/test_syntax/tests/snapshots/pass/comment_indent_in_parens.expr.result-ast
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,22 @@
),
[
@2-7 SpaceAfter(
NumLiteral(
"0",
ParensAround(
SpaceAfter(
NumLiteral(
"0",
),
[
LineComment(
"",
),
],
),
),
[
LineComment(
"",
),
LineComment(
"",
),
],
),
],
Expand Down
Loading

0 comments on commit e50fe80

Please sign in to comment.