[PATCH 3/3] utils: checkstyle: Add a python formatter

[Laurent Pinchart]

Hi Stefan,

On Mon, Sep 02, 2024 at 04:14:20PM +0200, Stefan Klug wrote:
> On Fri, Aug 30, 2024 at 06:06:28PM +0300, Laurent Pinchart wrote:
> > On Fri, Aug 30, 2024 at 02:53:00PM +0200, Stefan Klug wrote:
> > > Reporting style issues on python files is great, automatically fixing
> > > them is even better. Add a call to autopep8 for python files.
> > > 
> > > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > > ---
> > >  utils/checkstyle.py | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > > 
> > > diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> > > index 5901f1a71562..ed15145b64a4 100755
> > > --- a/utils/checkstyle.py
> > > +++ b/utils/checkstyle.py
> > > @@ -954,6 +954,21 @@ class StripTrailingSpaceFormatter(Formatter):
> > >          return ''.join(lines)
> > >  
> > >  
> > > +class Pep8Formatter(Formatter):
> > > +    patterns = ('*.py',)
> > > +
> > > +    @classmethod
> > > +    def format(cls, filename, data):
> > > +        try:
> > > +            ret = subprocess.run(['autopep8', '--ignore=E501', '-'],
> > > +                                 input=data.encode('utf-8'), stdout=subprocess.PIPE)
> > > +        except FileNotFoundError:
> > > +            issues.append(StyleIssue(0, None, None, 'Please install autopep8 to format python additions'))
> > > +            return issues
> > > +
> > > +        return ret.stdout.decode('utf-8')
> > > +
> > > +
> > 
> > Testing this, on a simple patch that breaks the coding style, I get
> > 
> > ----------------------------------------------------------------------------------
> > dea6e2fea04866b477ca9bd622c885ca74b50d9c utils: checkstyle.py: Test PEP8 formatter
> > ----------------------------------------------------------------------------------
> > --- utils/checkstyle.py
> > +++ utils/checkstyle.py
> > @@ -728,7 +730,7 @@
> >              issues.append(StyleIssue(0, None, None, 'Please install pycodestyle to validate python additions'))
> >              return issues
> > 
> > -        results = ret.stdout.decode('utf-8').splitlines( )
> > +        results = ret.stdout.decode('utf-8').splitlines()
> >          for item in results:
> >              search = re.search(Pep8Checker.results_regex, item)
> >              line_number = int(search.group(1))
> > #731: E201 whitespace after '('
> > +        results = ret.stdout.decode('utf-8').splitlines( )
> >                                                           ^
> > ---
> > 2 potential issues detected, please review
> > 
> > [master dea6e2fea048] utils: checkstyle.py: Test PEP8 formatter
> > 
> > 
> > The checker and formatter generate duplicate issues. Would you consider
> > than as an issue ? Should we disable the checker when the dependencies
> > of the formatter are available ? Or could the checker report additional
> > issues ?
> 
> Hm. I saw cases (E.g. only a single blank line before a class) where the
> formatter (which added the missing line above the class) created a
> change that didn't intersect with the modification, but the Issue was
> correctly detected. For a moment I thought about adding a line above and
> below in the hunk intersect function, but that felt wrong. Skipping the
> checker would silence that issue, which is also not good. Adding more
> logic just for python seems to be too much effort.  Maybe just leave the
> duplicate issues, as we only do limited python development?

Do you plan to stop working on the tuning tool ? :-) If this is caused
by an off-by-one but I'd rather fix it instead of carrying the annoyance
of duplicated issues. Can you provide a sample commit that reproduces
the problem ?

> > >  # ------------------------------------------------------------------------------
> > >  # Style checking
> > >  #

-- 
Regards,

Laurent Pinchart