From 7766f641fb28951c8333539876c2879ff458ccc0 Mon Sep 17 00:00:00 2001 From: Naveen Kumar Sangi Date: Fri, 30 Dec 2016 10:55:05 +0530 Subject: [PATCH] GitCommitBear.py: Require issue ref in commit body By default, this ensures that the git commit contains a short reference to an issue. If keyword regex is chosen, it verifies the presence of matching expression. Fixes https://github.com/coala/coala-bears/issues/1112 --- bears/vcs/git/GitCommitBear.py | 121 ++++++++++++++++++++++- tests/vcs/git/GitCommitBearTest.py | 149 ++++++++++++++++++++++++++++- 2 files changed, 268 insertions(+), 2 deletions(-) diff --git a/bears/vcs/git/GitCommitBear.py b/bears/vcs/git/GitCommitBear.py index 547b2bd336..59f5cfdd8f 100644 --- a/bears/vcs/git/GitCommitBear.py +++ b/bears/vcs/git/GitCommitBear.py @@ -3,6 +3,8 @@ import shutil import os +from urllib.parse import urlparse + from coalib.bears.GlobalBear import GlobalBear from dependency_management.requirements.PipRequirement import PipRequirement from coala_utils.ContextManagers import change_directory @@ -20,6 +22,15 @@ class GitCommitBear(GlobalBear): LICENSE = 'AGPL-3.0' ASCIINEMA_URL = 'https://asciinema.org/a/e146c9739ojhr8396wedsvf0d' CAN_DETECT = {'Formatting'} + SUPPORTED_HOST_KEYWORD_REGEX = { + 'github': (r'[Cc]lose[sd]?' + r'|[Rr]esolve[sd]?' + r'|[Ff]ix(?:e[sd])?'), + 'gitlab': (r'[Cc]los(?:e[sd]?|ing)' + r'|[Rr]esolv(?:e[sd]?|ing)' + r'|[Ff]ix(?:e[sd]|ing)?') + } + CONCATENATION_KEYWORDS = [r',', r'\sand\s'] @classmethod def check_prerequisites(cls): @@ -40,6 +51,12 @@ def get_body_checks_metadata(cls): cls.check_body, omit={'self', 'body'}) + @classmethod + def get_issue_checks_metadata(cls): + return FunctionMetadata.from_function( + cls.check_issue_reference, + omit={'self', 'body'}) + @classmethod def get_metadata(cls): return FunctionMetadata.merge( @@ -47,7 +64,27 @@ def get_metadata(cls): cls.run, omit={'self', 'dependency_results'}), cls.get_shortlog_checks_metadata(), - cls.get_body_checks_metadata()) + cls.get_body_checks_metadata(), + cls.get_issue_checks_metadata()) + + @staticmethod + def get_host_from_remotes(): + """ + Retrieve the first host from the list of git remotes. + """ + remotes, _ = run_shell_command( + "git config --get-regex '^remote.*.url$'") + + remotes = [url.split()[-1] for url in remotes.splitlines()] + if len(remotes) == 0: + return None + + url = remotes[0] + if 'git@' in url: + netloc = re.findall(r'@(\S+):', url)[0] + else: + netloc = urlparse(url)[1] + return netloc.split('.')[0] def run(self, allow_empty_commit_message: bool = False, **kwargs): """ @@ -79,6 +116,9 @@ def run(self, allow_empty_commit_message: bool = False, **kwargs): yield from self.check_body( stdout[1:], **self.get_body_checks_metadata().filter_parameters(kwargs)) + yield from self.check_issue_reference( + '\n'.join(stdout[1:]), + **self.get_issue_checks_metadata().filter_parameters(kwargs)) def check_shortlog(self, shortlog, shortlog_length: int=50, @@ -203,3 +243,82 @@ def check_body(self, body, yield Result(self, 'Body of HEAD commit contains too long lines. ' 'Commit body lines should not exceed {} ' 'characters.'.format(body_line_length)) + + def check_issue_reference(self, body, + body_close_issue: bool=False, + body_close_issue_full_url: bool=False, + body_close_issue_on_last_line: bool=False): + """ + Check for matching issue related references and URLs. + + :param body: + Body of the commit message of HEAD. + :param body_close_issue: + Whether to check for the presence of issue reference within + the commit body by retrieving host information from git + configuration. GitHub and GitLab support auto closing issues with + commit messages. Checks for matching keywords in the commit body. + By default, if none of ``body_close_issue_full_url`` and + ``body_close_issue_on_last_line`` are enabled, checks for presence + of short references like ``closes #213``. Otherwise behaves + according to other chosen flags. + More on keywords follows. + [GitHub](https://help.github.com/articles/closing-issues-via-commit-messages/) + [GitLab](https://docs.gitlab.com/ce/user/project/issues/automatic_issue_closing.html) + :param body_close_issue_full_url: + Checks the presence of issue close reference with a full URL + related to some issue. Works along with ``body_close_issue``. + :param body_close_issue_on_last_line: + When enabled, checks for issue close reference presence on the + last line of the commit body. Works along with + ``body_close_issue``. + """ + if not body_close_issue: + return + + host = self.get_host_from_remotes() + if host not in self.SUPPORTED_HOST_KEYWORD_REGEX: + return + + if body_close_issue_on_last_line: + body = body.splitlines()[-1] + result_message = ('Body of HEAD commit does not contain any {} ' + 'reference in the last line.') + else: + result_message = ('Body of HEAD commit does not contain any {} ' + 'reference.') + + if body_close_issue_full_url: + result_info = 'full issue' + compiled_issue_ref_regex = re.compile( + r'https?://{}\S+/issues/\w+'.format(re.escape(host))) + else: + result_info = 'issue' + compiled_issue_ref_regex = re.compile(r'#\w+') + + concat_regex = '|'.join(kw for kw in self.CONCATENATION_KEYWORDS) + compiled_joint_regex = re.compile( + r'(?:{0})\s+' + r'((?:\S(?!{1}))*\S(?:\s*(?:{1})\s*(?!{0})(?:\S(?!{1}))*\S)*)' + r''.format( + self.SUPPORTED_HOST_KEYWORD_REGEX[host], + concat_regex)) + + matches = compiled_joint_regex.findall(body) + + if len(matches) == 0: + yield Result(self, result_message.format(result_info)) + return + + compiled_issue_num_regex = re.compile(r'(?:#|\D+/)[1-9][0-9]*') + compiled_concat_regex = re.compile( + r'\s*(?:{})\s*'.format(concat_regex)) + + for match in matches: + for issue in re.split(compiled_concat_regex, match): + if not compiled_issue_ref_regex.match(issue): + yield Result(self, 'Invalid {} reference: ' + '{}'.format(result_info, issue)) + elif not compiled_issue_num_regex.fullmatch(issue): + yield Result(self, 'Invalid issue number: ' + '{}'.format(issue)) diff --git a/tests/vcs/git/GitCommitBearTest.py b/tests/vcs/git/GitCommitBearTest.py index a9997d4872..6f203327e7 100644 --- a/tests/vcs/git/GitCommitBearTest.py +++ b/tests/vcs/git/GitCommitBearTest.py @@ -95,12 +95,14 @@ 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('shortlog_length', metadata.optional_params) self.assertIn('body_line_length', metadata.optional_params) + self.assertIn('body_close_issue', metadata.optional_params) def test_git_failure(self): # In this case use a reference to a non-existing commit, so just try @@ -267,6 +269,151 @@ def test_body_checks(self): []) self.assertTrue(self.msg_queue.empty()) + def test_check_issue_reference(self): + # Commit with no remotes configured + self.git_commit('Shortlog\n\n' + 'First line, blablablablablabla.\n' + 'Another line, blablablablablabla.\n' + 'Closes #01112') + self.assertEqual(self.run_uut(body_close_issue=True), []) + + # Adding BitBucket remote for testing + self.run_git_command('remote', 'add', 'test', + 'https://bitbucket.com/user/repo.git') + + # Unsupported Host - Bitbucket + self.git_commit('Shortlog\n\n' + 'First line, blablablablablabla.\n' + 'Another line, blablablablablabla.\n' + 'Closes #1112') + self.assertEqual(self.run_uut( + body_close_issue=True, + body_close_issue_full_url=True), []) + + # Adding GitHub remote for testing, ssh way :P + self.run_git_command('remote', 'set-url', 'test', + 'git@github.com:user/repo.git') + + # GitHub host with an issue + self.git_commit('Shortlog\n\n' + 'First line, blablablablablabla.\n' + 'Another line, blablablablablabla.\n' + 'Fixed https://github.com/usr/repo/issues/1112\n' + 'and https://github.com/usr/repo/issues/1312') + self.assertEqual(self.run_uut( + body_close_issue=True, + body_close_issue_full_url=True), []) + + # No keywords and no issues + self.git_commit('Shortlog\n\n' + 'This line is ok.\n' + 'This line is by far too long (in this case).\n' + 'This one too, blablablablablablablablabla.') + self.assertEqual(self.run_uut(body_close_issue=True,), + ['Body of HEAD commit does not contain any issue ' + 'reference.']) + self.assert_no_msgs() + + # Has keyword but no valid issue URL + self.git_commit('Shortlog\n\n' + 'First line, blablablablablabla.\n' + 'Another line, blablablablablabla.\n' + 'Fix https://github.com/user/repo.git') + self.assertEqual(self.run_uut( + body_close_issue=True, + body_close_issue_full_url=True), + ['Invalid full issue reference: ' + 'https://github.com/user/repo.git']) + self.assert_no_msgs() + + # GitHub host with short issue tag + self.git_commit('Shortlog\n\n' + 'First line, blablablablablabla.\n' + 'Another line, blablablablablabla.\n' + 'Fix #1112, #1115 and #123') + self.assertEqual(self.run_uut(body_close_issue=True,), []) + + # GitHub host with invalid short issue tag + self.git_commit('Shortlog\n\n' + 'First line, blablablablablabla.\n' + 'Another line, blablablablablabla.\n' + 'Fix #01112 and #111') + self.assertEqual(self.run_uut(body_close_issue=True,), + ['Invalid issue number: #01112']) + self.assert_no_msgs() + + # GitHub host with no full issue reference + self.git_commit('Shortlog\n\n' + 'First line, blablablablablabla.\n' + 'Another line, blablablablablabla.\n' + 'Fix #1112') + self.assertEqual(self.run_uut( + body_close_issue=True, + body_close_issue_full_url=True), + ['Invalid full issue reference: #1112']) + self.assert_no_msgs() + + # Adding GitLab remote for testing + self.run_git_command('remote', 'set-url', 'test', + 'https://gitlab.com/user/repo.git') + + # GitLab chosen and has an issue + self.git_commit('Shortlog\n\n' + 'First line, blablablablablabla.\n' + 'Another line, blablablablablabla.\n' + 'Resolve https://gitlab.com/usr/repo/issues/1112\n' + 'and https://gitlab.com/usr/repo/issues/1312') + self.assertEqual(self.run_uut( + body_close_issue=True, + body_close_issue_full_url=True), []) + + # Invalid issue number in URL + self.git_commit('Shortlog\n\n' + 'First line, blablablablablabla.\n' + 'Another line, blablablablablabla.\n' + 'Closing https://gitlab.com/user/repo/issues/123\n' + 'and https://gitlab.com/user/repo/issues/not_num') + self.assertEqual(self.run_uut( + body_close_issue=True, + body_close_issue_full_url=True), + ['Invalid issue number: ' + 'https://gitlab.com/user/repo/issues/not_num']) + self.assert_no_msgs() + + # Invalid URL + self.git_commit('Shortlog\n\n' + 'First line, blablablablablabla.\n' + 'Another line, blablablablablabla.\n' + 'Fix www.google.com/issues/hehehe') + self.assertEqual(self.run_uut( + body_close_issue=True, + body_close_issue_full_url=True), + ['Invalid full issue reference: ' + 'www.google.com/issues/hehehe']) + self.assert_no_msgs() + + # One of the short references is broken + self.git_commit('Shortlog\n\n' + 'First line, blablablablablabla.\n' + 'Another line, blablablablablabla.\n' + 'Resolve #11 and close #notnum') + self.assertEqual(self.run_uut(body_close_issue=True,), + ['Invalid issue number: #notnum']) + self.assert_no_msgs() + + # Last line enforce full URL + self.git_commit('Shortlog\n\n' + 'First line, blablablablablabla.\n' + 'Fix http://gitlab.com/user/repo/issues/1112\n' + 'Another line, blablablablablabla.\n') + self.assertEqual(self.run_uut( + body_close_issue=True, + body_close_issue_full_url=True, + body_close_issue_on_last_line=True), + ['Body of HEAD commit does not contain any full issue' + ' reference in the last line.']) + self.assert_no_msgs() + def test_different_path(self): no_git_dir = mkdtemp() self.git_commit('Add a very long shortlog for a bad project history.')