Skip to content

Commit

Permalink
change parsing rules for echo in pipelines
Browse files Browse the repository at this point in the history
  • Loading branch information
@giacomocavalieri
giacomocavalieri committed Oct 21, 2024
1 parent b6549d6 commit d380a57
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 192 deletions.
15 changes: 0 additions & 15 deletions compiler-core/src/erlang/tests/echo.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,6 @@ pub fn main() {
);
}

#[test]
pub fn echo_with_a_function_in_a_pipeline() {
assert_erl!(
r#"
pub fn main() {
[1, 2, 3]
|> echo wibble
|> wibble
}
pub fn wibble(n) { n }
"#
)
}

#[test]
pub fn echo_in_a_pipeline() {
assert_erl!(
Expand Down
129 changes: 12 additions & 117 deletions compiler-core/src/format/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6504,39 +6504,6 @@ fn echo_inside_a_pipeline() {
);
}

#[test]
fn echo_with_value_inside_a_pipeline() {
assert_format!(
r#"fn main() {
wibble
|> echo value
|> wobble
}
"#
);
}

#[test]
fn echo_with_long_value_inside_a_pipeline() {
assert_format_rewrite!(
r#"fn main() {
wibble
|> echo this_is_a_looooooooong_value(that_get_split_on_multiple_lines, i_wonder_how_this_will_look)
|> wobble
}
"#,
r#"fn main() {
wibble
|> echo this_is_a_looooooooong_value(
that_get_split_on_multiple_lines,
i_wonder_how_this_will_look,
)
|> wobble
}
"#,
);
}

#[test]
fn echo_inside_a_single_line_pipeline() {
assert_format!(
Expand All @@ -6547,35 +6514,6 @@ fn echo_inside_a_single_line_pipeline() {
);
}

#[test]
fn echo_with_value_inside_a_single_line_pipeline() {
assert_format!(
r#"fn main() {
wibble |> echo value |> wobble
}
"#
);
}

#[test]
fn echo_with_long_value_inside_a_single_line_pipeline() {
assert_format_rewrite!(
r#"fn main() {
wibble |> echo this_is_a_looooooooong_value(that_get_split_on_multiple_lines, i_wonder_how_this_will_look) |> wobble
}
"#,
r#"fn main() {
wibble
|> echo this_is_a_looooooooong_value(
that_get_split_on_multiple_lines,
i_wonder_how_this_will_look,
)
|> wobble
}
"#
);
}

#[test]
fn echo_as_last_item_of_pipeline() {
assert_format!(
Expand All @@ -6586,35 +6524,6 @@ fn echo_as_last_item_of_pipeline() {
);
}

#[test]
fn echo_with_value_as_last_item_of_pipeline() {
assert_format!(
r#"fn main() {
wibble |> wobble |> echo value
}
"#
);
}

#[test]
fn echo_with_long_value_as_last_item_of_pipeline() {
assert_format_rewrite!(
r#"fn main() {
wibble |> wobble |> echo this_is_a_looooooooong_value(that_get_split_on_multiple_lines, i_wonder_how_this_will_look)
}
"#,
r#"fn main() {
wibble
|> wobble
|> echo this_is_a_looooooooong_value(
that_get_split_on_multiple_lines,
i_wonder_how_this_will_look,
)
}
"#
);
}

#[test]
fn echo_as_last_item_of_multiline_pipeline() {
assert_format!(
Expand All @@ -6628,48 +6537,34 @@ fn echo_as_last_item_of_multiline_pipeline() {
}

#[test]
fn echo_with_value_as_last_item_of_multiline_pipeline() {
assert_format!(
r#"fn main() {
wibble
|> wobble
|> echo value
}
"#
);
}

#[test]
fn echo_with_long_value_as_last_item_of_multiline_pipeline() {
fn echo_with_related_expression_on_following_line() {
assert_format_rewrite!(
r#"fn main() {
wibble
|> wobble
|> echo this_is_a_looooooooong_value(that_get_split_on_multiple_lines, i_wonder_how_this_will_look)
panic as echo
"wibble"
}
"#,
r#"fn main() {
wibble
|> wobble
|> echo this_is_a_looooooooong_value(
that_get_split_on_multiple_lines,
i_wonder_how_this_will_look,
)
panic as echo "wibble"
}
"#
);
}

#[test]
fn echo_with_related_expression_on_following_line() {
fn echo_with_following_value_in_a_pipeline() {
assert_format_rewrite!(
r#"fn main() {
panic as echo
"wibble"
[]
|> echo wibble
|> wobble
}
"#,
r#"fn main() {
panic as echo "wibble"
[]
|> echo
wibble
|> wobble
}
"#
);
Expand Down
15 changes: 0 additions & 15 deletions compiler-core/src/javascript/tests/echo.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,6 @@ pub fn main() {
);
}

#[test]
pub fn echo_with_a_function_in_a_pipeline() {
assert_js!(
r#"
pub fn main() {
[1, 2, 3]
|> echo wibble
|> wibble
}
pub fn wibble(n) { n }
"#
)
}

