From f98d234852b4a4cf1e23cdd19468c049a589cf07 Mon Sep 17 00:00:00 2001 From: rohilla21 Date: Wed, 14 Feb 2018 09:01:09 +0530 Subject: [PATCH] GitCommitBear.py: Detect revert commits This detects revert commits and checks they have a reason as well. Closes https://github.com/coala/coala-bears/issues/337 --- bears/vcs/git/GitCommitBear.py | 55 ++++++++++++++++++++++++++- tests/vcs/git/GitCommitBearTest.py | 60 +++++++++++++++++++++++++++++- 2 files changed, 111 insertions(+), 4 deletions(-) diff --git a/bears/vcs/git/GitCommitBear.py b/bears/vcs/git/GitCommitBear.py index 22414ee03d..e3c3d46c3e 100644 --- a/bears/vcs/git/GitCommitBear.py +++ b/bears/vcs/git/GitCommitBear.py @@ -67,6 +67,12 @@ def check_prerequisites(cls): else: return True + @classmethod + def get_revert_checks_metadata(cls): + return FunctionMetadata.from_function( + cls.check_revert, + omit={'self', 'shortlog', 'body'}) + @classmethod def get_shortlog_checks_metadata(cls): return FunctionMetadata.from_function( @@ -91,6 +97,7 @@ def get_metadata(cls): FunctionMetadata.from_function( cls.run, omit={'self', 'dependency_results'}), + cls.get_revert_checks_metadata(), cls.get_shortlog_checks_metadata(), cls.get_body_checks_metadata(), cls.get_issue_checks_metadata()) @@ -112,8 +119,7 @@ def get_host_from_remotes(): netloc = parsed_url.host return netloc.split('.')[0] - def run(self, - allow_empty_commit_message: bool = False, + def run(self, allow_empty_commit_message: bool = False, **kwargs): """ Check the current git commit message at HEAD. @@ -141,6 +147,10 @@ def run(self, yield Result(self, 'HEAD commit has no message.') return + yield from self.check_revert( + shortlog, + body, + **self.get_revert_checks_metadata().filter_parameters(kwargs)) yield from self.check_shortlog( shortlog, **self.get_shortlog_checks_metadata().filter_parameters(kwargs)) @@ -151,6 +161,47 @@ def run(self, body, **self.get_issue_checks_metadata().filter_parameters(kwargs)) + def check_revert(self, shortlog, body, + allow_revert_commits_without_reason: bool = False,): + """ + Validates revert commits. + + :param shortlog: The shortlog message string. + :param body: The body of the commit message of HEAD. + :param allow_revert_commits_without_reason: Whether reason is + needed for revert commits. + """ + REVERT_REGEX = re.compile(r'(Revert ")([\S+\s*]*)("$)') + HEADLINE_REGEX = re.compile(r'(^This reverts commit )([0-9a-f]{40})') + + if REVERT_REGEX.match(shortlog): + reverted_shortlog = re.search(REVERT_REGEX, shortlog).group(2) + + body = body.strip('\n') + blankline = body.find('\n\n') + headline = body[:blankline] if blankline != -1 else body + + if not HEADLINE_REGEX.match(headline): + yield Result(self, 'Invalid revert commit.') + return + + commit_sha = re.search(HEADLINE_REGEX, headline).group(2) + command = ('git log -n 1 ' + + str(commit_sha) + ' --pretty=%B') + rstdout, _ = run_shell_command(command) + rstdout = rstdout.rstrip('\n') + rpos = rstdout.find('\n') + old_shortlog = rstdout[:rpos] if rpos != -1 else rstdout + + if old_shortlog != reverted_shortlog: + yield Result(self, 'Shortlog of revert commit does ' + 'not match the original commit.') + + reason = body[blankline+1:] if blankline != -1 else '' + + if not allow_revert_commits_without_reason and len(reason) == 0: + yield Result(self, 'Revert commit does not have a reason.') + def check_shortlog(self, shortlog, shortlog_length: int = 50, shortlog_regex: str = '', diff --git a/tests/vcs/git/GitCommitBearTest.py b/tests/vcs/git/GitCommitBearTest.py index 87f747cb34..a0aa112aea 100644 --- a/tests/vcs/git/GitCommitBearTest.py +++ b/tests/vcs/git/GitCommitBearTest.py @@ -95,11 +95,13 @@ def test_get_metadata(self): metadata = GitCommitBear.get_metadata() self.assertEqual( metadata.name, - "") + "") # Test if at least one parameter of each signature is used. self.assertIn('allow_empty_commit_message', metadata.optional_params) + self.assertIn('allow_revert_commits_without_reason', + metadata.optional_params) self.assertIn('shortlog_length', metadata.optional_params) self.assertIn('body_line_length', metadata.optional_params) self.assertIn('body_close_issue', metadata.optional_params) @@ -125,6 +127,60 @@ def test_empty_message(self): []) self.assert_no_msgs() + def test_revert_checks(self): + self.git_commit('Test commit') + commit_hash, _ = run_shell_command('git rev-parse HEAD') + self.git_commit('Revert "Test commit"\n\n' + 'Abcdef blablabla commit ' + str(commit_hash) + '\n\n' + 'Reason for the revert commit is explained here.\n') + self.assertEqual(self.run_uut(), + ['Invalid revert commit.']) + self.assert_no_msgs() + + self.git_commit('Test commit1\n\n' + 'This is body of test commit1\n') + commit_hash, _ = run_shell_command('git rev-parse HEAD') + + self.git_commit('Revert "Another Test commit2"\n\n' + 'This reverts commit ' + str(commit_hash) + '\n\n' + 'Reason for the revert commit is explained here.\n') + self.assertEqual( + self.run_uut(), + ['Shortlog of revert commit does not match the original commit.']) + self.assert_no_msgs() + + self.git_commit('Test commit3') + commit_hash, _ = run_shell_command('git rev-parse HEAD') + self.git_commit('Revert "Test commit3"\n\n' + 'This reverts commit ' + str(commit_hash) + '\n\n') + self.assertEqual(self.run_uut(), + ['Revert commit does not have a reason.']) + self.assert_no_msgs() + + self.git_commit('Revert "Test commit3"\n\n' + 'This reverts commit ' + str(commit_hash) + '\n\n') + self.assertEqual( + self.run_uut(allow_revert_commits_without_reason=True), + []) + self.assert_no_msgs() + + self.git_commit('Revert "Test commit3"\n\n' + 'This reverts commit ' + str(commit_hash) + '\n\n' + 'Reason for the revert commit is explained here.\n') + self.assertEqual(self.run_uut(), []) + self.assert_no_msgs() + + self.git_commit('Test commit4\n\n' + 'This is body of test commit1\n') + commit_hash, _ = run_shell_command('git rev-parse HEAD') + self.git_commit('Revert "Another Test commit5"\n\n' + 'This reverts commit ' + str(commit_hash) + '\n\n' + 'Reason for the revert commit is explained here.\n') + self.assertEqual( + self.run_uut(), + ['Shortlog of revert commit does not match the original commit.']) + self.assert_no_msgs() + @unittest.mock.patch('bears.vcs.git.GitCommitBear.run_shell_command', return_value=('one-liner-message\n', '')) def test_pure_oneliner_message(self, patch):