Skip to content

Commit 72d7fc2

Browse files
authored
Merge pull request #709 from wado-lang/claude/robust-formatter-design-YKPwu
Normalize impl syntax to always emit explicit type params
2 parents 96eca08 + 6b395f4 commit 72d7fc2

File tree

5 files changed

+597
-55
lines changed

5 files changed

+597
-55
lines changed

wado-compiler/src/unparse.rs

Lines changed: 29 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -710,68 +710,22 @@ impl<'a> Unparser<'a> {
710710

711711
/// Output an inherent impl type with type param bounds inlined into type args.
712712
/// E.g.: `impl<T: Ord> Array<T>` → `impl Array<T: Ord>`
713-
fn unparse_impl_inherent_type(&mut self, ty: &Type, type_params: &[crate::ast::GenericParam]) {
714-
let Type::Generic(g) = ty else {
715-
// No type args to inline into; just output the type normally
716-
self.unparse_type(ty);
717-
return;
718-
};
719-
720-
self.output.push_str(&g.name);
721-
self.output.push('<');
722-
for (i, arg) in g.args.iter().enumerate() {
723-
if i > 0 {
724-
self.output.push_str(", ");
725-
}
726-
// If this arg is a named type param with bounds, inline the bounds
727-
if let Type::Named(n) = arg
728-
&& let Some(param) = type_params.iter().find(|p| p.name == n.name)
729-
{
730-
self.output.push_str(&param.name);
731-
if !param.bounds.is_empty() {
732-
self.output.push_str(": ");
733-
for (j, bound) in param.bounds.iter().enumerate() {
734-
if j > 0 {
735-
self.output.push_str(" + ");
736-
}
737-
self.output.push_str(&bound.name);
738-
if !bound.assoc_types.is_empty() {
739-
self.output.push('<');
740-
for (k, assoc) in bound.assoc_types.iter().enumerate() {
741-
if k > 0 {
742-
self.output.push_str(", ");
743-
}
744-
self.output.push_str(&assoc.name);
745-
self.output.push_str(" = ");
746-
self.unparse_type(&assoc.ty);
747-
}
748-
self.output.push('>');
749-
}
750-
}
751-
}
752-
continue;
753-
}
754-
self.unparse_type(arg);
755-
}
756-
self.output.push('>');
757-
}
758-
759713
fn unparse_impl(&mut self, i: &ImplBlock) {
760714
self.write_indent();
761715
self.output.push_str("impl");
762716

717+
// Always emit explicit type params: `impl<T> Foo<T>`, not compact `impl Foo<T>`
718+
self.unparse_generic_params(&i.type_params);
719+
763720
// Handle `impl Trait for Type` vs `impl Type`
764721
if let Some(trait_type) = &i.trait_type {
765-
// Trait impl: use Rust-style generic params at `impl` level
766-
self.unparse_generic_params(&i.type_params);
767722
self.output.push(' ');
768723
self.unparse_type(trait_type);
769724
self.output.push_str(" for ");
770725
self.unparse_type(&i.ty);
771726
} else {
772-
// Inherent impl: use compact form with bounds inlined in type args
773727
self.output.push(' ');
774-
self.unparse_impl_inherent_type(&i.ty, &i.type_params);
728+
self.unparse_type(&i.ty);
775729
}
776730

777731
if i.is_synthesize_request {
@@ -1429,7 +1383,22 @@ impl<'a> Unparser<'a> {
14291383
self.unparse_expr(expr);
14301384
}
14311385
ConditionElement::Expr(expr) => {
1386+
// In a let-chain, elements are joined by `&&`.
1387+
// If this element is itself a `&&` or `||` expression,
1388+
// we must wrap it in parens to preserve the AST structure.
1389+
// Without parens, `let PAT = E && (a && b)` would be
1390+
// re-parsed as three chain elements instead of two.
1391+
let needs_parens = matches!(
1392+
expr,
1393+
Expr::Binary(b) if matches!(b.op, BinaryOp::And | BinaryOp::Or)
1394+
);
1395+
if needs_parens {
1396+
self.output.push('(');
1397+
}
14321398
self.unparse_expr(expr);
1399+
if needs_parens {
1400+
self.output.push(')');
1401+
}
14331402
}
14341403
}
14351404
}
@@ -3248,7 +3217,17 @@ fn unparse_condition_into(cond: &Condition, output: &mut String) {
32483217
unparse_expr_into(expr, output, false);
32493218
}
32503219
ConditionElement::Expr(expr) => {
3220+
let needs_parens = matches!(
3221+
expr,
3222+
Expr::Binary(b) if matches!(b.op, BinaryOp::And | BinaryOp::Or)
3223+
);
3224+
if needs_parens {
3225+
output.push('(');
3226+
}
32513227
unparse_expr_into(expr, output, false);
3228+
if needs_parens {
3229+
output.push(')');
3230+
}
32523231
}
32533232
}
32543233
}

wado-compiler/tests/format.fixtures.golden/all.clean.wado

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ struct MyArray<T> {
8686
}
8787

8888
/// Inherent impl: compact form with bounds inlined
89-
impl MyArray<T: Ord> {
89+
impl<T: Ord> MyArray<T> {
9090
/// Sort method
9191
fn sort(&mut self) {
9292
}

wado-compiler/tests/format.fixtures.golden/mess.clean.wado

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ struct MyArray<T> {
171171

172172
// Block comment /* */ before impl
173173
/* inherent impl with trait bound */
174-
impl MyArray<T: Ord> {
174+
impl<T: Ord> MyArray<T> {
175175
/* sort in ascending order */
176176
fn sort(&mut self) {
177177
}

wado-compiler/tests/format.fixtures.golden/no_prelude.clean.wado

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ variant Shape {
2626
}
2727

2828
/// Inherent impl: compact form with bounds inlined
29-
impl Array<T: Ord> {
29+
impl<T: Ord> Array<T> {
3030
/// Sort method
3131
fn sort(&mut self) {
3232
}
@@ -104,7 +104,7 @@ struct DnsError {
104104

105105
// Block comment /* */ before impl
106106
/* inherent impl with trait bound */
107-
impl Array<T: Ord> {
107+
impl<T: Ord> Array<T> {
108108
/* sort in ascending order */
109109
fn sort(&mut self) {
110110
}

0 commit comments

Comments
 (0)