Teach check-webkit-style how to check the syntax of JSON files
authoraroben@apple.com <aroben@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Dec 2011 22:37:51 +0000 (22:37 +0000)
committeraroben@apple.com <aroben@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Dec 2011 22:37:51 +0000 (22:37 +0000)
Fixes <http://webkit.org/b/73590> check-webkit-style doesn't flag JSON syntax errors

Reviewed by Darin Adler.

* Scripts/webkitpy/style/checker.py:
(_all_categories): Added JSONChecker's categories to the set of all categories.
(FileType): Added a JSON type. Incremented other types.
(CheckerDispatcher._file_type): Use the JSON file type for .json files.
(CheckerDispatcher._create_checker): Use a JSONChecker for JSON files.

* Scripts/webkitpy/style/checker_unittest.py:
(CheckerDispatcherDispatchTest.assert_checker_json): Added this helper method.
(CheckerDispatcherDispatchTest.test_json_paths): Added. Based on test_python_paths.

* Scripts/webkitpy/style/checkers/jsonchecker.py: Added. (I didn't name this just "json",
which would have matched our other checkers, because I couldn't figure out how to call
"json.loads" without hitting namespace conflicts.)
(JSONChecker.__init__): Turn of line filtering so that we always check the whole file, not
just the modified lines from a patch.
(JSONChecker.check): Try to parse the lines as JSON. Mark an error if there was an
exception.
(JSONChecker.line_number_from_json_exception): Parse the json modules exception message to
try to extract a line number.

* Scripts/webkitpy/style/checkers/jsonchecker_unittest.py: Added.
(MockErrorHandler.__init__):
(MockErrorHandler.turn_off_line_filtering):
(MockErrorHandler.__call__):
Helper class. Copied from xml_unittest.py.

(JSONCheckerTest.test_line_number_from_json_exception): Test the
line_number_from_json_exception helper method.

(JSONCheckerTest.assert_no_error):
(JSONCheckerTest.assert_error):
Helper methods to assert that we did or didn't get an error.

(JSONCheckerTest.mock_handle_style_error): Helper method.

(JSONCheckerTest.test_conflict_marker):
(JSONCheckerTest.test_single_quote):
(JSONCheckerTest.test_init):
(JSONCheckerTest.test_missing_closing_brace):
(JSONCheckerTest.test_no_error):
Test various cases.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@101711 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/style/checker.py
Tools/Scripts/webkitpy/style/checker_unittest.py
Tools/Scripts/webkitpy/style/checkers/jsonchecker.py [new file with mode: 0644]
Tools/Scripts/webkitpy/style/checkers/jsonchecker_unittest.py [new file with mode: 0755]

