* [PATCH v7] BaseTools/Scripts/PatchCheck.py: Extended patch style check for c code
@ 2016-12-16 23:11 Daniil Egranov
2016-12-19 8:31 ` Gao, Liming
0 siblings, 1 reply; 3+ messages in thread
From: Daniil Egranov @ 2016-12-16 23:11 UTC (permalink / raw)
To: edk2-devel; +Cc: leif.lindholm, liming.gao, Daniil Egranov
Changed output format to make it more comapct.
Disabled colored output by default. For terminals which support ANSI
escape codes, colored output can be enabled with the "--color" command
line parameter.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov <daniil.egranov@arm.com>
---
Changelog:
v6
Added reporting a source code line number in case of error when
checking a patch file or using a git sha1 commit string. In case of
a patch file, both patch and source code line numbers will be provided.
Corrected handling the case with #define foo(..) macro. The absence of
the space before open parenthesis is not reported as the error anymore.
Added checking for lower case "void" and "static" words.
Added colors for error messages (experimental?)
v5
Corrected code checking for multi-line and commented lines. Both
multi-line and open comment flags will be reset when leaving
diff "+" area of the patch.
Changed version of the tool to 0.2.
v4
Corrected maximum code line size to 120 characters.
v3
Corrected space detection before parentheses.
v2:
Fixed several indentation cases
v1:
Fixed reporting signature error for a cover letter.
Fixed reporting line size error for a file change information
included in the commit message.
Fixed line number reported in PatchCheck error messages. It
points to the correct line in the diff file.
The patch extends style checking for c code:
Added check for code indentation.
Added report line size greater than 80 characters.
Added checking for space before '('.
Added checking for space before '{'.
Added checking for '}' to be on a new line and have spaces
for "} else {" or "} while ()" cases.
BaseTools/Scripts/PatchCheck.py | 379 ++++++++++++++++++++++++++++++++++++----
1 file changed, 341 insertions(+), 38 deletions(-)
diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index 7c30082..eae084c 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -15,7 +15,7 @@
from __future__ import print_function
-VersionNumber = '0.1'
+VersionNumber = '0.2'
__copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation All rights reserved."
import email
@@ -25,6 +25,44 @@ import re
import subprocess
import sys
+class MsgFormat:
+ ERROR, WARNING, REPORT, LINK, NORMAL = range(5)
+ color_on = False
+
+class ColorTypes:
+ RED = '\033[91m'
+ GREEN = '\033[92m'
+ BLUE = '\033[94m'
+ CYAN = '\033[96m'
+ WHITE = '\033[97m'
+ YELLOW = '\033[93m'
+ MAGENTA = '\033[95m'
+ GREY = '\033[90m'
+ BLACK = '\033[90m'
+ DEFAULT = '\033[0m'
+
+class TextColor:
+ def __init__(self, color_on, msg_format):
+ self.color = ''
+ if color_on == True:
+ if msg_format == MsgFormat.ERROR:
+ self.color = ColorTypes.RED
+ elif msg_format == MsgFormat.WARNING:
+ self.color = ColorTypes.YELLOW
+ elif msg_format == MsgFormat.REPORT:
+ self.color = ColorTypes.CYAN
+ elif msg_format == MsgFormat.LINK:
+ self.color = ColorTypes.BLUE
+ elif msg_format == MsgFormat.NORMAL:
+ self.color = ColorTypes.DEFAULT
+ else:
+ self.color = ColorTypes.DEFAULT
+ else:
+ self.color = ''
+
+ def __str__(self):
+ return self.color
+
class Verbose:
SILENT, ONELINE, NORMAL = range(3)
level = NORMAL
@@ -32,7 +70,7 @@ class Verbose:
class CommitMessageCheck:
"""Checks the contents of a git commit message."""
- def __init__(self, subject, message):
+ def __init__(self, subject, message, message_offset, cover):
self.ok = True
if subject is None and message is None:
@@ -41,9 +79,15 @@ class CommitMessageCheck:
self.subject = subject
self.msg = message
+ self.msg_offset = message_offset
+ self.cover = cover
+
+ if not cover:
+ self.check_contributed_under()
+ self.check_signed_off_by()
+ else:
+ print('The commit message is cover letter.')
- self.check_contributed_under()
- self.check_signed_off_by()
self.check_misc_signatures()
self.check_overall_format()
self.report_message_result()
@@ -60,9 +104,11 @@ class CommitMessageCheck:
else:
return_code = 1
if not self.ok:
- print(self.url)
+ print('\n' + self.url)
def error(self, *err):
+ start = TextColor(MsgFormat.color_on, MsgFormat.ERROR)
+ end = TextColor(MsgFormat.color_on, MsgFormat.NORMAL)
if self.ok and Verbose.level > Verbose.ONELINE:
print('The commit message format is not valid:')
self.ok = False
@@ -71,7 +117,7 @@ class CommitMessageCheck:
count = 0
for line in err:
prefix = (' *', ' ')[count > 0]
- print(prefix, line)
+ print(start, prefix, line, end)
count += 1
def check_contributed_under(self):
@@ -180,7 +226,12 @@ class CommitMessageCheck:
for sig in self.sig_types:
self.find_signatures(sig)
+ diff_change_info_re = \
+ re.compile(r'.*\|\s+(\d|Bin)*\s.*[\+\-]')
+
def check_overall_format(self):
+ start = TextColor(MsgFormat.color_on, MsgFormat.REPORT)
+ end = TextColor(MsgFormat.color_on, MsgFormat.NORMAL)
lines = self.msg.splitlines()
if len(lines) >= 1 and lines[0].endswith('\r\n'):
@@ -197,9 +248,10 @@ class CommitMessageCheck:
self.error('Empty commit message!')
return
- if count >= 1 and len(lines[0]) >= 72:
+ if count >= 1 and len(lines[0]) > 72:
self.error('First line of commit message (subject line) ' +
- 'is too long.')
+ 'is too long (%d) (max 72 characters): ' %(len(lines[0])) +
+ str(start) + lines[0] + str(end))
if count >= 1 and len(lines[0].strip()) == 0:
self.error('First line of commit message (subject line) ' +
@@ -210,10 +262,13 @@ class CommitMessageCheck:
'empty.')
for i in range(2, count):
- if (len(lines[i]) >= 76 and
+ if (len(lines[i]) > 76 and
len(lines[i].split()) > 1 and
- not lines[i].startswith('git-svn-id:')):
- self.error('Line %d of commit message is too long.' % (i + 1))
+ not lines[i].startswith('git-svn-id:') and
+ self.diff_change_info_re.search(lines[i]) is None):
+ self.error('Line %d of commit message is too long (%d) (max 76 characters): ' \
+ % (i + self.msg_offset - 1, len(lines[i])) +
+ str(start) + lines[i] + str(end))
last_sig_line = None
for i in range(count - 1, 0, -1):
@@ -236,13 +291,21 @@ class CommitMessageCheck:
class GitDiffCheck:
"""Checks the contents of a git diff."""
- def __init__(self, diff):
+ def __init__(self, diff, offset, patch_type):
self.ok = True
self.format_ok = True
self.lines = diff.splitlines(True)
self.count = len(self.lines)
self.line_num = 0
self.state = START
+ self.comments_open = False
+ self.multiline_string = False
+ self.current_indent_size = 0
+ self.parentheses_count = 0
+ self.offset = offset
+ self.file_type = "other_type"
+ self.patch_type = patch_type
+ self.src_line_num = 0;
while self.line_num < self.count and self.format_ok:
line_num = self.line_num
self.run()
@@ -255,6 +318,14 @@ class GitDiffCheck:
if self.ok:
print('The code passed all checks.')
+ def clean_counts(self):
+ self.current_indent_size = -1
+ self.parentheses_count = 0
+ self.comments_open = False
+ self.multiline_string = False
+
+ diff_src_line_start_re = re.compile(r'(\+\d+)')
+
def run(self):
line = self.lines[self.line_num]
@@ -281,9 +352,28 @@ class GitDiffCheck:
self.format_error("didn't find diff command")
self.line_num += 1
elif self.state == PRE_PATCH:
- if line.startswith('+++ b/'):
+ if line.startswith('+++ '):
self.set_filename(line[6:].rstrip())
+ print("\nChecking patch for %s ..." %(self.hunk_filename))
+ if self.hunk_filename.endswith(".c") or \
+ self.hunk_filename.endswith(".h"):
+ self.file_type = "c_type"
+ elif self.hunk_filename.endswith(".dec") or \
+ self.hunk_filename.endswith(".dsc") or \
+ self.hunk_filename.endswith(".fdf"):
+ self.file_type = "edk2_type"
+ elif self.hunk_filename.endswith(".S"):
+ self.file_type = "asm_arm_type"
+ elif self.hunk_filename.endswith(".asm"):
+ self.file_type = "asm_type"
+ else:
+ self.file_type = "other_type"
if line.startswith('@@ '):
+ sls = self.diff_src_line_start_re.search(line)
+ if sls is not None:
+ self.src_line_num = int(sls.group(0))
+ else:
+ self.src_line_num = 0
self.state = PATCH
self.binary = False
elif line.startswith('GIT binary patch'):
@@ -299,15 +389,25 @@ class GitDiffCheck:
self.line_num += 1
elif self.state == PATCH:
if self.binary:
+ self.clean_counts()
pass
if line.startswith('-'):
+ self.clean_counts()
pass
elif line.startswith('+'):
- self.check_added_line(line[1:])
+ # indentation resets every time we leave "+" diff area
+ self.check_added_line(line[1:], self.line_num + self.offset,
+ self.src_line_num)
+ self.src_line_num +=1
elif line.startswith(r'\ No newline '):
+ self.clean_counts()
pass
elif not line.startswith(' '):
+ self.clean_counts()
self.format_error("unexpected patch line")
+ else:
+ self.src_line_num +=1
+ self.clean_counts()
self.line_num += 1
pre_patch_prefixes = (
@@ -332,13 +432,19 @@ class GitDiffCheck:
else:
self.force_crlf = True
- def added_line_error(self, msg, line):
+ def added_line_error(self, msg, line, diff_line_num, src_line_num, patch_type):
+ start = TextColor(MsgFormat.color_on, MsgFormat.REPORT)
+ end = TextColor(MsgFormat.color_on, MsgFormat.NORMAL)
lines = [ msg ]
- if self.hunk_filename is not None:
- lines.append('File: ' + self.hunk_filename)
- lines.append('Line: ' + line)
-
self.error(*lines)
+ if self.hunk_filename is not None:
+ print('File: ' + self.hunk_filename)
+ line_info = 'Line '
+ if patch_type == 'patch_file_type':
+ line_info += ('%d(patch) ' %(diff_line_num))
+ line_info += ('%d(source) ' %(src_line_num))
+ line_info += (': ' +str(start) + line.lstrip() + str(end))
+ print(line_info)
old_debug_re = \
re.compile(r'''
@@ -348,28 +454,195 @@ class GitDiffCheck:
''',
re.VERBOSE)
- def check_added_line(self, line):
+ space_check_parentheses_re = re.compile(r'#define\s*[a-zA-Z0-9_]\S*\(|[a-zA-Z0-9_]\S*\((?<!\)\()(?<!\(\()')
+ space_check_brace_open_re = re.compile(r'[a-zA-Z0-9_]\S*\{')
+ space_check_brace_close_re = re.compile(r'[\}]\S.*')
+ brace_open_check_re = re.compile(r'[a-zA-Z0-9_]*\{')
+ brace_close_check_re = re.compile(r'.*\}.*')
+ multiline_check_re = re.compile(r'[a-zA-Z0-9_]*(\\|\&|\,|\+|\-|\*|\||\=)\Z')
+ void_cap_check_re = re.compile(r'(\s|\b|\()void(\s+|\*|\,|\))')
+ static_cap_check_re = re.compile(r'(\s|\b)static\s+')
+
+ def check_added_line(self, line, diff_line_num, src_line_num):
eol = ''
for an_eol in self.line_endings:
if line.endswith(an_eol):
eol = an_eol
line = line[:-len(eol)]
- stripped = line.rstrip()
+ rstripped = line.rstrip()
+ lstripped = line.lstrip()
if self.force_crlf and eol != '\r\n':
self.added_line_error('Line ending (%s) is not CRLF' % repr(eol),
- line)
+ line, diff_line_num, src_line_num, self.patch_type)
if '\t' in line:
- self.added_line_error('Tab character used', line)
- if len(stripped) < len(line):
- self.added_line_error('Trailing whitespace found', line)
+ self.added_line_error('Tab character used',
+ line, diff_line_num, src_line_num, self.patch_type)
+ if len(rstripped) < len(line):
+ self.added_line_error('Trailing whitespace found',
+ line, diff_line_num, src_line_num, self.patch_type)
+
+ # empty "+" line, skip it
+ if len(line) == 0:
+ return
- mo = self.old_debug_re.search(line)
- if mo is not None:
- self.added_line_error('EFI_D_' + mo.group(1) + ' was used, '
- 'but DEBUG_' + mo.group(1) +
- ' is now recommended', line)
+ # the file type specific checks
+ if self.file_type == "c_type":
+
+ # set initial indentation as the number of spaces after the first "+"
+ # indentation does not apply on lines with 0 characters
+ # adjust the indentation if first "+" has "}"
+ if self.current_indent_size == -1:
+ if len(lstripped) != 0:
+ self.current_indent_size = len(line) - len(lstripped)
+ else:
+ self.clean_counts()
+
+ if lstripped.startswith("}"):
+ self.current_indent_size = line.find("}") + 2
+
+ indent_size = len(line) - len(lstripped)
+ indent_size_adjust = 0
+ force_brace_check = False
+
+ if len(line) > 120:
+ self.added_line_error('Line is too long (%d) (max 120 characters)',
+ line, diff_line_num, src_line_num, self.patch_type)
+
+ # skip comments for code checking
+ if lstripped.startswith("//"):
+ return
+ if lstripped.startswith("/*") and not self.comments_open:
+ self.comments_open = True
+
+ if rstripped.endswith("*/") and self.comments_open:
+ self.comments_open = False
+ return
+
+ if rstripped.endswith("*/") and not self.comments_open:
+ # found unknown close comment, reset the indentation as
+ # no idea where it started
+ self.current_indent_size = -1
+ return
+
+ if not self.comments_open:
+ mo = self.old_debug_re.search(line)
+ if mo is not None:
+ self.added_line_error('EFI_D_' + mo.group(1) + ' was used, '
+ 'but DEBUG_' + mo.group(1) +
+ ' is now recommended',
+ line, diff_line_num, src_line_num, self.patch_type)
+
+ # track down multiple braces in the same line
+ braces_count = line.count('{') - line.count('}')
+
+ # check if line has text in double quotes as we do not
+ # need to check text messages
+ text_str_start = line.find('\"')
+
+ # track down a special case with "} else ..{" braces
+ if line.find("else") != -1:
+ force_brace_check = True
+
+ # check for closing brace which affects indentation
+ if braces_count < 0 and not force_brace_check:
+ self.current_indent_size -= 2 * abs(braces_count)
+ elif force_brace_check:
+ if line.find("}") != -1:
+ self.current_indent_size -= 2
+ braces_count = 1
+
+ # for the switch()/case:, adjust "case:" and "default:" indentation
+ # as they aligned with the "switch()" indentation
+ if lstripped.startswith("case ") or \
+ lstripped.startswith("default:"):
+ indent_size_adjust = -2
+
+ # indentation code check related to the diff structure
+ # do not check code indentation between parentheses
+ if indent_size != self.current_indent_size + indent_size_adjust \
+ and len(line) != 0 and self.parentheses_count == 0 \
+ and not self.multiline_string:
+ self.added_line_error('Invalid indentation. Expecting %d but found %d spaces' \
+ %(self.current_indent_size + indent_size_adjust, \
+ indent_size),
+ line, diff_line_num, src_line_num, self.patch_type)
+
+ # check for missing space before open brace
+ if braces_count > 0 or force_brace_check:
+ if line.find("{") != -1:
+ self.current_indent_size += 2 * braces_count
+ if not line.startswith("{"):
+ spb_check = self.space_check_brace_open_re.search(line)
+ if spb_check is not None:
+ self.added_line_error('Missing space before \'{\'',
+ line, diff_line_num, src_line_num, self.patch_type)
+
+ # check for missing space before open parenthesis
+ spo = self.space_check_parentheses_re.finditer(line)
+ if spo is not None and not self.multiline_string:
+ # check if it's not inside of the double quoted string or
+ # (aa)(bb)fff case or parenthesis belong to #define macro
+ for parstr in spo:
+ if not (text_str_start != -1 and text_str_start < parstr.start()) and \
+ not parstr.group(0).startswith("#define"):
+ self.added_line_error('Missing space before \'(\'',
+ line, diff_line_num, src_line_num, self.patch_type)
+ break;
+
+ # check for closing brace cases
+ if braces_count < 0 or \
+ (force_brace_check and line.find("}") != -1):
+ if not lstripped.startswith("}") and rstripped.endswith("}"):
+ self.added_line_error('The \'}\' is not on its own line',
+ line, diff_line_num, src_line_num, self.patch_type)
+ if not lstripped.endswith("}"):
+ scb = self.space_check_brace_close_re.search(lstripped)
+ if scb is not None \
+ and lstripped[scb.start() + 1] != ';' \
+ and lstripped[scb.start() + 1] != ',' \
+ and lstripped[scb.start() + 1] != '}':
+ self.added_line_error('Missing space after \'}\'',
+ line, diff_line_num, src_line_num, self.patch_type)
+
+ # check for using "void" instead of "VOID"
+ vcc = self.void_cap_check_re.search(line)
+ if vcc is not None and \
+ not (text_str_start != -1 and text_str_start < vcc.start()):
+ self.added_line_error('Using \"void\" instead of \"VOID\"',
+ line, diff_line_num, src_line_num, self.patch_type)
+
+ #check for using "static" instead of "STATIC"
+ scc = self.static_cap_check_re.search(line)
+ if scc is not None and \
+ not (text_str_start != -1 and text_str_start < scc.start()):
+ self.added_line_error('Using \"static\" instead of \"STATIC\"',
+ line, diff_line_num, src_line_num, self.patch_type)
+
+ # track down parentheses, no indentation check between them
+ if not (self.parentheses_count == 0 and lstripped.startswith(')') \
+ and not self.multiline_string and not self.comments_open):
+ self.parentheses_count += line.count('(') - line.count(')')
+
+ # multiline strings indentation is unknown
+ if not self.comments_open:
+ mlc = self.multiline_check_re.search(line)
+ if mlc is not None:
+ self.multiline_string = True
+ else:
+ self.multiline_string = False
+
+ elif self.file_type == "edk2_type":
+ pass
+ elif self.file_type == "asm_arm_type":
+ pass
+ elif self.file_type == "asm_type":
+ pass
+ elif self.file_type == "other_type":
+ pass
+ else:
+ pass
split_diff_re = re.compile(r'''
(?P<cmd>
@@ -388,6 +661,8 @@ class GitDiffCheck:
self.error(err, err2)
def error(self, *err):
+ start = TextColor(MsgFormat.color_on, MsgFormat.ERROR)
+ end = TextColor(MsgFormat.color_on, MsgFormat.NORMAL)
if self.ok and Verbose.level > Verbose.ONELINE:
print('Code format is not valid:')
self.ok = False
@@ -396,7 +671,7 @@ class GitDiffCheck:
count = 0
for line in err:
prefix = (' *', ' ')[count > 0]
- print(prefix, line)
+ print(start, prefix, line, end)
count += 1
class CheckOnePatch:
@@ -406,16 +681,17 @@ class CheckOnePatch:
patch content.
"""
- def __init__(self, name, patch):
+ def __init__(self, name, patch, patch_type):
self.patch = patch
self.find_patch_pieces()
- msg_check = CommitMessageCheck(self.commit_subject, self.commit_msg)
+ msg_check = CommitMessageCheck(self.commit_subject, self.commit_msg, \
+ self.commit_msg_offset, self.cover)
msg_ok = msg_check.ok
diff_ok = True
if self.diff is not None:
- diff_check = GitDiffCheck(self.diff)
+ diff_check = GitDiffCheck(self.diff, self.diff_offset, patch_type)
diff_ok = diff_check.ok
self.ok = msg_ok and diff_ok
@@ -458,6 +734,16 @@ class CheckOnePatch:
''',
re.VERBOSE)
+ cover_letter_re = re.compile(r'Subject:\s*\[PATCH.*.0/.*',
+ re.IGNORECASE | re.MULTILINE)
+
+ def is_cover_letter(self, patch):
+ cl = self.cover_letter_re.search(patch)
+ if cl is None:
+ return False
+
+ return True
+
def find_patch_pieces(self):
if sys.version_info < (3, 0):
patch = self.patch.encode('ascii', 'ignore')
@@ -465,10 +751,13 @@ class CheckOnePatch:
patch = self.patch
self.commit_msg = None
+ self.commit_msg_offset = 0
self.stat = None
self.commit_subject = None
self.commit_prefix = None
self.diff = None
+ self.diff_offset = 0
+ self.cover = False
if patch.startswith('diff --git'):
self.diff = patch
@@ -480,6 +769,14 @@ class CheckOnePatch:
assert(parts[0].get_content_type() == 'text/plain')
content = parts[0].get_payload(decode=True).decode('utf-8', 'ignore')
+ # find offset of content
+ self.commit_msg_offset = len(patch.splitlines()) - len(content.splitlines())
+
+ # find offset of first diff section
+ mo = self.git_diff_re.search(patch)
+ if mo is not None:
+ self.diff_offset = len(patch[:mo.start()].splitlines()) + 1
+
mo = self.git_diff_re.search(content)
if mo is not None:
self.diff = content[mo.start():]
@@ -492,6 +789,7 @@ class CheckOnePatch:
self.stat = mo.group('stat')
self.commit_msg = mo.group('commit_message')
+ self.cover = self.is_cover_letter(patch)
self.commit_subject = pmail['subject'].replace('\r\n', '')
self.commit_subject = self.commit_subject.replace('\n', '')
self.commit_subject = self.subject_prefix_re.sub('', self.commit_subject, 1)
@@ -514,9 +812,9 @@ class CheckGitCommits:
print()
else:
blank_line = True
- print('Checking git commit:', commit)
+ print('Checking git commit:', commit, '\n')
patch = self.read_patch_from_git(commit)
- self.ok &= CheckOnePatch(commit, patch).ok
+ self.ok &= CheckOnePatch(commit, patch, "git_commit_type").ok
def read_commit_list_from_git(self, rev_spec, max_count):
# Run git to get the commit patch
@@ -554,8 +852,8 @@ class CheckOnePatchFile:
patch = f.read().decode('utf-8', 'ignore')
f.close()
if Verbose.level > Verbose.ONELINE:
- print('Checking patch file:', patch_filename)
- self.ok = CheckOnePatch(patch_filename, patch).ok
+ print('Checking patch file:', patch_filename, '\n')
+ self.ok = CheckOnePatch(patch_filename, patch, "patch_file_type").ok
class CheckOneArg:
"""Performs a patch check for a single command line argument.
@@ -618,11 +916,16 @@ class PatchCheckApp:
group.add_argument("--silent",
action="store_true",
help="Print nothing")
+ group.add_argument("--color",
+ action="store_true",
+ help="Enable color messages")
self.args = parser.parse_args()
if self.args.oneline:
Verbose.level = Verbose.ONELINE
if self.args.silent:
Verbose.level = Verbose.SILENT
+ if self.args.color:
+ MsgFormat.color_on = True
if __name__ == "__main__":
sys.exit(PatchCheckApp().retval)
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v7] BaseTools/Scripts/PatchCheck.py: Extended patch style check for c code
2016-12-16 23:11 [PATCH v7] BaseTools/Scripts/PatchCheck.py: Extended patch style check for c code Daniil Egranov
@ 2016-12-19 8:31 ` Gao, Liming
2016-12-21 4:45 ` Jordan Justen
0 siblings, 1 reply; 3+ messages in thread
From: Gao, Liming @ 2016-12-19 8:31 UTC (permalink / raw)
To: Daniil Egranov, edk2-devel@lists.01.org
Cc: leif.lindholm@linaro.org, Justen, Jordan L
Daniil:
This version has addressed my comments. I see Jordan also provides some comments. Are they handled in this patch?
Thanks
Liming
> -----Original Message-----
> From: Daniil Egranov [mailto:daniil.egranov@arm.com]
> Sent: Saturday, December 17, 2016 7:11 AM
> To: edk2-devel@lists.01.org
> Cc: leif.lindholm@linaro.org; Gao, Liming <liming.gao@intel.com>; Daniil
> Egranov <daniil.egranov@arm.com>
> Subject: [PATCH v7] BaseTools/Scripts/PatchCheck.py: Extended patch style
> check for c code
>
> Changed output format to make it more comapct.
> Disabled colored output by default. For terminals which support ANSI
> escape codes, colored output can be enabled with the "--color" command
> line parameter.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Daniil Egranov <daniil.egranov@arm.com>
> ---
> Changelog:
>
> v6
> Added reporting a source code line number in case of error when
> checking a patch file or using a git sha1 commit string. In case of
> a patch file, both patch and source code line numbers will be provided.
> Corrected handling the case with #define foo(..) macro. The absence of
> the space before open parenthesis is not reported as the error anymore.
> Added checking for lower case "void" and "static" words.
> Added colors for error messages (experimental?)
>
> v5
> Corrected code checking for multi-line and commented lines. Both
> multi-line and open comment flags will be reset when leaving
> diff "+" area of the patch.
> Changed version of the tool to 0.2.
>
> v4
> Corrected maximum code line size to 120 characters.
>
> v3
> Corrected space detection before parentheses.
>
> v2:
> Fixed several indentation cases
>
> v1:
> Fixed reporting signature error for a cover letter.
> Fixed reporting line size error for a file change information
> included in the commit message.
> Fixed line number reported in PatchCheck error messages. It
> points to the correct line in the diff file.
> The patch extends style checking for c code:
> Added check for code indentation.
> Added report line size greater than 80 characters.
> Added checking for space before '('.
> Added checking for space before '{'.
> Added checking for '}' to be on a new line and have spaces
> for "} else {" or "} while ()" cases.
>
> BaseTools/Scripts/PatchCheck.py | 379
> ++++++++++++++++++++++++++++++++++++----
> 1 file changed, 341 insertions(+), 38 deletions(-)
>
> diff --git a/BaseTools/Scripts/PatchCheck.py
> b/BaseTools/Scripts/PatchCheck.py
> index 7c30082..eae084c 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -15,7 +15,7 @@
>
> from __future__ import print_function
>
> -VersionNumber = '0.1'
> +VersionNumber = '0.2'
> __copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation All rights
> reserved."
>
> import email
> @@ -25,6 +25,44 @@ import re
> import subprocess
> import sys
>
> +class MsgFormat:
> + ERROR, WARNING, REPORT, LINK, NORMAL = range(5)
> + color_on = False
> +
> +class ColorTypes:
> + RED = '\033[91m'
> + GREEN = '\033[92m'
> + BLUE = '\033[94m'
> + CYAN = '\033[96m'
> + WHITE = '\033[97m'
> + YELLOW = '\033[93m'
> + MAGENTA = '\033[95m'
> + GREY = '\033[90m'
> + BLACK = '\033[90m'
> + DEFAULT = '\033[0m'
> +
> +class TextColor:
> + def __init__(self, color_on, msg_format):
> + self.color = ''
> + if color_on == True:
> + if msg_format == MsgFormat.ERROR:
> + self.color = ColorTypes.RED
> + elif msg_format == MsgFormat.WARNING:
> + self.color = ColorTypes.YELLOW
> + elif msg_format == MsgFormat.REPORT:
> + self.color = ColorTypes.CYAN
> + elif msg_format == MsgFormat.LINK:
> + self.color = ColorTypes.BLUE
> + elif msg_format == MsgFormat.NORMAL:
> + self.color = ColorTypes.DEFAULT
> + else:
> + self.color = ColorTypes.DEFAULT
> + else:
> + self.color = ''
> +
> + def __str__(self):
> + return self.color
> +
> class Verbose:
> SILENT, ONELINE, NORMAL = range(3)
> level = NORMAL
> @@ -32,7 +70,7 @@ class Verbose:
> class CommitMessageCheck:
> """Checks the contents of a git commit message."""
>
> - def __init__(self, subject, message):
> + def __init__(self, subject, message, message_offset, cover):
> self.ok = True
>
> if subject is None and message is None:
> @@ -41,9 +79,15 @@ class CommitMessageCheck:
>
> self.subject = subject
> self.msg = message
> + self.msg_offset = message_offset
> + self.cover = cover
> +
> + if not cover:
> + self.check_contributed_under()
> + self.check_signed_off_by()
> + else:
> + print('The commit message is cover letter.')
>
> - self.check_contributed_under()
> - self.check_signed_off_by()
> self.check_misc_signatures()
> self.check_overall_format()
> self.report_message_result()
> @@ -60,9 +104,11 @@ class CommitMessageCheck:
> else:
> return_code = 1
> if not self.ok:
> - print(self.url)
> + print('\n' + self.url)
>
> def error(self, *err):
> + start = TextColor(MsgFormat.color_on, MsgFormat.ERROR)
> + end = TextColor(MsgFormat.color_on, MsgFormat.NORMAL)
> if self.ok and Verbose.level > Verbose.ONELINE:
> print('The commit message format is not valid:')
> self.ok = False
> @@ -71,7 +117,7 @@ class CommitMessageCheck:
> count = 0
> for line in err:
> prefix = (' *', ' ')[count > 0]
> - print(prefix, line)
> + print(start, prefix, line, end)
> count += 1
>
> def check_contributed_under(self):
> @@ -180,7 +226,12 @@ class CommitMessageCheck:
> for sig in self.sig_types:
> self.find_signatures(sig)
>
> + diff_change_info_re = \
> + re.compile(r'.*\|\s+(\d|Bin)*\s.*[\+\-]')
> +
> def check_overall_format(self):
> + start = TextColor(MsgFormat.color_on, MsgFormat.REPORT)
> + end = TextColor(MsgFormat.color_on, MsgFormat.NORMAL)
> lines = self.msg.splitlines()
>
> if len(lines) >= 1 and lines[0].endswith('\r\n'):
> @@ -197,9 +248,10 @@ class CommitMessageCheck:
> self.error('Empty commit message!')
> return
>
> - if count >= 1 and len(lines[0]) >= 72:
> + if count >= 1 and len(lines[0]) > 72:
> self.error('First line of commit message (subject line) ' +
> - 'is too long.')
> + 'is too long (%d) (max 72 characters): ' %(len(lines[0])) +
> + str(start) + lines[0] + str(end))
>
> if count >= 1 and len(lines[0].strip()) == 0:
> self.error('First line of commit message (subject line) ' +
> @@ -210,10 +262,13 @@ class CommitMessageCheck:
> 'empty.')
>
> for i in range(2, count):
> - if (len(lines[i]) >= 76 and
> + if (len(lines[i]) > 76 and
> len(lines[i].split()) > 1 and
> - not lines[i].startswith('git-svn-id:')):
> - self.error('Line %d of commit message is too long.' % (i + 1))
> + not lines[i].startswith('git-svn-id:') and
> + self.diff_change_info_re.search(lines[i]) is None):
> + self.error('Line %d of commit message is too long (%d) (max 76
> characters): ' \
> + % (i + self.msg_offset - 1, len(lines[i])) +
> + str(start) + lines[i] + str(end))
>
> last_sig_line = None
> for i in range(count - 1, 0, -1):
> @@ -236,13 +291,21 @@ class CommitMessageCheck:
> class GitDiffCheck:
> """Checks the contents of a git diff."""
>
> - def __init__(self, diff):
> + def __init__(self, diff, offset, patch_type):
> self.ok = True
> self.format_ok = True
> self.lines = diff.splitlines(True)
> self.count = len(self.lines)
> self.line_num = 0
> self.state = START
> + self.comments_open = False
> + self.multiline_string = False
> + self.current_indent_size = 0
> + self.parentheses_count = 0
> + self.offset = offset
> + self.file_type = "other_type"
> + self.patch_type = patch_type
> + self.src_line_num = 0;
> while self.line_num < self.count and self.format_ok:
> line_num = self.line_num
> self.run()
> @@ -255,6 +318,14 @@ class GitDiffCheck:
> if self.ok:
> print('The code passed all checks.')
>
> + def clean_counts(self):
> + self.current_indent_size = -1
> + self.parentheses_count = 0
> + self.comments_open = False
> + self.multiline_string = False
> +
> + diff_src_line_start_re = re.compile(r'(\+\d+)')
> +
> def run(self):
> line = self.lines[self.line_num]
>
> @@ -281,9 +352,28 @@ class GitDiffCheck:
> self.format_error("didn't find diff command")
> self.line_num += 1
> elif self.state == PRE_PATCH:
> - if line.startswith('+++ b/'):
> + if line.startswith('+++ '):
> self.set_filename(line[6:].rstrip())
> + print("\nChecking patch for %s ..." %(self.hunk_filename))
> + if self.hunk_filename.endswith(".c") or \
> + self.hunk_filename.endswith(".h"):
> + self.file_type = "c_type"
> + elif self.hunk_filename.endswith(".dec") or \
> + self.hunk_filename.endswith(".dsc") or \
> + self.hunk_filename.endswith(".fdf"):
> + self.file_type = "edk2_type"
> + elif self.hunk_filename.endswith(".S"):
> + self.file_type = "asm_arm_type"
> + elif self.hunk_filename.endswith(".asm"):
> + self.file_type = "asm_type"
> + else:
> + self.file_type = "other_type"
> if line.startswith('@@ '):
> + sls = self.diff_src_line_start_re.search(line)
> + if sls is not None:
> + self.src_line_num = int(sls.group(0))
> + else:
> + self.src_line_num = 0
> self.state = PATCH
> self.binary = False
> elif line.startswith('GIT binary patch'):
> @@ -299,15 +389,25 @@ class GitDiffCheck:
> self.line_num += 1
> elif self.state == PATCH:
> if self.binary:
> + self.clean_counts()
> pass
> if line.startswith('-'):
> + self.clean_counts()
> pass
> elif line.startswith('+'):
> - self.check_added_line(line[1:])
> + # indentation resets every time we leave "+" diff area
> + self.check_added_line(line[1:], self.line_num + self.offset,
> + self.src_line_num)
> + self.src_line_num +=1
> elif line.startswith(r'\ No newline '):
> + self.clean_counts()
> pass
> elif not line.startswith(' '):
> + self.clean_counts()
> self.format_error("unexpected patch line")
> + else:
> + self.src_line_num +=1
> + self.clean_counts()
> self.line_num += 1
>
> pre_patch_prefixes = (
> @@ -332,13 +432,19 @@ class GitDiffCheck:
> else:
> self.force_crlf = True
>
> - def added_line_error(self, msg, line):
> + def added_line_error(self, msg, line, diff_line_num, src_line_num,
> patch_type):
> + start = TextColor(MsgFormat.color_on, MsgFormat.REPORT)
> + end = TextColor(MsgFormat.color_on, MsgFormat.NORMAL)
> lines = [ msg ]
> - if self.hunk_filename is not None:
> - lines.append('File: ' + self.hunk_filename)
> - lines.append('Line: ' + line)
> -
> self.error(*lines)
> + if self.hunk_filename is not None:
> + print('File: ' + self.hunk_filename)
> + line_info = 'Line '
> + if patch_type == 'patch_file_type':
> + line_info += ('%d(patch) ' %(diff_line_num))
> + line_info += ('%d(source) ' %(src_line_num))
> + line_info += (': ' +str(start) + line.lstrip() + str(end))
> + print(line_info)
>
> old_debug_re = \
> re.compile(r'''
> @@ -348,28 +454,195 @@ class GitDiffCheck:
> ''',
> re.VERBOSE)
>
> - def check_added_line(self, line):
> + space_check_parentheses_re = re.compile(r'#define\s*[a-zA-Z0-
> 9_]\S*\(|[a-zA-Z0-9_]\S*\((?<!\)\()(?<!\(\()')
> + space_check_brace_open_re = re.compile(r'[a-zA-Z0-9_]\S*\{')
> + space_check_brace_close_re = re.compile(r'[\}]\S.*')
> + brace_open_check_re = re.compile(r'[a-zA-Z0-9_]*\{')
> + brace_close_check_re = re.compile(r'.*\}.*')
> + multiline_check_re = re.compile(r'[a-zA-Z0-9_]*(\\|\&|\,|\+|\-
> |\*|\||\=)\Z')
> + void_cap_check_re = re.compile(r'(\s|\b|\()void(\s+|\*|\,|\))')
> + static_cap_check_re = re.compile(r'(\s|\b)static\s+')
> +
> + def check_added_line(self, line, diff_line_num, src_line_num):
> eol = ''
> for an_eol in self.line_endings:
> if line.endswith(an_eol):
> eol = an_eol
> line = line[:-len(eol)]
>
> - stripped = line.rstrip()
> + rstripped = line.rstrip()
> + lstripped = line.lstrip()
>
> if self.force_crlf and eol != '\r\n':
> self.added_line_error('Line ending (%s) is not CRLF' % repr(eol),
> - line)
> + line, diff_line_num, src_line_num, self.patch_type)
> if '\t' in line:
> - self.added_line_error('Tab character used', line)
> - if len(stripped) < len(line):
> - self.added_line_error('Trailing whitespace found', line)
> + self.added_line_error('Tab character used',
> + line, diff_line_num, src_line_num, self.patch_type)
> + if len(rstripped) < len(line):
> + self.added_line_error('Trailing whitespace found',
> + line, diff_line_num, src_line_num, self.patch_type)
> +
> + # empty "+" line, skip it
> + if len(line) == 0:
> + return
>
> - mo = self.old_debug_re.search(line)
> - if mo is not None:
> - self.added_line_error('EFI_D_' + mo.group(1) + ' was used, '
> - 'but DEBUG_' + mo.group(1) +
> - ' is now recommended', line)
> + # the file type specific checks
> + if self.file_type == "c_type":
> +
> + # set initial indentation as the number of spaces after the first "+"
> + # indentation does not apply on lines with 0 characters
> + # adjust the indentation if first "+" has "}"
> + if self.current_indent_size == -1:
> + if len(lstripped) != 0:
> + self.current_indent_size = len(line) - len(lstripped)
> + else:
> + self.clean_counts()
> +
> + if lstripped.startswith("}"):
> + self.current_indent_size = line.find("}") + 2
> +
> + indent_size = len(line) - len(lstripped)
> + indent_size_adjust = 0
> + force_brace_check = False
> +
> + if len(line) > 120:
> + self.added_line_error('Line is too long (%d) (max 120 characters)',
> + line, diff_line_num, src_line_num, self.patch_type)
> +
> + # skip comments for code checking
> + if lstripped.startswith("//"):
> + return
> + if lstripped.startswith("/*") and not self.comments_open:
> + self.comments_open = True
> +
> + if rstripped.endswith("*/") and self.comments_open:
> + self.comments_open = False
> + return
> +
> + if rstripped.endswith("*/") and not self.comments_open:
> + # found unknown close comment, reset the indentation as
> + # no idea where it started
> + self.current_indent_size = -1
> + return
> +
> + if not self.comments_open:
> + mo = self.old_debug_re.search(line)
> + if mo is not None:
> + self.added_line_error('EFI_D_' + mo.group(1) + ' was used, '
> + 'but DEBUG_' + mo.group(1) +
> + ' is now recommended',
> + line, diff_line_num, src_line_num, self.patch_type)
> +
> + # track down multiple braces in the same line
> + braces_count = line.count('{') - line.count('}')
> +
> + # check if line has text in double quotes as we do not
> + # need to check text messages
> + text_str_start = line.find('\"')
> +
> + # track down a special case with "} else ..{" braces
> + if line.find("else") != -1:
> + force_brace_check = True
> +
> + # check for closing brace which affects indentation
> + if braces_count < 0 and not force_brace_check:
> + self.current_indent_size -= 2 * abs(braces_count)
> + elif force_brace_check:
> + if line.find("}") != -1:
> + self.current_indent_size -= 2
> + braces_count = 1
> +
> + # for the switch()/case:, adjust "case:" and "default:" indentation
> + # as they aligned with the "switch()" indentation
> + if lstripped.startswith("case ") or \
> + lstripped.startswith("default:"):
> + indent_size_adjust = -2
> +
> + # indentation code check related to the diff structure
> + # do not check code indentation between parentheses
> + if indent_size != self.current_indent_size + indent_size_adjust \
> + and len(line) != 0 and self.parentheses_count == 0 \
> + and not self.multiline_string:
> + self.added_line_error('Invalid indentation. Expecting %d but
> found %d spaces' \
> + %(self.current_indent_size + indent_size_adjust, \
> + indent_size),
> + line, diff_line_num, src_line_num, self.patch_type)
> +
> + # check for missing space before open brace
> + if braces_count > 0 or force_brace_check:
> + if line.find("{") != -1:
> + self.current_indent_size += 2 * braces_count
> + if not line.startswith("{"):
> + spb_check = self.space_check_brace_open_re.search(line)
> + if spb_check is not None:
> + self.added_line_error('Missing space before \'{\'',
> + line, diff_line_num, src_line_num, self.patch_type)
> +
> + # check for missing space before open parenthesis
> + spo = self.space_check_parentheses_re.finditer(line)
> + if spo is not None and not self.multiline_string:
> + # check if it's not inside of the double quoted string or
> + # (aa)(bb)fff case or parenthesis belong to #define macro
> + for parstr in spo:
> + if not (text_str_start != -1 and text_str_start < parstr.start())
> and \
> + not parstr.group(0).startswith("#define"):
> + self.added_line_error('Missing space before \'(\'',
> + line, diff_line_num, src_line_num, self.patch_type)
> + break;
> +
> + # check for closing brace cases
> + if braces_count < 0 or \
> + (force_brace_check and line.find("}") != -1):
> + if not lstripped.startswith("}") and rstripped.endswith("}"):
> + self.added_line_error('The \'}\' is not on its own line',
> + line, diff_line_num, src_line_num, self.patch_type)
> + if not lstripped.endswith("}"):
> + scb = self.space_check_brace_close_re.search(lstripped)
> + if scb is not None \
> + and lstripped[scb.start() + 1] != ';' \
> + and lstripped[scb.start() + 1] != ',' \
> + and lstripped[scb.start() + 1] != '}':
> + self.added_line_error('Missing space after \'}\'',
> + line, diff_line_num, src_line_num, self.patch_type)
> +
> + # check for using "void" instead of "VOID"
> + vcc = self.void_cap_check_re.search(line)
> + if vcc is not None and \
> + not (text_str_start != -1 and text_str_start < vcc.start()):
> + self.added_line_error('Using \"void\" instead of \"VOID\"',
> + line, diff_line_num, src_line_num, self.patch_type)
> +
> + #check for using "static" instead of "STATIC"
> + scc = self.static_cap_check_re.search(line)
> + if scc is not None and \
> + not (text_str_start != -1 and text_str_start < scc.start()):
> + self.added_line_error('Using \"static\" instead of \"STATIC\"',
> + line, diff_line_num, src_line_num, self.patch_type)
> +
> + # track down parentheses, no indentation check between them
> + if not (self.parentheses_count == 0 and lstripped.startswith(')') \
> + and not self.multiline_string and not self.comments_open):
> + self.parentheses_count += line.count('(') - line.count(')')
> +
> + # multiline strings indentation is unknown
> + if not self.comments_open:
> + mlc = self.multiline_check_re.search(line)
> + if mlc is not None:
> + self.multiline_string = True
> + else:
> + self.multiline_string = False
> +
> + elif self.file_type == "edk2_type":
> + pass
> + elif self.file_type == "asm_arm_type":
> + pass
> + elif self.file_type == "asm_type":
> + pass
> + elif self.file_type == "other_type":
> + pass
> + else:
> + pass
>
> split_diff_re = re.compile(r'''
> (?P<cmd>
> @@ -388,6 +661,8 @@ class GitDiffCheck:
> self.error(err, err2)
>
> def error(self, *err):
> + start = TextColor(MsgFormat.color_on, MsgFormat.ERROR)
> + end = TextColor(MsgFormat.color_on, MsgFormat.NORMAL)
> if self.ok and Verbose.level > Verbose.ONELINE:
> print('Code format is not valid:')
> self.ok = False
> @@ -396,7 +671,7 @@ class GitDiffCheck:
> count = 0
> for line in err:
> prefix = (' *', ' ')[count > 0]
> - print(prefix, line)
> + print(start, prefix, line, end)
> count += 1
>
> class CheckOnePatch:
> @@ -406,16 +681,17 @@ class CheckOnePatch:
> patch content.
> """
>
> - def __init__(self, name, patch):
> + def __init__(self, name, patch, patch_type):
> self.patch = patch
> self.find_patch_pieces()
>
> - msg_check = CommitMessageCheck(self.commit_subject,
> self.commit_msg)
> + msg_check = CommitMessageCheck(self.commit_subject,
> self.commit_msg, \
> + self.commit_msg_offset, self.cover)
> msg_ok = msg_check.ok
>
> diff_ok = True
> if self.diff is not None:
> - diff_check = GitDiffCheck(self.diff)
> + diff_check = GitDiffCheck(self.diff, self.diff_offset, patch_type)
> diff_ok = diff_check.ok
>
> self.ok = msg_ok and diff_ok
> @@ -458,6 +734,16 @@ class CheckOnePatch:
> ''',
> re.VERBOSE)
>
> + cover_letter_re = re.compile(r'Subject:\s*\[PATCH.*.0/.*',
> + re.IGNORECASE | re.MULTILINE)
> +
> + def is_cover_letter(self, patch):
> + cl = self.cover_letter_re.search(patch)
> + if cl is None:
> + return False
> +
> + return True
> +
> def find_patch_pieces(self):
> if sys.version_info < (3, 0):
> patch = self.patch.encode('ascii', 'ignore')
> @@ -465,10 +751,13 @@ class CheckOnePatch:
> patch = self.patch
>
> self.commit_msg = None
> + self.commit_msg_offset = 0
> self.stat = None
> self.commit_subject = None
> self.commit_prefix = None
> self.diff = None
> + self.diff_offset = 0
> + self.cover = False
>
> if patch.startswith('diff --git'):
> self.diff = patch
> @@ -480,6 +769,14 @@ class CheckOnePatch:
> assert(parts[0].get_content_type() == 'text/plain')
> content = parts[0].get_payload(decode=True).decode('utf-8', 'ignore')
>
> + # find offset of content
> + self.commit_msg_offset = len(patch.splitlines()) -
> len(content.splitlines())
> +
> + # find offset of first diff section
> + mo = self.git_diff_re.search(patch)
> + if mo is not None:
> + self.diff_offset = len(patch[:mo.start()].splitlines()) + 1
> +
> mo = self.git_diff_re.search(content)
> if mo is not None:
> self.diff = content[mo.start():]
> @@ -492,6 +789,7 @@ class CheckOnePatch:
> self.stat = mo.group('stat')
> self.commit_msg = mo.group('commit_message')
>
> + self.cover = self.is_cover_letter(patch)
> self.commit_subject = pmail['subject'].replace('\r\n', '')
> self.commit_subject = self.commit_subject.replace('\n', '')
> self.commit_subject = self.subject_prefix_re.sub('',
> self.commit_subject, 1)
> @@ -514,9 +812,9 @@ class CheckGitCommits:
> print()
> else:
> blank_line = True
> - print('Checking git commit:', commit)
> + print('Checking git commit:', commit, '\n')
> patch = self.read_patch_from_git(commit)
> - self.ok &= CheckOnePatch(commit, patch).ok
> + self.ok &= CheckOnePatch(commit, patch, "git_commit_type").ok
>
> def read_commit_list_from_git(self, rev_spec, max_count):
> # Run git to get the commit patch
> @@ -554,8 +852,8 @@ class CheckOnePatchFile:
> patch = f.read().decode('utf-8', 'ignore')
> f.close()
> if Verbose.level > Verbose.ONELINE:
> - print('Checking patch file:', patch_filename)
> - self.ok = CheckOnePatch(patch_filename, patch).ok
> + print('Checking patch file:', patch_filename, '\n')
> + self.ok = CheckOnePatch(patch_filename, patch, "patch_file_type").ok
>
> class CheckOneArg:
> """Performs a patch check for a single command line argument.
> @@ -618,11 +916,16 @@ class PatchCheckApp:
> group.add_argument("--silent",
> action="store_true",
> help="Print nothing")
> + group.add_argument("--color",
> + action="store_true",
> + help="Enable color messages")
> self.args = parser.parse_args()
> if self.args.oneline:
> Verbose.level = Verbose.ONELINE
> if self.args.silent:
> Verbose.level = Verbose.SILENT
> + if self.args.color:
> + MsgFormat.color_on = True
>
> if __name__ == "__main__":
> sys.exit(PatchCheckApp().retval)
> --
> 2.7.4
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v7] BaseTools/Scripts/PatchCheck.py: Extended patch style check for c code
2016-12-19 8:31 ` Gao, Liming
@ 2016-12-21 4:45 ` Jordan Justen
0 siblings, 0 replies; 3+ messages in thread
From: Jordan Justen @ 2016-12-21 4:45 UTC (permalink / raw)
To: Gao, Liming, Daniil Egranov, edk2-devel@lists.01.org
Cc: leif.lindholm@linaro.org
On 2016-12-19 00:31:13, Gao, Liming wrote:
> Daniil:
> This version has addressed my comments. I see Jordan also provides
> some comments. Are they handled in this patch?
>
No, they were not addressed. Here are my replies:
v5: https://lists.01.org/pipermail/edk2-devel/2016-December/005779.html
v6: https://lists.01.org/pipermail/edk2-devel/2016-December/005866.html
>
> > -----Original Message-----
> > From: Daniil Egranov [mailto:daniil.egranov@arm.com]
> > Sent: Saturday, December 17, 2016 7:11 AM
> > To: edk2-devel@lists.01.org
> > Cc: leif.lindholm@linaro.org; Gao, Liming <liming.gao@intel.com>; Daniil
> > Egranov <daniil.egranov@arm.com>
> > Subject: [PATCH v7] BaseTools/Scripts/PatchCheck.py: Extended patch style
> > check for c code
> >
> > Changed output format to make it more comapct.
> > Disabled colored output by default. For terminals which support ANSI
> > escape codes, colored output can be enabled with the "--color" command
> > line parameter.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Daniil Egranov <daniil.egranov@arm.com>
> > ---
> > Changelog:
> >
> > v6
> > Added reporting a source code line number in case of error when
> > checking a patch file or using a git sha1 commit string. In case of
> > a patch file, both patch and source code line numbers will be provided.
> > Corrected handling the case with #define foo(..) macro. The absence of
> > the space before open parenthesis is not reported as the error anymore.
> > Added checking for lower case "void" and "static" words.
> > Added colors for error messages (experimental?)
> >
> > v5
> > Corrected code checking for multi-line and commented lines. Both
> > multi-line and open comment flags will be reset when leaving
> > diff "+" area of the patch.
> > Changed version of the tool to 0.2.
> >
> > v4
> > Corrected maximum code line size to 120 characters.
> >
> > v3
> > Corrected space detection before parentheses.
> >
> > v2:
> > Fixed several indentation cases
> >
> > v1:
> > Fixed reporting signature error for a cover letter.
> > Fixed reporting line size error for a file change information
> > included in the commit message.
> > Fixed line number reported in PatchCheck error messages. It
> > points to the correct line in the diff file.
> > The patch extends style checking for c code:
> > Added check for code indentation.
> > Added report line size greater than 80 characters.
> > Added checking for space before '('.
> > Added checking for space before '{'.
> > Added checking for '}' to be on a new line and have spaces
> > for "} else {" or "} while ()" cases.
> >
> > BaseTools/Scripts/PatchCheck.py | 379
> > ++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 341 insertions(+), 38 deletions(-)
> >
> > diff --git a/BaseTools/Scripts/PatchCheck.py
> > b/BaseTools/Scripts/PatchCheck.py
> > index 7c30082..eae084c 100755
> > --- a/BaseTools/Scripts/PatchCheck.py
> > +++ b/BaseTools/Scripts/PatchCheck.py
> > @@ -15,7 +15,7 @@
> >
> > from __future__ import print_function
> >
> > -VersionNumber = '0.1'
> > +VersionNumber = '0.2'
> > __copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation All rights
> > reserved."
> >
> > import email
> > @@ -25,6 +25,44 @@ import re
> > import subprocess
> > import sys
> >
> > +class MsgFormat:
> > + ERROR, WARNING, REPORT, LINK, NORMAL = range(5)
> > + color_on = False
> > +
> > +class ColorTypes:
> > + RED = '\033[91m'
> > + GREEN = '\033[92m'
> > + BLUE = '\033[94m'
> > + CYAN = '\033[96m'
> > + WHITE = '\033[97m'
> > + YELLOW = '\033[93m'
> > + MAGENTA = '\033[95m'
> > + GREY = '\033[90m'
> > + BLACK = '\033[90m'
> > + DEFAULT = '\033[0m'
> > +
> > +class TextColor:
> > + def __init__(self, color_on, msg_format):
> > + self.color = ''
> > + if color_on == True:
> > + if msg_format == MsgFormat.ERROR:
> > + self.color = ColorTypes.RED
> > + elif msg_format == MsgFormat.WARNING:
> > + self.color = ColorTypes.YELLOW
> > + elif msg_format == MsgFormat.REPORT:
> > + self.color = ColorTypes.CYAN
> > + elif msg_format == MsgFormat.LINK:
> > + self.color = ColorTypes.BLUE
> > + elif msg_format == MsgFormat.NORMAL:
> > + self.color = ColorTypes.DEFAULT
> > + else:
> > + self.color = ColorTypes.DEFAULT
> > + else:
> > + self.color = ''
> > +
> > + def __str__(self):
> > + return self.color
> > +
> > class Verbose:
> > SILENT, ONELINE, NORMAL = range(3)
> > level = NORMAL
> > @@ -32,7 +70,7 @@ class Verbose:
> > class CommitMessageCheck:
> > """Checks the contents of a git commit message."""
> >
> > - def __init__(self, subject, message):
> > + def __init__(self, subject, message, message_offset, cover):
> > self.ok = True
> >
> > if subject is None and message is None:
> > @@ -41,9 +79,15 @@ class CommitMessageCheck:
> >
> > self.subject = subject
> > self.msg = message
> > + self.msg_offset = message_offset
> > + self.cover = cover
> > +
> > + if not cover:
> > + self.check_contributed_under()
> > + self.check_signed_off_by()
> > + else:
> > + print('The commit message is cover letter.')
> >
> > - self.check_contributed_under()
> > - self.check_signed_off_by()
> > self.check_misc_signatures()
> > self.check_overall_format()
> > self.report_message_result()
> > @@ -60,9 +104,11 @@ class CommitMessageCheck:
> > else:
> > return_code = 1
> > if not self.ok:
> > - print(self.url)
> > + print('\n' + self.url)
> >
> > def error(self, *err):
> > + start = TextColor(MsgFormat.color_on, MsgFormat.ERROR)
> > + end = TextColor(MsgFormat.color_on, MsgFormat.NORMAL)
> > if self.ok and Verbose.level > Verbose.ONELINE:
> > print('The commit message format is not valid:')
> > self.ok = False
> > @@ -71,7 +117,7 @@ class CommitMessageCheck:
> > count = 0
> > for line in err:
> > prefix = (' *', ' ')[count > 0]
> > - print(prefix, line)
> > + print(start, prefix, line, end)
> > count += 1
> >
> > def check_contributed_under(self):
> > @@ -180,7 +226,12 @@ class CommitMessageCheck:
> > for sig in self.sig_types:
> > self.find_signatures(sig)
> >
> > + diff_change_info_re = \
> > + re.compile(r'.*\|\s+(\d|Bin)*\s.*[\+\-]')
> > +
> > def check_overall_format(self):
> > + start = TextColor(MsgFormat.color_on, MsgFormat.REPORT)
> > + end = TextColor(MsgFormat.color_on, MsgFormat.NORMAL)
> > lines = self.msg.splitlines()
> >
> > if len(lines) >= 1 and lines[0].endswith('\r\n'):
> > @@ -197,9 +248,10 @@ class CommitMessageCheck:
> > self.error('Empty commit message!')
> > return
> >
> > - if count >= 1 and len(lines[0]) >= 72:
> > + if count >= 1 and len(lines[0]) > 72:
> > self.error('First line of commit message (subject line) ' +
> > - 'is too long.')
> > + 'is too long (%d) (max 72 characters): ' %(len(lines[0])) +
> > + str(start) + lines[0] + str(end))
> >
> > if count >= 1 and len(lines[0].strip()) == 0:
> > self.error('First line of commit message (subject line) ' +
> > @@ -210,10 +262,13 @@ class CommitMessageCheck:
> > 'empty.')
> >
> > for i in range(2, count):
> > - if (len(lines[i]) >= 76 and
> > + if (len(lines[i]) > 76 and
> > len(lines[i].split()) > 1 and
> > - not lines[i].startswith('git-svn-id:')):
> > - self.error('Line %d of commit message is too long.' % (i + 1))
> > + not lines[i].startswith('git-svn-id:') and
> > + self.diff_change_info_re.search(lines[i]) is None):
> > + self.error('Line %d of commit message is too long (%d) (max 76
> > characters): ' \
> > + % (i + self.msg_offset - 1, len(lines[i])) +
> > + str(start) + lines[i] + str(end))
> >
> > last_sig_line = None
> > for i in range(count - 1, 0, -1):
> > @@ -236,13 +291,21 @@ class CommitMessageCheck:
> > class GitDiffCheck:
> > """Checks the contents of a git diff."""
> >
> > - def __init__(self, diff):
> > + def __init__(self, diff, offset, patch_type):
> > self.ok = True
> > self.format_ok = True
> > self.lines = diff.splitlines(True)
> > self.count = len(self.lines)
> > self.line_num = 0
> > self.state = START
> > + self.comments_open = False
> > + self.multiline_string = False
> > + self.current_indent_size = 0
> > + self.parentheses_count = 0
> > + self.offset = offset
> > + self.file_type = "other_type"
> > + self.patch_type = patch_type
> > + self.src_line_num = 0;
> > while self.line_num < self.count and self.format_ok:
> > line_num = self.line_num
> > self.run()
> > @@ -255,6 +318,14 @@ class GitDiffCheck:
> > if self.ok:
> > print('The code passed all checks.')
> >
> > + def clean_counts(self):
> > + self.current_indent_size = -1
> > + self.parentheses_count = 0
> > + self.comments_open = False
> > + self.multiline_string = False
> > +
> > + diff_src_line_start_re = re.compile(r'(\+\d+)')
> > +
> > def run(self):
> > line = self.lines[self.line_num]
> >
> > @@ -281,9 +352,28 @@ class GitDiffCheck:
> > self.format_error("didn't find diff command")
> > self.line_num += 1
> > elif self.state == PRE_PATCH:
> > - if line.startswith('+++ b/'):
> > + if line.startswith('+++ '):
> > self.set_filename(line[6:].rstrip())
> > + print("\nChecking patch for %s ..." %(self.hunk_filename))
> > + if self.hunk_filename.endswith(".c") or \
> > + self.hunk_filename.endswith(".h"):
> > + self.file_type = "c_type"
> > + elif self.hunk_filename.endswith(".dec") or \
> > + self.hunk_filename.endswith(".dsc") or \
> > + self.hunk_filename.endswith(".fdf"):
> > + self.file_type = "edk2_type"
> > + elif self.hunk_filename.endswith(".S"):
> > + self.file_type = "asm_arm_type"
> > + elif self.hunk_filename.endswith(".asm"):
> > + self.file_type = "asm_type"
> > + else:
> > + self.file_type = "other_type"
> > if line.startswith('@@ '):
> > + sls = self.diff_src_line_start_re.search(line)
> > + if sls is not None:
> > + self.src_line_num = int(sls.group(0))
> > + else:
> > + self.src_line_num = 0
> > self.state = PATCH
> > self.binary = False
> > elif line.startswith('GIT binary patch'):
> > @@ -299,15 +389,25 @@ class GitDiffCheck:
> > self.line_num += 1
> > elif self.state == PATCH:
> > if self.binary:
> > + self.clean_counts()
> > pass
> > if line.startswith('-'):
> > + self.clean_counts()
> > pass
> > elif line.startswith('+'):
> > - self.check_added_line(line[1:])
> > + # indentation resets every time we leave "+" diff area
> > + self.check_added_line(line[1:], self.line_num + self.offset,
> > + self.src_line_num)
> > + self.src_line_num +=1
> > elif line.startswith(r'\ No newline '):
> > + self.clean_counts()
> > pass
> > elif not line.startswith(' '):
> > + self.clean_counts()
> > self.format_error("unexpected patch line")
> > + else:
> > + self.src_line_num +=1
> > + self.clean_counts()
> > self.line_num += 1
> >
> > pre_patch_prefixes = (
> > @@ -332,13 +432,19 @@ class GitDiffCheck:
> > else:
> > self.force_crlf = True
> >
> > - def added_line_error(self, msg, line):
> > + def added_line_error(self, msg, line, diff_line_num, src_line_num,
> > patch_type):
> > + start = TextColor(MsgFormat.color_on, MsgFormat.REPORT)
> > + end = TextColor(MsgFormat.color_on, MsgFormat.NORMAL)
> > lines = [ msg ]
> > - if self.hunk_filename is not None:
> > - lines.append('File: ' + self.hunk_filename)
> > - lines.append('Line: ' + line)
> > -
> > self.error(*lines)
> > + if self.hunk_filename is not None:
> > + print('File: ' + self.hunk_filename)
> > + line_info = 'Line '
> > + if patch_type == 'patch_file_type':
> > + line_info += ('%d(patch) ' %(diff_line_num))
> > + line_info += ('%d(source) ' %(src_line_num))
> > + line_info += (': ' +str(start) + line.lstrip() + str(end))
> > + print(line_info)
> >
> > old_debug_re = \
> > re.compile(r'''
> > @@ -348,28 +454,195 @@ class GitDiffCheck:
> > ''',
> > re.VERBOSE)
> >
> > - def check_added_line(self, line):
> > + space_check_parentheses_re = re.compile(r'#define\s*[a-zA-Z0-
> > 9_]\S*\(|[a-zA-Z0-9_]\S*\((?<!\)\()(?<!\(\()')
> > + space_check_brace_open_re = re.compile(r'[a-zA-Z0-9_]\S*\{')
> > + space_check_brace_close_re = re.compile(r'[\}]\S.*')
> > + brace_open_check_re = re.compile(r'[a-zA-Z0-9_]*\{')
> > + brace_close_check_re = re.compile(r'.*\}.*')
> > + multiline_check_re = re.compile(r'[a-zA-Z0-9_]*(\\|\&|\,|\+|\-
> > |\*|\||\=)\Z')
> > + void_cap_check_re = re.compile(r'(\s|\b|\()void(\s+|\*|\,|\))')
> > + static_cap_check_re = re.compile(r'(\s|\b)static\s+')
> > +
> > + def check_added_line(self, line, diff_line_num, src_line_num):
> > eol = ''
> > for an_eol in self.line_endings:
> > if line.endswith(an_eol):
> > eol = an_eol
> > line = line[:-len(eol)]
> >
> > - stripped = line.rstrip()
> > + rstripped = line.rstrip()
> > + lstripped = line.lstrip()
> >
> > if self.force_crlf and eol != '\r\n':
> > self.added_line_error('Line ending (%s) is not CRLF' % repr(eol),
> > - line)
> > + line, diff_line_num, src_line_num, self.patch_type)
> > if ' ' in line:
> > - self.added_line_error('Tab character used', line)
> > - if len(stripped) < len(line):
> > - self.added_line_error('Trailing whitespace found', line)
> > + self.added_line_error('Tab character used',
> > + line, diff_line_num, src_line_num, self.patch_type)
> > + if len(rstripped) < len(line):
> > + self.added_line_error('Trailing whitespace found',
> > + line, diff_line_num, src_line_num, self.patch_type)
> > +
> > + # empty "+" line, skip it
> > + if len(line) == 0:
> > + return
> >
> > - mo = self.old_debug_re.search(line)
> > - if mo is not None:
> > - self.added_line_error('EFI_D_' + mo.group(1) + ' was used, '
> > - 'but DEBUG_' + mo.group(1) +
> > - ' is now recommended', line)
> > + # the file type specific checks
> > + if self.file_type == "c_type":
> > +
> > + # set initial indentation as the number of spaces after the first "+"
> > + # indentation does not apply on lines with 0 characters
> > + # adjust the indentation if first "+" has "}"
> > + if self.current_indent_size == -1:
> > + if len(lstripped) != 0:
> > + self.current_indent_size = len(line) - len(lstripped)
> > + else:
> > + self.clean_counts()
> > +
> > + if lstripped.startswith("}"):
> > + self.current_indent_size = line.find("}") + 2
> > +
> > + indent_size = len(line) - len(lstripped)
> > + indent_size_adjust = 0
> > + force_brace_check = False
> > +
> > + if len(line) > 120:
> > + self.added_line_error('Line is too long (%d) (max 120 characters)',
> > + line, diff_line_num, src_line_num, self.patch_type)
> > +
> > + # skip comments for code checking
> > + if lstripped.startswith("//"):
> > + return
> > + if lstripped.startswith("/*") and not self.comments_open:
> > + self.comments_open = True
> > +
> > + if rstripped.endswith("*/") and self.comments_open:
> > + self.comments_open = False
> > + return
> > +
> > + if rstripped.endswith("*/") and not self.comments_open:
> > + # found unknown close comment, reset the indentation as
> > + # no idea where it started
> > + self.current_indent_size = -1
> > + return
> > +
> > + if not self.comments_open:
> > + mo = self.old_debug_re.search(line)
> > + if mo is not None:
> > + self.added_line_error('EFI_D_' + mo.group(1) + ' was used, '
> > + 'but DEBUG_' + mo.group(1) +
> > + ' is now recommended',
> > + line, diff_line_num, src_line_num, self.patch_type)
> > +
> > + # track down multiple braces in the same line
> > + braces_count = line.count('{') - line.count('}')
> > +
> > + # check if line has text in double quotes as we do not
> > + # need to check text messages
> > + text_str_start = line.find('\"')
> > +
> > + # track down a special case with "} else ..{" braces
> > + if line.find("else") != -1:
> > + force_brace_check = True
> > +
> > + # check for closing brace which affects indentation
> > + if braces_count < 0 and not force_brace_check:
> > + self.current_indent_size -= 2 * abs(braces_count)
> > + elif force_brace_check:
> > + if line.find("}") != -1:
> > + self.current_indent_size -= 2
> > + braces_count = 1
> > +
> > + # for the switch()/case:, adjust "case:" and "default:" indentation
> > + # as they aligned with the "switch()" indentation
> > + if lstripped.startswith("case ") or \
> > + lstripped.startswith("default:"):
> > + indent_size_adjust = -2
> > +
> > + # indentation code check related to the diff structure
> > + # do not check code indentation between parentheses
> > + if indent_size != self.current_indent_size + indent_size_adjust \
> > + and len(line) != 0 and self.parentheses_count == 0 \
> > + and not self.multiline_string:
> > + self.added_line_error('Invalid indentation. Expecting %d but
> > found %d spaces' \
> > + %(self.current_indent_size + indent_size_adjust, \
> > + indent_size),
> > + line, diff_line_num, src_line_num, self.patch_type)
> > +
> > + # check for missing space before open brace
> > + if braces_count > 0 or force_brace_check:
> > + if line.find("{") != -1:
> > + self.current_indent_size += 2 * braces_count
> > + if not line.startswith("{"):
> > + spb_check = self.space_check_brace_open_re.search(line)
> > + if spb_check is not None:
> > + self.added_line_error('Missing space before \'{\'',
> > + line, diff_line_num, src_line_num, self.patch_type)
> > +
> > + # check for missing space before open parenthesis
> > + spo = self.space_check_parentheses_re.finditer(line)
> > + if spo is not None and not self.multiline_string:
> > + # check if it's not inside of the double quoted string or
> > + # (aa)(bb)fff case or parenthesis belong to #define macro
> > + for parstr in spo:
> > + if not (text_str_start != -1 and text_str_start < parstr.start())
> > and \
> > + not parstr.group(0).startswith("#define"):
> > + self.added_line_error('Missing space before \'(\'',
> > + line, diff_line_num, src_line_num, self.patch_type)
> > + break;
> > +
> > + # check for closing brace cases
> > + if braces_count < 0 or \
> > + (force_brace_check and line.find("}") != -1):
> > + if not lstripped.startswith("}") and rstripped.endswith("}"):
> > + self.added_line_error('The \'}\' is not on its own line',
> > + line, diff_line_num, src_line_num, self.patch_type)
> > + if not lstripped.endswith("}"):
> > + scb = self.space_check_brace_close_re.search(lstripped)
> > + if scb is not None \
> > + and lstripped[scb.start() + 1] != ';' \
> > + and lstripped[scb.start() + 1] != ',' \
> > + and lstripped[scb.start() + 1] != '}':
> > + self.added_line_error('Missing space after \'}\'',
> > + line, diff_line_num, src_line_num, self.patch_type)
> > +
> > + # check for using "void" instead of "VOID"
> > + vcc = self.void_cap_check_re.search(line)
> > + if vcc is not None and \
> > + not (text_str_start != -1 and text_str_start < vcc.start()):
> > + self.added_line_error('Using \"void\" instead of \"VOID\"',
> > + line, diff_line_num, src_line_num, self.patch_type)
> > +
> > + #check for using "static" instead of "STATIC"
> > + scc = self.static_cap_check_re.search(line)
> > + if scc is not None and \
> > + not (text_str_start != -1 and text_str_start < scc.start()):
> > + self.added_line_error('Using \"static\" instead of \"STATIC\"',
> > + line, diff_line_num, src_line_num, self.patch_type)
> > +
> > + # track down parentheses, no indentation check between them
> > + if not (self.parentheses_count == 0 and lstripped.startswith(')') \
> > + and not self.multiline_string and not self.comments_open):
> > + self.parentheses_count += line.count('(') - line.count(')')
> > +
> > + # multiline strings indentation is unknown
> > + if not self.comments_open:
> > + mlc = self.multiline_check_re.search(line)
> > + if mlc is not None:
> > + self.multiline_string = True
> > + else:
> > + self.multiline_string = False
> > +
> > + elif self.file_type == "edk2_type":
> > + pass
> > + elif self.file_type == "asm_arm_type":
> > + pass
> > + elif self.file_type == "asm_type":
> > + pass
> > + elif self.file_type == "other_type":
> > + pass
> > + else:
> > + pass
> >
> > split_diff_re = re.compile(r'''
> > (?P<cmd>
> > @@ -388,6 +661,8 @@ class GitDiffCheck:
> > self.error(err, err2)
> >
> > def error(self, *err):
> > + start = TextColor(MsgFormat.color_on, MsgFormat.ERROR)
> > + end = TextColor(MsgFormat.color_on, MsgFormat.NORMAL)
> > if self.ok and Verbose.level > Verbose.ONELINE:
> > print('Code format is not valid:')
> > self.ok = False
> > @@ -396,7 +671,7 @@ class GitDiffCheck:
> > count = 0
> > for line in err:
> > prefix = (' *', ' ')[count > 0]
> > - print(prefix, line)
> > + print(start, prefix, line, end)
> > count += 1
> >
> > class CheckOnePatch:
> > @@ -406,16 +681,17 @@ class CheckOnePatch:
> > patch content.
> > """
> >
> > - def __init__(self, name, patch):
> > + def __init__(self, name, patch, patch_type):
> > self.patch = patch
> > self.find_patch_pieces()
> >
> > - msg_check = CommitMessageCheck(self.commit_subject,
> > self.commit_msg)
> > + msg_check = CommitMessageCheck(self.commit_subject,
> > self.commit_msg, \
> > + self.commit_msg_offset, self.cover)
> > msg_ok = msg_check.ok
> >
> > diff_ok = True
> > if self.diff is not None:
> > - diff_check = GitDiffCheck(self.diff)
> > + diff_check = GitDiffCheck(self.diff, self.diff_offset, patch_type)
> > diff_ok = diff_check.ok
> >
> > self.ok = msg_ok and diff_ok
> > @@ -458,6 +734,16 @@ class CheckOnePatch:
> > ''',
> > re.VERBOSE)
> >
> > + cover_letter_re = re.compile(r'Subject:\s*\[PATCH.*.0/.*',
> > + re.IGNORECASE | re.MULTILINE)
> > +
> > + def is_cover_letter(self, patch):
> > + cl = self.cover_letter_re.search(patch)
> > + if cl is None:
> > + return False
> > +
> > + return True
> > +
> > def find_patch_pieces(self):
> > if sys.version_info < (3, 0):
> > patch = self.patch.encode('ascii', 'ignore')
> > @@ -465,10 +751,13 @@ class CheckOnePatch:
> > patch = self.patch
> >
> > self.commit_msg = None
> > + self.commit_msg_offset = 0
> > self.stat = None
> > self.commit_subject = None
> > self.commit_prefix = None
> > self.diff = None
> > + self.diff_offset = 0
> > + self.cover = False
> >
> > if patch.startswith('diff --git'):
> > self.diff = patch
> > @@ -480,6 +769,14 @@ class CheckOnePatch:
> > assert(parts[0].get_content_type() == 'text/plain')
> > content = parts[0].get_payload(decode=True).decode('utf-8', 'ignore')
> >
> > + # find offset of content
> > + self.commit_msg_offset = len(patch.splitlines()) -
> > len(content.splitlines())
> > +
> > + # find offset of first diff section
> > + mo = self.git_diff_re.search(patch)
> > + if mo is not None:
> > + self.diff_offset = len(patch[:mo.start()].splitlines()) + 1
> > +
> > mo = self.git_diff_re.search(content)
> > if mo is not None:
> > self.diff = content[mo.start():]
> > @@ -492,6 +789,7 @@ class CheckOnePatch:
> > self.stat = mo.group('stat')
> > self.commit_msg = mo.group('commit_message')
> >
> > + self.cover = self.is_cover_letter(patch)
> > self.commit_subject = pmail['subject'].replace('\r\n', '')
> > self.commit_subject = self.commit_subject.replace('\n', '')
> > self.commit_subject = self.subject_prefix_re.sub('',
> > self.commit_subject, 1)
> > @@ -514,9 +812,9 @@ class CheckGitCommits:
> > print()
> > else:
> > blank_line = True
> > - print('Checking git commit:', commit)
> > + print('Checking git commit:', commit, '\n')
> > patch = self.read_patch_from_git(commit)
> > - self.ok &= CheckOnePatch(commit, patch).ok
> > + self.ok &= CheckOnePatch(commit, patch, "git_commit_type").ok
> >
> > def read_commit_list_from_git(self, rev_spec, max_count):
> > # Run git to get the commit patch
> > @@ -554,8 +852,8 @@ class CheckOnePatchFile:
> > patch = f.read().decode('utf-8', 'ignore')
> > f.close()
> > if Verbose.level > Verbose.ONELINE:
> > - print('Checking patch file:', patch_filename)
> > - self.ok = CheckOnePatch(patch_filename, patch).ok
> > + print('Checking patch file:', patch_filename, '\n')
> > + self.ok = CheckOnePatch(patch_filename, patch, "patch_file_type").ok
> >
> > class CheckOneArg:
> > """Performs a patch check for a single command line argument.
> > @@ -618,11 +916,16 @@ class PatchCheckApp:
> > group.add_argument("--silent",
> > action="store_true",
> > help="Print nothing")
> > + group.add_argument("--color",
> > + action="store_true",
> > + help="Enable color messages")
> > self.args = parser.parse_args()
> > if self.args.oneline:
> > Verbose.level = Verbose.ONELINE
> > if self.args.silent:
> > Verbose.level = Verbose.SILENT
> > + if self.args.color:
> > + MsgFormat.color_on = True
> >
> > if __name__ == "__main__":
> > sys.exit(PatchCheckApp().retval)
> > --
> > 2.7.4
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-12-21 4:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-16 23:11 [PATCH v7] BaseTools/Scripts/PatchCheck.py: Extended patch style check for c code Daniil Egranov
2016-12-19 8:31 ` Gao, Liming
2016-12-21 4:45 ` Jordan Justen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox