aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Anthony <nickanthony@google.com>2020-11-19 10:39:36 -0500
committerNick Anthony <nickanthony@google.com>2020-11-19 13:19:27 -0500
commitfc3ee954f27b6048eded4d2e6f8c8a30c7fb9309 (patch)
tree9a81fa91e5836a9a7a51e8254b8838b117eea9ee
parent2f5ba44e2682e8054b0b48df4c450aa045089e1c (diff)
downloadrepohooks-fc3ee954f27b6048eded4d2e6f8c8a30c7fb9309.tar.gz
Fix issue with triple quoted relnotes
This accounts for the case where we use triple quotes, but check for singular quotes. Triple quotes are supported, so we need to make sure we account for them when checking correctness. This also fixes a triple quote use case that is technically correct, but not supported. Bug: 173702182 Test: ./hooks_unittest.py Change-Id: I0b6e6deeac0addc3939b27db36a6ba47ff4bd2da
-rw-r--r--rh/hooks.py38
-rwxr-xr-xrh/hooks_unittest.py34
2 files changed, 58 insertions, 14 deletions
diff --git a/rh/hooks.py b/rh/hooks.py
index a622bf8..66aa030 100644
--- a/rh/hooks.py
+++ b/rh/hooks.py
@@ -671,7 +671,6 @@ def check_commit_msg_relnote_field_format(project, commit, desc, _diff,
break
# Check 3: Check that multiline Relnotes contain matching quotes.
-
first_quote_found = False
second_quote_found = False
for cur_line in desc_lines:
@@ -688,8 +687,13 @@ def check_commit_msg_relnote_field_format(project, commit, desc, _diff,
# Check that the `Relnote:` tag exists and it contains a starting quote.
if check_re_relnote.match(cur_line) and contains_quote:
first_quote_found = True
+ # A single-line Relnote containing a start and ending triple quote
+ # is valid.
+ if cur_line.count('"""') == 2:
+ second_quote_found = True
+ break
# A single-line Relnote containing a start and ending quote
- # is valid as well.
+ # is valid.
if cur_line.count('"') - cur_line.count('\\"') == 2:
second_quote_found = True
break
@@ -703,7 +707,7 @@ def check_commit_msg_relnote_field_format(project, commit, desc, _diff,
# Check 4: Check that non-starting or non-ending quotes are escaped with a
# backslash.
line_needs_checking = False
- uses_invalide_quotes = False
+ uses_invalid_quotes = False
for cur_line in desc_lines:
if check_re_other_fields.findall(cur_line):
line_needs_checking = False
@@ -711,27 +715,33 @@ def check_commit_msg_relnote_field_format(project, commit, desc, _diff,
# Determine if we are parsing the base `Relnote:` line.
if on_relnote_line and '"' in cur_line:
line_needs_checking = True
+ # We don't think anyone will type '"""' and then forget to
+ # escape it, so we're not checking for this.
+ if '"""' in cur_line:
+ break
if line_needs_checking:
stripped_line = re.sub('^%s:' % field, '', cur_line,
flags=re.IGNORECASE).strip()
for i, character in enumerate(stripped_line):
- # Case 1: Valid quote at the beginning of the
- # base `Relnote:` line.
- if on_relnote_line and i == 0:
- continue
- # Case 2: Invalid quote at the beginning of following lines.
- if not on_relnote_line and i == 0 and character == '"':
- uses_invalide_quotes = True
- break
+ if i == 0:
+ # Case 1: Valid quote at the beginning of the
+ # base `Relnote:` line.
+ if on_relnote_line:
+ continue
+ # Case 2: Invalid quote at the beginning of following
+ # lines, where we are not terminating the release note.
+ if character == '"' and stripped_line != '"':
+ uses_invalid_quotes = True
+ break
# Case 3: Check all other cases.
if (character == '"'
and 0 < i < len(stripped_line) - 1
- and stripped_line[i-1] != "\""
+ and stripped_line[i-1] != '"'
and stripped_line[i-1] != "\\"):
- uses_invalide_quotes = True
+ uses_invalid_quotes = True
break
- if uses_invalide_quotes:
+ if uses_invalid_quotes:
ret.append(rh.results.HookResult(('commit msg: "%s:" '
'tag using unescaped '
'quotes') % (field,),
diff --git a/rh/hooks_unittest.py b/rh/hooks_unittest.py
index 74971b8..87d2820 100755
--- a/rh/hooks_unittest.py
+++ b/rh/hooks_unittest.py
@@ -567,6 +567,33 @@ class BuiltinHooksTests(unittest.TestCase):
'a correctly formatted second line."\n\n'
'Bug: 1234'
'Here is some extra "quoted" content.'),
+ ('subj\n\nRelnote: """This is a release note.\n\n'
+ 'This relnote contains an empty line.\n'
+ 'Then a non-empty line.\n\n'
+ 'And another empty line."""\n\n'
+ 'Bug: 1234'),
+ ('subj\n\nRelnote: """This is a release note.\n\n'
+ 'This relnote contains an empty line.\n'
+ 'Then an acceptable "quoted" line.\n\n'
+ 'And another empty line."""\n\n'
+ 'Bug: 1234'),
+ ('subj\n\nRelnote: """This is a release note."""\n\n'
+ 'Bug: 1234'),
+ ('subj\n\nRelnote: """This is a release note.\n'
+ 'It has a second line."""\n\n'
+ 'Bug: 1234'),
+ ('subj\n\nRelnote: """This is a release note.\n'
+ 'It has a second line, but does not end here.\n'
+ '"""\n\n'
+ 'Bug: 1234'),
+ ('subj\n\nRelnote: """This is a release note.\n'
+ '"It" has a second line, but does not end here.\n'
+ '"""\n\n'
+ 'Bug: 1234'),
+ ('subj\n\nRelnote: "This is a release note.\n'
+ 'It has a second line, but does not end here.\n'
+ '"\n\n'
+ 'Bug: 1234'),
))
# Check some bad messages.
@@ -607,6 +634,13 @@ class BuiltinHooksTests(unittest.TestCase):
'It contains a correct second line.\n'
'But incorrect "quotes" on the third line."\n'
'Bug: 1234'),
+ ('subj\n\nRelnote: """This is a release note.\n'
+ 'It has a second line, but no closing triple quote.\n\n'
+ 'Bug: 1234'),
+ ('subj\n\nRelnote: "This is a release note.\n'
+ '"It" has a second line, but does not end here.\n'
+ '"\n\n'
+ 'Bug: 1234'),
))
def test_commit_msg_relnote_for_current_txt(self, _mock_check, _mock_run):