index 46fff6f..91f5ac3 100644 (file)
@@ -1,3 +1,53 @@
+2011-12-01  Adam Roben  <aroben@apple.com>
+
+        Teach check-webkit-style how to check the syntax of JSON files
+
+        Fixes <http://webkit.org/b/73590> check-webkit-style doesn't flag JSON syntax errors
+
+        Reviewed by Darin Adler.
+
+        * Scripts/webkitpy/style/checker.py:
+        (_all_categories): Added JSONChecker's categories to the set of all categories.
+        (FileType): Added a JSON type. Incremented other types.
+        (CheckerDispatcher._file_type): Use the JSON file type for .json files.
+        (CheckerDispatcher._create_checker): Use a JSONChecker for JSON files.
+
+        * Scripts/webkitpy/style/checker_unittest.py:
+        (CheckerDispatcherDispatchTest.assert_checker_json): Added this helper method.
+        (CheckerDispatcherDispatchTest.test_json_paths): Added. Based on test_python_paths.
+
+        * Scripts/webkitpy/style/checkers/jsonchecker.py: Added. (I didn't name this just "json",
+        which would have matched our other checkers, because I couldn't figure out how to call
+        "json.loads" without hitting namespace conflicts.)
+        (JSONChecker.__init__): Turn of line filtering so that we always check the whole file, not
+        just the modified lines from a patch.
+        (JSONChecker.check): Try to parse the lines as JSON. Mark an error if there was an
+        exception.
+        (JSONChecker.line_number_from_json_exception): Parse the json modules exception message to
+        try to extract a line number.
+
+        * Scripts/webkitpy/style/checkers/jsonchecker_unittest.py: Added.
+        (MockErrorHandler.__init__):
+        (MockErrorHandler.turn_off_line_filtering):
+        (MockErrorHandler.__call__):
+        Helper class. Copied from xml_unittest.py.
+
+        (JSONCheckerTest.test_line_number_from_json_exception): Test the
+        line_number_from_json_exception helper method.
+
+        (JSONCheckerTest.assert_no_error):
+        (JSONCheckerTest.assert_error):
+        Helper methods to assert that we did or didn't get an error.
+
+        (JSONCheckerTest.mock_handle_style_error): Helper method.
+
+        (JSONCheckerTest.test_conflict_marker):
+        (JSONCheckerTest.test_single_quote):
+        (JSONCheckerTest.test_init):
+        (JSONCheckerTest.test_missing_closing_brace):
+        (JSONCheckerTest.test_no_error):
+        Test various cases.
+
 2011-12-01  Martin Robinson  <mrobinson@igalia.com>
 
         [GTK] Add a helper function to find the current executable's path
index 07c8964..d770da3 100644 (file)
@@ -39,6 +39,7 @@ from checkers.common import categories as CommonCategories
 from checkers.common import CarriageReturnChecker
 from checkers.changelog import ChangeLogChecker
 from checkers.cpp import CppChecker
+from checkers.jsonchecker import JSONChecker
 from checkers.python import PythonChecker
 from checkers.test_expectations import TestExpectationsChecker
 from checkers.text import TextChecker
@@ -229,6 +230,8 @@ _CPP_FILE_EXTENSIONS = [
     'h',
     ]
 
