fix: Incorrect remark parsing in CA statements

eclipse-che4z · Jun 9, 2022 · 4e4a516 · 4e4a516
1 parent 9156494
commit 4e4a516
Show file tree

Hide file tree

Showing 11 changed files with 140 additions and 80 deletions.
diff --git a/clients/vscode-hlasmplugin/CHANGELOG.md b/clients/vscode-hlasmplugin/CHANGELOG.md
@@ -22,6 +22,7 @@
 - URIs and paths are now represented by a designated data type
 - Return correct variable type for values provided in the macro's name field
 - Revise machine instructions
+- Incorrect remark parsing in CA statements
 
 ## [1.2.0](https://github.com/eclipse/che-che4z-lsp-for-hlasm/compare/1.1.0...1.2.0) (2022-05-11)
 

diff --git a/parser_library/src/parsing/grammar/ca_expr_rules.g4 b/parser_library/src/parsing/grammar/ca_expr_rules.g4
@@ -366,6 +366,11 @@ string_ch_v returns [concat_point_ptr point]
 
 string_ch_v_c returns [concat_chain chain]
 	:
-	| cl=string_ch_v_c string_ch_v		{$cl.chain.push_back(std::move($string_ch_v.point)); $chain = std::move($cl.chain);};
+	| cl=string_ch_v_c string_ch_v
+	{
+		if (auto& v = $string_ch_v.point; v)
+			$cl.chain.push_back(std::move(v));
+		$chain = std::move($cl.chain);
+	};
 	finally
 	{concatenation_point::clear_concat_chain($chain);}
diff --git a/parser_library/src/parsing/grammar/ca_operand_rules.g4 b/parser_library/src/parsing/grammar/ca_operand_rules.g4
@@ -15,7 +15,6 @@
  //rules for CA operands
 parser grammar ca_operand_rules; 
 
