Reland "Add presubmit to detect inappropriate punctuation usage in strings"

Google material standards usually prefer curly double/single quotes to
neutral quotes. The presubmit detects these punctuations in string
resources and then prints warnings.

Bug: 1100941
Change-Id: Id85ef0a9f299fccc0fabe795e6e1bb385d5379ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2319541Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Lijin Shen <lazzzis@google.com>
Cr-Commit-Position: refs/heads/master@{#791966}

tools/android/checkxmlstyle/checkxmlstyle.py

@@ -33,10 +33,9 @@ def CheckStyleOnCommit(input_api, output_api):
   return _CommonChecks(input_api, output_api)
 
 
-def IncludedFiles(input_api):
+def IncludedFiles(input_api, allow_list=helpers.INCLUDED_PATHS):
   # Filter out XML files outside included paths and files that were deleted.
-  files = lambda f: input_api.FilterSourceFile(
-      f, white_list=helpers.INCLUDED_PATHS)
+  files = lambda f: input_api.FilterSourceFile(f, allow_list)
   return input_api.AffectedFiles(include_deletes=False, file_filter=files)
 
 
@@ -52,6 +51,7 @@ def _CommonChecks(input_api, output_api):
   result.extend(_CheckTextAppearance(input_api, output_api))
   result.extend(_CheckLineSpacingAttribute(input_api, output_api))
   result.extend(_CheckButtonCompatWidgetUsage(input_api, output_api))
+  result.extend(_CheckStringResourcePunctuations(input_api, output_api))
   # Add more checks here
   return result
 
@@ -468,6 +468,64 @@ def _CheckButtonCompatWidgetUsage(input_api, output_api):
 
   return []
 
+
+### String resource check ###
+def _CheckStringResourcePunctuations(input_api, output_api):
+  """Check whether inappropriate punctuations are used"""
+  warnings = []
+  result = []
+  # Removing placeholders for parsing purpose:
+  # placeholders will be parsed as children of the parent node.
+  ph = re.compile(r'<ph>.*</ph>')
+  quote_re = re.compile(u'[\u0022\u0027\u0060\u00B4]')
+  for f in IncludedFiles(input_api, helpers.INCLUDED_GRD_PATHS):
+    contents = input_api.ReadFile(f)
+
+    contents = re.sub(ph, '', contents)
+    tree = ET.fromstring(contents)
+
+    # some grds don't contain release and messages tags
+    if tree.find('release') is None:
+      continue
+    if tree.find('release').find('messages') is None:
+      continue
+
+    messages = tree.find('release').find('messages')
+
+    quotes = set()
+    for child in messages:
+      if child.tag == 'message':
+        lines = child.text.split('\n')
+        quotes.update(l for l in lines if quote_re.search(l))
+
+    # Only report the lines in the changed contents of the current workspace
+    for line_number, line in f.ChangedContents():
+      lineWithoutPh = re.sub(ph, '', line)
+      if lineWithoutPh in quotes:
+        warnings.append('  %s:%d\n    \t%s' %
+                        (f.LocalPath(), line_number, line))
+
+  if warnings:
+    result += [
+        output_api.PresubmitPromptWarning(
+            u'''
+  Android String Resources Check failed:
+    Your new string is using one or more of generic quotes(\u0022 \\u0022, \u0027 \\u0027,
+    \u0060 \\u0060, \u00B4 \\u00B4), which is not encouraged. Instead, quotations marks
+    (\u201C \\u201C, \u201D \\u201D, \u2018 \\u2018, \u2019 \\u2019) are usually preferred.
+
+    Use prime (\u2032 \\u2032) only in abbreviations for feet, arcminutes, and minutes.
+    Use Double-prime (\u2033 \\u2033) only in abbreviations for inches, arcminutes, and minutes.
+
+    Please reach out to the UX designer/writer in your team to double check
+    which punctuation should be correctly used. Ignore this warning if UX has confirmed.
+
+    Reach out to writing-strings@chromium.org if you have any question about writing strings.
+  '''.encode('utf-8'), warnings)
+    ]
+  return result
+
+
 ### helpers ###
 def _colorXml2Dict(content):
   dct = dict()

tools/android/checkxmlstyle/checkxmlstyle_test.py

@@ -434,5 +434,48 @@ class UnfavoredWidgetsTest(unittest.TestCase):
                      result[0].items[1].splitlines()[0])
 
 
+class StringResourcesTest(unittest.TestCase):
+  def testInfavoredQuotations(self):
+    xmlChanges = (u'''<grit><release><messages>
+      <message name="IDS_TEST_0">
+          <ph><ex>Hi</ex></ph>, it\u0027s a good idea
+      </message>
+      <message name="IDS_TEST_1">
+          <ph><ex>Yes</ex></ph>, it\u2019s a good idea
+      </message>
+      <message name="IDS_TEST_2">
+        Go to \u0022Settings\u0022 and
+        \u0022Menus\u0022
+      </message>
+      <message name="IDS_TEST_3">
+        Go to \u201CSettings\u201D
+        \u0022Menus\u0023
+      </message>
+      <message name="IDS_TEST_4">
+        Go to \u201CSettings\u201D
+        \u201CMenus\u201D
+      </message>
+        <part file="site_settings.grdp" />
+          </messages></release></grit>'''.encode('utf-8')).splitlines()
+
+    mock_input_api = MockInputApi()
+    mock_input_api.files = [
+        MockFile('ui/android/string/chrome_android_string.grd', xmlChanges)
+    ]
+    result = checkxmlstyle._CheckStringResourcePunctuations(
+        mock_input_api, MockOutputApi())
+
+    self.assertEqual(1, len(result))
+    self.assertEqual(4, len(result[0].items))
+    self.assertEqual('  ui/android/string/chrome_android_string.grd:3',
+                     result[0].items[0].splitlines()[0])
+    self.assertEqual('  ui/android/string/chrome_android_string.grd:9',
+                     result[0].items[1].splitlines()[0])
+    self.assertEqual('  ui/android/string/chrome_android_string.grd:10',
+                     result[0].items[2].splitlines()[0])
+    self.assertEqual('  ui/android/string/chrome_android_string.grd:14',
+                     result[0].items[3].splitlines()[0])
+
+
 if __name__ == '__main__':
   unittest.main()

tools/android/checkxmlstyle/helpers.py

@@ -23,6 +23,9 @@ TEXT_APPEARANCE_STYLE_PATTERN = re.compile(r'^TextAppearance\.')
 INCLUDED_PATHS = [
     r'^(chrome|ui|components|content)[\\/](.*[\\/])?java[\\/]res.+\.xml$'
 ]
+INCLUDED_GRD_PATHS = [
+    r'^(chrome|ui|components|content)[\\/](.*[\\/])?android[\\/](.*)\.grd$'
+]
 # TODO(lazzzis): check color references in java source files
 COLOR_REFERENCE_PATTERN = re.compile('''
     @color/   # starts with '@color'