Add global presubmit that JSON and IDL files can be parsed.

Sometimes, changes to JSON/IDL files make them invalid. It's easier to check these at presubmit time than discovering they can't be parsed at runtime.

This just moves the check from chrome/common/extensions/api to top-level.
This presubmit check excludes files in test/data directories.

BUG=366395

Review URL: https://codereview.chromium.org/239283008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@274432 0039d316-1c4b-4281-b951-d872f2087c98

PRESUBMIT.py

@@ -1063,6 +1063,106 @@ def _CheckUserActionUpdate(input_api, output_api):
   return []
 
 
+def _GetJSONParseError(input_api, filename, eat_comments=True):
+  try:
+    contents = input_api.ReadFile(filename)
+    if eat_comments:
+      json_comment_eater = input_api.os_path.join(
+          input_api.PresubmitLocalPath(),
+          'tools', 'json_comment_eater', 'json_comment_eater.py')
+      process = input_api.subprocess.Popen(
+          [input_api.python_executable, json_comment_eater],
+          stdin=input_api.subprocess.PIPE,
+          stdout=input_api.subprocess.PIPE,
+          universal_newlines=True)
+      (contents, _) = process.communicate(input=contents)
+
+    input_api.json.loads(contents)
+  except ValueError as e:
+    return e
+  return None
+
+
+def _GetIDLParseError(input_api, filename):
+  try:
+    contents = input_api.ReadFile(filename)
+    idl_schema = input_api.os_path.join(
+        input_api.PresubmitLocalPath(),
+        'tools', 'json_schema_compiler', 'idl_schema.py')
+    process = input_api.subprocess.Popen(
+        [input_api.python_executable, idl_schema],
+        stdin=input_api.subprocess.PIPE,
+        stdout=input_api.subprocess.PIPE,
+        stderr=input_api.subprocess.PIPE,
+        universal_newlines=True)
+    (_, error) = process.communicate(input=contents)
+    return error or None
+  except ValueError as e:
+    return e
+
+
+def _CheckParseErrors(input_api, output_api):
+  """Check that IDL and JSON files do not contain syntax errors."""
+  actions = {
+    '.idl': _GetIDLParseError,
+    '.json': _GetJSONParseError,
+  }
+  # These paths contain test data and other known invalid JSON files.
+  excluded_patterns = [
+    'test/data/',
+    '^components/policy/resources/policy_templates.json$',
+  ]
+  # Most JSON files are preprocessed and support comments, but these do not.
+  json_no_comments_patterns = [
+    '^testing/',
+  ]
+  # Only run IDL checker on files in these directories.
+  idl_included_patterns = [
+    '^chrome/common/extensions/api/',
+    '^extensions/common/api/',
+  ]
+
+  def get_action(affected_file):
+    filename = affected_file.LocalPath()
+    return actions.get(input_api.os_path.splitext(filename)[1])
+
+  def MatchesFile(patterns, path):
+    for pattern in patterns:
+      if input_api.re.search(pattern, path):
+        return True
+    return False
+
+  def FilterFile(affected_file):
+    action = get_action(affected_file)
+    if not action:
+      return False
+    path = affected_file.LocalPath()
+
+    if MatchesFile(excluded_patterns, path):
+      return False
+
+    if (action == _GetIDLParseError and
+        not MatchesFile(idl_included_patterns, path)):
+      return False
+    return True
+
+  results = []
+  for affected_file in input_api.AffectedFiles(
+      file_filter=FilterFile, include_deletes=False):
+    action = get_action(affected_file)
+    kwargs = {}
+    if (action == _GetJSONParseError and
+        MatchesFile(json_no_comments_patterns, affected_file.LocalPath())):
+      kwargs['eat_comments'] = False
+    parse_error = action(input_api,
+                         affected_file.AbsoluteLocalPath(),
+                         **kwargs)
+    if parse_error:
+      results.append(output_api.PresubmitError('%s could not be parsed: %s' %
+          (affected_file.LocalPath(), parse_error)))
+  return results
+
+
 def _CheckJavaStyle(input_api, output_api):
   """Runs checkstyle on changed java files and returns errors if any exist."""
   original_sys_path = sys.path
