clang-format: Set InsertNewlineAtEOF: true #33896

maflcko commented at 7:49 AM on November 18, 2025: member

Now that the minimum supported clang version is 17, the InsertNewlineAtEOF setting can be set to true in the clang-format file. (https://releases.llvm.org/16.0.0/tools/clang/docs/ClangFormatStyleOptions.html#insertnewlineateof)

This is in line with the already existing newline linter. Can be tested via:

truncate --size=-1 src/init.cpp
git diff

# Should fail:
cargo run --manifest-path ./test/lint/test_runner/Cargo.toml -- --lint=trailing_newline

# Restore newline:
git diff -U0 | ./contrib/devtools/clang-format-diff.py -p1 -i -v

clang-format: Set InsertNewlineAtEOF: true fa1bf6818f

DrahtBot commented at 7:49 AM on November 18, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33896.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hodlinator, stickies-v, hebasto, l0rinc, janb84, achow101

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

maflcko added the label Refactoring on Nov 18, 2025

hodlinator approved

hodlinator commented at 9:48 AM on November 18, 2025: contributor

ACK fa1bf6818f0910f997e5235a197ff51f2e18780d

Makes sense for this project to conform to the UNIXy style of ending all lines with \n.

Previous setting of having it false comes from 13f36c020f0329b5e975282b45292fdf2a495e31 which just added a bunch of newer settings to src/.clang-format with default clang-format values, rather than being a conscious decision by this project.

Have recently been discussing how GitHub and Git also seem to agree on this: #28792 (review)

in src/.clang-format:126 in fa1bf6818f

 122 | @@ -123,7 +123,7 @@ IndentRequiresClause: true
 123 |  IndentWidth:     4
 124 |  IndentWrappedFunctionNames: false
 125 |  InsertBraces:    false
 126 | -InsertNewlineAtEOF: false
 127 | +InsertNewlineAtEOF: true

l0rinc commented at 10:01 AM on November 18, 2025:

llvm@17 has a KeepEmptyLinesAtEOF: false as well - added exactly to avoid the problem we're trying to avoid here: https://github.com/llvm/llvm-project/issues/56054 Should we make it explicit here?

diff --git a/src/.clang-format b/src/.clang-format
--- a/src/.clang-format	(revision 9d5bad11b6cbf50c776f9e9a583ac1fb79271fe8)
+++ b/src/.clang-format	(date 1763460129408)
@@ -123,7 +123,7 @@
 IndentWidth:     4
 IndentWrappedFunctionNames: false
 InsertBraces:    false
-InsertNewlineAtEOF: false
+InsertNewlineAtEOF: true
 InsertTrailingCommas: None
 IntegerLiteralSeparator:
   Binary:          0
@@ -135,6 +135,7 @@
 JavaScriptQuotes: Leave
 JavaScriptWrapImports: true
 KeepEmptyLinesAtTheStartOfBlocks: false
+KeepEmptyLinesAtEOF: true
 LambdaBodyIndentation: Signature
 LineEnding:      DeriveLF
 MacroBlockBegin: ''

Note that these were replaced again in llvm@19: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#keepemptylines

maflcko commented at 10:24 AM on November 18, 2025:

llvm@17 has a KeepEmptyLinesAtEOF: false as well - added exactly to avoid the problem we're trying to avoid here: llvm/llvm-project#56054 Should we make it explicit here?

Not sure I understand the problem you are referring to. InsertNewlineAtEOF is already set to true in this pull request. Can you add exact steps to reproduce the problem?

l0rinc commented at 10:31 AM on November 18, 2025:

InsertNewlineAtEOF is already set to true in this pull

Yes, please scroll to the bottom of the patch above, I'm also setting KeepEmptyLinesAtEOF there.

My understanding from https://github.com/llvm/llvm-project/issues/63150 is that InsertNewlineAtEOF alone doesn't work in some versions

The clang format option InsertNewlineAtEOF : true is not being applied, instead the newline is being removed

, so they have added KeepEmptyLinesAtEOF in 17 https://reviews.llvm.org/D152305:

Adds an option KeepEmptyLinesAtEOF to keep empty lines (up to MaxEmptyLinesToKeep) before EOF. This remedies the probably unintentional change in behavior introduced in rG3d3ea8, which started to always remove empty lines before EOF.

As mentioned, it's a bit hacky, since they have deprecated KeepEmptyLinesAtEOF in 19 - but we have some time until we upgrade to 19, we should probably temporarily set it

hodlinator commented at 10:38 AM on November 18, 2025:

As I understand it some people wanted to keep additional empty lines at the end of the file (\n\n). https://github.com/llvm/llvm-project/issues/63150#issuecomment-1579150950

I don't think this is a concern for us?

l0rinc commented at 10:43 AM on November 18, 2025:

The issue claims:

For C and C++ an empty line at the end of each file (header and implementation) is mandatory.

Hmmm, so maybe the problem was that multiple empty lines were changes to none? I can try it out a bit later, but I'm working on something else locally...

maflcko commented at 11:16 AM on November 18, 2025:

I don't think this is a concern for us?

Agree, I don't see the issue for us, but happy to test, if there are steps to test.

Hmmm, so maybe the problem was that multiple empty lines were changes to none?

I can't reproduce this locally:

echo '' >> src/init.cpp
echo '' >> src/init.cpp
git diff

# Restore newline:
git diff -U0 | ./contrib/devtools/clang-format-diff.py -p1 -i -v

I'll leave this as-is for now, but I am happy to review a separate pull changing the value of a separate setting, if there is need to.

l0rinc commented at 12:40 PM on November 18, 2025:

Thanks for checking

stickies-v approved

stickies-v commented at 11:34 AM on November 18, 2025: contributor

ACK fa1bf6818f0910f997e5235a197ff51f2e18780d

hebasto approved

hebasto commented at 11:40 AM on November 18, 2025: member

ACK fa1bf6818f0910f997e5235a197ff51f2e18780d.

l0rinc commented at 12:41 PM on November 18, 2025: contributor

ACK fa1bf6818f0910f997e5235a197ff51f2e18780d

janb84 commented at 12:47 PM on November 18, 2025: contributor

ACK fa1bf6818f0910f997e5235a197ff51f2e18780d

PR unifies the enforcement of the new line at end of file to also include the rule in the clang-format rules (is already in the linter and in the .editorconfig)

Tested and works as advertised + learned of the existence a new script in contrib :)

achow101 commented at 12:17 AM on November 20, 2025: member

ACK fa1bf6818f0910f997e5235a197ff51f2e18780d

achow101 merged this on Nov 20, 2025

achow101 closed this on Nov 20, 2025

maflcko deleted the branch on Nov 20, 2025