🖊️ Improved error message for missing colons in level 17 (hedyorg#5465)

- Add preprocessing rules which take an argument in order to allow for a rule to turn into an error without copying the old rule definition. For example, `error_ifelse<old_rule_to_error ifelse>` is used in level 8 to transform the old flat if-else to an error.
- Add errors for if-else statements, loops and function definitions missing a colon in level 17

Fixes hedyorg#5222

**How to test**
- Automated tests are added for the new preprocessing functionality and for the commands missing colons in level 17.
- To see the error message, run the following program in level 17 and try removing any of the colons at a time. The message should direct you to the right line.
```
define function with b:
    i = 1
    while i < b:
        print i
        i = i + 1

b = 10
if b is 1:
    call function with b
elif b is 2:
    call function with b
else:
    call function with b

if b is pressed:
    call function with b
elif c is pressed:
    call function with b
else:
    call function with b

```

content/error-messages.txt

@@ -35,6 +35,7 @@ gettext('Runtime Value Error')
 gettext('Runtime Values Error')
 gettext('Runtime Index Error')
 gettext('Else Without If Error')
+gettext('Missing Colon Error')
 gettext('ask_needs_var')
 gettext('no_more_flat_if')
 gettext('echo_out')

exceptions.py

@@ -388,4 +388,9 @@ def __init__(self, line_number):
         super().__init__('Else Without If Error', line_number=line_number)
 
 
+class MissingColonException(HedyException):
+    def __init__(self, command, line_number):
+        super().__init__('Missing Colon Error', command=command, line_number=line_number)
+
+
 HEDY_EXCEPTIONS = {name: cls for name, cls in globals().items() if inspect.isclass(cls)}

grammars/level16-Additions.lark

@@ -10,7 +10,7 @@ change_list_item: var_access _LEFT_SQUARE_BRACKET (INT | var_access) _RIGHT_SQUA
 assign_list: var (_IS | _EQUALS) _LEFT_SQUARE_BRACKET ((quoted_text | NUMBER) (_COMMA (quoted_text | NUMBER))*)? _RIGHT_SQUARE_BRACKET
 error_assign_list_missing_brackets: var  (_IS | _EQUALS) (_LEFT_SQUARE_BRACKET)? ((quoted_text | NUMBER) _COMMA (quoted_text | NUMBER) (_COMMA (quoted_text | NUMBER))*)? (_RIGHT_SQUARE_BRACKET)?
 
-error_list_access_at: var_access _AT (INT | random)
+error_list_access_at<old_rule_to_error list_access>
 
 _print_ask_argument: (_SPACE | quoted_text | list_access | error_list_access_at | expression | print_expression)*
 assign: var (_IS| _EQUALS) (list_access | error_list_access_at | atom | expression)

grammars/level17-Additions.lark

@@ -1,19 +1,42 @@
-// We add the : at the end of if and else and elif and for/while
+// In this level we add a colon to if-else statements, function definitions, and loops.
+// The `<rule_name>` syntax specifies a preprocessor rule which would be applied in the grammar merger.
+// The merger will find the appropriate function for this annotation and modify it accordingly.
+// The preprocessor can also accept a parameter via the syntax <rule_name argument>.
 
-command: += if_pressed (if_pressed_elifs)* if_pressed_elses? | ifs (elifs)* elses? -= ifs elses?
+command: += (if_pressed | if_pressed_no_colon) (if_pressed_elifs | if_pressed_elifs_no_colon)* (if_pressed_elses | if_pressed_elses_no_colon)? | (ifs | ifs_no_colon) (elifs | elifs_no_colon)* (elses | elses_no_colon)? | for_list_no_colon | for_loop_no_colon | while_loop_no_colon | define_no_colon -= ifs elses?
 
-// These will be handled by the preprocessor step in the merger
-// It will find the appropiate function for this anotation and modify it accordingly
 if_pressed<needs_colon>
+if_pressed_no_colon<old_rule_to_error if_pressed>
+
 if_pressed_else<needs_colon>
+
 if_pressed_elses<needs_colon>
+if_pressed_elses_no_colon<old_rule_to_error if_pressed_elses>
+
 if_pressed_elifs: _EOL _ELIF (LETTER_OR_NUMERAL | NAME) _IS _PRESSED _COLON _EOL (_SPACE command) (_EOL _SPACE command)* _EOL? _END_BLOCK
+if_pressed_elifs_no_colon: _EOL _ELIF (LETTER_OR_NUMERAL | NAME) _IS _PRESSED _EOL (_SPACE command) (_EOL _SPACE command)* _EOL? _END_BLOCK
+
+
 
 ifs<needs_colon>
+ifs_no_colon<old_rule_to_error ifs>
+
 elses<needs_colon>
+elses_no_colon<old_rule_to_error elses>
+
 elifs: _EOL _ELIF _conditions _COLON _EOL (_SPACE command) (_EOL _SPACE command)* _EOL? _END_BLOCK
+elifs_no_colon: _EOL _ELIF _conditions _EOL (_SPACE command) (_EOL _SPACE command)* _EOL? _END_BLOCK
+
+
 
 for_list<needs_colon>
+for_list_no_colon<old_rule_to_error for_list>
+
 for_loop<needs_colon>
+for_loop_no_colon<old_rule_to_error for_loop>
+
 while_loop<needs_colon>
+while_loop_no_colon<old_rule_to_error while_loop>
+
 define<needs_colon>
+define_no_colon<old_rule_to_error define>

grammars/level8-Additions.lark

@@ -1,7 +1,7 @@
 command: += error_repeat_dep_8 | error_ifelse | if_pressed_without_else | if_pressed if_pressed_elses? | ifs elses? -= error_invalid_space | ifelse | error_else_no_if
 
 repeat.1: _REPEAT (INT | var_access) _TIMES _SPACE? _EOL (_SPACE command) (_EOL _SPACE command)* _EOL? _END_BLOCK
-error_repeat_dep_8: _REPEAT (INT | var_access) _TIMES _SPACE command
+error_repeat_dep_8<old_rule_to_error repeat>
 
 // FH jan 2022 (#1817) we can allow unquoted rhs's again cause there now is a line break after
 // not sure of we want it, but we could
@@ -18,6 +18,6 @@ if_pressed_elses: _EOL (_SPACE)* _ELSE (_SPACE)* _EOL (_SPACE command) (_EOL _SP
 //'if' cannot be used in Python, hence the name of the rule is 'ifs'
 ifs: _IF condition _EOL (_SPACE command) (_EOL _SPACE command)* _EOL? _END_BLOCK
 
-//old 'flat' ifelse:
-error_ifelse : _if_clause (_SPACE+ _EOL* | _SPACE* _EOL+) _ELSE (_SPACE+ _EOL* | _SPACE* _EOL+) _if_less_command
+//the old 'flat' if-else is now an error
+error_ifelse<old_rule_to_error ifelse>
 

hedy.py

@@ -8,9 +8,9 @@
 from lark import Tree, Transformer, visitors, v_args
 from os import path, getenv
 
-import warnings
 import hedy
 import hedy_error
+import hedy_grammar
 import hedy_translation
 from utils import atomic_write_file
 from hedy_content import ALL_KEYWORD_LANGUAGES
@@ -169,22 +169,6 @@
         indent_keywords[lang_].append(keywords.get(keyword))
 
 
-# These are the preprocessor rules that we use to specify changes in the rules that
-# are expected to work across several rules
-# Example
-# for<needs_colon> instead of defining the whole rule again.
-
-
-def needs_colon(rule):
-    pos = rule.find('_EOL (_SPACE command)')
-    return f'{rule[0:pos]} _COLON {rule[pos:]}'
-
-
-PREPROCESS_RULES = {
-    'needs_colon': needs_colon
-}
-
-
 def make_value_error(command, tip, lang, value='{}'):
     return make_error_text(exceptions.RuntimeValueException(command=command, value=value, tip=tip), lang)
 
@@ -229,6 +213,9 @@ class Command:
     repeat = 'repeat'
     for_list = 'for in'
     for_loop = 'for in range'
+    if_ = 'if'
+    else_ = 'else'
+    elif_ = 'elif'
     addition = '+'
     subtraction = '-'
     multiplication = '*'
@@ -244,6 +231,7 @@ class Command:
     call = 'call'
     returns = 'return'
     play = 'play'
+    while_ = 'while'
 
 
 translatable_commands = {Command.print: ['print'],
@@ -1276,6 +1264,40 @@ def error_if_pressed_missing_else(self, meta, args):
         raise exceptions.MissingElseForPressitException(
             command='ifpressed_else', level=self.level, line_number=meta.line)
 
+    def if_pressed_no_colon(self, meta, args):
+        raise exceptions.MissingColonException(command=Command.if_, line_number=meta.line)
+
+    def if_pressed_elifs_no_colon(self, meta, args):
+        # if_pressed_elifs starts with _EOL, so we need to add +1 to its line
+        raise exceptions.MissingColonException(command=Command.elif_, line_number=meta.line+1)
+
+    def if_pressed_elses_no_colon(self, meta, args):
+        # if_pressed_elses starts with _EOL, so we need to add +1 to its line
+        raise exceptions.MissingColonException(command=Command.else_, line_number=meta.line+1)
+
+    def ifs_no_colon(self, meta, args):
+        raise exceptions.MissingColonException(command=Command.if_, line_number=meta.line)
+
+    def elifs_no_colon(self, meta, args):
+        # elifs starts with _EOL, so we need to add +1 to its line
+        raise exceptions.MissingColonException(command=Command.elif_, line_number=meta.line+1)
+
+    def elses_no_colon(self, meta, args):
+        # elses starts with _EOL, so we need to add +1 to its line
+        raise exceptions.MissingColonException(command=Command.else_, line_number=meta.line+1)
+
+    def for_list_no_colon(self, meta, args):
+        raise exceptions.MissingColonException(command=Command.for_list, line_number=meta.line)
+
+    def for_loop_no_colon(self, meta, args):
+        raise exceptions.MissingColonException(command=Command.for_loop, line_number=meta.line)
+
+    def while_loop_no_colon(self, meta, args):
+        raise exceptions.MissingColonException(command=Command.while_, line_number=meta.line)
+
+    def define_no_colon(self, meta, args):
+        raise exceptions.MissingColonException(command=Command.define, line_number=meta.line)
+
     # other rules are inherited from Filter
 
 
@@ -2742,129 +2764,6 @@ def print_empty_brackets(self, meta, args):
         return self.print(meta, args)
 
 
-def get_rule_from_string(s):
-    parts = s.split(':')
-    # get part before and after : (this is a join because there can be : in the rule)
-    if len(parts) <= 1:
-        return s, s
-    return parts[0], ''.join(parts[1])
-
-
-def merge_grammars(grammar_text_1, grammar_text_2):
-    # this function takes two grammar files and merges them into one
-    # rules that are redefined in the second file are overridden
-    # rules that are new in the second file are added (remaining_rules_grammar_2)
-    merged_grammar = []
-
-    deletables = []
-    # this list collects rules we no longer need,
-    # they will be removed when we encounter them
-
-    rules_grammar_1 = grammar_text_1.split('\n')
-    remaining_rules_grammar_2 = grammar_text_2.split('\n')
-    for line_1 in rules_grammar_1:
-        if line_1 == '' or line_1[0] == '/':  # skip comments and empty lines:
-            continue
-
-        name_1, definition_1 = get_rule_from_string(line_1)
-
-        rules_grammar_2 = grammar_text_2.split('\n')
-        override_found = False
-        for line_2 in rules_grammar_2:
-            if line_2 == '' or line_2[0] == '/':  # skip comments and empty lines:
-                continue
-
-            needs_preprocessing = re.match(r'((\w|_)+)<((\w|_)+)>', line_2)
-            if needs_preprocessing:
-                name_2 = f'{needs_preprocessing.group(1)}'
-                processor = needs_preprocessing.group(3)
-            else:
-                name_2, definition_2 = get_rule_from_string(line_2)
-
-            if name_1 == name_2:
-                override_found = True
-                if needs_preprocessing:
-                    definition_2 = PREPROCESS_RULES[processor](definition_1)
-                    line_2_processed = f'{name_2}: {definition_2}'
-                else:
-                    line_2_processed = line_2
-                if definition_1.strip() == definition_2.strip():
-                    warn_message = f"The rule {name_1} is duplicated: {definition_1} and {definition_2}. Please check!"
-                    warnings.warn(warn_message)
-                # Used to compute the rules that use the merge operators in the grammar, namely +=, -= and >>
-                new_rule, new_deletables = merge_rules_operator(definition_1, definition_2, name_1, line_2_processed)
-                if new_deletables:
-                    deletables += new_deletables
-                # Already processed, so remove it
-                remaining_rules_grammar_2.remove(line_2)
-                break
-
-        # new rule found? print that. nothing found? print org rule
-        if override_found:
-            merged_grammar.append(new_rule)
-        else:
-            merged_grammar.append(line_1)
-
-    # all rules that were not overlapping are new in the grammar, add these too
-    for rule in remaining_rules_grammar_2:
-        if not (rule == '' or rule[0] == '/'):
-            merged_grammar.append(rule)
-
-    merged_grammar = sorted(merged_grammar)
-    # filter deletable rules
-    rules_to_keep = [rule for rule in merged_grammar if get_rule_from_string(rule)[0] not in deletables]
-    return '\n'.join(rules_to_keep)
-
-
-ADD_GRAMMAR_MERGE_OP = '+='
-REMOVE_GRAMMAR_MERGE_OP = '-='
-LAST_GRAMMAR_MERGE_OP = '>>'
-GRAMMAR_MERGE_OPERATORS = [ADD_GRAMMAR_MERGE_OP, REMOVE_GRAMMAR_MERGE_OP, LAST_GRAMMAR_MERGE_OP]
-
-
-def merge_rules_operator(prev_definition, new_definition, name, complete_line):
-    op_to_arg = get_operator_to_argument(new_definition)
-
-    add_arg = op_to_arg.get(ADD_GRAMMAR_MERGE_OP, '')
-    remove_arg = op_to_arg.get(REMOVE_GRAMMAR_MERGE_OP, '')
-    last_arg = op_to_arg.get(LAST_GRAMMAR_MERGE_OP, '')
-    remaining_commands = get_remaining_rules(prev_definition, remove_arg, last_arg)
-    ordered_commands = split_rule(remaining_commands, add_arg, last_arg)
-
-    new_rule = f"{name}: {' | '.join(ordered_commands)}" if bool(op_to_arg) else complete_line
-    deletable = split_rule(remove_arg)
-    return new_rule, deletable
-
-
-def get_operator_to_argument(definition):
-    # Creates a map of all used operators and their respective arguments e.g. {'+=': 'print | play', '>>': 'echo'}
-    operator_to_index = [(op, definition.find(op)) for op in GRAMMAR_MERGE_OPERATORS if op in definition]
-    result = {}
-    for i, (op, index) in enumerate(operator_to_index):
-        start_index = index + len(op)
-        if i + 1 < len(operator_to_index):
-            _, next_index = operator_to_index[i + 1]
-            result[op] = definition[start_index:next_index].strip()
-        else:
-            result[op] = definition[start_index:].strip()
-    return result
-
-
-def get_remaining_rules(orig_def, *sub_def):
-    original_commands = split_rule(orig_def)
-    commands_after_minus = split_rule(*sub_def)
-    misses = [c for c in commands_after_minus if c not in original_commands]
-    if misses:
-        raise Exception(f"Command(s) {'|'.join(misses)} do not exist in the previous definition")
-    remaining_commands = [cmd for cmd in original_commands if cmd not in commands_after_minus]
-    remaining_commands = ' | '.join(remaining_commands)  # turn the result list into a string
-    return remaining_commands
-
-
-def split_rule(*rules):
-    return [c.strip() for rule in rules for c in rule.split('|') if c.strip() != '']
-
-
 # this is only a couple of MB in total, safe to cache
 @cache
 def create_grammar(level, lang, skip_faulty):
@@ -2874,7 +2773,7 @@ def create_grammar(level, lang, skip_faulty):
     # then keep merging new grammars in
     for i in range(2, level + 1):
         grammar_text_i = get_additional_rules_for_level(i)
-        merged_grammars = merge_grammars(merged_grammars, grammar_text_i)
+        merged_grammars = hedy_grammar.merge_grammars(merged_grammars, grammar_text_i)
 
     # keyword and other terminals never have mergable rules, so we can just add them at the end
     keywords = get_keywords_for_language(lang)