Skip to content

Commit

Permalink
fix(formatter): add support for multiline type expressions (#3271)
Browse files Browse the repository at this point in the history 
There are two changes as part of this fix:

1. Several of the parser functions for type expressions could not handle
trailing commas, they have been updated.
2. The formatting functions for type expressions now match the multiline
behavior of the equivalent literal formatting
  • Loading branch information
@nathanielc
nathanielc authored Oct 23, 2020
1 parent a8915a4 commit 6075f35
Show file tree
Hide file tree
Showing 5 changed files with 350 additions and 128 deletions.
8 changes: 4 additions & 4 deletions libflux/go/libflux/buildinfo.gen.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ var sourceHashes = map[string]string{
"libflux/src/core/ast/walk/mod.rs": "855701066e8d11a620855d2db986ce3df27555a91e2fcaf7a0cd3b12aa4cc4f0",
"libflux/src/core/ast/walk/tests.rs": "f7b2d7dd5643bb795a86c04b6979b136b0de46b52b213caff094aed6d204a05d",
"libflux/src/core/build.rs": "7e8c3626b9034dc4a31c2b748b2a174260949d852455299d071a0fd128c18a5a",
"libflux/src/core/formatter/mod.rs": "1224b0712cfc846b63f811fd8a25a920a99761eaeb203d7dd7ef900165977a57",
"libflux/src/core/formatter/tests.rs": "1257c154505d5351df67332d83b8fb1f8bae6e16a47be27dfb5e0869726b87d6",
"libflux/src/core/formatter/mod.rs": "d4698c4dd480a9fda8b67abc783127370890d1e1a0a0e7b8fba5a1030b9ce809",
"libflux/src/core/formatter/tests.rs": "57ceffbb94160ed7c4f092148253b086315a6654488fdfe895727789dc93f59f",
"libflux/src/core/lib.rs": "ee7c9cfd10e82f1127ac988a28c2f78dfe031c6cd37d236401e2997b22bcdab5",
"libflux/src/core/parser/mod.rs": "59bffe16d75270ee49d14cab13123a992f7c501a1ddf1c5c9c027c3eb1bcb861",
"libflux/src/core/parser/mod.rs": "8109f5152bf181ae22ff8da59a2d65c431d5b34efecb2a32fa2241d8c0707011",
"libflux/src/core/parser/strconv.rs": "748c82f6efc2eafaafb872db5b4185ce29aafa8f03ba02c4b84f4a9614e832d2",
"libflux/src/core/parser/tests.rs": "812fdcec360698b8ca2f74be00122146caaf7d1b7c658693f18f5f0780e43d26",
"libflux/src/core/parser/tests.rs": "539590f6d586730e6aa71bdc646d5f697bb1cf954b4cee86938debe85b6a33dd",
"libflux/src/core/scanner/mod.rs": "b99242c81a244fc8c1a925e3657fc8f80c5d23d2a9020b750b59ced9de560c5a",
"libflux/src/core/scanner/scanner.h": "1c2c1fda1d170efd6e1d2efd4c6d5589972656880c240e8ba2aa12e44b55cc87",
"libflux/src/core/scanner/scanner.rl": "7c1d866dec7989d85cd5199ec670d20012305f45850688233a119c355212ea36",
Expand Down
177 changes: 108 additions & 69 deletions libflux/src/core/formatter/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ impl Formatter {
}

let mut prev: i8 = -1;
for (i, value) in n.body.iter().enumerate() {
let cur = n.body.get(i).unwrap().typ();
for (i, stmt) in (&n.body).iter().enumerate() {
let cur = stmt.typ();
if i != 0 {
self.write_rune(sep);
// separate different statements with double newline
Expand All @@ -160,7 +160,7 @@ impl Formatter {
}
}
self.write_indent();
self.format_node(&Node::from_stmt(value));
self.format_node(&Node::from_stmt(stmt));
prev = cur;
}

Expand Down Expand Up @@ -247,7 +247,7 @@ impl Formatter {
self.format_identifier(&n.id);
self.format_comments(&n.colon);
if n.colon == None {
self.write_string(" ");
self.write_rune(' ');
}
self.write_string(": ");
self.format_type_expression(&n.ty);
Expand All @@ -256,37 +256,75 @@ impl Formatter {
fn format_type_expression(&mut self, n: &ast::TypeExpression) {
self.format_monotype(&n.monotype);
if !n.constraints.is_empty() {
self.write_string(" where ");
self.format_constraints(&n.constraints);
let multiline = n.constraints.len() > 4 || n.base.is_multiline();
self.write_string(" where");
if multiline {
self.write_rune('\n');
self.indent();
self.write_indent();
} else {
self.write_rune(' ');
}
let sep = match multiline {
true => ",\n",
false => ", ",
};
for (i, c) in (&n.constraints).iter().enumerate() {
if i != 0 {
self.write_string(sep);
if multiline {
self.write_indent();
}
}
self.format_constraint(c);
}
if multiline {
self.unindent();
}
}
}

fn format_monotype(&mut self, n: &ast::MonoType) {
match n {
ast::MonoType::Tvar(tv) => self.format_tvar(tv),
ast::MonoType::Basic(nt) => self.format_basic(&nt),
ast::MonoType::Array(arr) => self.format_array(&*arr),
ast::MonoType::Record(rec) => self.format_record(&rec),
ast::MonoType::Function(fun) => self.format_function(&*fun),
ast::MonoType::Basic(nt) => self.format_basic_type(&nt),
ast::MonoType::Array(arr) => self.format_array_type(&*arr),
ast::MonoType::Record(rec) => self.format_record_type(&rec),
ast::MonoType::Function(fun) => self.format_function_type(&*fun),
}
}
fn format_function(&mut self, n: &ast::FunctionType) {
self.write_string("(");
if !n.parameters.is_empty() {
self.format_parameters(&n.parameters)
fn format_function_type(&mut self, n: &ast::FunctionType) {
let multiline = n.parameters.len() > 4 || n.base.is_multiline();
self.format_comments(&n.base.comments);
self.write_rune('(');
if multiline {
self.write_rune('\n');
self.indent();
self.write_indent();
}
self.write_string(")");
self.write_string(" => ");
self.format_monotype(&n.monotype)
}
fn format_parameters(&mut self, n: &[ast::ParameterType]) {
self.format_parameter(&n[0]);
for p in &n[1..] {
self.write_string(", ");
self.format_parameter(p);
let sep = match multiline {
true => ",\n",
false => ", ",
};
for (i, p) in (&n.parameters).iter().enumerate() {
if i != 0 {
self.write_string(sep);
if multiline {
self.write_indent();
}
}
self.format_parameter_type(p);
}
if multiline {
self.write_string(sep);
self.unindent();
self.write_indent();
}
self.write_rune(')');
self.write_string(" => ");
self.format_monotype(&n.monotype);
}
fn format_parameter(&mut self, n: &ast::ParameterType) {
fn format_parameter_type(&mut self, n: &ast::ParameterType) {
match &n {
ast::ParameterType::Required {
base: _,
Expand All @@ -302,7 +340,7 @@ impl Formatter {
name,
monotype,
} => {
self.write_string("?");
self.write_rune('?');
self.format_identifier(&name);
self.write_string(": ");
self.format_monotype(&monotype);
Expand All @@ -322,49 +360,55 @@ impl Formatter {
}
}
}
fn format_record(&mut self, n: &ast::RecordType) {
self.write_string("{");
match &n.tvar {
Some(tv) => {
self.format_identifier(tv);
self.write_string(" with ");
self.format_properties(&n.properties);
fn format_record_type(&mut self, n: &ast::RecordType) {
let multiline = n.properties.len() > 4 || n.base.is_multiline();
self.format_comments(&n.base.comments);
self.write_rune('{');
if let Some(tv) = &n.tvar {
self.format_identifier(tv);
self.write_string(" with");
if !multiline {
self.write_rune(' ');
}
None => {
if !n.properties.is_empty() {
self.format_properties(&n.properties);
}
if multiline {
self.write_rune('\n');
self.indent();
self.write_indent();
}
let sep = match multiline {
true => ",\n",
false => ", ",
};
for (i, p) in (&n.properties).iter().enumerate() {
if i != 0 {
self.write_string(sep);
if multiline {
self.write_indent();
}
}
self.format_property_type(p);
}
self.write_string("}");
}
fn format_properties(&mut self, n: &[ast::PropertyType]) {
self.format_property_type(&n[0]);
for p in &n[1..] {
self.write_string(", ");
self.format_property_type(&p);
if multiline {
self.write_string(sep);
self.unindent();
self.write_indent();
}
self.write_rune('}');
}
fn format_property_type(&mut self, n: &ast::PropertyType) {
self.format_identifier(&n.name);
self.write_string(": ");
self.format_monotype(&n.monotype);
}
fn format_array(&mut self, n: &ast::ArrayType) {
self.write_string("[");
fn format_array_type(&mut self, n: &ast::ArrayType) {
self.write_rune('[');
self.format_monotype(&n.element);
self.write_string("]");
self.write_rune(']');
}
fn format_basic(&mut self, n: &ast::NamedType) {
fn format_basic_type(&mut self, n: &ast::NamedType) {
self.format_identifier(&n.name);
}
fn format_constraints(&mut self, n: &[ast::TypeConstraint]) {
self.format_constraint(&n[0]);
for c in &n[1..] {
self.write_string(", ");
self.format_constraint(c);
}
}
fn format_constraint(&mut self, n: &ast::TypeConstraint) {
self.format_identifier(&n.tvar);
self.write_string(": ");
Expand Down Expand Up @@ -394,12 +438,11 @@ impl Formatter {
self.format_comments(&n.lparen);
self.write_rune('(');
let sep = ", ";
for (i, _) in n.params.iter().enumerate() {
for (i, property) in (&n.params).iter().enumerate() {
if i != 0 {
self.write_string(sep)
}
// treat properties differently than in general case
let property = n.params.get(i).unwrap();
self.format_function_argument(property);
self.format_comments(&property.comma);
}
Expand Down Expand Up @@ -495,18 +538,17 @@ impl Formatter {
fn format_interpolated_part(&mut self, n: &ast::InterpolatedPart) {
self.write_string("${");
self.format_node(&Node::from_expr(&n.expression));
self.write_string("}")
self.write_rune('}')
}

fn format_array_expression(&mut self, n: &ast::ArrayExpr) {
self.format_comments(&n.lbrack);
self.write_rune('[');
let sep = ", ";
for (i, _) in n.elements.iter().enumerate() {
for (i, item) in (&n.elements).iter().enumerate() {
if i != 0 {
self.write_string(sep)
}
let item = n.elements.get(i).unwrap();
self.format_node(&Node::from_expr(&item.expression));
self.format_comments(&item.comma);
}
Expand All @@ -532,8 +574,8 @@ impl Formatter {
}

let mut prev: i8 = -1;
for (i, smt) in n.body.iter().enumerate() {
let cur = smt.typ();
for (i, stmt) in n.body.iter().enumerate() {
let cur = stmt.typ();
self.write_rune(sep);

if i != 0 {
Expand All @@ -543,7 +585,7 @@ impl Formatter {
}
}
self.write_indent();
self.format_node(&Node::from_stmt(smt));
self.format_node(&Node::from_stmt(stmt));
prev = cur;
}
if !n.body.is_empty() {
Expand Down Expand Up @@ -698,20 +740,17 @@ impl Formatter {
self.indent();
self.write_indent();
}
let sep: &str;
if multiline {
sep = ",\n"
} else {
sep = ", "
}
for (i, _) in n.properties.iter().enumerate() {
let sep = match multiline {
true => ",\n",
false => ", ",
};
for (i, property) in (&n.properties).iter().enumerate() {
if i != 0 {
self.write_string(sep);
if multiline {
self.write_indent()
}
}
let property = n.properties.get(i).unwrap();
self.format_node(&Node::Property(property));
self.format_comments(&property.comma);
}
Expand Down
51 changes: 51 additions & 0 deletions libflux/src/core/formatter/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -602,3 +602,54 @@ fn parens() {
assert_unchanged("() => ({_value: 1})");
assert_unchanged("() => \n\t// comment\n\t({_value: 1})");
}

#[test]
fn type_expressions() {
assert_unchanged(r#"builtin foo : (a: int, b: string) => int"#);
assert_unchanged(
r#"builtin foo : (
a: int,
b: string,
) => int"#,
);
assert_unchanged(r#"builtin foo : {a: int, b: string}"#);
assert_unchanged(
r#"builtin foo : {
a: int,
b: string,
}"#,
);
assert_unchanged(
r#"builtin foo : {X with
a: int,
b: string,
}"#,
);
assert_format(
r#"builtin foo : {a: A, b: B, c: C, d: D, e: E} where A: Numeric, B: Numeric, C: Numeric, D: Numeric, E: Numeric"#,
r#"builtin foo : {
a: A,
b: B,
c: C,
d: D,
e: E,
} where
A: Numeric,
B: Numeric,
C: Numeric,
D: Numeric,
E: Numeric"#,
);
assert_unchanged(
r#"builtin foo : (
a: int,
b: string,
c: A,
d: [int],
e: [[B]],
fn: () => int,
) => {x with a: int, b: string} where
A: Timeable,
B: Record"#,
);
}
Loading

0 comments on commit 6075f35

Please sign in to comment.