Skip to content

Commit

Permalink
checkpatch.py: Port checkpatch related changes from the OVS repo.
Browse files Browse the repository at this point in the history
This picks up the following OVS changes:
  00d3d4a7d375 ("checkpatch: Avoid catastrophic backtracking.")
  d25c6bd8df37 ("checkpatch: Reorganize flagged words using a list.")
  9a50170a805a ("checkpatch: Add suggestions to the spell checker.")
  799f697e51ec ("checkpatch: Print subject field if misspelled or missing.")
  1b8fa4a66aa4 ("checkpatch: Add checks for the subject line.")
  bf843fd439b2 ("checkpatch: Don't spell check Fixes tag.")
  74bfe3701407 ("checkpatch: Add argument to skip committer signoff check.")
  21c61243fb75 ("checkpatch: Fix personal word list storage.")
  915b97971d58 ("checkpatch.py: Load codespell dictionary.")

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Mark Michelson <mmichels@redhat.com>
  • Loading branch information
dceara authored and putnopvut committed Jan 17, 2024
1 parent 6ce267a commit bf334c6
Show file tree
Hide file tree
Showing 2 changed files with 140 additions and 9 deletions.
45 changes: 45 additions & 0 deletions tests/checkpatch.at
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,14 @@ OVS_START_SHELL_HELPERS
try_checkpatch() {
# Take the patch to test from $1. Remove an initial four-space indent
# from it and, if it is just headers with no body, add a null body.
# If it does not have a 'Subject', add a valid one.
echo "$1" | sed 's/^ //' > test.patch
if grep 'Subject\:' test.patch >/dev/null 2>&1; then :
else
sed -i'' -e '1i\
Subject: Patch this is.
' test.patch
fi
if grep '---' expout >/dev/null 2>&1; then :
else
printf '\n---\n' >> test.patch
Expand Down Expand Up @@ -236,6 +243,22 @@ done
AT_CLEANUP


AT_SETUP([checkpatch - catastrophic backtracking])
dnl Special case this rather than using the above construct because sometimes a
dnl warning needs to be generated for line lengths (f.e. when the 'while'
dnl keyword is used).
try_checkpatch \
"COMMON_PATCH_HEADER
+ if (!b_ctx_in->chassis_rec || !b_ctx_in->br_int || !b_ctx_in->ovs_idl_txn)
" \
"ERROR: Inappropriate bracing around statement
#8 FILE: A.c:1:
if (!b_ctx_in->chassis_rec || !b_ctx_in->br_int || !b_ctx_in->ovs_idl_txn)
"

AT_CLEANUP


AT_SETUP([checkpatch - parenthesized constructs - for])
try_checkpatch \
"COMMON_PATCH_HEADER
Expand Down Expand Up @@ -560,3 +583,25 @@ try_checkpatch \
"

AT_CLEANUP

AT_SETUP([checkpatch - subject])
try_checkpatch \
"Author: A
Commit: A
Subject: netdev: invalid case and dot ending

Signed-off-by: A" \
"WARNING: The subject summary should start with a capital.
WARNING: The subject summary should end with a dot.
Subject: netdev: invalid case and dot ending"

try_checkpatch \
"Author: A
Commit: A
Subject: netdev: This is a way to long commit summary and therefor it should report a WARNING!

Signed-off-by: A" \
"WARNING: The subject, '<area>: <summary>', is over 70 characters, i.e., 85.
Subject: netdev: This is a way to long commit summary and therefor it should report a WARNING!"

AT_CLEANUP
104 changes: 95 additions & 9 deletions utilities/checkpatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@
def open_spell_check_dict():
import enchant

try:
import codespell_lib
codespell_dir = os.path.dirname(codespell_lib.__file__)
codespell_file = os.path.join(codespell_dir, 'data', 'dictionary.txt')
if not os.path.exists(codespell_file):
codespell_file = ''
except:
codespell_file = ''

try:
extra_keywords = ['ovs', 'vswitch', 'vswitchd', 'ovs-vswitchd',
'netdev', 'selinux', 'ovs-ctl', 'dpctl', 'ofctl',
Expand Down Expand Up @@ -92,9 +101,18 @@ def open_spell_check_dict():
'syscall', 'lacp', 'ipf', 'skb', 'valgrind']

global spell_check_dict

spell_check_dict = enchant.Dict("en_US")

if codespell_file:
with open(codespell_file) as f:
for line in f.readlines():
words = line.strip().split('>')[1].strip(', ').split(',')
for word in words:
spell_check_dict.add_to_session(word.strip())

for kw in extra_keywords:
spell_check_dict.add(kw)
spell_check_dict.add_to_session(kw)

return True
except:
Expand Down Expand Up @@ -190,6 +208,7 @@ def reset_counters():
skip_gerrit_change_id_check = False
skip_block_whitespace_check = False
skip_signoff_check = False
skip_committer_signoff_check = False

# Don't enforce character limit on files that include these characters in their
# name, as they may have legitimate reasons to have longer lines.
Expand Down Expand Up @@ -282,9 +301,13 @@ def balanced_parens(line):
if len(line) == MAX_LINE_LEN - 1 and line[-1] == ')':
return True

if __regex_ends_with_bracket.search(line) is None and \
__regex_if_macros.match(line) is None:
return False
if __regex_ends_with_bracket.search(line) is None:
if line.endswith("\\") and \
__regex_if_macros.match(line) is not None:
return True
else:
return False

if __regex_conditional_else_bracing.match(line) is not None:
return False
if __regex_conditional_else_bracing2.match(line) is not None:
Expand Down Expand Up @@ -410,9 +433,15 @@ def check_spelling(line, comment):
if not spell_check_dict or not spellcheck:
return False

if line.startswith('Fixes: '):
return False

words = filter_comments(line, True) if comment else line
words = words.replace(':', ' ').split(' ')

flagged_words = []
num_suggestions = 3

for word in words:
skip = False
strword = re.subn(r'\W+', '', word)[0].replace(',', '')
Expand All @@ -437,9 +466,15 @@ def check_spelling(line, comment):
skip = True

if not skip:
print_warning("Check for spelling mistakes (e.g. \"%s\")"
% strword)
return True
flagged_words.append(strword)

if len(flagged_words) > 0:
for mistake in flagged_words:
print_warning("Possible misspelled word: \"%s\"" % mistake)
print("Did you mean: ",
spell_check_dict.suggest(mistake)[:num_suggestions])

return True

return False

Expand Down Expand Up @@ -780,6 +815,36 @@ def run_file_checks(text):
check['check'](text)


def run_subject_checks(subject, spellcheck=False):
warnings = False

if spellcheck and check_spelling(subject, False):
warnings = True

summary = subject[subject.rindex(': ') + 2:]
area_summary = subject[subject.index(': ') + 2:]
area_summary_len = len(area_summary)
if area_summary_len > 70:
print_warning("The subject, '<area>: <summary>', is over 70 "
"characters, i.e., %u." % area_summary_len)
warnings = True

if summary[0].isalpha() and summary[0].islower():
print_warning(
"The subject summary should start with a capital.")
warnings = True

if subject[-1] not in [".", "?", "!"]:
print_warning(
"The subject summary should end with a dot.")
warnings = True

if warnings:
print(subject)

return warnings


def ovs_checkpatch_parse(text, filename, author=None, committer=None):
global print_file_name, total_line, checking_file, \
empty_return_check_state
Expand All @@ -800,6 +865,7 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None):
r'^@@ ([0-9-+]+),([0-9-+]+) ([0-9-+]+),([0-9-+]+) @@')
is_author = re.compile(r'^(Author|From): (.*)$', re.I | re.M | re.S)
is_committer = re.compile(r'^(Commit: )(.*)$', re.I | re.M | re.S)
is_subject = re.compile(r'^(Subject: )(.*)$', re.I | re.M | re.S)
is_signature = re.compile(r'^(Signed-off-by: )(.*)$',
re.I | re.M | re.S)
is_co_author = re.compile(r'^(Co-authored-by: )(.*)$',
Expand Down Expand Up @@ -874,7 +940,8 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None):
break
if (committer
and author != committer
and committer not in signatures):
and committer not in signatures
and not skip_committer_signoff_check):
print_error("Committer %s needs to sign off."
% committer)

