wmllint: Handle nested events and other things with filters in the death check (closes #4085)

The main reason for moving this to a separate function was to make the
per-event variables local to that function.

(cherry picked from commit 7e69da7f68)

data/tools/wesnoth/wmliterator3.py

@@ -448,7 +448,11 @@ Important Attributes:
         return isAttribute(self)
 
     def iterScope(self):
-        """Return an iterator for the current scope"""
+        """Return an iterator for the current scope.
+
+        If the iterator is currently on a line that opens a new scope,
+        this returns an iterator of the outer scope not the inner one.
+        """
         if not self.scopes:
             return WmlIterator(self.lines, self.fname)
         scopeItor = self.scopes[-1].copy()

data/tools/wmllint

@@ -1345,43 +1345,78 @@ def local_sanity_check(filename, nav, key, prefix, value, comment):
 def global_sanity_check_events(filename, lines):
     """Part of global_sanity_check which finds each [event] tag.
 
-    todo: handle nested events, and ignore [filter] tags that belong to a
-    non-event tag.
+    To handle nested events, this is an multi-pass implementation. This
+    function will make one complete pass itself. Each subfunction called is
+    given its own copy of the iterator, to do its own pass on the contents of
+    the event without changing the iterator for this function.
     """
-
-    # Detect units that speak in their death events
-    filter_subject = None
-    die_event = False
     deathcheck = True
     for nav in WmllintIterator(lines, filename):
         if "wmllint: deathcheck off" in nav.text:
             deathcheck = False
-            continue
         elif "wmllint: deathcheck on" in nav.text:
             deathcheck = True
-        if "[/event]" in nav.text:
-            filter_subject = None
-            die_event = False
-        elif not nav.ancestors():
-            continue
-        elif "[event]" in nav.ancestors():
-            parent = nav.ancestors()[-1]
-            if parent == "[event]":
-                # Check if it's a death event
+        # Now the tests. These run even when the opening tag is on a line which
+        # turns that specific check off, in case the check is re-enabled before
+        # the closing tag.
+        if "[event]" in nav.text:
+            sanity_check_speaks_in_death_event(nav.copy(), deathcheck)
+
+def sanity_check_speaks_in_death_event(opening_tag, deathcheck):
+    """Detect units that speak in their death events
+
+    Given an iterator pointing to an [event]'s opening tag, check whether it's
+    a die event, and if so check whether the already-dead unit speaks during
+    it.
+
+    This will move the iterator that is passed into it, the caller should make
+    a copy if required.
+
+    opening_tag -- an iterator on the [event] tag's line
+    deathcheck -- the status of the global "deathcheck on/off" flag at
+    the start of this event
+    """
+    filter_subject = None
+    die_event = False
+    if not opening_tag.hasNext():
+        # Either this is an empty tag, or the closing tag must be
+        # missing. There will be a separate error reported for that.
+        return
+    opening_tag.__next__()
+    event_scope = opening_tag.iterScope()
+    base_depth = len(event_scope.ancestors())
+
+    # First pass - find out if the event is a death event, and if it has a
+    # filter. Here the base_depth is used to ignore nested events and other
+    # tags that have a [filter] subtag.
+    for nav in event_scope.copy():
+        ancestors = nav.ancestors()
+        if len(ancestors) == base_depth + 1:
+            fields = parse_attribute(nav.text)
+            if fields:
+                (key, prefix, value, comment) = fields
+                if key == 'name' and value == 'die':
+                    die_event = True
+        if len(ancestors) == base_depth + 2:
+            if ancestors[-1] == "[filter]":
                 fields = parse_attribute(nav.text)
                 if fields:
                     (key, prefix, value, comment) = fields
-                    if key == 'name' and value == 'die':
-                        die_event = True
-            elif die_event and not filter_subject and parent == "[filter]":
-                # Check to see if it has a filter subject
-                if "id" in nav.text:
-                    try:
-                        (key,prefix,value,comment) = parse_attribute(nav.text)
+                    if key == 'id':
                         filter_subject = value
-                    except TypeError:
-                        pass
-            elif die_event and filter_subject and parent == "[message]":
+
+    # Second pass - check if the subject speaks in this event. This will
+    # descend in to nested events, but as the unit is already dead it shouldn't
+    # speak in those either.
+    if die_event and filter_subject:
+        for nav in event_scope:
+            if "wmllint: deathcheck off" in nav.text:
+                deathcheck = False
+                continue
+            elif "wmllint: deathcheck on" in nav.text:
+                deathcheck = True
+            parent = nav.ancestors()[-1]
+            if parent == "[message]":
                 # Who is speaking?
                 fields = parse_attribute(nav.text)
                 if fields:
@@ -1389,7 +1424,7 @@ def global_sanity_check_events(filename, lines):
                     if key in ("id", "speaker"):
                         if deathcheck and ((value == filter_subject) or (value == "unit")):
                             print('"%s", line %d: %s speaks in his/her "die" event rather than "last breath"' \
-                                  % (filename, nav.lineno+1, value))
+                                  % (nav.fname, nav.lineno+1, value))
 
 def global_sanity_check(filename, lines):
     "Perform sanity and consistency checks on input files."