From 0ec64204b0adad8f367ba9b8e1e9333722acf0f2 Mon Sep 17 00:00:00 2001
From: Tommy Ip <hkmp7tommy@gmail.com>
Date: Sun, 5 Mar 2017 16:21:32 +0000
Subject: [PATCH] [WIP] Add support for linting a range of commits natively.

Fixes #14.
---
 gitlint/cli.py            | 24 +++++++++---
 gitlint/git.py            | 81 +++++++++++++++++++--------------------
 gitlint/tests/test_cli.py | 10 ++---
 gitlint/tests/test_git.py | 75 ++++++++++++++++--------------------
 4 files changed, 96 insertions(+), 94 deletions(-)

diff --git a/gitlint/cli.py b/gitlint/cli.py
index 81992b07..e61b7e9f 100644
--- a/gitlint/cli.py
+++ b/gitlint/cli.py
@@ -70,6 +70,7 @@ def build_config(ctx, target, config_path, c, extra_path, ignore, verbose, silen
 @click.option('-c', multiple=True,
               help="Config flags in format <rule>.<option>=<value> (e.g.: -c T1.line-length=80). " +
                    "Flag can be used multiple times to set multiple config values.")  # pylint: disable=bad-continuation
+@click.option('--commits', default='HEAD^...', help="The range of commits to lint.")
 @click.option('-e', '--extra-path', help="Path to a directory with extra user-defined rules",
               type=click.Path(exists=True, resolve_path=True, file_okay=False, readable=True))
 @click.option('--ignore', default="", help="Ignore rules (comma-separated by id or name).")
@@ -79,14 +80,14 @@ def build_config(ctx, target, config_path, c, extra_path, ignore, verbose, silen
 @click.option('-d', '--debug', help="Enable debugging output.", is_flag=True)
 @click.version_option(version=gitlint.__version__)
 @click.pass_context
-def cli(ctx, target, config, c, extra_path, ignore, verbose, silent, debug):
+def cli(ctx, target, config, c, commits, extra_path, ignore, verbose, silent, debug):
     """ Git lint tool, checks your git commit messages for styling issues """
 
     # Get the lint config from the commandline parameters and
     # store it in the context (click allows storing an arbitrary object in ctx.obj).
     config, config_builder = build_config(ctx, target, config, c, extra_path, ignore, verbose, silent, debug)
 
-    ctx.obj = (config, config_builder)
+    ctx.obj = (config, config_builder, commits)
 
     # If no subcommand is specified, then just lint
     if ctx.invoked_subcommand is None:
@@ -101,7 +102,7 @@ def lint(ctx):
     try:
         if sys.stdin.isatty():
             # If target has not been set explicitly before, fallback to the current directory
-            gitcontext = GitContext.from_local_repository(lint_config.target)
+            gitcontext = GitContext.from_local_repository(lint_config.target, ctx.obj[2])
         else:
             stdin_str = ustr(sys.stdin.read())
             gitcontext = GitContext.from_commit_msg(stdin_str)
@@ -117,8 +118,21 @@ def lint(ctx):
 
     # Let's get linting!
     linter = GitLinter(lint_config)
-    violations = linter.lint(last_commit)
-    linter.print_violations(violations)
+    number_of_commits = len(gitcontext.commits)
+    first_violation = True
+
+    for commit in gitcontext.commits:
+        violations = linter.lint(commit)
+        if violations:
+            # Display the commit hash & new lines intelligently
+            if number_of_commits > 1 and commit.sha:
+                click.echo(u"{0}Commit {1}:".format(
+                    "\n" if not first_violation or commit is last_commit else "",
+                    commit.sha[:10]
+                ))
+            linter.print_violations(violations)
+            first_violation = False
+
     exit_code = min(MAX_VIOLATION_ERROR_CODE, len(violations))
     ctx.exit(exit_code)
 
diff --git a/gitlint/git.py b/gitlint/git.py
index d459b788..2396fded 100644
--- a/gitlint/git.py
+++ b/gitlint/git.py
@@ -54,26 +54,18 @@ class GitCommit(object):
         In the context of gitlint, only the git context and commit message are required.
     """
 
-    def __init__(self, context, message, date=None, author_name=None, author_email=None, parents=None,
+    def __init__(self, context, message, sha=None, date=None, author_name=None, author_email=None, parents=None,
                  is_merge_commit=False, changed_files=None):
         self.context = context
         self.message = message
+        self.sha = sha
+        self.date = date
         self.author_name = author_name
         self.author_email = author_email
-        self.date = date
-
         # parent commit hashes
-        if not parents:
-            self.parents = []
-        else:
-            self.parents = parents
-
+        self.parents = parents or []
         self.is_merge_commit = is_merge_commit
-
-        if not changed_files:
-            self.changed_files = []
-        else:
-            self.changed_files = changed_files
+        self.changed_files = changed_files or []
 
     def __unicode__(self):
         format_str = u"Author: %s <%s>\nDate:   %s\n%s"  # pragma: no cover
@@ -88,7 +80,8 @@ def __repr__(self):
     def __eq__(self, other):
         # skip checking the context as context refers back to this obj, this will trigger a cyclic dependency
         return isinstance(other, GitCommit) and self.message == other.message and \
-               self.author_name == other.author_name and self.author_email == other.author_email and \
+               self.sha == other.sha and self.author_name == other.author_name and \
+               self.author_email == other.author_email and \
                self.date == other.date and self.parents == other.parents and \
                self.is_merge_commit == other.is_merge_commit and self.changed_files == other.changed_files  # noqa
 
@@ -117,10 +110,12 @@ def from_commit_msg(commit_msg_str):
         return context
 
     @staticmethod
-    def from_local_repository(repository_path):
+    def from_local_repository(repository_path, refspec="HEAD^..."):
         """ Retrieves the git context from a local git repository.
         :param repository_path: Path to the git repository to retrieve the context from
+        :param refspec: The commit(s) to retrieve
         """
+
         context = GitContext()
         try:
             # Special arguments passed to sh: http://amoffat.github.io/sh/special_arguments.html
@@ -129,18 +124,35 @@ def from_local_repository(repository_path):
                 '_cwd': repository_path
             }
 
-            # Get info from the local git repository
-            # https://git-scm.com/docs/pretty-formats
-            commit_msg = sh.git.log("-1", "--pretty=%B", **sh_special_args)
-            commit_author_name = sh.git.log("-1", "--pretty=%aN", **sh_special_args)
-            commit_author_email = sh.git.log("-1", "--pretty=%aE", **sh_special_args)
-            # %aI -> ISO 8601-like format, while %aI is strict ISO 8601, it seems to be less widely supporte
-            commit_date_str = sh.git.log("-1", "--pretty=%ai", **sh_special_args)
-            commit_parents = sh.git.log("-1", "--pretty=%P", **sh_special_args).split(" ")
-            commit_is_merge_commit = len(commit_parents) > 1
-
-            # changed files in last commit
-            changed_files_str = sh.git("diff-tree", "--no-commit-id", "--name-only", "-r", "HEAD", **sh_special_args)
+            sha_list = sh.git("rev-list", refspec, **sh_special_args).split()
+            for sha in sha_list:
+                # Get info from the local git repository: https://git-scm.com/docs/pretty-formats
+                raw_commit = sh.git.log(sha, "-1", "--pretty=%aN,%aE,%ai,%P%n%B",
+                                        **sh_special_args).split("\n")
+                meta_data, commit_msg = raw_commit[0], "\n".join(raw_commit[1:])
+
+                name, email, date, parents = meta_data.split(",")
+
+                commit_parents = parents.split(" ")
+                commit_is_merge_commit = len(commit_parents) > 1
+
+                # changed files in last commit
+                changed_files = sh.git("diff-tree", "--no-commit-id", "--name-only",
+                                       "-r", sha, **sh_special_args).split()
+
+                # "YYYY-MM-DD HH:mm:ss Z" -> ISO 8601-like format
+                # Use arrow for datetime parsing, because apparently python is quirky around ISO-8601 dates:
+                # http://stackoverflow.com/a/30696682/381010
+                commit_date = arrow.get(ustr(date), "YYYY-MM-DD HH:mm:ss Z").datetime
+
+                # Create Git commit object with the retrieved info
+                commit_msg_obj = GitCommitMessage.from_full_message(commit_msg)
+                commit = GitCommit(context, commit_msg_obj, sha=sha, author_name=name,
+                                   author_email=email, date=commit_date, changed_files=changed_files,
+                                   parents=commit_parents, is_merge_commit=commit_is_merge_commit)
+
+                context.commits.append(commit)
+
         except CommandNotFound:
             error_msg = u"'git' command not found. You need to install git to use gitlint on a local repository. " + \
                         u"See https://git-scm.com/book/en/v2/Getting-Started-Installing-Git on how to install git."
@@ -153,21 +165,6 @@ def from_local_repository(repository_path):
                 error_msg = u"An error occurred while executing '{0}': {1}".format(e.full_cmd, error_msg)
             raise GitContextError(error_msg)
 
-        # "YYYY-MM-DD HH:mm:ss Z" -> ISO 8601-like format
-        # Use arrow for datetime parsing, because apparently python is quirky around ISO-8601 dates:
-        # http://stackoverflow.com/a/30696682/381010
-        commit_date = arrow.get(ustr(commit_date_str), "YYYY-MM-DD HH:mm:ss Z").datetime
-
-        # Create Git commit object with the retrieved info
-        changed_files = [changed_file for changed_file in changed_files_str.strip().split("\n")]
-        commit_msg_obj = GitCommitMessage.from_full_message(commit_msg)
-        commit = GitCommit(context, commit_msg_obj, author_name=commit_author_name, author_email=commit_author_email,
-                           date=commit_date, changed_files=changed_files, parents=commit_parents,
-                           is_merge_commit=commit_is_merge_commit)
-
-        # Create GitContext info with the commit object and return
-
-        context.commits.append(commit)
         return context
 
     def __eq__(self, other):
diff --git a/gitlint/tests/test_cli.py b/gitlint/tests/test_cli.py
index 83bd70ed..09fdb65c 100644
--- a/gitlint/tests/test_cli.py
+++ b/gitlint/tests/test_cli.py
@@ -38,13 +38,11 @@ def test_lint(self, sys, sh):
         sys.stdin.isatty.return_value = True
 
         def git_log_side_effect(*args, **_kwargs):
-            return_values = {'--pretty=%B': u"commït-title\n\ncommït-body", '--pretty=%aN': u"test åuthor",
-                             '--pretty=%aE': u"test-email@föo.com", '--pretty=%ai': "2016-12-03 15:28:15 01:00",
-                             '--pretty=%P': u"åbc"}
-            return return_values[args[1]]
+            return (u"test åuthor,test-email@föo.com,2016-12-03 15:28:15 01:00,åbc\n"
+                    u"commït-title\n\ncommït-body")
 
         sh.git.log.side_effect = git_log_side_effect
-        sh.git.return_value = u"file1.txt\npåth/to/file2.txt\n"
+        sh.git.side_effect = ["6f29bf81a8322a04071bb794666e48c443a90360", u"file1.txt\npåth/to/file2.txt\n"]
 
         with patch('gitlint.display.stderr', new=StringIO()) as stderr:
             result = self.cli.invoke(cli.cli)
@@ -200,7 +198,7 @@ def test_generate_config_negative(self):
     @patch('gitlint.cli.sys')
     def test_git_error(self, sys, sh):
         sys.stdin.isatty.return_value = True
-        sh.git.log.side_effect = CommandNotFound("git")
+        sh.git.side_effect = CommandNotFound("git")
         result = self.cli.invoke(cli.cli)
         self.assertEqual(result.exit_code, self.GIT_CONTEXT_ERROR_CODE)
 
diff --git a/gitlint/tests/test_git.py b/gitlint/tests/test_git.py
index f8a48293..38dd4bf7 100644
--- a/gitlint/tests/test_git.py
+++ b/gitlint/tests/test_git.py
@@ -12,14 +12,14 @@
 class GitTests(BaseTestCase):
     @patch('gitlint.git.sh')
     def test_get_latest_commit(self, sh):
+        sample_sha = "d8ac47e9f2923c7f22d8668e3a1ed04eb4cdbca9"
+
         def git_log_side_effect(*args, **_kwargs):
-            return_values = {'--pretty=%B': u"cömmit-title\n\ncömmit-body", '--pretty=%aN': u"test åuthor",
-                             '--pretty=%aE': u"test-emåil@foo.com", '--pretty=%ai': u"2016-12-03 15:28:15 01:00",
-                             '--pretty=%P': u"åbc"}
-            return return_values[args[1]]
+            return (u"test åuthor,test-emåil@foo.com,2016-12-03 15:28:15 01:00,åbc\n"
+                    u"cömmit-title\n\ncömmit-body")
 
+        sh.git.side_effect = [sample_sha, u"file1.txt\npåth/to/file2.txt\n"]
         sh.git.log.side_effect = git_log_side_effect
-        sh.git.return_value = u"file1.txt\npåth/to/file2.txt\n"
 
         context = GitContext.from_local_repository(u"fake/påth")
         expected_sh_special_args = {
@@ -27,13 +27,12 @@ def git_log_side_effect(*args, **_kwargs):
             '_cwd': u"fake/påth"
         }
         # assert that commit info was read using git command
-        expected_calls = [call('-1', '--pretty=%B', _cwd=u"fake/påth", _tty_out=False),
-                          call('-1', '--pretty=%aN', _cwd=u"fake/påth", _tty_out=False),
-                          call('-1', '--pretty=%aE', _cwd=u"fake/påth", _tty_out=False),
-                          call('-1', '--pretty=%ai', _cwd=u"fake/påth", _tty_out=False),
-                          call('-1', '--pretty=%P', _cwd=u"fake/påth", _tty_out=False)]
-
-        self.assertListEqual(sh.git.log.mock_calls, expected_calls)
+        expected_calls = [
+            call("rev-list", "HEAD^...", **expected_sh_special_args),
+            call.log(sample_sha, "-1", "--pretty=%aN,%aE,%ai,%P%n%B", _cwd=u"fake/påth", _tty_out=False),
+            call('diff-tree', '--no-commit-id', '--name-only', '-r', sample_sha, **expected_sh_special_args)
+        ]
+        self.assertListEqual(sh.git.mock_calls, expected_calls)
 
         last_commit = context.commits[-1]
         self.assertEqual(last_commit.message.title, u"cömmit-title")
@@ -44,22 +43,18 @@ def git_log_side_effect(*args, **_kwargs):
                                                              tzinfo=dateutil.tz.tzoffset("+0100", 3600)))
         self.assertListEqual(last_commit.parents, [u"åbc"])
         self.assertFalse(last_commit.is_merge_commit)
-
-        # assert that changed files are read using git command
-        sh.git.assert_called_once_with('diff-tree', '--no-commit-id', '--name-only', '-r', 'HEAD',
-                                       **expected_sh_special_args)
         self.assertListEqual(last_commit.changed_files, ["file1.txt", u"påth/to/file2.txt"])
 
     @patch('gitlint.git.sh')
     def test_get_latest_commit_merge_commit(self, sh):
+        sample_sha = "d8ac47e9f2923c7f22d8668e3a1ed04eb4cdbca9"
+
         def git_log_side_effect(*args, **_kwargs):
-            return_values = {'--pretty=%B': u"Merge \"foo bår commit\"", '--pretty=%aN': u"test åuthor",
-                             '--pretty=%aE': u"test-emåil@foo.com", '--pretty=%ai': u"2016-12-03 15:28:15 01:00",
-                             '--pretty=%P': u"åbc def"}
-            return return_values[args[1]]
+            return (u"test åuthor,test-emåil@foo.com,2016-12-03 15:28:15 01:00,åbc def\n"
+                    "Merge \"foo bår commit\"")
 
+        sh.git.side_effect = [sample_sha, u"file1.txt\npåth/to/file2.txt\n"]
         sh.git.log.side_effect = git_log_side_effect
-        sh.git.return_value = u"file1.txt\npåth/to/file2.txt\n"
 
         context = GitContext.from_local_repository(u"fåke/path")
         expected_sh_special_args = {
@@ -67,13 +62,14 @@ def git_log_side_effect(*args, **_kwargs):
             '_cwd': u"fåke/path"
         }
         # assert that commit info was read using git command
-        expected_calls = [call('-1', '--pretty=%B', _cwd=u"fåke/path", _tty_out=False),
-                          call('-1', '--pretty=%aN', _cwd=u"fåke/path", _tty_out=False),
-                          call('-1', '--pretty=%aE', _cwd=u"fåke/path", _tty_out=False),
-                          call('-1', '--pretty=%ai', _cwd=u"fåke/path", _tty_out=False),
-                          call('-1', '--pretty=%P', _cwd=u"fåke/path", _tty_out=False)]
-
-        self.assertListEqual(sh.git.log.mock_calls, expected_calls)
+        expected_calls = [
+            call("rev-list", "HEAD^...", **expected_sh_special_args),
+            call.log(sample_sha, "-1", "--pretty=%aN,%aE,%ai,%P%n%B", _cwd=u"fåke/path", _tty_out=False),
+            call('diff-tree', '--no-commit-id', '--name-only', '-r', sample_sha, **expected_sh_special_args)
+        ]
+        print(sh.git.mock_calls[1])
+        print(expected_calls[1])
+        self.assertEqual(sh.git.mock_calls, expected_calls)
 
         last_commit = context.commits[-1]
         self.assertEqual(last_commit.message.title, u"Merge \"foo bår commit\"")
@@ -84,45 +80,42 @@ def git_log_side_effect(*args, **_kwargs):
                                                              tzinfo=dateutil.tz.tzoffset("+0100", 3600)))
         self.assertListEqual(last_commit.parents, [u"åbc", "def"])
         self.assertTrue(last_commit.is_merge_commit)
-
-        # assert that changed files are read using git command
-        sh.git.assert_called_once_with('diff-tree', '--no-commit-id', '--name-only', '-r', 'HEAD',
-                                       **expected_sh_special_args)
         self.assertListEqual(last_commit.changed_files, ["file1.txt", u"påth/to/file2.txt"])
 
     @patch('gitlint.git.sh')
     def test_get_latest_commit_command_not_found(self, sh):
-        sh.git.log.side_effect = CommandNotFound("git")
+        sh.git.side_effect = CommandNotFound("git")
         expected_msg = "'git' command not found. You need to install git to use gitlint on a local repository. " + \
                        "See https://git-scm.com/book/en/v2/Getting-Started-Installing-Git on how to install git."
         with self.assertRaisesRegex(GitContextError, expected_msg):
             GitContext.from_local_repository(u"fåke/path")
 
         # assert that commit message was read using git command
-        sh.git.log.assert_called_once_with('-1', '--pretty=%B', _tty_out=False, _cwd=u"fåke/path")
+        sh.git.assert_called_once_with("rev-list", "HEAD^...", _tty_out=False, _cwd=u"fåke/path")
 
     @patch('gitlint.git.sh')
     def test_get_latest_commit_git_error(self, sh):
         # Current directory not a git repo
         err = b"fatal: Not a git repository (or any of the parent directories): .git"
-        sh.git.log.side_effect = ErrorReturnCode("git log -1 --pretty=%B", b"", err)
+        sh.git.side_effect = ErrorReturnCode("git rev-list HEAD^...", b"", err)
 
         with self.assertRaisesRegex(GitContextError, u"fåke/path is not a git repository."):
             GitContext.from_local_repository(u"fåke/path")
 
         # assert that commit message was read using git command
-        sh.git.log.assert_called_once_with('-1', '--pretty=%B', _tty_out=False, _cwd=u"fåke/path")
-
-        sh.git.log.reset_mock()
+        sh.git.assert_called_once_with("rev-list", "HEAD^...",
+                                       _tty_out=False, _cwd=u"fåke/path")
+        sh.git.reset_mock()
         err = b"fatal: Random git error"
-        sh.git.log.side_effect = ErrorReturnCode("git log -1 --pretty=%B", b"", err)
+        sh.git.side_effect = ErrorReturnCode("git rev-list HEAD^...", b"", err)
 
-        expected_msg = u"An error occurred while executing 'git log -1 --pretty=%B': {0}".format(err)
+        expected_msg = u"An error occurred while executing 'git rev-list HEAD^...': {0}".format(err)
         with self.assertRaisesRegex(GitContextError, expected_msg):
             GitContext.from_local_repository(u"fåke/path")
 
         # assert that commit message was read using git command
-        sh.git.log.assert_called_once_with('-1', '--pretty=%B', _tty_out=False, _cwd=u"fåke/path")
+        sh.git.assert_called_once_with("rev-list", "HEAD^...",
+                                       _tty_out=False, _cwd=u"fåke/path")
 
     def test_from_commit_msg_full(self):
         gitcontext = GitContext.from_commit_msg(self.get_sample("commit_message/sample1"))