dburkart / check-sieve

Syntax checker for mail sieves.
MIT License
34 stars 7 forks source link

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

Open johnlettman opened 10 months ago

johnlettman commented 10 months ago

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:

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:

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: https://github.com/dburkart/check-sieve/blob/40e51c69a309beca893cd0a9a17db36cee71d883/src/AST/Validation/Command.cc#L76

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

This leaves the following style guides:

Return Type Placement

Next, the return type of a function is on the same line as the function declaration: https://github.com/dburkart/check-sieve/blob/40e51c69a309beca893cd0a9a17db36cee71d883/src/AST/Validation/Command.cc#L122

The following style guides also implement this preference:

Summary

The following style guides appear to be the most compatible:

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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