From 980f534b55c6eab7ef047a0c3b5f0aa36ec91861 Mon Sep 17 00:00:00 2001 From: Kennedy Tedesco Date: Sun, 15 Jan 2023 23:23:49 -0300 Subject: [PATCH 1/2] new: parse short `match` correctly --- .../internal/expression/control_flow.rs | 29 +- src/parser/issue.rs | 39 --- src/tree/expression/control_flow.rs | 8 +- tests/samples/0070/tree.txt | 40 +-- tests/samples/0106/code.ara | 9 + tests/samples/0106/tree.txt | 263 ++++++++++++++++++ 6 files changed, 311 insertions(+), 77 deletions(-) create mode 100644 tests/samples/0106/code.ara create mode 100644 tests/samples/0106/tree.txt diff --git a/src/parser/internal/expression/control_flow.rs b/src/parser/internal/expression/control_flow.rs index 020253f..73abb57 100644 --- a/src/parser/internal/expression/control_flow.rs +++ b/src/parser/internal/expression/control_flow.rs @@ -10,20 +10,27 @@ use crate::tree::expression::control_flow::MatchExpression; use crate::tree::utils::CommaSeparated; pub fn match_expression(state: &mut State) -> ParseResult { + let r#match = utils::skip_keyword(state, TokenKind::Match)?; + + let expression = if state.iterator.current().kind == TokenKind::LeftBrace { + None + } else { + Some(Box::new(expression::create(state)?)) + }; + Ok(MatchExpression { comments: state.iterator.comments(), - r#match: utils::skip_keyword(state, TokenKind::Match)?, - expression: Box::new(expression::create(state)?), + r#match, + expression, body: MatchBodyExpression { left_brace: utils::skip_left_brace(state)?, arms: { let mut items = Vec::new(); let mut commas = Vec::new(); - let mut default_arm = None; while state.iterator.current().kind != TokenKind::RightBrace { let current = state.iterator.current(); - let (condition, default) = if current.kind == TokenKind::Default { + let (condition, _default) = if current.kind == TokenKind::Default { ( MatchArmConditionExpression::Default(utils::skip_keyword( state, @@ -48,20 +55,6 @@ pub fn match_expression(state: &mut State) -> ParseResult { expression: expression::create(state)?, }; - if default { - if let Some(default_arm) = &default_arm { - crate::parser_report!( - state, - match_expression_cannot_have_multiple_default_arms( - default_arm, - &arm, - ) - ); - } else { - default_arm = Some(arm.clone()); - } - } - items.push(arm); let current = state.iterator.current(); diff --git a/src/parser/issue.rs b/src/parser/issue.rs index 9891608..c8654e0 100644 --- a/src/parser/issue.rs +++ b/src/parser/issue.rs @@ -347,25 +347,6 @@ pub enum ParserIssueCode { /// - Remove the modifier ModifierCannotBeUsedOnInterfaceConstant = 22, - /// Match expression cannot have multiple default arms ( code = 23 ) - /// - /// Example: - /// - /// ```ara - /// function foo(string $input): string { - /// match $input { - /// "foo" => "bar", - /// default => "baz", - /// default => "qux", - /// } - /// } - /// ``` - /// - /// Possible solution(s): - /// - /// - Remove one of the default arms - MatchExpressionCannotHaveMultipleDefaultArms = 23, - /// Promoted property cannot be variadic ( code = 24 ) /// /// Example: @@ -919,26 +900,6 @@ pub(crate) fn modifier_cannot_be_used_on_interface_constant( .with_note("only the `final`, and `public` modifiers can be used on an interface constant.") } -pub(crate) fn match_expression_cannot_have_multiple_default_arms( - state: &ParserState, - first: &dyn Node, - second: &dyn Node, -) -> Issue { - let origin = state.source.name(); - - Issue::error( - ParserIssueCode::MatchExpressionCannotHaveMultipleDefaultArms, - "match expression cannot have multiple default arms", - ) - .with_source(origin, second.initial_position(), second.final_position()) - .with_annotation(Annotation::primary( - origin, - first.initial_position(), - first.final_position(), - )) - .with_note("a match expression can only have one default arm") -} - pub(crate) fn promoted_property_cannot_be_variadic( state: &ParserState, class_name: Option<&Identifier>, diff --git a/src/tree/expression/control_flow.rs b/src/tree/expression/control_flow.rs index bcd391a..eafebb3 100644 --- a/src/tree/expression/control_flow.rs +++ b/src/tree/expression/control_flow.rs @@ -13,7 +13,7 @@ use crate::tree::Node; pub struct MatchExpression { pub comments: CommentGroup, pub r#match: Keyword, - pub expression: Box, + pub expression: Option>, pub body: MatchBodyExpression, } @@ -54,7 +54,11 @@ impl Node for MatchExpression { } fn children(&self) -> Vec<&dyn Node> { - vec![&self.r#match, self.expression.as_ref(), &self.body] + let mut children: Vec<&dyn Node> = vec![&self.r#match, &self.body]; + if let Some(expression) = &self.expression { + children.push(expression.as_ref()); + } + children } fn get_description(&self) -> String { diff --git a/tests/samples/0070/tree.txt b/tests/samples/0070/tree.txt index e7ec354..6451482 100644 --- a/tests/samples/0070/tree.txt +++ b/tests/samples/0070/tree.txt @@ -1388,11 +1388,13 @@ DefinitionTree { value: "match", position: 669, }, - expression: Variable( - Variable { - position: 675, - name: "$a", - }, + expression: Some( + Variable( + Variable { + position: 675, + name: "$a", + }, + ), ), body: MatchBodyExpression { left_brace: 678, @@ -1524,20 +1526,22 @@ DefinitionTree { value: "match", position: 751, }, - expression: Parenthesized( - ParenthesizedExpression { - comments: CommentGroup { - comments: [], - }, - left_parenthesis: 757, - expression: Variable( - Variable { - position: 758, - name: "$a", + expression: Some( + Parenthesized( + ParenthesizedExpression { + comments: CommentGroup { + comments: [], }, - ), - right_parenthesis: 760, - }, + left_parenthesis: 757, + expression: Variable( + Variable { + position: 758, + name: "$a", + }, + ), + right_parenthesis: 760, + }, + ), ), body: MatchBodyExpression { left_brace: 762, diff --git a/tests/samples/0106/code.ara b/tests/samples/0106/code.ara new file mode 100644 index 0000000..e18f655 --- /dev/null +++ b/tests/samples/0106/code.ara @@ -0,0 +1,9 @@ +function bar(): void { + $c = match { + $a == $b => 'a is equal to b', + $a > $b => 'a is greater than b', + $a < $b => 'a is less than b', + default => 'a and b are not comparable', + default => 'a and b are not comparable', // The analyzer should warn about this + }; +} diff --git a/tests/samples/0106/tree.txt b/tests/samples/0106/tree.txt new file mode 100644 index 0000000..4425efc --- /dev/null +++ b/tests/samples/0106/tree.txt @@ -0,0 +1,263 @@ +DefinitionTree { + definitions: [ + Function( + FunctionDefinition { + comments: CommentGroup { + comments: [], + }, + attributes: [], + function: Keyword { + value: "function", + position: 0, + }, + name: Identifier { + position: 9, + value: "bar", + }, + templates: None, + parameters: FunctionLikeParameterListDefinition { + comments: CommentGroup { + comments: [], + }, + left_parenthesis: 12, + parameters: CommaSeparated { + inner: [], + commas: [], + }, + right_parenthesis: 13, + }, + return_type: FunctionLikeReturnTypeDefinition { + colon: 14, + type_definition: Void( + Keyword { + value: "void", + position: 16, + }, + ), + }, + body: BlockStatement { + comments: CommentGroup { + comments: [], + }, + left_brace: 21, + statements: [ + Expression( + ExpressionStatement { + comments: CommentGroup { + comments: [], + }, + expression: AssignmentOperation( + Assignment { + comments: CommentGroup { + comments: [], + }, + left: Variable( + Variable { + position: 27, + name: "$c", + }, + ), + equals: 30, + right: Match( + MatchExpression { + comments: CommentGroup { + comments: [], + }, + match: Keyword { + value: "match", + position: 32, + }, + expression: None, + body: MatchBodyExpression { + left_brace: 38, + arms: CommaSeparated { + inner: [ + MatchArmExpression { + condition: Expressions( + CommaSeparated { + inner: [ + ComparisonOperation( + Equal { + comments: CommentGroup { + comments: [], + }, + left: Variable( + Variable { + position: 48, + name: "$a", + }, + ), + double_equals: 51, + right: Variable( + Variable { + position: 54, + name: "$b", + }, + ), + }, + ), + ], + commas: [], + }, + ), + arrow: 57, + expression: Literal( + String( + LiteralString { + comments: CommentGroup { + comments: [], + }, + value: "'a is equal to b'", + position: 60, + }, + ), + ), + }, + MatchArmExpression { + condition: Expressions( + CommaSeparated { + inner: [ + ComparisonOperation( + GreaterThan { + comments: CommentGroup { + comments: [], + }, + left: Variable( + Variable { + position: 87, + name: "$a", + }, + ), + greater_than: 90, + right: Variable( + Variable { + position: 92, + name: "$b", + }, + ), + }, + ), + ], + commas: [], + }, + ), + arrow: 95, + expression: Literal( + String( + LiteralString { + comments: CommentGroup { + comments: [], + }, + value: "'a is greater than b'", + position: 98, + }, + ), + ), + }, + MatchArmExpression { + condition: Expressions( + CommaSeparated { + inner: [ + ComparisonOperation( + LessThan { + comments: CommentGroup { + comments: [], + }, + left: Variable( + Variable { + position: 129, + name: "$a", + }, + ), + less_than: 132, + right: Variable( + Variable { + position: 134, + name: "$b", + }, + ), + }, + ), + ], + commas: [], + }, + ), + arrow: 137, + expression: Literal( + String( + LiteralString { + comments: CommentGroup { + comments: [], + }, + value: "'a is less than b'", + position: 140, + }, + ), + ), + }, + MatchArmExpression { + condition: Default( + Keyword { + value: "default", + position: 168, + }, + ), + arrow: 176, + expression: Literal( + String( + LiteralString { + comments: CommentGroup { + comments: [], + }, + value: "'a and b are not comparable'", + position: 179, + }, + ), + ), + }, + MatchArmExpression { + condition: Default( + Keyword { + value: "default", + position: 217, + }, + ), + arrow: 225, + expression: Literal( + String( + LiteralString { + comments: CommentGroup { + comments: [], + }, + value: "'a and b are not comparable'", + position: 228, + }, + ), + ), + }, + ], + commas: [ + 77, + 119, + 158, + 207, + 256, + ], + }, + right_brace: 301, + }, + }, + ), + }, + ), + semicolon: 302, + }, + ), + ], + right_brace: 304, + }, + }, + ), + ], + eof: 306, +} \ No newline at end of file From ff0d2f3863fee5ffe86ffd9755b9f1c8842c3760 Mon Sep 17 00:00:00 2001 From: Kennedy Tedesco Date: Mon, 16 Jan 2023 12:17:35 -0300 Subject: [PATCH 2/2] No need to return a tuple here --- .../internal/expression/control_flow.rs | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/parser/internal/expression/control_flow.rs b/src/parser/internal/expression/control_flow.rs index 73abb57..f355cd7 100644 --- a/src/parser/internal/expression/control_flow.rs +++ b/src/parser/internal/expression/control_flow.rs @@ -30,23 +30,17 @@ pub fn match_expression(state: &mut State) -> ParseResult { while state.iterator.current().kind != TokenKind::RightBrace { let current = state.iterator.current(); - let (condition, _default) = if current.kind == TokenKind::Default { - ( - MatchArmConditionExpression::Default(utils::skip_keyword( - state, - TokenKind::Default, - )?), - true, - ) + let condition = if current.kind == TokenKind::Default { + MatchArmConditionExpression::Default(utils::skip_keyword( + state, + TokenKind::Default, + )?) } else { - ( - MatchArmConditionExpression::Expressions(utils::comma_separated( - state, - &expression::create, - TokenKind::DoubleArrow, - )?), - false, - ) + MatchArmConditionExpression::Expressions(utils::comma_separated( + state, + &expression::create, + TokenKind::DoubleArrow, + )?) }; let arm = MatchArmExpression {