@@ -1162,6 +1262,7 @@ def _CommonChecks(input_api, output_api):
   results.extend(_CheckCygwinShell(input_api, output_api))
   results.extend(_CheckUserActionUpdate(input_api, output_api))
   results.extend(_CheckNoDeprecatedCSS(input_api, output_api))
+  results.extend(_CheckParseErrors(input_api, output_api))
 
   if any('PRESUBMIT.py' == f.LocalPath() for f in input_api.AffectedFiles()):
     results.extend(input_api.canned_checks.RunUnitTestsInDirectory(

PRESUBMIT_test.py

@@ -3,23 +3,43 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import glob
+import json
 import os
 import re
+import subprocess
+import sys
 import unittest
 
 import PRESUBMIT
 
 
+_TEST_DATA_DIR = 'base/test/data/presubmit'
+
+
 class MockInputApi(object):
   def __init__(self):
+    self.json = json
     self.re = re
     self.os_path = os.path
+    self.python_executable = sys.executable
+    self.subprocess = subprocess
     self.files = []
     self.is_committing = False
 
   def AffectedFiles(self):
     return self.files
 
+  def PresubmitLocalPath(self):
+    return os.path.dirname(__file__)
+
+  def ReadFile(self, filename, mode='rU'):
+    for file_ in self.files:
+      if file_.LocalPath() == filename:
+        return '\n'.join(file_.NewContents())
+    # Otherwise, file is not in our mock API.
+    raise IOError, "No such file or directory: '%s'" % filename
+
 
 class MockOutputApi(object):
   class PresubmitResult(object):
@@ -431,5 +451,201 @@ def testFilesToCheckForIncomingDeps(self):
     self.assertEqual(expected, files_to_check);
 
 
+class JSONParsingTest(unittest.TestCase):
+  def testSuccess(self):
+    input_api = MockInputApi()
+    filename = 'valid_json.json'
+    contents = ['// This is a comment.',
+                '{',
+                '  "key1": ["value1", "value2"],',
+                '  "key2": 3  // This is an inline comment.',
+                '}'
+                ]
+    input_api.files = [MockFile(filename, contents)]
+    self.assertEqual(None,
+                     PRESUBMIT._GetJSONParseError(input_api, filename))
+
+  def testFailure(self):
+    input_api = MockInputApi()
+    test_data = [
+      ('invalid_json_1.json',
+       ['{ x }'],
+       'Expecting property name: line 1 column 2 (char 2)'),
+      ('invalid_json_2.json',
+       ['// Hello world!',
+        '{ "hello": "world }'],
+       'Unterminated string starting at: line 2 column 12 (char 12)'),
+      ('invalid_json_3.json',
+       ['{ "a": "b", "c": "d", }'],
+       'Expecting property name: line 1 column 22 (char 22)'),
+      ('invalid_json_4.json',
+       ['{ "a": "b" "c": "d" }'],
+       'Expecting , delimiter: line 1 column 11 (char 11)'),
+      ]
+
+    input_api.files = [MockFile(filename, contents)
+                       for (filename, contents, _) in test_data]
+
+    for (filename, _, expected_error) in test_data:
+      actual_error = PRESUBMIT._GetJSONParseError(input_api, filename)
+      self.assertEqual(expected_error, str(actual_error))
+
+  def testNoEatComments(self):
+    input_api = MockInputApi()
+    file_with_comments = 'file_with_comments.json'
+    contents_with_comments = ['// This is a comment.',
+                              '{',
+                              '  "key1": ["value1", "value2"],',
+                              '  "key2": 3  // This is an inline comment.',
+                              '}'
+                              ]
+    file_without_comments = 'file_without_comments.json'
+    contents_without_comments = ['{',
+                                 '  "key1": ["value1", "value2"],',
+                                 '  "key2": 3',
+                                 '}'
+                                 ]
+    input_api.files = [MockFile(file_with_comments, contents_with_comments),
+                       MockFile(file_without_comments,
+                                contents_without_comments)]
+
+    self.assertEqual('No JSON object could be decoded',
+                     str(PRESUBMIT._GetJSONParseError(input_api,
+                                                      file_with_comments,
+                                                      eat_comments=False)))
+    self.assertEqual(None,
+                     PRESUBMIT._GetJSONParseError(input_api,
+                                                  file_without_comments,
+                                                  eat_comments=False))
+
+
+class IDLParsingTest(unittest.TestCase):
+  def testSuccess(self):
+    input_api = MockInputApi()
+    filename = 'valid_idl_basics.idl'
+    contents = ['// Tests a valid IDL file.',
+                'namespace idl_basics {',
+                '  enum EnumType {',
+                '    name1,',
+                '    name2',
+                '  };',
+                '',
+                '  dictionary MyType1 {',
+                '    DOMString a;',
+                '  };',
+                '',
+                '  callback Callback1 = void();',
+                '  callback Callback2 = void(long x);',
+                '  callback Callback3 = void(MyType1 arg);',
+                '  callback Callback4 = void(EnumType type);',
+                '',
+                '  interface Functions {',
+                '    static void function1();',
+                '    static void function2(long x);',
+                '    static void function3(MyType1 arg);',
+                '    static void function4(Callback1 cb);',
+                '    static void function5(Callback2 cb);',
+                '    static void function6(Callback3 cb);',
+                '    static void function7(Callback4 cb);',
+                '  };',
+                '',
+                '  interface Events {',
+                '    static void onFoo1();',
+                '    static void onFoo2(long x);',
+                '    static void onFoo2(MyType1 arg);',
+                '    static void onFoo3(EnumType type);',
+                '  };',
+                '};'
+                ]
+    input_api.files = [MockFile(filename, contents)]
+    self.assertEqual(None,
+                     PRESUBMIT._GetIDLParseError(input_api, filename))
+
+  def testFailure(self):
+    input_api = MockInputApi()
+    test_data = [
+      ('invalid_idl_1.idl',
+       ['//',
+        'namespace test {',
+        '  dictionary {',
+        '    DOMString s;',
+        '  };',
+        '};'],
+       'Unexpected "{" after keyword "dictionary".\n'),
+      # TODO(yoz): Disabled because it causes the IDL parser to hang.
+      # See crbug.com/363830.
+      # ('invalid_idl_2.idl',
+      #  (['namespace test {',
+      #    '  dictionary MissingSemicolon {',
+      #    '    DOMString a',
+      #    '    DOMString b;',
+      #    '  };',
+      #    '};'],
+      #   'Unexpected symbol DOMString after symbol a.'),
+      ('invalid_idl_3.idl',
+       ['//',
+        'namespace test {',
+        '  enum MissingComma {',
+        '    name1',
+        '    name2',
+        '  };',
+        '};'],
+       'Unexpected symbol name2 after symbol name1.'),
+      ('invalid_idl_4.idl',
+       ['//',
+        'namespace test {',
+        '  enum TrailingComma {',
+        '    name1,',
+        '    name2,',
+        '  };',
+        '};'],
+       'Trailing comma in block.'),
+      ('invalid_idl_5.idl',
+       ['//',
+        'namespace test {',
+        '  callback Callback1 = void(;',
+        '};'],
+       'Unexpected ";" after "(".'),
+      ('invalid_idl_6.idl',
+       ['//',
+        'namespace test {',
+        '  callback Callback1 = void(long );',
+        '};'],
+       'Unexpected ")" after symbol long.'),
+      ('invalid_idl_7.idl',
+       ['//',
+        'namespace test {',
+        '  interace Events {',
+        '    static void onFoo1();',
+        '  };',
+        '};'],
+       'Unexpected symbol Events after symbol interace.'),
+      ('invalid_idl_8.idl',
+       ['//',
+        'namespace test {',
+        '  interface NotEvent {',
+        '    static void onFoo1();',
+        '  };',
+        '};'],
+       'Did not process Interface Interface(NotEvent)'),
+      ('invalid_idl_9.idl',
+       ['//',
+        'namespace test {',
+        '  interface {',
+        '    static void function1();',
+        '  };',
+        '};'],
+       'Interface missing name.'),
+      ]
+
+    input_api.files = [MockFile(filename, contents)
+                       for (filename, contents, _) in test_data]
+
+    for (filename, _, expected_error) in test_data:
+      actual_error = PRESUBMIT._GetIDLParseError(input_api, filename)
+      self.assertTrue(expected_error in str(actual_error),
+                      "'%s' not found in '%s'" % (expected_error, actual_error))
+
+
 if __name__ == '__main__':
   unittest.main()