Merge bitcoin/bitcoin#32477: lint: Check for missing trailing newline

fa9198af55 lint: Check for missing trailing newline (MarcoFalke)
fa2b2aa27c lint: Add archived notes to default excludes (MarcoFalke)

Pull request description:

  A missing trailing newline is harmless, but a bit problematic:

  * `git` shows a warning by default
  * After another line is appended, the diff will be verbose and `git blame` will be wrong for the "untouched" line.

  Fix the problems by just requiring what is already the default, see also https://github.com/bitcoin/bitcoin/blob/663a9cabf811e2fc53102bc6da00d09fc99d1d81/.editorconfig#L9 and https://github.com/bitcoin/bitcoin/blob/663a9cabf811e2fc53102bc6da00d09fc99d1d81/test/lint/test_runner/src/main.rs#L327

ACKs for top commit:
  l0rinc:
    utACK fa9198af55
  fanquake:
    ACK fa9198af55

Tree-SHA512: d144eebdeee68fc3404aa4a66ecd5c130f907ed4b869bd300f6e9ed74d125561d1f4cdd6dd20d9e969471a7d007399f928f072d1c1f626275ca31f32bc23fdbc

INSTALL.md

@@ -1 +1 @@
-See [doc/build-\*.md](/doc)
+See [doc/build-\*.md](/doc)

doc/release-notes-31278.md

@@ -1,3 +1,3 @@
 RPC and Startup Option
 ---
-The `-paytxfee` startup option and the `settxfee` RPC are now deprecated and will be removed in Bitcoin Core 31.0. They allowed the user to set a static fee rate for wallet transactions, which could potentially lead to overpaying or underpaying. Users should instead rely on fee estimation or specify a fee rate per transaction using the `fee_rate` argument in RPCs such as `fundrawtransaction`, `sendtoaddress`, `send`, `sendall`, and `sendmany`. (#31278)
+The `-paytxfee` startup option and the `settxfee` RPC are now deprecated and will be removed in Bitcoin Core 31.0. They allowed the user to set a static fee rate for wallet transactions, which could potentially lead to overpaying or underpaying. Users should instead rely on fee estimation or specify a fee rate per transaction using the `fee_rate` argument in RPCs such as `fundrawtransaction`, `sendtoaddress`, `send`, `sendall`, and `sendmany`. (#31278)

src/test/fuzz/bech32.cpp

@@ -1,4 +1,4 @@
-// Copyright (c) 2019-2021 The Bitcoin Core developers
+// Copyright (c) 2019-present The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -66,4 +66,4 @@ FUZZ_TARGET(bech32_roundtrip)
             assert(decoded.data == converted_input);
         }
     }
-}
+}

test/lint/test_runner/src/main.rs

@@ -3,8 +3,8 @@
 // file COPYING or https://opensource.org/license/mit/.
 
 use std::env;
-use std::fs;
+use std::fs::{self, File};
-use std::io::ErrorKind;
+use std::io::{ErrorKind, Read, Seek, SeekFrom};
 use std::path::PathBuf;
 use std::process::{Command, ExitCode, Stdio};
 
@@ -88,6 +88,11 @@ fn get_linter_list() -> Vec<&'static Linter> {
             name: "trailing_whitespace",
             lint_fn: lint_trailing_whitespace
         },
+        &Linter {
+            description: "Check for trailing newline",
+            name: "trailing_newline",
+            lint_fn: lint_trailing_newline
+        },
         &Linter {
             description: "Run all linters of the form: test/lint/lint-*.py",
             name: "all_python_linters",
@@ -209,10 +214,13 @@ fn get_subtrees() -> Vec<&'static str> {
     ]
 }
 
-/// Return the pathspecs to exclude all subtrees
+/// Return the pathspecs to exclude by default
-fn get_pathspecs_exclude_subtrees() -> Vec<String> {
+fn get_pathspecs_default_excludes() -> Vec<String> {
     get_subtrees()
         .iter()
+        .chain(&[
+            "doc/release-notes/release-notes-*", // archived notes
+        ])
         .map(|s| format!(":(exclude){}", s))
         .collect()
 }
@@ -333,7 +341,7 @@ fn lint_py_lint() -> LintResult {
     let files = check_output(
         git()
             .args(["ls-files", "--", "*.py"])
-            .args(get_pathspecs_exclude_subtrees()),
+            .args(get_pathspecs_default_excludes()),
     )?;
 
     let mut cmd = Command::new(bin_name);
@@ -459,7 +467,7 @@ expected to follow the naming "/doc/release-notes-<PR number>.md".
 
 /// Return the pathspecs for whitespace related excludes
 fn get_pathspecs_exclude_whitespace() -> Vec<String> {
-    let mut list = get_pathspecs_exclude_subtrees();
+    let mut list = get_pathspecs_default_excludes();
     list.extend(
         [
             // Permanent excludes
@@ -468,7 +476,6 @@ fn get_pathspecs_exclude_whitespace() -> Vec<String> {
             "contrib/windeploy/win-codesign.cert",
             "doc/README_windows.txt",
             // Temporary excludes, or existing violations
-            "doc/release-notes/release-notes-0.*",
             "contrib/init/bitcoind.openrc",
             "contrib/macdeploy/macdeployqtplus",
             "src/crypto/sha256_sse4.cpp",
@@ -512,6 +519,44 @@ sourced files to the exclude list.
     }
 }
 
+fn lint_trailing_newline() -> LintResult {
+    let files = check_output(
+        git()
+            .args([
+                "ls-files", "--", "*.py", "*.cpp", "*.h", "*.md", "*.rs", "*.sh", "*.cmake",
+            ])
+            .args(get_pathspecs_default_excludes()),
+    )?;
+    let mut missing_newline = false;
+    for path in files.lines() {
+        let mut file = File::open(path).expect("must be able to open file");
+        if file.seek(SeekFrom::End(-1)).is_err() {
+            continue; // Allow fully empty files
+        }
+        let mut buffer = [0u8; 1];
+        file.read_exact(&mut buffer)
+            .expect("must be able to read the last byte");
+        if buffer[0] != b'\n' {
+            missing_newline = true;
+            println!("{path}");
+        }
+    }
+    if missing_newline {
+        Err(r#"
+A trailing newline is required, because git may warn about it missing. Also, it can make diffs
+verbose and can break git blame after appending lines.
+
+Thus, it is best to add the trailing newline now.
+
+Please add any false positives to the exclude list.
+            "#
+        .trim()
+        .to_string())
+    } else {
+        Ok(())
+    }
+}
+
 fn lint_tabs_whitespace() -> LintResult {
     let tabs = git()
         .args(["grep", "-I", "--line-number", "--perl-regexp", "^\\t", "--"])
@@ -569,7 +614,7 @@ fn lint_includes_build_config() -> LintResult {
                     "*.cpp",
                     "*.h",
                 ])
-                .args(get_pathspecs_exclude_subtrees())
+                .args(get_pathspecs_default_excludes())
                 .args([
                     // These are exceptions which don't use bitcoin-build-config.h, rather CMakeLists.txt adds
                     // these cppflags manually.