From 6564cc416d7a4c0b54ae3b28717b43bb65f801d7 Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan@rename-it.nl>
Date: Sun, 19 Oct 2008 17:47:49 +0200
Subject: [PATCH] Clarified error messages for missing semicolon.

---
 README                          |  9 ++++++++
 src/lib-sieve/sieve-validator.c | 40 ++++++++++++++++++++++++---------
 tests/compile/errors.svtest     | 33 +++++++++++++++++++++++++--
 3 files changed, 70 insertions(+), 12 deletions(-)

diff --git a/README b/README
index 65084d62b..c48f35033 100644
--- a/README
+++ b/README
@@ -182,6 +182,15 @@ Currently undocumented (will be merged with sieve-test).
 
 Various example scripts are bundled in the directory 'examples'. 
 
+Known issues
+------------
+
+Most open issues are outlined in the TODO file. The more generic ones are (re-)
+listed here:
+
+- Compile errors are sometimes a bit obscure and long. This needs work. 
+  Suggestions for improvement are welcome. 
+
 Authors
 -------
 
diff --git a/src/lib-sieve/sieve-validator.c b/src/lib-sieve/sieve-validator.c
index c8093812a..08c8f72cd 100644
--- a/src/lib-sieve/sieve-validator.c
+++ b/src/lib-sieve/sieve-validator.c
@@ -857,24 +857,44 @@ static bool sieve_validate_arguments_context
  */ 
                  
 static bool sieve_validate_command_subtests
-(struct sieve_validator *validator, struct sieve_command_context *cmd, 
+(struct sieve_validator *valdtr, struct sieve_command_context *cmd, 
 	const unsigned int count) 
 {
 	switch ( count ) {
 	
 	case 0:
 	 	if ( sieve_ast_test_count(cmd->ast_node) > 0 ) {
-			sieve_command_validate_error(validator, cmd, 
-				"the %s %s accepts no sub-tests, but tests are specified anyway "
-				"(forgot semicolon?)", 
-				cmd->command->identifier, sieve_command_type_name(cmd->command));
-			
+			/* Unexpected command specified */
+			enum sieve_command_type ctype = SCT_NONE;
+			struct sieve_command_registration *cmd_reg;
+			struct sieve_ast_node *test = sieve_ast_test_first(cmd->ast_node);
+
+			cmd_reg = sieve_validator_find_command_registration
+				(valdtr, test->identifier);
+	
+			/* First check what we are dealing with */
+			if ( cmd_reg != NULL && cmd_reg->command != NULL )
+				ctype = cmd_reg->command->type;
+
+			switch ( ctype ) {
+			case SCT_TEST:
+				sieve_command_validate_error(valdtr, cmd, 
+					"the %s %s accepts no sub-tests, but one is specified anyway", 
+					cmd->command->identifier, sieve_command_type_name(cmd->command));
+				break;
+			case SCT_NONE:
+			case SCT_COMMAND:
+				sieve_command_validate_error(valdtr, cmd, 
+					"missing semicolon ';' after %s %s", 
+					cmd->command->identifier, sieve_command_type_name(cmd->command));
+				break;
+			}
 			return FALSE;
 		}
 		break;
 	case 1:
 		if ( sieve_ast_test_count(cmd->ast_node) == 0 ) {
-			sieve_command_validate_error(validator, cmd, 
+			sieve_command_validate_error(valdtr, cmd, 
 				"the %s %s requires one sub-test, but none is specified", 
 				cmd->command->identifier, sieve_command_type_name(cmd->command));
 				
@@ -883,7 +903,7 @@ static bool sieve_validate_command_subtests
 		} else if ( sieve_ast_test_count(cmd->ast_node) > 1 || 
 			cmd->ast_node->test_list ) {
 			
-			sieve_command_validate_error(validator, cmd, 
+			sieve_command_validate_error(valdtr, cmd, 
 				"the %s %s requires one sub-test, but a list of tests is specified", 
 				cmd->command->identifier, sieve_command_type_name(cmd->command));
 				
@@ -893,7 +913,7 @@ static bool sieve_validate_command_subtests
 		
 	default:
 		if ( sieve_ast_test_count(cmd->ast_node) == 0 ) {
-			sieve_command_validate_error(validator, cmd, 
+			sieve_command_validate_error(valdtr, cmd, 
 				"the %s %s requires a list of sub-tests, but none is specified", 
 				cmd->command->identifier, sieve_command_type_name(cmd->command));
 			
@@ -902,7 +922,7 @@ static bool sieve_validate_command_subtests
 		} else if ( sieve_ast_test_count(cmd->ast_node) == 1 && 
 			!cmd->ast_node->test_list ) {
 			
-			sieve_command_validate_error(validator, cmd, 
+			sieve_command_validate_error(valdtr, cmd, 
 				"the %s %s requires a list of sub-tests, "
 				"but a single test is specified", 
 				cmd->command->identifier, sieve_command_type_name(cmd->command) );
diff --git a/tests/compile/errors.svtest b/tests/compile/errors.svtest
index 98430a163..81a389bf0 100644
--- a/tests/compile/errors.svtest
+++ b/tests/compile/errors.svtest
@@ -80,7 +80,7 @@ test "Header errors" {
 	}
 
 	if not test_error :index 7 :matches 
-		"*header test accepts no sub-tests* are specified*" {
+		"*header test accepts no sub-tests* is specified*" {
 		test_fail "error 7 is invalid";
 	}
 
@@ -313,6 +313,36 @@ test "Tagged argument errors (FIXME: count only)" {
     }
 }
 
+/*
+ * Typos
+ */
+
+test "Typos" {
+	if test_compile "errors/typos.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";
+	}
+
+	if not test_error :index 1 :matches
+		"missing semicolon * fileinto *" {
+		test_fail "error 1 is invalid";
+	}
+
+	if not test_error :index 2 :matches
+		"*fileinto command * no *tests* one is specified*" {
+		test_fail "error 2 is invalid";
+	}
+
+	if not test_error :index 3 :matches
+		"missing semicolon * fileinto *" {
+		test_fail "error 3 is invalid";
+	}
+}
+
+
 /*
  * Unsupported language features
  */
@@ -326,4 +356,3 @@ test "Unsupported language features (FIXME: count only)" {
         test_fail "wrong number of errors reported";
     }
 }
-
-- 
GitLab