#[test]
pub fn echo_in_a_pipeline() {
assert_js!(
Expand Down
58 changes: 44 additions & 14 deletions compiler-core/src/parse.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,15 @@ where
let mut estack = vec![];
let mut last_op_start = 0;
let mut last_op_end = 0;

// This is used to keep track if we've just ran into a `|>` operator in
// order to properly parse an echo based on its position: if it is in a
// pipeline then it isn't expected to be followed by an expression.
// Otherwise, it's expected to be followed by an expression.
let mut right_after_pipe = false;

loop {
match self.parse_expression_unit()? {
match self.parse_expression_unit(right_after_pipe)? {
Some(unit) => {
self.post_process_expression_unit(&unit, is_let_binding)?;
estack.push(unit)
Expand All @@ -420,6 +427,8 @@ where

if let Some((op_s, t, op_e)) = self.tok0.take() {
if let Some(p) = precedence(&t) {
right_after_pipe = t == Token::Pipe;

// Is Op
self.advance();
last_op_start = op_s;
Expand Down Expand Up @@ -473,7 +482,10 @@ where
// unit().unit().unit()
// A(a.., label: tuple(1))
// { expression_sequence }
fn parse_expression_unit(&mut self) -> Result<Option<UntypedExpr>, ParseError> {
fn parse_expression_unit(
&mut self,
right_after_pipe: bool,
) -> Result<Option<UntypedExpr>, ParseError> {
let mut expr = match self.tok0.take() {
Some((start, Token::String { value }, end)) => {
self.advance();
Expand Down Expand Up @@ -511,7 +523,7 @@ where
self.advance();
let mut message = None;
if self.maybe_one(&Token::As).is_some() {
let msg_expr = self.expect_expression_unit()?;
let msg_expr = self.expect_expression_unit(false)?;
end = msg_expr.location().end;
message = Some(Box::new(msg_expr));
}
Expand All @@ -526,7 +538,7 @@ where
self.advance();
let mut label = None;
if self.maybe_one(&Token::As).is_some() {
let msg_expr = self.expect_expression_unit()?;
let msg_expr = self.expect_expression_unit(false)?;
end = msg_expr.location().end;
label = Some(Box::new(msg_expr));
}
Expand All @@ -538,11 +550,26 @@ where

Some((start, Token::Echo, end)) => {
self.advance();
let expression = self.parse_expression_unit()?;
let end = expression.as_ref().map_or(end, |e| e.location().end);
UntypedExpr::Echo {
location: SrcSpan { start, end },
expression: expression.map(Box::new),
if right_after_pipe {
// If an echo is used as a step in a pipeline (`|> echo`)
// then it cannot be followed by an expression.
UntypedExpr::Echo {
location: SrcSpan { start, end },
expression: None,
}
} else {
// Otherwise it must be followed by an expression.
// However, you might have noticed we're not erroring if the
// expression is not there. Instead we move this error to
// the analysis phase so that a wrong usage of echo won't
// stop analysis from happening everywhere and be fault
// tolerant like everything else.
let expression = self.parse_expression_unit(false)?;
let end = expression.as_ref().map_or(end, |e| e.location().end);
UntypedExpr::Echo {
location: SrcSpan { start, end },
expression: expression.map(Box::new),
}
}
}

Expand Down Expand Up @@ -647,7 +674,7 @@ where
&|s| {
Parser::parse_bit_array_segment(
s,
&Parser::parse_expression_unit,
&(|this| this.parse_expression_unit(false)),
&Parser::expect_expression,
&bit_array_expr_int,
)
Expand Down Expand Up @@ -731,7 +758,7 @@ where
// Boolean negation
Some((start, Token::Bang, _end)) => {
self.advance();
match self.parse_expression_unit()? {
match self.parse_expression_unit(false)? {
Some(value) => UntypedExpr::NegateBool {
location: SrcSpan {
start,
Expand All @@ -751,7 +778,7 @@ where
// Int negation
Some((start, Token::Minus, _end)) => {
self.advance();
match self.parse_expression_unit()? {
match self.parse_expression_unit(false)? {
Some(value) => UntypedExpr::NegateInt {
location: SrcSpan {
start,
Expand Down Expand Up @@ -3093,8 +3120,11 @@ where
}
}

fn expect_expression_unit(&mut self) -> Result<UntypedExpr, ParseError> {
if let Some(e) = self.parse_expression_unit()? {
fn expect_expression_unit(
&mut self,
right_after_pipe: bool,
) -> Result<UntypedExpr, ParseError> {
if let Some(e) = self.parse_expression_unit(right_after_pipe)? {
Ok(e)
} else {
self.next_tok_unexpected(vec!["An expression".into()])
Expand Down
47 changes: 47 additions & 0 deletions ...rse/snapshots/gleam_core__parse__tests__echo_cannot_have_an_expression_in_a_pipeline.snap
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
---
source: compiler-core/src/parse/tests.rs
expression: "[] |> echo fun |> wibble"
---
[
Expression(
PipeLine {
expressions: [
List {
location: SrcSpan {
start: 0,
end: 2,
},
elements: [],
tail: None,
},
Echo {
location: SrcSpan {
start: 6,
end: 10,
},
expression: None,
},
],
},
),
Expression(
PipeLine {
expressions: [
Var {
location: SrcSpan {
start: 11,
end: 14,
},
name: "fun",
},
Var {
location: SrcSpan {
start: 18,
end: 24,
},
name: "wibble",
},
],
},
),
]
Loading

0 comments on commit d380a57

Please sign in to comment.