Expand All @@ -899,6 +966,8 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None):
committer = is_committer.match(line).group(2)
elif is_author.match(line):
author = is_author.match(line).group(2)
elif is_subject.match(line):
run_subject_checks(line, spellcheck)
elif is_signature.match(line):
m = is_signature.match(line)
signatures.append(m.group(2))
Expand Down Expand Up @@ -990,7 +1059,8 @@ def usage():
-S|--spellcheck Check C comments and commit-message for possible
spelling mistakes
-t|--skip-trailing-whitespace Skips the trailing whitespace test
--skip-gerrit-change-id Skips the gerrit change id test"""
--skip-gerrit-change-id Skips the gerrit change id test
--skip-committer-signoff Skips the committer sign-off test"""
% sys.argv[0])


Expand All @@ -1017,6 +1087,19 @@ def ovs_checkpatch_file(filename):
result = ovs_checkpatch_parse(part.get_payload(decode=False), filename,
mail.get('Author', mail['From']),
mail['Commit'])

if not mail['Subject'] or not mail['Subject'].strip():
if mail['Subject']:
mail.replace_header('Subject', sys.argv[-1])
else:
mail.add_header('Subject', sys.argv[-1])

print("Subject missing! Your provisional subject is",
mail['Subject'])

if run_subject_checks('Subject: ' + mail['Subject'], spellcheck):
result = True

ovs_checkpatch_print_result()
return result

Expand Down Expand Up @@ -1048,6 +1131,7 @@ def partition(pred, iterable):
"skip-signoff-lines",
"skip-trailing-whitespace",
"skip-gerrit-change-id",
"skip-committer-signoff",
"spellcheck",
"quiet"])
except:
Expand All @@ -1068,6 +1152,8 @@ def partition(pred, iterable):
skip_trailing_whitespace_check = True
elif o in ("--skip-gerrit-change-id"):
skip_gerrit_change_id_check = True
elif o in ("--skip-committer-signoff"):
skip_committer_signoff_check = True
elif o in ("-f", "--check-file"):
checking_file = True
elif o in ("-S", "--spellcheck"):
Expand Down

0 comments on commit bf334c6

Please sign in to comment.