Feature: Implement a C/C++ style guide with linting

[johnlettman]

Implementing a style guide and enforcing a uniform syntax style would significantly enhance development, particularly as the codebase expands.

Linting and Formatting Tools

Presently, there are several tools available and up to the task:

• clang-format: A clang-based formatter that formats C and C++ code according to a style guide, such as Google, LLVM, Mozilla, Webkit, and Chromium.

  ClangFormat describes a set of tools that are built on top of LibFormat. It can support your workflow in a variety of ways including a standalone tool and editor integrations.

• clang-tidy: A clang-based C and C++ linter tool to detect typical syntax and semantic errors.

  clang-tidy is a clang-based C++ “linter” tool. Its purpose is to provide an extensible framework for diagnosing and fixing typical programming errors, like style violations, interface misuse, or bugs that can be deduced via static analysis. clang-tidy is modular and provides a convenient interface for writing new checks.

• cppcheck: A static analysis tool for C and C++ code to detect bugs, undefined behavior, and dangerous constructs.

  Cppcheck is a static analysis tool for C/C++ code. It provides unique code analysis to detect bugs and focuses on detecting undefined behaviour and dangerous coding constructs. The goal is to have very few false positives. Cppcheck is designed to be able to analyze your C/C++ code even if it has non-standard syntax (common in embedded projects).

• cpplint: A C and C++ linter tool following Google's C++ style guide.

  Cpplint is a command-line tool to check C/C++ files for style issues following Google's C++ style guide. Cpplint is developed and maintained by Google Inc. at google/styleguide, also see the wikipedia entry
  While Google maintains cpplint, Google is not (very) responsive to issues and pull requests, this fork aims to be (somewhat) more open to add fixes to cpplint to enable fixes, when those fixes make cpplint usable in wider contexts. Also see discussion here Add python 3 support to cpplint and cpplint_unittest google/styleguide#528.

• uncrustify: A source code formatter (or, "beautifier") supporting C, C++, C#, Objective-C, D, Java, Pawn, and Vala.

  A source code beautifier for C, C++, C#, Objective-C, D, Java, Pawn and Vala.

Conventions and Style Guides

Maintaining a shared programming style and implementing that standard using automation and analysis tools requires a well-formed style guide. The following major style guides exist:

• LLVM
  Sample: https://github.com/motine/cppstylelineup/blob/master/llvm.cpp
• Google
  Sample: https://github.com/motine/cppstylelineup/blob/master/google.cpp
• Chromium
  Sample: https://github.com/motine/cppstylelineup/blob/master/chromium.cpp
• Mozilla
  Sample: https://github.com/motine/cppstylelineup/blob/master/mozilla.cpp
• WebKit
  Sample: https://github.com/motine/cppstylelineup/blob/master/webkit.cpp
• Microsoft
  Sample: https://github.com/motine/cppstylelineup/blob/master/microsoft.cpp
• GNU
  Sample: https://github.com/motine/cppstylelineup/blob/master/gnu.cpp

Recommendation

Based on conventions in the wild, the trend seems to lean toward the Google style guide; however, I leave this personal preference to @dburkart.

Existing Style

I will now examine and compare the existing code to the listed conventions.

Brace Placement

Starting with functions, the opening brace { is on the same line as the function declaration:

check-sieve/src/AST/Validation/Command.cc

Line 76 in 40e51c6

bool Command::validate(const ASTNode *node) {

This alone eliminates the following style guides based on brace placement:

• GNU: next line braces
• Microsoft: next line braces
• Mozilla: next line braces
• WebKit: next line braces

This leaves the following style guides:

• Chromium
• Google
• LLVM

Return Type Placement

Next, the return type of a function is on the same line as the function declaration:

check-sieve/src/AST/Validation/Command.cc

Line 122 in 40e51c6

bool Command::_validateDeleteHeadersCommand(const ASTNode *node) {

The following style guides also implement this preference:

• Chromium
• Google
• LLVM
• Microsoft ❌
• WebKit ❌

Summary

The following style guides appear to be the most compatible:

• Chromium
• Google
• LLVM

Overall Summary

Implementing clang-format, clang-tidy, and cppcheck would greatly improve QA across the codebase. These can be passively enforced via GitHub actions to contextualize pull requests.

Note

Implementing the change will also require a refactor of the existing code -- creating a particularly messy commit diff.

[johnlettman]

Currently, I am poking at a branch that uses clang-format, clang-tidy, and cppcheck with the Google Style Guide. This can also touch on #5 by tweaking parts of the build system simultaneously, given we'd be tugging on the Makefiles.

also sorry for the wall of text

[dburkart]

Starting with functions, the opening brace { is on the same line as the function declaration:

I do like next-line braces for functions, although I recognize I don't consistently do it (since I haven't been using a tool).

I think I like the WebKit style the best... but I'm probably biased from having worked on that project while at Apple...

[johnlettman]

The WebKit standard is exceptionally well-defined and implements many great style choices, I don't blame you at all 😄