-
 ca_op returns [operand_ptr op]
 	: expr_list seq_symbol
 	{

diff --git a/parser_library/src/parsing/grammar/hlasmparser.g4 b/parser_library/src/parsing/grammar/hlasmparser.g4
@@ -218,26 +218,6 @@ remark_o returns [std::optional<range> value]
 	: SPACE remark							{$value = provider.get_range( $remark.ctx);}
 	| ;
 
-remark_eol returns [std::optional<range> value]
-	:
-	(
-		SPACE
-		{
-			auto s = _input->LT(1);
-		}
-		l=(DOT|ASTERISK|MINUS|PLUS|LT|GT|COMMA|LPAR|RPAR|SLASH|EQUALS|AMPERSAND|APOSTROPHE|IDENTIFIER|NUM|VERTICAL|ORDSYMBOL|SPACE|ATTR)*
-		{$value = provider.get_range(s, $l);}
-	)?
-	(
-		CONTINUATION
-		|
-		EOF
-	)
-	;
-
-
-
-
 	//***** highlighting rules
 comma 
 	: COMMA {collector.add_hl_symbol(token_info(provider.get_range( $COMMA),hl_scopes::operator_symbol)); };

diff --git a/parser_library/src/parsing/grammar/operand_field_rules.g4 b/parser_library/src/parsing/grammar/operand_field_rules.g4
@@ -129,57 +129,96 @@ operand_asm returns [operand_ptr op]
 	: asm_op							{$op = std::move($asm_op.op);}
 	|									{$op = std::make_unique<semantics::empty_operand>(provider.get_empty_range( _localctx->getStart()));};
 
-
 //////////////////////////////////////// ca
 
-op_rem_body_ca
+op_rem_body_ca locals [bool pending_empty_op = false, std::vector<range> remarks, std::vector<operand_ptr> operands, antlr4::Token* first_token = nullptr]
 	:
-	SPACE* EOF {collector.set_operand_remark_field(provider.get_range(_localctx));}
-	|
-	SPACE+ op_rem_body_alt_ca
-	{
-		auto line_range = provider.get_range($op_rem_body_alt_ca.ctx);
-		collector.set_operand_remark_field(std::move($op_rem_body_alt_ca.line.operands), std::move($op_rem_body_alt_ca.line.remarks), line_range);
-	} EOF
-	| remark_o 
+	EOF
 	{
-		auto remarks = $remark_o.value ? remark_list{*$remark_o.value} : remark_list{};
-		auto line_range = provider.get_range($remark_o.ctx);
-		collector.set_operand_remark_field(operand_list(), std::move(remarks), line_range);
-	} EOF;
-
-op_rem_body_alt_ca returns [op_rem line]
-	:
+		collector.set_operand_remark_field(provider.get_range(_localctx));
+	}
+	|
+	SPACE+
 	(
+		EOF
+		{
+			collector.set_operand_remark_field(provider.get_range(_localctx));
+		}
+		|
+		{
+			$first_token = _input->LT(1);
+		}
 		(
-			ca_op? comma
+			comma
 			{
-				if ($ca_op.ctx && $ca_op.op)
-					$line.operands.push_back(std::move($ca_op.op));
-				else
-					$line.operands.push_back(std::make_unique<semantics::empty_operand>(provider.get_empty_range($comma.ctx->getStart())));
+                $operands.push_back(std::make_unique<semantics::empty_operand>(provider.get_empty_range($comma.start)));
+				$pending_empty_op = true;
 			}
-		)+
-		{enable_continuation();}
+			|
+			{
+				$pending_empty_op = false;
+			}
+			ca_op
+			{
+                if ($ca_op.op)
+                    $operands.push_back(std::move($ca_op.op));
+                else
+                    $operands.push_back(std::make_unique<semantics::empty_operand>(provider.get_empty_range($ca_op.start)));
+			}
+		)
 		(
-			r1=remark_o CONTINUATION
+			comma
 			{
-				if($r1.value) $line.remarks.push_back(std::move(*$r1.value));
+				if ($pending_empty_op)
+					$operands.push_back(std::make_unique<semantics::empty_operand>(provider.get_empty_range($comma.start)));
+				$pending_empty_op = true;
 			}
-		)?
-		{disable_continuation();}
-	)*
-	(
-		last_ca_op=ca_op? last_remark=remark_o
+			|
+			{
+				if (_input->LA(-1) == hlasmparser::COMMA)
+					enable_continuation();
+			}
+			(
+				SPACE
+				remark
+				{
+					$remarks.push_back(provider.get_range($remark.ctx));
+				}
+				CONTINUATION?
+				|
+				CONTINUATION
+			)
+			{
+				disable_continuation();
+			}
+			|
+			{
+				if (!$pending_empty_op)
+					throw NoViableAltException(this);
+			}
+			ca_op
+			{
+				$pending_empty_op = false;
+			}
+			{
+                if ($ca_op.op)
+                    $operands.push_back(std::move($ca_op.op));
+                else
+                    $operands.push_back(std::make_unique<semantics::empty_operand>(provider.get_empty_range($ca_op.start)));
+			}
+		)*
+		EOF
 		{
-			if ($last_ca_op.ctx)
-				$line.operands.push_back(std::move($last_ca_op.op));
-			else
-				$line.operands.push_back(std::make_unique<semantics::empty_operand>(provider.get_empty_range($remark_o.ctx->getStart())));
-			if ($last_remark.value)
-				$line.remarks.push_back(std::move(*$last_remark.value));
+			if ($pending_empty_op)
+				$operands.push_back(std::make_unique<semantics::empty_operand>(provider.get_empty_range(_input->LT(-1))));
 		}
 	);
+	finally
+	{
+		disable_continuation();
+		if ($first_token)
+			collector.set_operand_remark_field(std::move($operands), std::move($remarks), provider.get_range($first_token, _input->LT(-1)));
+	}
 
 //////////////////////////////////////// mac
 

diff --git a/parser_library/src/processing/opencode_provider.cpp b/parser_library/src/processing/opencode_provider.cpp
@@ -242,6 +242,7 @@ std::shared_ptr<const context::hlasm_statement> opencode_provider::process_looka
 
     return result;
 }
+
 std::shared_ptr<const context::hlasm_statement> opencode_provider::process_ordinary(const statement_processor& proc,
     semantics::collector& collector,
     const std::optional<std::string>& op_text,

diff --git a/parser_library/test/expressions/arithmetic_expression_test.cpp b/parser_library/test/expressions/arithmetic_expression_test.cpp
@@ -74,9 +74,9 @@ TEST(arithmetic_expressions, invalid_self_defining_term)
 )";
     analyzer a(input);
     a.analyze();
-
     a.collect_diags();
-    ASSERT_EQ(a.diags().size(), (size_t)3);
+
+    EXPECT_TRUE(matches_message_codes(a.diags(), { "S0002", "S0002", "CE012", "CE012" }));
 }
 
 TEST(arithmetic_expressions, substitution_to_character_expression)
@@ -108,9 +108,9 @@ TEST(arithmetic_expressions, subscript_use)
 )";
     analyzer a(input);
     a.analyze();
-
     a.collect_diags();
-    ASSERT_EQ(a.diags().size(), (size_t)1);
+
+    EXPECT_TRUE(matches_message_codes(a.diags(), { "S0002", "E013" }));
 }
 
 TEST(arithmetic_expressions, unary_operators)
