Fixed bugs in parser recovery and added corresponding tests in the test suite.

Makefile.am

@@ -39,6 +39,7 @@ test_cases = \
 	tests/compile/compile.svtest \
 	tests/compile/errors.svtest \
 	tests/compile/warnings.svtest \
+	tests/compile/recover.svtest \
 	tests/execute/errors.svtest \
 	tests/execute/actions.svtest \
 	tests/execute/smtp.svtest \

src/lib-sieve/sieve-parser.c

@@ -157,9 +157,13 @@ static int sieve_parse_arguments
 	                       necessarily error-free state */
 
 	/* Parse arguments */
-	while ( arg_present && result > 0 && 
-		(parser->valid || sieve_errors_more_allowed(parser->ehandler)) ) {
+	while ( arg_present && result > 0 ) {
 		struct sieve_ast_argument *arg;
+
+		if ( !parser->valid && !sieve_errors_more_allowed(parser->ehandler) ) {
+			result = 0;
+			break;
+		}
 		
 		switch ( sieve_lexer_current_token(lexer) ) {
 		
@@ -184,10 +188,14 @@ static int sieve_parse_arguments
 				
 				sieve_lexer_skip_token(lexer);
 				 
-				while ( !add_failed && sieve_lexer_current_token(lexer) == STT_COMMA &&
-					(parser->valid || sieve_errors_more_allowed(parser->ehandler)) ) {
-			
+				while ( !add_failed && sieve_lexer_current_token(lexer) == STT_COMMA ) {
 					sieve_lexer_skip_token(lexer);
+
+					/* Check parser status */
+					if ( !parser->valid && !sieve_errors_more_allowed(parser->ehandler) ) {
+						result = sieve_parser_recover(parser, STT_RSQUARE);
+						break;						
+					}
 				
 					if ( sieve_lexer_current_token(lexer) == STT_STRING ) {
 						/* Add the string to the list */
@@ -339,10 +347,15 @@ static int sieve_parse_arguments
 			if ( (result=sieve_parse_arguments(parser, test, depth+1)) > 0 ) {
 			
 				/* More tests ? */
-				while ( sieve_lexer_current_token(lexer) == STT_COMMA && 
-					(parser->valid && sieve_errors_more_allowed(parser->ehandler)) ) {
+				while ( sieve_lexer_current_token(lexer) == STT_COMMA ) { 
 					sieve_lexer_skip_token(lexer);
-					
+
+					/* Check parser status */
+					if ( !parser->valid && !sieve_errors_more_allowed(parser->ehandler) ) {
+						result = sieve_parser_recover(parser, STT_RBRACKET);
+						break;
+					}
+
 					/* Test starts with identifier */
 					if ( sieve_lexer_current_token(lexer) == STT_IDENTIFIER ) {
 						test = sieve_ast_test_create
@@ -355,7 +368,6 @@ static int sieve_parse_arguments
 						/* Parse test arguments, which may include more tests (recurse) */
 						if ( (result=sieve_parse_arguments(parser, test, depth+1)) <= 0 ) {
 							if ( result < 0 ) return result;
-
 							result = sieve_parser_recover(parser, STT_RBRACKET);
 							break;
 						}
@@ -430,12 +442,20 @@ static int sieve_parse_commands
 	int result = TRUE;
 
 	while ( result > 0 && 
-		sieve_lexer_current_token(lexer) == STT_IDENTIFIER && 
-		(parser->valid || sieve_errors_more_allowed(parser->ehandler)) ) {
-		struct sieve_ast_node *command = 
-			sieve_ast_command_create
-				(block, sieve_lexer_token_ident(lexer), 
-					sieve_lexer_current_line(parser->lexer));
+		sieve_lexer_current_token(lexer) == STT_IDENTIFIER ) {
+		struct sieve_ast_node *command;
+
+		/* Check parser status */
+		if ( !parser->valid && !sieve_errors_more_allowed(parser->ehandler) ) {
+			result = sieve_parser_recover(parser, STT_SEMICOLON);
+			break;
+		}
+
+		/* Create command node */
+		command = sieve_ast_command_create
+			(block, sieve_lexer_token_ident(lexer), 
+				sieve_lexer_current_line(parser->lexer));
+		sieve_lexer_skip_token(lexer);
 	
 		if ( command == NULL ) {
 			sieve_parser_error(parser, 
@@ -444,11 +464,6 @@ static int sieve_parse_commands
 			return -1;
 		}
 
-		/* Defined state */
-		result = TRUE;		
-		
-		sieve_lexer_skip_token(lexer);
-		
 		result = sieve_parse_arguments(parser, command, 1);
 
 		/* Check whether the command is properly terminated 
@@ -471,7 +486,9 @@ static int sieve_parse_commands
 		}
 
 		/* Don't bother to continue if we are not in a defined state */
-		if ( result <= 0 ) return result;
+		if ( result <= 0 ) {
+			return result;
+		}
 			
 		switch ( sieve_lexer_current_token(lexer) ) {
 		
@@ -510,7 +527,7 @@ static int sieve_parse_commands
 			} else {
 				if ( result < 0 ) return result;
 
-				if ( (result=sieve_parser_recover(parser, STT_RCURLY)) == 0 ) 
+				if ( (result=sieve_parser_recover(parser, STT_RCURLY)) > 0 ) 
 					sieve_lexer_skip_token(lexer);
 			}
 
@@ -643,8 +660,9 @@ static int sieve_parser_recover
 	
 	/* Special case: COMMAND */
 	if (end_token == STT_SEMICOLON && 
-		sieve_lexer_current_token(lexer) == STT_LCURLY)
+		sieve_lexer_current_token(lexer) == STT_LCURLY) {
 		return TRUE;
+	}
 	
 	/* End not found before eof or end of surrounding grammatical structure 
 	 */

tests/compile/recover.svtest

+require "vnd.dovecot.testsuite";
+
+require "relational";
+require "comparator-i;ascii-numeric";
+
+/*
+ * Test parser's recover capability
+ */
+
+/*
+ * Commands 
+ */
+
+/* Missing semicolon */
+
+test "Missing semicolons" {
+    if test_script_compile "recover/commands-semicolon.sieve" {
+        test_fail "compile should have failed.";
+    }
+
+    if not test_error :count "eq" :comparator "i;ascii-numeric" "3" {
+        test_fail "wrong number of errors reported";
+    }
+}
+
+/* End of block recovery*/
+
+test "Missing semicolon at end of block" {
+    if test_script_compile "recover/commands-endblock.sieve" {
+        test_fail "compile should have failed.";
+    }
+
+    if not test_error :count "eq" :comparator "i;ascii-numeric" "4" {
+        test_fail "wrong number of errors reported";
+    }
+}
+
+/*
+ * Tests
+ */
+
+test "Spurious comma at end of test list" {
+    if test_script_compile "recover/tests-endcomma.sieve" {
+        test_fail "compile should have failed.";
+    }
+
+    if not test_error :count "eq" :comparator "i;ascii-numeric" "3" {
+        test_fail "wrong number of errors reported";
+    }
+}

tests/compile/recover/commands-endblock.sieve

+if true {
+	if true {
+		keep
+	}
+}
+
+if true {
+	keep,
+	keep
+}
+
+if true {
+	if anyof(true,true,false) {
+		keep;
+	}
+}
+
+if true {
+	if anyof(true,true,false) {
+		keep;
+		discard
+	}
+}
+

tests/compile/recover/commands-semicolon.sieve

+keep;
+discard;
+keep
+redirect "frop@frop.nl";
+discard;
+keep
+redirect "frml@frop.nl";

tests/compile/recover/tests-endcomma.sieve

+if true {
+if true {
+if anyof(true,true,true,) {
+}}}
+
+if true {
+if anyof(true,true) {
+if anyof(true,true,true,) {
+if anyof(true,true,true) {
+}}}}