Improve detection of invalid placeholders in localazy strings (#7624)

tools/localazy/checkForbiddenTerms.py

@@ -59,11 +59,34 @@ _PLACEHOLDER_MATCHES = re.compile(r'%%|%[0-9$.A-Za-z]*')
 def find_invalid_placeholders(value):
     """Return the list of malformed placeholder tokens found in `value`."""
     invalid = []
-    for token in _PLACEHOLDER_MATCHES.findall(value):
+    # The % character is special: when using the strings without placeholder replacement, it will be just printed as-is.
+    # However, if the string is used with placeholder replacement, a single '%' will be interpreted as an invalid placeholder,
+    # and will cause a crash at runtime unless it's escaped as '%%'.
+    has_unescaped_percent = False
+    has_escaped_percent = False
+    has_valid_placeholders = False
+    placeholders = _PLACEHOLDER_MATCHES.findall(value)
+    for token in placeholders:
         if token == '%%':
+            has_escaped_percent = True
             continue  # escaped percent is fine
+        if token == '%':
+            has_unescaped_percent = True # We'll check later if having this token alongside valid placeholders is an error
+            continue
         if not _VALID_PLACEHOLDER.fullmatch(token):
             invalid.append(token)
+        else:
+            has_valid_placeholders = True
+
+    # If we found a single unescaped '%' and there are valid placeholders, we assume it'll be interpreted as an invalid placeholder,
+    # so this is an error.
+    if has_unescaped_percent and has_valid_placeholders:
+        invalid.append('%')
+
+    # If we found an escaped percent but no valid placeholders, this is probably a mistake, so we warn about it.
+    if has_escaped_percent and not has_valid_placeholders:
+        print('Warning: string `' + value + '` contains escaped percent (%%) but no valid placeholders. This is probably a mistake.', file=sys.stderr)
+
     return invalid
 
 
@@ -85,9 +108,10 @@ for elem in content.getElementsByTagName('string'):
         matches = re.search(term, value)
         if matches and name not in exceptions:
             errors.append('Forbidden term "' + term + '" in string: "' + name + '": ' + value)
-        invalid_placeholders = find_invalid_placeholders(value)
+
-        for placeholder in invalid_placeholders:
+    invalid_placeholders = find_invalid_placeholders(value)
-            errors.append('Invalid placeholder "' + placeholder + '" in string: "' + name + '": ' + value)
+    for placeholder in invalid_placeholders:
+        errors.append('Invalid placeholder "' + placeholder + '" in string: "' + name + '": ' + value)
 
 ### Plurals
 for elem in content.getElementsByTagName('plurals'):
@@ -106,9 +130,10 @@ for elem in content.getElementsByTagName('plurals'):
             matches = re.search(term, value)
             if matches and name not in exceptions:
                 errors.append('Forbidden term "' + term + '" in plural: "' + name + '": ' + value)
-            invalid_placeholders = find_invalid_placeholders(value)
+
-            for placeholder in invalid_placeholders:
+        invalid_placeholders = find_invalid_placeholders(value)
-                errors.append('Invalid placeholder "' + placeholder + '" in plural: "' + name + '": ' + value)
+        for placeholder in invalid_placeholders:
+            errors.append('Invalid placeholder "' + placeholder + '" in plural: "' + name + '": ' + value)
 
 # If errors is not empty print the report
 if errors: