[libcamera-devel] [PATCH 2/2] utils: checkstyle: Catch LOG() usage without an explicit category
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Feb 13 12:08:44 CET 2019
Hi Kieran,
On Wed, Feb 13, 2019 at 08:17:32AM +0000, Kieran Bingham wrote:
> On 12/02/2019 22:25, Laurent Pinchart wrote:
> > Add support for checkers not related to code formatting to the
> > checkstyle.py script, and create a first checker that catches usage of
> > the LOG() macro without an explicit category.
>
> Ah, now I see why you said it was easy to add extra checkers.
>
> It's easy when you know the architecture of the script at least :)
Feel free to get yourself familiar with it too ;-)
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > utils/checkstyle.py | 55 ++++++++++++++++++++++++++++++++++++++-------
> > 1 file changed, 47 insertions(+), 8 deletions(-)
> >
> > diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> > index f3005d1fbb52..9abd2687b1f9 100755
> > --- a/utils/checkstyle.py
> > +++ b/utils/checkstyle.py
> > @@ -146,7 +146,7 @@ class DiffHunk(object):
> > s += Colours.reset()
> > s += '\n'
> >
> > - return s
> > + return s[:-1]
>
> I think this chops off the last character in s. Why are you doing that ?
> (especially as without seeing the rest of the context, you've only
> 'just' added a '\n'
>
> Or is that it - to remove the last \n ?
>
>
> Aha ok it clearly is to remove the \n.
Yes, as the string is printed with the print() function, an extra \n is
added.
> > def append(self, line):
> > if line[0] == ' ':
> > @@ -229,6 +229,29 @@ available_formatters = {
> > # Style checking
> > #
> >
> > +class LogCategoryChecker(object):
> > + log_regex = re.compile('\\bLOG\((Debug|Info|Warning|Error|Fatal)\)')
> > +
> > + def __init__(self, content):
> > + self.__content = content
> > +
> > + def check(self, line_numbers):
> > + issues = []
> > + for line_number in line_numbers:
> > + line = self.__content[line_number-1]
> > + if not LogCategoryChecker.log_regex.search(line):
> > + continue
> > +
> > + issues.append([line_number, line, 'LOG() should use categories'])
> > +
> > + return issues
> > +
> > +
> > +available_checkers = {
> > + 'log_category': LogCategoryChecker,
> > +}
> > +
> > +
> > def check_file(top_level, commit, filename, formatters):
> > # Extract the line numbers touched by the commit.
> > diff = subprocess.run(['git', 'diff', '%s~..%s' % (commit, commit), '--',
> > @@ -260,20 +283,36 @@ def check_file(top_level, commit, filename, formatters):
> > formatted = formatted.splitlines(True)
> > diff = difflib.unified_diff(after, formatted)
> >
> > - # Split the diff in hunks, recording line number ranges for each hunk.
> > + # Split the diff in hunks, recording line number ranges for each hunk, and
> > + # filter out hunks that are not touched by the commit.
> > formatted_diff = parse_diff(diff)
> > -
> > - # Filter out hunks that are not touched by the commit.
> > formatted_diff = [hunk for hunk in formatted_diff if hunk.intersects(lines)]
> > - if len(formatted_diff) == 0:
> > +
> > + # Check for code issues not related to formatting.
> > + issues = []
> > + for checker in available_checkers:
> > + checker = available_checkers[checker](after)
> > + for hunk in commit_diff:
> > + issues += checker.check(hunk.side('to').touched)
> > +
> > + # Print the detected issues.
> > + if len(issues) == 0 and len(formatted_diff) == 0:
> > return 0
> >
> > print('%s---' % Colours.fg(Colours.Red), filename)
> > print('%s+++' % Colours.fg(Colours.Green), filename)
>
> Interestingly, these both print red and green respectively...
>
> > - for hunk in formatted_diff:
> > - print(hunk)
> >
> > - return len(formatted_diff)
> > + if len(formatted_diff):
> > + for hunk in formatted_diff:
> > + print(hunk)
> > +
> > + if len(issues):
> > + issues.sort()
> > + for issue in issues:
> > + print('%s#%u: %s' % (Colours.fg(Colours.Yellow), issue[0], issue[2]))
>
> But this looks distinctly brown or is it orange ...
> I wonder if that's due to my shell perhaps.
It prints orange-brown here, probably the same colour as on your screen.
The colour being displayed depends on your terminal's theme.
> > + print('+%s%s' % (issue[1].rstrip(), Colours.reset()))
> > +
> > + return len(formatted_diff) + len(issues)
> >
> >
> > def check_style(top_level, commit, formatters):
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list