webkitpy: Simplify DiffParser, removing support for SVN patches.

BUG=676012

Review-Url: https://codereview.chromium.org/2742453003
Cr-Commit-Position: refs/heads/master@{#455629}

third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/diff_parser.py

@@ -33,53 +33,9 @@ import re
 
 _log = logging.getLogger(__name__)
 
-conversion_patterns = (
-    (re.compile(r"^diff --git \w/(.+) \w/(?P<FilePath>.+)"), lambda matched: "Index: " + matched.group('FilePath') + "\n"),
-    (re.compile(r"^new file.*"), lambda matched: "\n"),
-    (re.compile(r"^index (([0-9a-f]{7}\.\.[0-9a-f]{7})|([0-9a-f]{40}\.\.[0-9a-f]{40})) [0-9]{6}"),
-     lambda matched: ("=" * 67) + "\n"),
-    (re.compile(r"^--- \w/(?P<FilePath>.+)"), lambda matched: "--- " + matched.group('FilePath') + "\n"),
-    (re.compile(r"^\+\+\+ \w/(?P<FilePath>.+)"), lambda matched: "+++ " + matched.group('FilePath') + "\n"),
-)
+INDEX_PATTERN = re.compile(r'^diff --git \w/(.+) \w/(?P<FilePath>.+)')
+LINES_CHANGED_PATTERN = re.compile(r"^@@ -(?P<OldStartLine>\d+)(,\d+)? \+(?P<NewStartLine>\d+)(,\d+)? @@")
 
-index_pattern = re.compile(r"^Index: (?P<FilePath>.+)")
-lines_changed_pattern = re.compile(r"^@@ -(?P<OldStartLine>\d+)(,\d+)? \+(?P<NewStartLine>\d+)(,\d+)? @@")
-diff_git_pattern = re.compile(r"^diff --git \w/")
-
-
-def git_diff_to_svn_diff(line):
-    """Converts a git formatted diff line to a svn formatted line.
-
-    Args:
-      line: A string representing a line of the diff.
-    """
-    for pattern, conversion in conversion_patterns:
-        matched = pattern.match(line)
-        if matched:
-            return conversion(matched)
-    return line
-
-
-# This function exists so we can unittest get_diff_converter function
-def svn_diff_to_svn_diff(line):
-    return line
-
-
-def get_diff_converter(lines):
-    """Gets a converter function of diff lines.
-
-    Args:
-      lines: The lines of a diff file.
-             If this line is git formatted, we'll return a
-             converter from git to SVN.
-    """
-    for i, line in enumerate(lines[:-1]):
-        # Stop when we find the first patch
-        if line[:3] == "+++" and lines[i + 1] == "---":
-            break
-        if diff_git_pattern.match(line):
-            return git_diff_to_svn_diff
-    return svn_diff_to_svn_diff
 
 _INITIAL_STATE = 1
 _DECLARED_FILE_PATH = 2
@@ -140,12 +96,10 @@ class DiffParser(object):
         current_file = None
         old_diff_line = None
         new_diff_line = None
-        transform_line = get_diff_converter(diff_input)
         for line in diff_input:
-            line = line.rstrip("\n")
-            line = transform_line(line)
+            line = line.rstrip('\n')
 
-            file_declaration = index_pattern.match(line)
+            file_declaration = INDEX_PATTERN.match(line)
             if file_declaration:
                 filename = file_declaration.group('FilePath')
                 current_file = DiffFile(filename)
@@ -153,7 +107,7 @@ class DiffParser(object):
                 state = _DECLARED_FILE_PATH
                 continue
 
-            lines_changed = lines_changed_pattern.match(line)
+            lines_changed = LINES_CHANGED_PATTERN.match(line)
             if lines_changed:
                 if state != _DECLARED_FILE_PATH and state != _PROCESSING_CHUNK:
                     _log.error('Unexpected line change without file path declaration: %r', line)

third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/diff_parser_unittest.py

@@ -26,44 +26,47 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-import cStringIO as StringIO
 import re
 import unittest
 
-from webkitpy.common.checkout import diff_parser
+from webkitpy.common.checkout.diff_parser import DiffParser
 from webkitpy.common.checkout.diff_test_data import DIFF_TEST_DATA
 
 
 class DiffParserTest(unittest.TestCase):
-    maxDiff = None
 
     def test_diff_parser(self, parser=None):
         if not parser:
-            parser = diff_parser.DiffParser(DIFF_TEST_DATA.splitlines())
+            parser = DiffParser(DIFF_TEST_DATA.splitlines())
         self.assertEqual(3, len(parser.files))
 
         self.assertIn('WebCore/style/StyleFlexibleBoxData.h', parser.files)
         diff = parser.files['WebCore/style/StyleFlexibleBoxData.h']
         self.assertEqual(7, len(diff.lines))
+
         # The first two unchanged lines.
         self.assertEqual((47, 47), diff.lines[0][0:2])
         self.assertEqual('', diff.lines[0][2])
         self.assertEqual((48, 48), diff.lines[1][0:2])
         self.assertEqual('    unsigned align : 3; // EBoxAlignment', diff.lines[1][2])
-        # The deleted line
+
+        # The deleted line.
         self.assertEqual((50, 0), diff.lines[3][0:2])
         self.assertEqual('    unsigned orient: 1; // EBoxOrient', diff.lines[3][2])
 
         # The first file looks OK. Let's check the next, more complicated file.
         self.assertIn('WebCore/style/StyleRareInheritedData.cpp', parser.files)
         diff = parser.files['WebCore/style/StyleRareInheritedData.cpp']
+
         # There are 3 chunks.
         self.assertEqual(7 + 7 + 9, len(diff.lines))
+
         # Around an added line.
         self.assertEqual((60, 61), diff.lines[9][0:2])
         self.assertEqual((0, 62), diff.lines[10][0:2])
         self.assertEqual((61, 63), diff.lines[11][0:2])
-        # Look through the last chunk, which contains both add's and delete's.
+
+        # Look through the last chunk, which contains both adds and deletes.
         self.assertEqual((81, 83), diff.lines[14][0:2])
         self.assertEqual((82, 84), diff.lines[15][0:2])
         self.assertEqual((83, 85), diff.lines[16][0:2])
@@ -79,99 +82,22 @@ class DiffParserTest(unittest.TestCase):
         self.assertEqual(1, len(diff.lines))
         self.assertEqual((0, 1), diff.lines[0][0:2])
 
-    def test_diff_converter(self):
-        comment_lines = [
-            "Hey people,\n",
-            "\n",
-            "See my awesome patch below!\n",
-            "\n",
-            " - Cool Hacker\n",
-            "\n",
-        ]
-
-        revision_lines = [
-            "Subversion Revision 289799\n",
-        ]
-
-        svn_diff_lines = [
-            "Index: Tools/Scripts/webkitpy/common/checkout/diff_parser.py\n",
-            "===================================================================\n",
-            "--- Tools/Scripts/webkitpy/common/checkout/diff_parser.py\n",
-            "+++ Tools/Scripts/webkitpy/common/checkout/diff_parser.py\n",
-            "@@ -59,6 +59,7 @@ def git_diff_to_svn_diff(line):\n",
-        ]
-        self.assertEqual(diff_parser.get_diff_converter(svn_diff_lines), diff_parser.svn_diff_to_svn_diff)
-        self.assertEqual(diff_parser.get_diff_converter(comment_lines + svn_diff_lines), diff_parser.svn_diff_to_svn_diff)
-        self.assertEqual(diff_parser.get_diff_converter(revision_lines + svn_diff_lines), diff_parser.svn_diff_to_svn_diff)
-
-        git_diff_lines = [
-            ("diff --git a/Tools/Scripts/webkitpy/common/checkout/diff_parser.py "
-             "b/Tools/Scripts/webkitpy/common/checkout/diff_parser.py\n"),
-            "index 3c5b45b..0197ead 100644\n",
-            "--- a/Tools/Scripts/webkitpy/common/checkout/diff_parser.py\n",
-            "+++ b/Tools/Scripts/webkitpy/common/checkout/diff_parser.py\n",
-            "@@ -59,6 +59,7 @@ def git_diff_to_svn_diff(line):\n",
-        ]
-        self.assertEqual(diff_parser.get_diff_converter(git_diff_lines), diff_parser.git_diff_to_svn_diff)
-        self.assertEqual(diff_parser.get_diff_converter(comment_lines + git_diff_lines), diff_parser.git_diff_to_svn_diff)
-        self.assertEqual(diff_parser.get_diff_converter(revision_lines + git_diff_lines), diff_parser.git_diff_to_svn_diff)
-
-    def test_git_mnemonicprefix(self):
-        p = re.compile(r' ([a|b])/')
-
-        prefixes = [
-            {'a': 'i', 'b': 'w'},  # git-diff (compares the (i)ndex and the (w)ork tree)
-            {'a': 'c', 'b': 'w'},  # git-diff HEAD (compares a (c)ommit and the (w)ork tree)
-            {'a': 'c', 'b': 'i'},  # git diff --cached (compares a (c)ommit and the (i)ndex)
-            {'a': 'o', 'b': 'w'},  # git-diff HEAD:file1 file2 (compares an (o)bject and a (w)ork tree entity)
-            {'a': '1', 'b': '2'},  # git diff --no-index a b (compares two non-git things (1) and (2))
-        ]
-
-        for prefix in prefixes:
-            patch = p.sub(lambda x: " %s/" % prefix[x.group(1)], DIFF_TEST_DATA)
-            self.test_diff_parser(diff_parser.DiffParser(patch.splitlines()))
-
-    def test_git_diff_to_svn_diff(self):
-        output = """\
-Index: Tools/Scripts/webkitpy/common/checkout/diff_parser.py
-===================================================================
---- Tools/Scripts/webkitpy/common/checkout/diff_parser.py
-+++ Tools/Scripts/webkitpy/common/checkout/diff_parser.py
-@@ -59,6 +59,7 @@ def git_diff_to_svn_diff(line):
- A
- B
- C
-+D
- E
- F
-"""
-
-        inputfmt = StringIO.StringIO("""\
-diff --git a/Tools/Scripts/webkitpy/common/checkout/diff_parser.py b/Tools/Scripts/webkitpy/common/checkout/diff_parser.py
-index 2ed552c4555db72df16b212547f2c125ae301a04..72870482000c0dba64ce4300ed782c03ee79b74f 100644
---- a/Tools/Scripts/webkitpy/common/checkout/diff_parser.py
-+++ b/Tools/Scripts/webkitpy/common/checkout/diff_parser.py
-@@ -59,6 +59,7 @@ def git_diff_to_svn_diff(line):
- A
- B
- C
-+D
- E
- F
-""")
-        shortfmt = StringIO.StringIO("""\
-diff --git a/Tools/Scripts/webkitpy/common/checkout/diff_parser.py b/Tools/Scripts/webkitpy/common/checkout/diff_parser.py
-index b48b162..f300960 100644
---- a/Tools/Scripts/webkitpy/common/checkout/diff_parser.py
-+++ b/Tools/Scripts/webkitpy/common/checkout/diff_parser.py
-@@ -59,6 +59,7 @@ def git_diff_to_svn_diff(line):
- A
- B
- C
-+D
- E
- F
-""")
-
-        self.assertMultiLineEqual(output, ''.join(diff_parser.git_diff_to_svn_diff(x) for x in shortfmt.readlines()))
-        self.assertMultiLineEqual(output, ''.join(diff_parser.git_diff_to_svn_diff(x) for x in inputfmt.readlines()))
+    def test_diff_parser_with_different_mnemonic_prefixes(self):
+        # This repeats test_diff_parser but with different versions
+        # of DIFF_TEST_DATA that use other prefixes instead of a/b.
+        prefixes = (
+            ('i', 'w'),  # git-diff (compares the (i)ndex and the (w)ork tree)
+            ('c', 'w'),  # git-diff HEAD (compares a (c)ommit and the (w)ork tree)
+            ('c', 'i'),  # git diff --cached (compares a (c)ommit and the (i)ndex)
+            ('o', 'w'),  # git-diff HEAD:file1 file2 (compares an (o)bject and a (w)ork tree entity)
+            ('1', '2'),  # git diff --no-index a b (compares two non-git things (1) and (2))
+        )
+        for a_replacement, b_replacement in prefixes:
+            patch = self._patch(a_replacement, b_replacement)
+            self.test_diff_parser(DiffParser(patch.splitlines()))
+
+    @staticmethod
+    def _patch(a_replacement='a', b_replacement='b'):
+        """Returns a version of the example patch with mnemonic prefixes a/b changed."""
+        patch = re.sub(r' a/', ' %s/' % a_replacement, DIFF_TEST_DATA)
+        return re.sub(r' b/', ' %s/' % b_replacement, patch)

third_party/WebKit/Tools/Scripts/webkitpy/style/patchreader.py

@@ -29,10 +29,8 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 import logging
-import re
 
 from webkitpy.common.checkout.diff_parser import DiffParser
-from webkitpy.common.system.filesystem import FileSystem
 
 
 _log = logging.getLogger(__name__)
@@ -49,26 +47,15 @@ class PatchReader(object):
         """
         self._text_file_reader = text_file_reader
 
-    def check(self, patch_string, fs=None):
-        """Check style in the given patch."""
-        fs = fs or FileSystem()
+    def check(self, patch_string):
+        """Checks style in the given patch."""
         patch_files = DiffParser(patch_string.splitlines()).files
 
-        # If the user uses git, checking subversion config file only once is enough.
-        # TODO(qyearsley): Simplify this since git is now the only supported SCM system.
-        call_only_once = True
-
         for path, diff_file in patch_files.iteritems():
             line_numbers = diff_file.added_or_modified_line_numbers()
             _log.debug('Found %s new or modified lines in: %s', len(line_numbers), path)
 
             if not line_numbers:
-                match = re.search(r"\s*png$", path)
-                if match and fs.exists(path):
-                    if call_only_once:
-                        self._text_file_reader.process_file(file_path=path, line_numbers=None)
-                        call_only_once = False
-                    continue
                 # Don't check files which contain only deleted lines
                 # as they can never add style errors. However, mark them as
                 # processed so that we count up number of such files.

third_party/WebKit/Tools/Scripts/webkitpy/style/patchreader_unittest.py

@@ -31,21 +31,16 @@
 
 import unittest
 
-from webkitpy.common.system.filesystem_mock import MockFileSystem
 from webkitpy.style.patchreader import PatchReader
 
 
 class PatchReaderTest(unittest.TestCase):
 
-    """Test the PatchReader class."""
-
     class MockTextFileReader(object):
 
         def __init__(self):
-            self.passed_to_process_file = []
-            """A list of (file_path, line_numbers) pairs."""
-            self.delete_only_file_count = 0
-            """A number of times count_delete_only_file() called"""
+            self.passed_to_process_file = []  # A list of (file_path, line_numbers) pairs.
+            self.delete_only_file_count = 0  # A number of times count_delete_only_file() called.
 
         def process_file(self, file_path, line_numbers):
             self.passed_to_process_file.append((file_path, line_numbers))
@@ -54,48 +49,41 @@ class PatchReaderTest(unittest.TestCase):
             self.delete_only_file_count += 1
 
     def setUp(self):
-        file_reader = self.MockTextFileReader()
-        self._file_reader = file_reader
-        self._patch_checker = PatchReader(file_reader)
-
-    def _call_check_patch(self, patch_string):
-        self._patch_checker.check(patch_string)
+        self._file_reader = self.MockTextFileReader()
 
     def _assert_checked(self, passed_to_process_file, delete_only_file_count):
-        self.assertEqual(self._file_reader.passed_to_process_file,
-                         passed_to_process_file)
-        self.assertEqual(self._file_reader.delete_only_file_count,
-                         delete_only_file_count)
+        self.assertEqual(self._file_reader.passed_to_process_file, passed_to_process_file)
+        self.assertEqual(self._file_reader.delete_only_file_count, delete_only_file_count)
 
     def test_check_patch(self):
-        # The modified line_numbers array for this patch is: [2].
-        self._call_check_patch("""diff --git a/__init__.py b/__init__.py
-index ef65bee..e3db70e 100644
---- a/__init__.py
-+++ b/__init__.py
-@@ -1,1 +1,2 @@
- # Required for Python to search this directory for module files
-+# New line
-""")
-        self._assert_checked([("__init__.py", [2])], 0)
+        PatchReader(self._file_reader).check(
+            'diff --git a/__init__.py b/__init__.py\n'
+            'index ef65bee..e3db70e 100644\n'
+            '--- a/__init__.py\n'
+            '+++ b/__init__.py\n'
+            '@@ -1,1 +1,2 @@\n'
+            ' # Required for Python to search this directory for module files\n'
+            '+# New line\n')
+        self._assert_checked(
+            passed_to_process_file=[('__init__.py', [2])],
+            delete_only_file_count=0)
 
     def test_check_patch_with_deletion(self):
-        self._call_check_patch("""Index: __init__.py
-===================================================================
---- __init__.py  (revision 3593)
-+++ __init__.py  (working copy)
-@@ -1 +0,0 @@
--foobar
-""")
-        # _mock_check_file should not be called for the deletion patch.
-        self._assert_checked([], 1)
+        PatchReader(self._file_reader).check(
+            'diff --git a/__init__.py b/__init.py\n'
+            'deleted file mode 100644\n'
+            'index ef65bee..0000000\n'
+            '--- a/__init__.py\n'
+            '+++ /dev/null\n'
+            '@@ -1 +0,0 @@\n'
+            '-foobar\n')
+        # The deleted file isn't be processed.
+        self._assert_checked(passed_to_process_file=[], delete_only_file_count=1)
 
     def test_check_patch_with_png_deletion(self):
-        fs = MockFileSystem()
-        diff_text = """Index: LayoutTests/platform/mac/foo-expected.png
-===================================================================
-Cannot display: file marked as a binary type.
-svn:mime-type = image/png
-"""
-        self._patch_checker.check(diff_text, fs)
-        self._assert_checked([], 1)
+        PatchReader(self._file_reader).check(
+            'diff --git a/foo-expected.png b/foo-expected.png\n'
+            'deleted file mode 100644\n'
+            'index ef65bee..0000000\n'
+            'Binary files a/foo-expected.png and /dev/null differ\n')
+        self._assert_checked(passed_to_process_file=[], delete_only_file_count=1)