Add .clang-format and reformat the codebase

[MakisH]

This pull request:

• adds a configuration file for clang-format 11, with some commented-out options for clang-format 12. Since the OpenFOAM style is quite unique, I wanted to have as many options available as possible. Closes Support formatting #126.
• reformats the codebase with the included .clang-format.
• adds the GitHub action clang-format-check.

@davidscn could you please have a look into the formatted code and write if you feel comfortable with this style? If you have any ideas on the remaining issues, they would be very welcome!

For anyone from OpenFOAM reading this: we know we have some technical debt here, please don't look too closely into the code. 🙈 We are in the process of fixing many of the open issues, but more feedback is always welcome! 😄

While these steps are rather straight-forward, the complicated part is to configure clang-format so that it applies the OpenFOAM style guide as closely as possible. The result is not perfect, but I think that it is good enough for us and that it reaches the current limits of clang-format style options (here v11, upcoming v13 release notes).

A very large part of the code stays the same (good!). The following are a few unresolved issues I identified from formatting this arbitrary sample OpenFOAM file:

Issue 1: Round braces in dedicated lines

Cannot tell clang-format that parentheses should open and close in dedicated lines. If you know any way, please answer my StackOverflow question 1.

- contents_.append
- (
-     autoPtr<DynamicList<label>>
-     (
-         new DynamicList<label>()
-     )
- );
---
+ contents_.append(
+     autoPtr<DynamicList<label>>(
+         new DynamicList<label>()));

Upside: IDEs understand that e.g. append is a function and colorize it properly.

Issue 2: Spaces around arithmetic operators.

Cannot tell clang-format that it should not add spaces around arithmetic operators. See also my StackOverflow question 2. Both spaces and no-spaces are actually allowed by the guidelines (and I prefer spaces).

- const point mid(0.5*(min+max));
---
+ const point mid(0.5 * (min + max));

Upside: Consistent formatting (single rule instead of either), more easily identifiable operands.

Issue 3: Aligning operations

Blocks with multiple operations are still aligned, but not exactly in the same way:

-    if
-    (
-        bb.min()[0] >= bb.max()[0]
-     || bb.min()[1] >= bb.max()[1]
-     || bb.min()[2] >= bb.max()[2]
-    )
---
+    if (
+        bb.min()[0] >= bb.max()[0]
+        || bb.min()[1] >= bb.max()[1]
+        || bb.min()[2] >= bb.max()[2])

Maybe there is a way to fix the alignment, but I could not succeed. The opening/closing braces in the same lines is again issue 1.

Issue 4: Space in streams

OpenFOAM always starts << in the fourth column after the start of the indentation level, often leaving no space between the target stream and the operator.

- Pout<< "    iter:" << i
-     << " hit face:" << faceString(hitFaceID)
---
+ Pout << "    iter:" << i
+      << " hit face:" << faceString(hitFaceID)

Upside: The added space could make this more readable. It also looks less like fortran fixed format.

Issue 5: Columns limit

I had to set ColumnLimit: 0. If I set it to 80, clang-format was formatting things that it shouldn't. I tried tuning the penalties, but I could not succeed. This leads to situations such as the following:

- nodes_[parentNodeIndex].subNodes_[octantToBeDivided]
-    = nodePlusOctant(sz, octantToBeDivided);
---
+ nodes_[parentNodeIndex].subNodes_[octantToBeDivided] = nodePlusOctant(sz, octantToBeDivided);

Issue 6: Indentation in initializer list

Similarly to issue 1, no dedicated line for ::

- :
-    shapes_(shapes),
-    bb_(bb), 
---
+ : shapes_(shapes),
+   bb_(bb),

At least the : remains not indented.

See also

Related upstream issue: https://develop.openfoam.com/Development/openfoam/-/issues/1634.

In the .clang-format itself, I include comments on how this relates to the style guide. I can provide more details in the upstream issue.

I started from the default LLVM style and changed each setting manually (they are not so many). The .clang-format of mdolab/dafoam helped me guess how to fix some issues (especially the ColumnLimit: 0, I would never guess that).

[davidscn]

