Skip to content

Commit a8d58ee

Browse files
giacomocavalierilpil
authored andcommitted
Improve formatting of 'as' messages when using comments
1 parent a7fc4ad commit a8d58ee

File tree

2 files changed

+270
-22
lines changed

2 files changed

+270
-22
lines changed

compiler-core/src/format.rs

Lines changed: 86 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -971,7 +971,10 @@ impl<'comments> Formatter<'comments> {
971971
.append(" =")
972972
.append(self.assigned_value(value));
973973

974-
commented(self.append_as_message(doc, message), comments)
974+
commented(
975+
self.append_as_message(doc, PrecedingAs::Expression, message),
976+
comments,
977+
)
975978
}
976979

977980
fn expr<'a>(&mut self, expr: &'a UntypedExpr) -> Document<'a> {
@@ -980,17 +983,13 @@ impl<'comments> Formatter<'comments> {
980983
let document = match expr {
981984
UntypedExpr::Placeholder { .. } => panic!("Placeholders should not be formatted"),
982985

983-
UntypedExpr::Panic { message: None, .. } => "panic".to_doc(),
984-
UntypedExpr::Panic {
985-
message: Some(message),
986-
..
987-
} => docvec!["panic as ", self.expr(message)],
986+
UntypedExpr::Panic { message, .. } => {
987+
self.append_as_message("panic".to_doc(), PrecedingAs::Keyword, message.as_deref())
988+
}
988989

989-
UntypedExpr::Todo { message: None, .. } => "todo".to_doc(),
990-
UntypedExpr::Todo {
991-
message: Some(message),
992-
..
993-
} => docvec!["todo as ", self.expr(message)],
990+
UntypedExpr::Todo { message, .. } => {
991+
self.append_as_message("todo".to_doc(), PrecedingAs::Keyword, message.as_deref())
992+
}
994993

995994
UntypedExpr::Echo {
996995
expression,
@@ -2581,7 +2580,8 @@ impl<'comments> Formatter<'comments> {
25812580
self.expr(&assert.value)
25822581
};
25832582

2584-
let doc = self.append_as_message(expression, assert.message.as_ref());
2583+
let doc =
2584+
self.append_as_message(expression, PrecedingAs::Expression, assert.message.as_ref());
25852585
commented(docvec!["assert ", doc], comments)
25862586
}
25872587

@@ -2875,17 +2875,43 @@ impl<'comments> Formatter<'comments> {
28752875
fn append_as_message<'a>(
28762876
&mut self,
28772877
doc: Document<'a>,
2878+
preceding_as: PrecedingAs,
28782879
message: Option<&'a UntypedExpr>,
28792880
) -> Document<'a> {
28802881
let Some(message) = message else { return doc };
28812882

2882-
docvec![
2883-
doc.group(),
2884-
break_("", " ").nest(INDENT),
2885-
"as ",
2886-
self.expr(message).group().nest(INDENT)
2887-
]
2888-
.group()
2883+
let comments = self.pop_comments(message.location().start);
2884+
let comments = printed_comments(comments, false);
2885+
2886+
let as_ = match preceding_as {
2887+
PrecedingAs::Keyword => " as".to_doc(),
2888+
PrecedingAs::Expression => docvec![break_("", " "), "as"].nest(INDENT),
2889+
};
2890+
2891+
let doc = match comments {
2892+
// If there's comments between the document and the message we want
2893+
// the `as` bit to be on the same line as the original document and
2894+
// go on a new indented line with the message and comments:
2895+
// ```gleam
2896+
// todo as
2897+
// // comment!
2898+
// "wibble"
2899+
// ```
2900+
Some(comments) => docvec![
2901+
doc.group(),
2902+
as_,
2903+
docvec![line(), comments, line(), self.expr(message).group()].nest(INDENT)
2904+
],
2905+
2906+
None => docvec![
2907+
doc.group(),
2908+
as_,
2909+
" ",
2910+
self.expr(message).group().nest(INDENT),
2911+
],
2912+
};
2913+
2914+
doc.group()
28892915
}
28902916

28912917
fn echo<'a>(
@@ -2894,7 +2920,11 @@ impl<'comments> Formatter<'comments> {
28942920
message: &'a Option<Box<UntypedExpr>>,
28952921
) -> Document<'a> {
28962922
let Some(expression) = expression else {
2897-
return self.append_as_message("echo".to_doc(), message.as_deref());
2923+
return self.append_as_message(
2924+
"echo".to_doc(),
2925+
PrecedingAs::Keyword,
2926+
message.as_deref(),
2927+
);
28982928
};
28992929

29002930
// When a binary expression gets broken on multiple lines we don't want
@@ -2918,14 +2948,48 @@ impl<'comments> Formatter<'comments> {
29182948
//
29192949
let doc = self.expr(expression);
29202950
if expression.is_binop() || expression.is_pipeline() {
2921-
let doc = self.append_as_message(doc.nest(INDENT), message.as_deref());
2951+
let doc = self.append_as_message(
2952+
doc.nest(INDENT),
2953+
PrecedingAs::Expression,
2954+
message.as_deref(),
2955+
);
29222956
docvec!["echo ", doc]
29232957
} else {
2924-
docvec!["echo ", self.append_as_message(doc, message.as_deref())]
2958+
docvec![
2959+
"echo ",
2960+
self.append_as_message(doc, PrecedingAs::Expression, message.as_deref())
2961+
]
29252962
}
29262963
}
29272964
}
29282965

