diff options
author | Nick Anthony <nickanthony@google.com> | 2020-11-19 10:39:36 -0500 |
---|---|---|
committer | Nick Anthony <nickanthony@google.com> | 2020-11-19 13:19:27 -0500 |
commit | fc3ee954f27b6048eded4d2e6f8c8a30c7fb9309 (patch) | |
tree | 9a81fa91e5836a9a7a51e8254b8838b117eea9ee | |
parent | 2f5ba44e2682e8054b0b48df4c450aa045089e1c (diff) | |
download | repohooks-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.py | 38 | ||||
-rwxr-xr-x | rh/hooks_unittest.py | 34 |
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): |