Very nice. I think the result is even better than the original and the listed issues are not really issues. I never really understood the overwhelming usage of braces on separate lines and I also like the spaces between operators. I understand your concerns regarding the column limit. However, note that (not 100% sure) with no column limit, both listed code styles are legal.

[olesenm]

Some good ground work there.

issue 1 (round braces):

• slight show-stopper there.
  issue 2 (spaces around arithmetic)
• I could live with either one
  issue 3
• outdenting would be really good, but at least the line splits seems to be working.
  issue 4 (space before stream operator):
• this will affect large chunks of code. Subsequent lines look pretty weird - they lineup with nothing.
  Issue 5 (column limits):
• this may not be a large issue, but does risk adding noise into commits as the statement length changes.
  Issues 6 (class initializer):
• looks ugly

I think that stackoverflow may not be enough. Probably have to try working directly with an llvm group (mailing-list) to see which modifications can be made in clang-format itself.

[MakisH]

Thank you very much for your feedback, @olesenm!

The situation is actually a bit better, GitHub had rendered the Markdown snippets a bit strange in issues 4 and 6, I fixed it and checked with the produced code.

issue 1 (round braces):

* slight show-stopper there.

I agree that this is the biggest issue and I think we can only work together with the LLVM project on this. Do you have any other, non-OpenFOAM-derived project in mind that uses this style? This could add some leverage.

  issue 4 (space before stream operator):

* this will affect large chunks of code. Subsequent lines look pretty weird - they lineup with nothing.

This was a GitHub rendering issue: in the code, all << align, they just all move one space after the stream name. For some streams, this may end up being exactly the same (by chance).

  Issue 5 (column limits):

* this may not be a large issue, but does risk adding noise into commits as the statement length changes.

I could also try a larger column limit, so that it (hopefully) affects less code. But this issue would definitely have an effect.

  Issues 6 (class initializer):

* looks ugly

Same GitHub rendering issue, I fixed it. All members align under each other, indented by two spaces (because of : ).

I think that stackoverflow may not be enough. Probably have to try working directly with an llvm group (mailing-list) to see which modifications can be made in clang-format itself.

I completely agree that this is the way to go. In the Additional style options they ask for:

Each new style option must ..

• be used in a project of significant size (have dozens of contributors)
• have a publicly accessible style guide
• have a person willing to contribute and maintain patches

I think that the first point is relatively easy, considering how many projects build upon OpenFOAM and the second point is covered.

Regarding the last point, I have no clue what it requires. Edit: I started a first step by posting in the clang-format Discord channel.

[olesenm]

• be used in a project of significant size (have dozens of contributors)
• have a publicly accessible style guide
• have a person willing to contribute and maintain patches

I think that the first point is relatively easy, considering how many projects build upon OpenFOAM and the second point is covered.

Regarding the last point, I have no clue what it requires. Edit: I started a first step by posting in the clang-format Discord channel.

I can imagine two main items

• line-break before brackets
• operator outdent
  would get the formatting most of the way.
  If you can figure out where these hooks should go, I would think that there wouldn't be much (any) future patching effort at the initial changeset.

Not sure if it helps, but here are some OpenFOAM indentation rules that might help for orientation (the kitware ones are horribly out of date).

Extracted:

(C_Switch_Offset, C_Param_Offset_Max) = (4, -1);

(C_INDENT, C_BRACE, C_BRA_NEWLINE, C_CONTINUED_OFFSET,
    C_Colon_Offset, C_Class_Offset, C_Namespace_Offset,
     C_Macro_Indent, C_Label_Offset, C_Label_Indents_Relative,
     C_Outer_Block_Offset
 ) = (4,0,0,4,0,4,0,4,0,0,0);

Does mostly ok, except that lines ending with 'function()' tend to mess up the indentation (after the first line) and comma lists (eg, a large enumeration) don't really indent properly at all. Having said that, these settings have served me quite well.

[MakisH]

Thank you once more for the feedback, we will continue the discussion in the OpenFOAM issue or in a new PR here. Automatic formatting is quite important for us at this point, so I will go ahead and merge this PR. We will be happy to reformat again if we have any .clang-format improvements.