Put policy_template placeholders in a ph node

Bug: 1000118
Change-Id: I09456c19aec5a5a9ea26cdc76d5c28b66807f1e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1872541
Commit-Queue: Yann Dago <ydago@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708365}

components/policy/resources/PRESUBMIT.py

@@ -6,7 +6,7 @@
 # mnissler@chromium.org, bartfab@chromium.org or atwilson@chromium.org.
 
 import sys
-import xml.dom.minidom
+from xml.dom import minidom
 
 def _GetPolicyTemplates(template_path):
   # Read list of policies in the template. eval() is used instead of a JSON
@@ -77,7 +77,7 @@ def _CheckPolicyHistograms(input_api, output_api, policies):
   histograms = input_api.os_path.join(
       root, 'tools', 'metrics', 'histograms', 'enums.xml')
   with open(histograms) as f:
-    tree = xml.dom.minidom.parseString(f.read())
+    tree = minidom.parseString(f.read())
   enums = (tree.getElementsByTagName('histogram-configuration')[0]
                .getElementsByTagName('enums')[0]
                .getElementsByTagName('enum'))
@@ -117,7 +117,7 @@ def _CheckPolicyAtomicGroupsHistograms(input_api, output_api, atomic_groups):
   histograms = input_api.os_path.join(
       root, 'tools', 'metrics', 'histograms', 'enums.xml')
   with open(histograms) as f:
-    tree = xml.dom.minidom.parseString(f.read())
+    tree = minidom.parseString(f.read())
   enums = (tree.getElementsByTagName('histogram-configuration')[0]
                .getElementsByTagName('enums')[0]
                .getElementsByTagName('enum'))
@@ -153,11 +153,28 @@ def _CheckPolicyAtomicGroupsHistograms(input_api, output_api, atomic_groups):
     results.append(output_api.PresubmitError(error_extra % atomic_group_id))
   return results
 
+def _CheckMissingPlaceholders(input_api, output_api, template_path):
+  with open(template_path) as f:
+    template_data = eval(f.read(), {})
 
-def _CommonChecks(input_api, output_api):
   results = []
-  results.extend(_CheckPolicyTemplatesSyntax(input_api, output_api))
+  items = template_data['policy_definitions'] \
+          + [msg for msg in template_data['messages'].values()]
+  for item in items:
+    for key in ['desc', 'text']:
+      if not key in item:
+        continue
+      node = minidom.parseString('<msg>%s</msg>' % item[key]).childNodes[0]
+      for child in node.childNodes:
+        if child.nodeType == minidom.Node.TEXT_NODE and '$' in child.data:
+          warning = ('Character \'$\' found outside of a placeholder in "%s". '
+                     'Should it be in a placeholder ?') % item[key]
+          results.append(output_api.PresubmitPromptWarning(warning))
+  return results
 
+
+def _CommonChecks(input_api, output_api):
+  results = []
   root = input_api.change.RepositoryRoot()
   template_path = template_path = input_api.os_path.join(
       root, 'components', 'policy', 'resources', 'policy_templates.json')
@@ -166,6 +183,8 @@ def _CommonChecks(input_api, output_api):
       root, 'chrome', 'test', 'data', 'policy', 'policy_test_cases.json')
   affected_files = input_api.change.AffectedFiles()
 
+  results.extend(_CheckMissingPlaceholders(input_api, output_api,
+      template_path))
   template_changed = any(f.AbsoluteLocalPath() == template_path \
     for f in affected_files)
   tests_changed = any(f.AbsoluteLocalPath() == test_cases_path \
@@ -179,6 +198,7 @@ def _CommonChecks(input_api, output_api):
       return results
     results.extend(_CheckPolicyTestCases(input_api, output_api, policies))
     if template_changed:
+      results.extend(_CheckPolicyTemplatesSyntax(input_api, output_api))
       results.extend(_CheckPolicyHistograms(input_api, output_api, policies))
 
   return results