+_JSON_FILE_EXTENSION = 'json'
+
 _PYTHON_FILE_EXTENSION = 'py'
 
 _TEXT_FILE_EXTENSIONS = [
@@ -303,6 +306,7 @@ def _all_categories():
     """Return the set of all categories used by check-webkit-style."""
     # Take the union across all checkers.
     categories = CommonCategories.union(CppChecker.categories)
+    categories = categories.union(JSONChecker.categories)
     categories = categories.union(TestExpectationsChecker.categories)
     categories = categories.union(ChangeLogChecker.categories)
 
@@ -445,11 +449,12 @@ class FileType:
     # Alphabetize remaining types
     CHANGELOG = 1
     CPP = 2
-    PYTHON = 3
-    TEXT = 4
-    WATCHLIST = 5
-    XML = 6
-    XCODEPROJ = 7
+    JSON = 3
+    PYTHON = 4
+    TEXT = 5
+    WATCHLIST = 6
+    XML = 7
+    XCODEPROJ = 8
 
 
 class CheckerDispatcher(object):
@@ -512,6 +517,8 @@ class CheckerDispatcher(object):
             # reading from stdin, cpp_style tests should not rely on
             # the extension.
             return FileType.CPP
+        elif file_extension == _JSON_FILE_EXTENSION:
+            return FileType.JSON
         elif file_extension == _PYTHON_FILE_EXTENSION:
             return FileType.PYTHON
         elif file_extension in _XML_FILE_EXTENSIONS:
@@ -542,6 +549,8 @@ class CheckerDispatcher(object):
             file_extension = self._file_extension(file_path)
             checker = CppChecker(file_path, file_extension,
                                  handle_style_error, min_confidence)
+        elif file_type == FileType.JSON:
+            checker = JSONChecker(file_path, handle_style_error)
         elif file_type == FileType.PYTHON:
             checker = PythonChecker(file_path, handle_style_error)
         elif file_type == FileType.XML:
index 447fd39..86c7ef7 100755 (executable)
@@ -53,6 +53,7 @@ from checker import StyleProcessor
 from checker import StyleProcessorConfiguration
 from checkers.changelog import ChangeLogChecker
 from checkers.cpp import CppChecker
+from checkers.jsonchecker import JSONChecker
 from checkers.python import PythonChecker
 from checkers.text import TextChecker
 from checkers.xml import XMLChecker
@@ -405,6 +406,10 @@ class CheckerDispatcherDispatchTest(unittest.TestCase):
         """Assert that the dispatched checker is a CppChecker."""
         self.assert_checker(file_path, CppChecker)
 
+    def assert_checker_json(self, file_path):
+        """Assert that the dispatched checker is a JSONChecker."""
+        self.assert_checker(file_path, JSONChecker)
+
     def assert_checker_python(self, file_path):
         """Assert that the dispatched checker is a PythonChecker."""
         self.assert_checker(file_path, PythonChecker)
@@ -467,6 +472,25 @@ class CheckerDispatcherDispatchTest(unittest.TestCase):
         self.assertEquals(checker.file_extension, file_extension)
         self.assertEquals(checker.file_path, file_path)
 
+    def test_json_paths(self):
+        """Test paths that should be checked as JSON."""
+        paths = [
+           "Source/WebCore/inspector/Inspector.json",
+           "Tools/BuildSlaveSupport/build.webkit.org-config/config.json",
+        ]
+
+        for path in paths:
+            self.assert_checker_json(path)
+
+        # Check checker attributes on a typical input.
+        file_base = "foo"
+        file_extension = "json"
+        file_path = file_base + "." + file_extension
+        self.assert_checker_json(file_path)
+        checker = self.dispatch(file_path)
+        self.assertEquals(checker._handle_style_error,
+                          self.mock_handle_style_error)
+
     def test_python_paths(self):
         """Test paths that should be checked as Python."""
         paths = [
diff --git a/Tools/Scripts/webkitpy/style/checkers/jsonchecker.py b/Tools/Scripts/webkitpy/style/checkers/jsonchecker.py
new file mode 100644 (file)
index 0000000..0c2c87f
--- /dev/null
@@ -0,0 +1,54 @@
+# Copyright (C) 2011 Apple Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1.  Redistributions of source code must retain the above copyright
+#     notice, this list of conditions and the following disclaimer.
+# 2.  Redistributions in binary form must reproduce the above copyright
+#     notice, this list of conditions and the following disclaimer in the
+#     documentation and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR
+# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+"""Checks WebKit style for JSON files."""
+
+import re
+
+try:
+    import json
+except ImportError:
+    # python 2.5 compatibility
+    import webkitpy.thirdparty.simplejson as json
+
+
+class JSONChecker(object):
+    """Processes JSON lines for checking style."""
+
+    categories = set(('json/syntax',))
+
+    def __init__(self, file_path, handle_style_error):
+        self._handle_style_error = handle_style_error
+        self._handle_style_error.turn_off_line_filtering()
+
+    def check(self, lines):
+        try:
+            json.loads('\n'.join(lines) + '\n')
+        except ValueError, e:
+            self._handle_style_error(self.line_number_from_json_exception(e), 'json/syntax', 5, e.message)
+
+    @staticmethod
+    def line_number_from_json_exception(error):
+        match = re.search(r': line (?P<line>\d+) column \d+', error.message)
+        if not match:
+            return 0
+        return int(match.group('line'))
diff --git a/Tools/Scripts/webkitpy/style/checkers/jsonchecker_unittest.py b/Tools/Scripts/webkitpy/style/checkers/jsonchecker_unittest.py
new file mode 100755 (executable)
index 0000000..95ab83a
--- /dev/null
@@ -0,0 +1,118 @@
+#!/usr/bin/env python
+#
+# Copyright (C) 2010 Apple Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1.  Redistributions of source code must retain the above copyright
+#     notice, this list of conditions and the following disclaimer.
+# 2.  Redistributions in binary form must reproduce the above copyright
+#     notice, this list of conditions and the following disclaimer in the
+#     documentation and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR
+# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+"""Unit test for jsonchecker.py."""
+
+import unittest
+
+import jsonchecker
+
+
+class MockErrorHandler(object):
+    def __init__(self, handle_style_error):
+        self.turned_off_filtering = False
+        self._handle_style_error = handle_style_error
+
+    def turn_off_line_filtering(self):
+        self.turned_off_filtering = True
+
+    def __call__(self, line_number, category, confidence, message):
+        self._handle_style_error(self, line_number, category, confidence, message)
+
+
+class JSONCheckerTest(unittest.TestCase):
+    """Tests JSONChecker class."""
+
+    def test_line_number_from_json_exception(self):
+        tests = (
+            (0, 'No JSON object could be decoded'),
+            (2, 'Expecting property name: line 2 column 1 (char 2)'),
+            (3, 'Expecting object: line 3 column 1 (char 15)'),
+            (9, 'Expecting property name: line 9 column 21 (char 478)'),
+        )
+        for expected_line, message in tests:
+            self.assertEqual(expected_line, jsonchecker.JSONChecker.line_number_from_json_exception(ValueError(message)))
+
+    def assert_no_error(self, json_data):
+        def handle_style_error(mock_error_handler, line_number, category, confidence, message):
+            self.fail('Unexpected error: %d %s %d %s' % (line_number, category, confidence, message))
+
+        error_handler = MockErrorHandler(handle_style_error)
+        checker = jsonchecker.JSONChecker('foo.json', error_handler)
+        checker.check(json_data.split('\n'))
+        self.assertTrue(error_handler.turned_off_filtering)
+
+    def assert_error(self, expected_line_number, expected_category, json_data):
+        def handle_style_error(mock_error_handler, line_number, category, confidence, message):
+            mock_error_handler.had_error = True
+            self.assertEquals(expected_line_number, line_number)
+            self.assertEquals(expected_category, category)
+            self.assertIn(category, jsonchecker.JSONChecker.categories)
+
+        error_handler = MockErrorHandler(handle_style_error)
+        error_handler.had_error = False
+
+        checker = jsonchecker.JSONChecker('foo.json', error_handler)
+        checker.check(json_data.split('\n'))
+        self.assertTrue(error_handler.had_error)
+        self.assertTrue(error_handler.turned_off_filtering)
+
+    def mock_handle_style_error(self):
+        pass
+
+    def test_conflict_marker(self):
+        self.assert_error(0, 'json/syntax', '<<<<<<< HEAD\n{\n}\n')
+
+    def test_single_quote(self):
+        self.assert_error(2, 'json/syntax', "{\n'slaves': []\n}\n")
+
+    def test_init(self):
+        error_handler = MockErrorHandler(self.mock_handle_style_error)
+        checker = jsonchecker.JSONChecker('foo.json', error_handler)
+        self.assertEquals(checker._handle_style_error, error_handler)
+
+    def test_missing_closing_brace(self):
+        self.assert_error(3, 'json/syntax', '{\n"slaves": []\n')
+
+    def test_no_error(self):
+        self.assert_no_error("""{
+    "slaves":     [ { "name": "test-slave", "platform": "*" },
+                    { "name": "apple-xserve-4", "platform": "mac-snowleopard" }
+                  ],
+
+    "builders":   [ { "name": "SnowLeopard Intel Release (Build)", "type": "Build", "builddir": "snowleopard-intel-release",
+                      "platform": "mac-snowleopard", "configuration": "release", "architectures": ["x86_64"],
+                      "slavenames": ["apple-xserve-4"]
+                    }
+                   ],
+
+    "schedulers": [ { "type": "PlatformSpecificScheduler", "platform": "mac-snowleopard", "branch": "trunk", "treeStableTimer": 45.0,
+                      "builderNames": ["SnowLeopard Intel Release (Build)", "SnowLeopard Intel Debug (Build)"]
+                    }
+                  ]
+}
+""")
+
+if __name__ == '__main__':
+    unittest.main()