@@ -250,9 +250,9 @@ TEST(arithmetic_expressions, multiple_operand_with_spaces)
 )";
     analyzer a(input);
     a.analyze();
-
     a.collect_diags();
-    ASSERT_EQ(a.diags().size(), (size_t)3);
+
+    EXPECT_TRUE(matches_message_codes(a.diags(), { "S0002", "S0002", "S0002", "CE012", "CE012" }));
 }
 
 TEST(arithmetic_expressions, conversion_from_binary)

diff --git a/parser_library/test/expressions/character_expression_test.cpp b/parser_library/test/expressions/character_expression_test.cpp
@@ -106,9 +106,9 @@ TEST(character_expresssion, invalid_string)
 )";
     analyzer a(input);
     a.analyze();
-
     a.collect_diags();
-    ASSERT_EQ(a.diags().size(), (size_t)2);
+
+    EXPECT_TRUE(matches_message_codes(a.diags(), { "S0008", "CE011" }));
 }
 
 TEST(character_expresssion, escaping)

diff --git a/parser_library/test/parsing/deferred_statement_test.cpp b/parser_library/test/parsing/deferred_statement_test.cpp
@@ -64,4 +64,22 @@ TEST(deferred_statement, nested_macro_def_with_continuation)
 
     a.collect_diags();
     EXPECT_TRUE(a.diags().empty());
-}
+}
+
+TEST(deferred_statement, recognize_comment)
+{
+    std::string input = R"(
+         MACRO
+         MAC   &X
+         AIF   (T'&X NE 'O').A                                         X
+               (T'&X NE 'O').B,
+.A       ANOP
+         MEND
+         MAC
+)";
+    analyzer a(input);
+    a.analyze();
+
+    a.collect_diags();
+    EXPECT_TRUE(a.diags().empty());
+}
diff --git a/parser_library/test/processing/ainsert_test.cpp b/parser_library/test/processing/ainsert_test.cpp
@@ -322,7 +322,19 @@ TEST(ainsert, grammar_unknown_label)
     analyzer a(input);
     a.analyze();
     a.collect_diags();
-    EXPECT_TRUE(matches_message_codes(a.diags(), { "E010", "E076", "E010", "E076", "E010", "E076", "E010", "E076" }));
+    EXPECT_TRUE(matches_message_codes(a.diags(),
+        {
+            "S0002",
+            "S0002",
+            "E010",
+            "E076",
+            "E010",
+            "E076",
+            "E010",
+            "E076",
+            "E010",
+            "E076",
+        }));
 }
 
 TEST(ainsert, grammar_unknown_variable)
@@ -372,15 +384,7 @@ TEST(ainsert, grammar_invalid_string)
     analyzer a(input);
     a.analyze();
     a.collect_diags();
-    EXPECT_TRUE(matches_message_codes(a.diags(),
-        {
-            "E022",
-            "E076",
-            "E022",
-            "E076",
-            "E022",
-            "E022",
-        }));
+    EXPECT_TRUE(matches_message_codes(a.diags(), { "S0002", "S0002", "S0002", "S0002", "E076", "E076" }));
 }
 
 /*
@@ -409,7 +413,7 @@ TEST(ainsert, grammar_non_matching_apostrophes_by_two_01)
     analyzer a(input);
     a.analyze();
     a.collect_diags();
-    EXPECT_TRUE(matches_message_codes(a.diags(), { "E022" }));
+    EXPECT_TRUE(matches_message_codes(a.diags(), { "S0002" }));
 }
 
 /*
@@ -434,7 +438,7 @@ TEST(ainsert, grammar_non_matching_apostrophes_by_two_02)
     analyzer a(input);
     a.analyze();
     a.collect_diags();
-    EXPECT_TRUE(matches_message_codes(a.diags(), { "E022" }));
+    EXPECT_TRUE(matches_message_codes(a.diags(), { "S0002" }));
 }
 
 /*

diff --git a/parser_library/test/processing/ca_instr_test.cpp b/parser_library/test/processing/ca_instr_test.cpp
@@ -249,6 +249,19 @@ TEST(AIF, extended)
     EXPECT_EQ(get_var_value<B_t>(a.hlasm_ctx(), "var3"), false);
 }
 
+TEST(AIF, missing_comma)
+{
+    std::string input(R"(
+         AIF ('&SYSPARM' EQ '').A('&SYSPARM' EQ '').A
+.A       ANOP
+)");
+    analyzer a(input);
+    a.analyze();
+    a.collect_diags();
+
+    EXPECT_TRUE(matches_message_codes(a.diags(), { "S0002" }));
+}
+
 TEST(AIF, extended_fail)
 {
     std::string input(R"(