On 05/15/2017 10:35 PM, Martin Sebor wrote: > On 05/15/2017 07:55 AM, Martin Liška wrote: >> Hello. >> >> Recently I've been working on bigger changes to dump infrastructure and I >> had to >> fix tens of formatting issues reported by check_GNU_style.sh script. The >> script works >> quite fine, but it's very unpleasant that it reports problematic lines in >> the patch, >> not in original patches. > > I was a little confused by this but in the end decided that you > must have meant "not in the original sources." I can see how > having the script generate a patch to fix the problems it finds > could be handy.
Sorry for the confusion. > > I decided to substitute part of functionality by Python >> script that uses a library that parses patches. So that reported errors can >> be >> easily converted to quickfix list for VIM. That makes navigation very easy. >> >> I'm attaching simple version that I made in couple of minutes and I would >> like to >> ask whether the bash script is broadly used and whether community would be >> interested >> in transformation of the script? > > I use the script less and less as I become more comfortable with > the GCC style, and I don't expect to be making significant changes > to it. I'm sure it can be improved and/or simplified but I have > developed a (shell/awk/sed) script of my own that automatically > fixes most of these little things for me while applying a patch, > so I don't have to worry about them while writing code. I'm also improving in the GNU coding style. However, doing for instance replaces that touch already not conformance lines, can be problem. Apart from that, I always forget when doing bigger changes. > > That said, I don't really see a very compelling reason to rewrite > this script in Python or other high-level language, or add another > dependency for contributors to have to remember to install to use > it. I'd rather see the effort invested into a smart commit hook > that would format all code for us the expected way automatically. > (That would save enough hassle to justify introducing a dependency > on almost any language or library ;-) I know it's quite far from being ideal, but it's not about doing a dependency, but helper that can help new (and also current) maintainers. Well, I did some effort in creation of contrib/clang-format, which works for quite well. But I see it problematic that sometimes it hits for instance function arguments and provides a different (also valid) line formatting. Same for argument initialization in constructors, etc. I would be also happy to have a hook, but.. I'm attaching second version of the patch I played with last evening. It covers all patterns as the old script, provides error formatting with term colors and generates quickfix lists with column information included. Moreover, I added unittest, where we can eventually add additional tests. May I install the patch? Martin > > Martin
#!/usr/bin/env python3 # # Checks some of the GNU style formatting rules in a set of patches. # # This file is part of GCC. # # GCC is free software; you can redistribute it and/or modify it under # the terms of the GNU General Public License as published by the Free # Software Foundation; either version 3, or (at your option) any later # version. # # GCC is distributed in the hope that it will be useful, but WITHOUT ANY # WARRANTY; without even the implied warranty of MERCHANTABILITY or # FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License # for more details. # # You should have received a copy of the GNU General Public License # along with GCC; see the file COPYING3. If not see # <http://www.gnu.org/licenses/>. */ # # TODO: comment pip3 install and mention usage patch, termcolor # TODO: add more tests import sys import re import argparse import unittest from termcolor import colored from unidiff import PatchSet from itertools import * ws_char = 'â' ts = 8 def error_string(s): return colored(s, 'red', attrs = ['bold']) class CheckError: def __init__(self, filename, lineno, console_error, error_message, column = -1): self.filename = filename self.lineno = lineno self.console_error = console_error self.error_message = error_message self.column = column def error_location(self): return '%s:%d:%d:' % (self.filename, self.lineno, self.column if self.column != -1 else -1) class LineLengthCheck: def __init__(self): self.limit = 80 self.expanded_tab = ' ' * ts def check(self, filename, lineno, line): line_expanded = line.replace('\t', self.expanded_tab) if len(line_expanded) > self.limit: return CheckError(filename, lineno, line_expanded[:self.limit] + error_string(line_expanded[self.limit:]), 'lines should not exceed 80 characters', self.limit) return None class SpacesCheck: def __init__(self): self.expanded_tab = ' ' * ts def check(self, filename, lineno, line): i = line.find(self.expanded_tab) if i != -1: return CheckError(filename, lineno, line.replace(self.expanded_tab, error_string(ws_char * ts)), 'blocks of 8 spaces should be replaced with tabs', i) class TrailingWhitespaceCheck: def __init__(self): self.re = re.compile('(\s+)$') def check(self, filename, lineno, line): m = self.re.search(line) if m != None: return CheckError(filename, lineno, line[:m.start(1)] + error_string(ws_char * len(m.group(1))) + line[m.end(1):], 'trailing whitespace', m.start(1)) class SentenceSeparatorCheck: def __init__(self): self.re = re.compile('\w\.(\s|\s{3,})\w') def check(self, filename, lineno, line): m = self.re.search(line) if m != None: return CheckError(filename, lineno, line[:m.start(1)] + error_string(ws_char * len(m.group(1))) + line[m.end(1):], 'dot, space, space, new sentence', m.start(1)) class SentenceEndOfCommentCheck: def __init__(self): self.re = re.compile('\w\.(\s{0,1}|\s{3,})\*/') def check(self, filename, lineno, line): m = self.re.search(line) if m != None: return CheckError(filename, lineno, line[:m.start(1)] + error_string(ws_char * len(m.group(1))) + line[m.end(1):], 'dot, space, space, end of comment', m.start(1)) class SentenceDotEndCheck: def __init__(self): self.re = re.compile('\w(\s*\*/)') def check(self, filename, lineno, line): m = self.re.search(line) if m != None: return CheckError(filename, lineno, line[:m.start(1)] + error_string(m.group(1)) + line[m.end(1):], 'dot, space, space, end of comment', m.start(1)) class FunctionParenthesisCheck: # TODO: filter out GTY stuff def __init__(self): self.re = re.compile('\w(\s{2,})?(\()') def check(self, filename, lineno, line): if '#define' in line: return None m = self.re.search(line) if m != None: return CheckError(filename, lineno, line[:m.start(2)] + error_string(m.group(2)) + line[m.end(2):], 'there should be exactly one space between function name ' \ 'and parenthesis', m.start(2)) class SquareBracketCheck: def __init__(self): self.re = re.compile('\w\s+(\[)') def check(self, filename, lineno, line): m = self.re.search(line) if m != None: return CheckError(filename, lineno, line[:m.start(1)] + error_string(m.group(1)) + line[m.end(1):], 'there should be no space before a left square bracket', m.start(1)) class ClosingParenthesisCheck: def __init__(self): self.re = re.compile('\S\s+(\))') def check(self, filename, lineno, line): m = self.re.search(line) if m != None: return CheckError(filename, lineno, line[:m.start(1)] + error_string(m.group(1)) + line[m.end(1):], 'there should be no space before closing parenthesis', m.start(1)) class BracesOnSeparateLineCheck: # This will give false positives for C99 compound literals. def __init__(self): self.re = re.compile('(\)|else)\s*({)') def check(self, filename, lineno, line): m = self.re.search(line) if m != None: return CheckError(filename, lineno, line[:m.start(2)] + error_string(m.group(2)) + line[m.end(2):], 'braces should be on a separate line', m.start(2)) class TrailinigOperatorCheck: def __init__(self): regex = '^\s.*(([^a-zA-Z_]\*)|([-%<=&|^?])|([^*]/)|([^:][+]))$' self.re = re.compile(regex) def check(self, filename, lineno, line): m = self.re.search(line) if m != None: return CheckError(filename, lineno, line[:m.start(1)] + error_string(m.group(1)) + line[m.end(1):], 'trailing operator', m.start(1)) class LineLengthTest(unittest.TestCase): def setUp(self): self.check = LineLengthCheck() def test_line_length_check_basic(self): r = self.check.check('foo', 123, self.check.limit * 'a' + ' = 123;') self.assertIsNotNone(r) self.assertEqual('foo', r.filename) self.assertEqual(80, r.column) self.assertEqual(r.console_error, self.check.limit * 'a' + error_string(' = 123;')) def main(): parser = argparse.ArgumentParser(description='Check GNU coding style.') parser.add_argument('file', help = 'File with a patch') parser.add_argument('-f', '--format', default = 'stdio', help = 'Display format', choices = ['stdio', 'quickfix']) args = parser.parse_args() checks = [LineLengthCheck(), SpacesCheck(), TrailingWhitespaceCheck(), SentenceSeparatorCheck(), SentenceEndOfCommentCheck(), SentenceDotEndCheck(), FunctionParenthesisCheck(), SquareBracketCheck(), ClosingParenthesisCheck(), BracesOnSeparateLineCheck(), TrailinigOperatorCheck()] errors = [] with open(args.file, 'rb') as diff_file: patch = PatchSet(diff_file, encoding = 'utf-8') for pfile in patch.added_files + patch.modified_files: t = pfile.target_file.lstrip('b/') # Skip testsuite files if 'testsuite' in t: continue for hunk in pfile: delta = 0 for line in hunk: if line.is_added and line.target_line_no != None: for check in checks: e = check.check(t, line.target_line_no, line.value) if e != None: errors.append(e) if args.format == 'stdio': fn = lambda x: x.error_message i = 1 for (k, errors) in groupby(sorted(errors, key = fn), fn): errors = list(errors) print('=== ERROR type #%d: %s (%d error(s)) ===' % (i, k, len(errors))) i += 1 for e in errors: print(e.error_location () + e.console_error) print() exit(0 if len(errors) == 0 else 1) elif args.format == 'quickfix': f = 'errors.err' with open(f, 'w+') as qf: for e in errors: qf.write('%s%s\n' % (e.error_location(), e.error_message)) if len(errors) == 0: exit(0) else: print('%d error(s) written to %s file.' % (len(errors), f)) exit(1) else: assert False if __name__ == '__main__': if len(sys.argv) > 1: main() else: unittest.main()
