From patchwork Mon Jan 21 17:35:37 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 308 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1608C60C83 for ; Mon, 21 Jan 2019 18:35:51 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 679BA53E for ; Mon, 21 Jan 2019 18:35:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1548092150; bh=a3UbgBi2SSOWy4/SRLaAMoerK7BZkXeUeJnWbFi8g8M=; h=From:To:Subject:Date:From; b=AolH/zUn6KdBURI/qw2l3aNQVrxbc8FNSaNoChR+/g2gPCRWXrF2g71fzydEf+UNC iYlZmwAiz8qknUp3dh2nCd16Vt8r7pCCZmK2h0Foaav1x08ZABHJcAFsM9TpA0Mtcr ryVceydLeSz/SvVpuEPUzBUlmPGuDyuOIywpmkeo= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 21 Jan 2019 19:35:37 +0200 Message-Id: <20190121173537.31734-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH] utils: checkstyle: Add support for clang-format X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Jan 2019 17:35:51 -0000 clang-format produces better results than astyle as it can better match the libcamera coding style. Default to clang-format over astyle, fall back to astyle if clang-format isn't found, and add a --formatter command line option to select a formatter manually. The initial clang-format options are copied from the Linux kernel style, with the C++ standard updated to C++11 and the line length limit removed to keep the original line breaks. Signed-off-by: Laurent Pinchart --- .clang-format | 118 +++++++++++++++++++++++++++++++++ Documentation/coding-style.rst | 25 +++++-- utils/checkstyle.py | 70 ++++++++++++++----- 3 files changed, 189 insertions(+), 24 deletions(-) create mode 100644 .clang-format diff --git a/.clang-format b/.clang-format new file mode 100644 index 000000000000..6edcd3ff583c --- /dev/null +++ b/.clang-format @@ -0,0 +1,118 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# clang-format configuration file. Intended for clang-format >= 4. +# +# For more information, see: +# +# Documentation/process/clang-format.rst +# https://clang.llvm.org/docs/ClangFormat.html +# https://clang.llvm.org/docs/ClangFormatStyleOptions.html +# +--- +AccessModifierOffset: -4 +AlignAfterOpenBracket: Align +AlignConsecutiveAssignments: false +AlignConsecutiveDeclarations: false +#AlignEscapedNewlines: Left # Unknown to clang-format-4.0 +AlignOperands: true +AlignTrailingComments: false +AllowAllParametersOfDeclarationOnNextLine: false +AllowShortBlocksOnASingleLine: false +AllowShortCaseLabelsOnASingleLine: false +AllowShortFunctionsOnASingleLine: None +AllowShortIfStatementsOnASingleLine: false +AllowShortLoopsOnASingleLine: false +AlwaysBreakAfterDefinitionReturnType: None +AlwaysBreakAfterReturnType: None +AlwaysBreakBeforeMultilineStrings: false +AlwaysBreakTemplateDeclarations: false +BinPackArguments: true +BinPackParameters: true +BraceWrapping: + AfterClass: false + AfterControlStatement: false + AfterEnum: false + AfterFunction: true + AfterNamespace: true + AfterObjCDeclaration: false + AfterStruct: false + AfterUnion: false + #AfterExternBlock: false # Unknown to clang-format-5.0 + BeforeCatch: false + BeforeElse: false + IndentBraces: false + #SplitEmptyFunction: true # Unknown to clang-format-4.0 + #SplitEmptyRecord: true # Unknown to clang-format-4.0 + #SplitEmptyNamespace: true # Unknown to clang-format-4.0 +BreakBeforeBinaryOperators: None +BreakBeforeBraces: Custom +#BreakBeforeInheritanceComma: false # Unknown to clang-format-4.0 +BreakBeforeTernaryOperators: false +BreakConstructorInitializersBeforeComma: false +#BreakConstructorInitializers: BeforeComma # Unknown to clang-format-4.0 +BreakAfterJavaFieldAnnotations: false +BreakStringLiterals: false +ColumnLimit: 0 +CommentPragmas: '^ IWYU pragma:' +#CompactNamespaces: false # Unknown to clang-format-4.0 +ConstructorInitializerAllOnOneLineOrOnePerLine: false +ConstructorInitializerIndentWidth: 8 +ContinuationIndentWidth: 8 +Cpp11BracedListStyle: false +DerivePointerAlignment: false +DisableFormat: false +ExperimentalAutoDetectBinPacking: false +#FixNamespaceComments: false # Unknown to clang-format-4.0 + +#IncludeBlocks: Preserve # Unknown to clang-format-5.0 +IncludeCategories: + - Regex: '.*' + Priority: 1 +IncludeIsMainRegex: '(_test)?$' +IndentCaseLabels: false +#IndentPPDirectives: None # Unknown to clang-format-5.0 +IndentWidth: 8 +IndentWrappedFunctionNames: false +JavaScriptQuotes: Leave +JavaScriptWrapImports: true +KeepEmptyLinesAtTheStartOfBlocks: false +MacroBlockBegin: '' +MacroBlockEnd: '' +MaxEmptyLinesToKeep: 1 +NamespaceIndentation: Inner +#ObjCBinPackProtocolList: Auto # Unknown to clang-format-5.0 +ObjCBlockIndentWidth: 8 +ObjCSpaceAfterProperty: true +ObjCSpaceBeforeProtocolList: true + +# Taken from git's rules +#PenaltyBreakAssignment: 10 # Unknown to clang-format-4.0 +PenaltyBreakBeforeFirstCallParameter: 30 +PenaltyBreakComment: 10 +PenaltyBreakFirstLessLess: 0 +PenaltyBreakString: 10 +PenaltyExcessCharacter: 100 +PenaltyReturnTypeOnItsOwnLine: 60 + +PointerAlignment: Right +ReflowComments: false +SortIncludes: false +#SortUsingDeclarations: false # Unknown to clang-format-4.0 +SpaceAfterCStyleCast: false +SpaceAfterTemplateKeyword: true +SpaceBeforeAssignmentOperators: true +#SpaceBeforeCtorInitializerColon: true # Unknown to clang-format-5.0 +#SpaceBeforeInheritanceColon: true # Unknown to clang-format-5.0 +SpaceBeforeParens: ControlStatements +#SpaceBeforeRangeBasedForLoopColon: true # Unknown to clang-format-5.0 +SpaceInEmptyParentheses: false +SpacesBeforeTrailingComments: 1 +SpacesInAngles: false +SpacesInContainerLiterals: false +SpacesInCStyleCastParentheses: false +SpacesInParentheses: false +SpacesInSquareBrackets: false +Standard: Cpp11 +TabWidth: 8 +UseTab: Always +... diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst index 30a1455d8c65..02cdc1677bf1 100644 --- a/Documentation/coding-style.rst +++ b/Documentation/coding-style.rst @@ -170,8 +170,11 @@ These rules match the `object ownership rules from the Chromium C++ Style Guide` Tools ----- -The 'astyle' code formatting tool can be used to reformat source files with the -libcamera coding style, defined by the following arguments. +The 'clang-format' code formatting tool can be used to reformat source files +with the libcamera coding style, defined in the .clang-format file at the root +of the source tree. + +Alternatively the 'astyle' tool can also be used, with the following arguments. :: @@ -184,11 +187,15 @@ libcamera coding style, defined by the following arguments. --align-reference=name --max-code-length=120 -As astyle is a code formatter, it operates on full files outputs reformatted -source code. While it can be used to reformat code before sending patches, it -may generate unrelated changes. To avoid this, libcamera provides a -'checkstyle.py' script wrapping astyle to only retain related changes. This -should be used to validate modifications before submitting them for review. +Use of astyle is discourage as clang-format better matches the libcamera coding +style. + +As both astyle and clang-format are code formatters, they operate on full files +and output reformatted source code. While they can be used to reformat code +before sending patches, it may generate unrelated changes. To avoid this, +libcamera provides a 'checkstyle.py' script wrapping the formatting tools to +only retain related changes. This should be used to validate modifications +before submitting them for review. The script operates on one or multiple git commits specified on the command line. It does not modify the git tree, the index or the working directory and @@ -277,4 +284,8 @@ diff that fixes the issues, on top of the corresponding commit. As the script is in early development false positive are expected. The flagged issues should be reviewed, but the diff doesn't need to be applied blindly. +The checkstyle.py script uses clang-format by default if found, and otherwise +falls back to astyle. The formatter can be manually selected with the +'--formatter' argument. + Happy hacking, libcamera awaits your patches! diff --git a/utils/checkstyle.py b/utils/checkstyle.py index a40d7dec879f..f3005d1fbb52 100755 --- a/utils/checkstyle.py +++ b/utils/checkstyle.py @@ -4,11 +4,11 @@ # # Author: Laurent Pinchart # -# checkstyle.py - A patch style checker script based on astyle +# checkstyle.py - A patch style checker script based on astyle or clang-format # # TODO: # -# - Support other formatting tools (clang-format, ...) +# - Support other formatting tools and checkers (cppcheck, cpplint, kwstyle, ...) # - Split large hunks to minimize context noise # - Improve style issues counting # @@ -33,6 +33,12 @@ astyle_options = ( '--max-code-length=120' ) +dependencies = { + 'astyle': False, + 'clang-format': False, + 'git': True, +} + source_extensions = ( '.c', '.cpp', @@ -192,30 +198,38 @@ def parse_diff(diff): # Code reformatting # -def formatter_astyle(data): +def formatter_astyle(filename, data): ret = subprocess.run(['astyle', *astyle_options], input=data.encode('utf-8'), stdout=subprocess.PIPE) return ret.stdout.decode('utf-8') -def formatter_strip_trailing_space(data): +def formatter_clang_format(filename, data): + ret = subprocess.run(['clang-format', '-style=file', + '-assume-filename=' + filename], + input=data.encode('utf-8'), stdout=subprocess.PIPE) + return ret.stdout.decode('utf-8') + + +def formatter_strip_trailing_space(filename, data): lines = data.split('\n') for i in range(len(lines)): lines[i] = lines[i].rstrip() + '\n' return ''.join(lines) -formatters = [ - formatter_astyle, - formatter_strip_trailing_space, -] +available_formatters = { + 'astyle': formatter_astyle, + 'clang-format': formatter_clang_format, + 'strip-trailing-spaces': formatter_strip_trailing_space, +} # ------------------------------------------------------------------------------ # Style checking # -def check_file(top_level, commit, filename): +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), '--', '%s/%s' % (top_level, filename)], @@ -239,7 +253,8 @@ def check_file(top_level, commit, filename): formatted = after for formatter in formatters: - formatted = formatter(formatted) + formatter = available_formatters[formatter] + formatted = formatter(filename, formatted) after = after.splitlines(True) formatted = formatted.splitlines(True) @@ -261,7 +276,7 @@ def check_file(top_level, commit, filename): return len(formatted_diff) -def check_style(top_level, commit): +def check_style(top_level, commit, formatters): # Get the commit title and list of files. ret = subprocess.run(['git', 'show', '--pretty=oneline','--name-only', commit], stdout=subprocess.PIPE) @@ -282,7 +297,7 @@ def check_style(top_level, commit): issues = 0 for f in files: - issues += check_file(top_level, commit, f) + issues += check_file(top_level, commit, f, formatters) if issues == 0: print("No style issue detected") @@ -330,17 +345,38 @@ def main(argv): # Parse command line arguments parser = argparse.ArgumentParser() + parser.add_argument('--formatter', '-f', type=str, choices=['astyle', 'clang-format'], + help='Code formatter. Default to clang-format if not specified.') parser.add_argument('revision_range', type=str, default='HEAD', nargs='?', help='Revision range (as defined by git rev-parse). Defaults to HEAD if not specified.') args = parser.parse_args(argv[1:]) # Check for required dependencies. - dependencies = ('astyle', 'git') + for command, mandatory in dependencies.items(): + found = shutil.which(command) + if mandatory and not found: + print("Executable %s not found" % command) + return 1 - for dependency in dependencies: - if not shutil.which(dependency): - print("Executable %s not found" % dependency) + dependencies[command] = found + + if args.formatter: + if not args.formatter in dependencies or \ + not dependencies[args.formatter]: + print("Formatter %s not available" % args.formatter) return 1 + formatter = args.formatter + else: + if dependencies['clang-format']: + formatter = 'clang-format' + elif dependencies['astyle']: + formatter = 'astyle' + else: + print("No formatter found, please install clang-format or astyle") + return 1 + + # Create the list of formatters to be applied. + formatters = [formatter, 'strip-trailing-spaces'] # Get the top level directory to pass absolute file names to git diff # commands, in order to support execution from subdirectories of the git @@ -352,7 +388,7 @@ def main(argv): revlist = extract_revlist(args.revision_range) for commit in revlist: - check_style(top_level, commit) + check_style(top_level, commit, formatters) print('') return 0