[PATCH 4/4] utils: checkstyle.py: Show location of coding style issue within line
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri May 31 19:08:53 CEST 2024
On Fri, May 31, 2024 at 03:14:14PM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2024-05-31 14:33:25)
> > On Fri, May 31, 2024 at 10:17:10PM +0900, Paul Elder wrote:
> > > On Fri, May 31, 2024 at 03:18:38PM +0300, Laurent Pinchart wrote:
> > > > The issue checkers display the line number and line content of each
> > > > offending line, but don't show the location of the issue within a line.
> > > > Improve checkstyle by adding a marker that points to the exact location.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > > ---
> > > > utils/checkstyle.py | 39 +++++++++++++++++++++++++--------------
> > > > 1 file changed, 25 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> > > > index 77d6f39011c1..ff885aa98d65 100755
> > > > --- a/utils/checkstyle.py
> > > > +++ b/utils/checkstyle.py
> > > > @@ -556,8 +556,9 @@ class StyleChecker(metaclass=ClassRegistry):
> > > >
> > > >
> > > > class StyleIssue(object):
> > > > - def __init__(self, line_number, line, msg):
> > > > + def __init__(self, line_number, position, line, msg):
> > > > self.line_number = line_number
> > > > + self.position = position
> > > > self.line = line
> > > > self.msg = msg
> > > >
> > > > @@ -584,7 +585,7 @@ class HexValueChecker(StyleChecker):
> > > > if value == value.lower():
> > > > continue
> > > >
> > > > - issues.append(StyleIssue(line_number, line,
> > > > + issues.append(StyleIssue(line_number, match.span(0), line,
> > > > f'Use lowercase hex constant {value.lower()}'))
> > > >
> > > > return issues
> > > > @@ -623,7 +624,7 @@ class IncludeChecker(StyleChecker):
> > > > header_type = 'C compatibility'
> > > > header = header[1:] + '.h'
> > > >
> > > > - issues.append(StyleIssue(line_number, line,
> > > > + issues.append(StyleIssue(line_number, match.span(1), line,
> > > > f'{header_type} header <{header}> is preferred'))
> > > >
> > > > return issues
> > > > @@ -641,10 +642,12 @@ class LogCategoryChecker(StyleChecker):
> > > > issues = []
> > > > for line_number in line_numbers:
> > > > line = self.__content[line_number-1]
> > > > - if not LogCategoryChecker.log_regex.search(line):
> > > > + match = LogCategoryChecker.log_regex.search(line)
> > > > + if not match:
> > > > continue
> > > >
> > > > - issues.append(StyleIssue(line_number, line, 'LOG() should use categories'))
> > > > + issues.append(StyleIssue(line_number, match.span(1), line,
> > > > + 'LOG() should use categories'))
> > > >
> > > > return issues
> > > >
> > > > @@ -660,8 +663,10 @@ class MesonChecker(StyleChecker):
> > > > issues = []
> > > > for line_number in line_numbers:
> > > > line = self.__content[line_number-1]
> > > > - if line.find('\t') != -1:
> > > > - issues.append(StyleIssue(line_number, line, 'meson.build should use spaces for indentation'))
> > > > + pos = line.find('\t')
> > > > + if pos != -1:
> > > > + issues.append(StyleIssue(line_number, [pos, pos], line,
> > > > + 'meson.build should use spaces for indentation'))
> > > > return issues
> > > >
> > > >
> > > > @@ -681,7 +686,7 @@ class Pep8Checker(StyleChecker):
> > > > ret = subprocess.run(['pycodestyle', '--ignore=E501', '-'],
> > > > input=data, stdout=subprocess.PIPE)
> > > > except FileNotFoundError:
> > > > - issues.append(StyleIssue(0, None, 'Please install pycodestyle to validate python additions'))
> > > > + issues.append(StyleIssue(0, None, None, 'Please install pycodestyle to validate python additions'))
> > > > return issues
> > > >
> > > > results = ret.stdout.decode('utf-8').splitlines()
> > > > @@ -693,7 +698,7 @@ class Pep8Checker(StyleChecker):
> > > >
> > > > if line_number in line_numbers:
> > > > line = self.__content[line_number - 1]
> > > > - issues.append(StyleIssue(line_number, line, msg))
> > > > + issues.append(StyleIssue(line_number, None, line, msg))
> > > >
> > > > return issues
> > > >
> > > > @@ -714,7 +719,7 @@ class ShellChecker(StyleChecker):
> > > > ret = subprocess.run(['shellcheck', '-Cnever', '-'],
> > > > input=data, stdout=subprocess.PIPE)
> > > > except FileNotFoundError:
> > > > - issues.append(StyleIssue(0, None, 'Please install shellcheck to validate shell script additions'))
> > > > + issues.append(StyleIssue(0, None, None, 'Please install shellcheck to validate shell script additions'))
> > > > return issues
> > > >
> > > > results = ret.stdout.decode('utf-8').splitlines()
> > > > @@ -727,11 +732,8 @@ class ShellChecker(StyleChecker):
> > > > line = results[nr + 1]
> > > > msg = results[nr + 2]
> > > >
> > > > - # Determined, but not yet used
> > > > - position = msg.find('^') + 1
> > > > -
> > > > if line_number in line_numbers:
> > > > - issues.append(StyleIssue(line_number, line, msg))
> > > > + issues.append(StyleIssue(line_number, None, line, msg))
> > > >
> > > > return issues
> > > >
> > > > @@ -973,6 +975,15 @@ def check_file(top_level, commit, filename, checkers):
> > > > print('%s+%s%s' % (Colours.fg(Colours.Yellow), issue.line.rstrip(),
> > > > Colours.reset()))
> > > >
> > > > + if issue.position is not None:
> > > > + # Align the position marker by using the original line with
> > > > + # all characters by tabs replaced with spaces. This ensures
> > >
> > > I'm confused by the english here.
> >
> > s/by tabs/but tabs/
> >
> > Is that clearer ?
>
> Oh I see. Do you mean all up to the point of the marker?
> I'd use "with all characters except for tabs replaced by spaces"
I'll use that.
> (optional by spaces/with spaces)
>
> > > > + # proper alignment regardless of how the code is indented.
> > > > + start = issue.position[0]
> > > > + prefix = ''.join([c if c == '\t' else ' ' for c in issue.line[:start]])
> > > > + length = issue.position[1] - start - 1
> > > > + print(' ' + prefix + '^' + '~' * length)
>
> I'm not sure I understand how this will correctly determine the position
> with combinations of tabs/spaces but let see.
>
> I expect you'll have tested it so I look forward to seeing the results.
$ ./utils/checkstyle.py 3755d966485d96e76eef7756222baf02dea960a4
--------------------------------------------------------------------------------------
3755d966485d96e76eef7756222baf02dea960a4 libcamera: software_isp: Add DebayerCpu class
--------------------------------------------------------------------------------------
--- src/libcamera/software_isp/debayer_cpu.cpp
+++ src/libcamera/software_isp/debayer_cpu.cpp
#14: C++ header <cmath> is preferred
+#include <math.h>
^~~~~~
---
1 potential issue detected, please review
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > > > +
> > > > return len(formatted_diff) + len(issues)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list