2966+
/// This is used to describe the kind of things that might preceding an `as`
2967+
/// message that can be added to various places: `panic`, `echo`, `let assert`,
2968+
/// `assert`, `todo`.
2969+
///
2970+
/// It might be preceded by a keyword, like with `echo` and `panic`, or by
2971+
/// an expression, like in `assert` or `let assert`.
2972+
///
2973+
enum PrecedingAs {
2974+
/// An expression is preceding the `as` message:
2975+
/// ```gleam
2976+
/// echo 1 as "message"
2977+
/// assert 1 == 2 as "message"
2978+
/// let assert Ok(_) = result as "message"
2979+
/// ```
2980+
///
2981+
Expression,
2982+
2983+
/// A keyword is preceding the `as` message:
2984+
/// ```gleam
2985+
/// 1 |> echo as "message"
2986+
/// panic as "message"
2987+
/// todo as "message"
2988+
/// ```
2989+
///
2990+
Keyword,
2991+
}
2992+
29292993
fn init_and_last<T>(vec: &[T]) -> Option<(&[T], &T)> {
29302994
match vec {
29312995
[] => None,

compiler-core/src/format/tests.rs

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6501,3 +6501,187 @@ fn comment_is_not_moved_after_assert() {
65016501
"
65026502
);
65036503
}
6504+
6505+
#[test]
6506+
fn todo_as_with_comment() {
6507+
assert_format!(
6508+
r#"pub fn main() {
6509+
todo as
6510+
// A little comment explaining something
6511+
"wibble"
6512+
}
6513+
"#
6514+
);
6515+
}
6516+
6517+
#[test]
6518+
fn todo_as_with_comment_on_the_same_line() {
6519+
assert_format_rewrite!(
6520+
r#"pub fn main() {
6521+
todo as // A little comment explaining something
6522+
"wibble"
6523+
}
6524+
"#,
6525+
r#"pub fn main() {
6526+
todo as
6527+
// A little comment explaining something
6528+
"wibble"
6529+
}
6530+
"#
6531+
);
6532+
}
6533+
6534+
#[test]
6535+
fn todo_as_with_comment_before_the_as() {
6536+
assert_format_rewrite!(
6537+
r#"pub fn main() {
6538+
todo // A little comment explaining something
6539+
as "wibble"
6540+
}
6541+
"#,
6542+
r#"pub fn main() {
6543+
todo as
6544+
// A little comment explaining something
6545+
"wibble"
6546+
}
6547+
"#
6548+
);
6549+
}
6550+
6551+
#[test]
6552+
fn panic_as_with_comment() {
6553+
assert_format!(
6554+
r#"pub fn main() {
6555+
panic as
6556+
// A little comment explaining something
6557+
"wibble"
6558+
}
6559+
"#
6560+
);
6561+
}
6562+
6563+
#[test]
6564+
fn panic_as_with_comment_on_the_same_line() {
6565+
assert_format_rewrite!(
6566+
r#"pub fn main() {
6567+
panic as // A little comment explaining something
6568+
"wibble"
6569+
}
6570+
"#,
6571+
r#"pub fn main() {
6572+
panic as
6573+
// A little comment explaining something
6574+
"wibble"
6575+
}
6576+
"#
6577+
);
6578+
}
6579+
6580+
#[test]
6581+
fn panic_as_with_comment_before_the_as() {
6582+
assert_format_rewrite!(
6583+
r#"pub fn main() {
6584+
panic // A little comment explaining something
6585+
as "wibble"
6586+
}
6587+
"#,
6588+
r#"pub fn main() {
6589+
panic as
6590+
// A little comment explaining something
6591+
"wibble"
6592+
}
6593+
"#
6594+
);
6595+
}
6596+
6597+
#[test]
6598+
fn echo_as_with_comment() {
6599+
assert_format!(
6600+
r#"pub fn main() {
6601+
echo 1 as
6602+
// A little comment explaining something
6603+
"wibble"
6604+
}
6605+
"#
6606+
);
6607+
}
6608+
6609+
#[test]
6610+
fn echo_as_with_comment_on_the_same_line() {
6611+
assert_format_rewrite!(
6612+
r#"pub fn main() {
6613+
echo 1 as // A little comment explaining something
6614+
"wibble"
6615+
}
6616+
"#,
6617+
r#"pub fn main() {
6618+
echo 1 as
6619+
// A little comment explaining something
6620+
"wibble"
6621+
}
6622+
"#
6623+
);
6624+
}
6625+
6626+
#[test]
6627+
fn echo_as_with_comment_before_the_as() {
6628+
assert_format_rewrite!(
6629+
r#"pub fn main() {
6630+
echo 1 // A little comment explaining something
6631+
as "wibble"
6632+
}
6633+
"#,
6634+
r#"pub fn main() {
6635+
echo 1 as
6636+
// A little comment explaining something
6637+
"wibble"
6638+
}
6639+
"#
6640+
);
6641+
}
6642+
6643+
#[test]
6644+
fn assert_as_with_comment() {
6645+
assert_format!(
6646+
r#"pub fn main() {
6647+
assert True as
6648+
// A little comment explaining something
6649+
"wibble"
6650+
}
6651+
"#
6652+
);
6653+
}
6654+
6655+
#[test]
6656+
fn assert_as_with_comment_on_the_same_line() {
6657+
assert_format_rewrite!(
6658+
r#"pub fn main() {
6659+
assert True as // A little comment explaining something
6660+
"wibble"
6661+
}
6662+
"#,
6663+
r#"pub fn main() {
6664+
assert True as
6665+
// A little comment explaining something
6666+
"wibble"
6667+
}
6668+
"#
6669+
);
6670+
}
6671+
6672+
#[test]
6673+
fn assert_as_with_comment_before_the_as() {
6674+
assert_format_rewrite!(
6675+
r#"pub fn main() {
6676+
assert True // A little comment explaining something
6677+
as "wibble"
6678+
}
6679+
"#,
6680+
r#"pub fn main() {
6681+
assert True as
6682+
// A little comment explaining something
6683+
"wibble"
6684+
}
6685+
"#
6686+
);
6687+
}

0 commit comments

Comments
 (0)