aboutsummaryrefslogtreecommitdiff
path: root/test/lint
diff options
context:
space:
mode:
authorMarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>2024-02-27 13:03:48 +0100
committerMarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>2024-03-12 13:38:39 +0100
commitfa5729436ca12b20cfa2cd1f0c6f54af7192f0a6 (patch)
treeb105c2a273657190a30d0b62c785c9984697c83e /test/lint
parentd14c7286b653ad09a5a196aa351b325f97db546b (diff)
lint: Fix lint-whitespace issues
Diffstat (limited to 'test/lint')
-rwxr-xr-xtest/lint/lint-whitespace.py136
-rw-r--r--test/lint/test_runner/src/main.rs81
2 files changed, 81 insertions, 136 deletions
diff --git a/test/lint/lint-whitespace.py b/test/lint/lint-whitespace.py
deleted file mode 100755
index f5e4a776d0..0000000000
--- a/test/lint/lint-whitespace.py
+++ /dev/null
@@ -1,136 +0,0 @@
-#!/usr/bin/env python3
-#
-# Copyright (c) 2017-2022 The Bitcoin Core developers
-# Distributed under the MIT software license, see the accompanying
-# file COPYING or http://www.opensource.org/licenses/mit-license.php.
-#
-# Check for new lines in diff that introduce trailing whitespace or
-# tab characters instead of spaces.
-
-# We can't run this check unless we know the commit range for the PR.
-
-import argparse
-import os
-import re
-import sys
-
-from subprocess import check_output
-
-EXCLUDED_DIRS = ["depends/patches/",
- "contrib/guix/patches/",
- "src/leveldb/",
- "src/crc32c/",
- "src/secp256k1/",
- "src/minisketch/",
- "doc/release-notes/",
- "src/qt/locale"]
-
-def parse_args():
- """Parse command line arguments."""
- parser = argparse.ArgumentParser(
- description="""
- Check for new lines in diff that introduce trailing whitespace
- or tab characters instead of spaces in unstaged changes, the
- previous n commits, or a commit-range.
- """,
- epilog=f"""
- You can manually set the commit-range with the COMMIT_RANGE
- environment variable (e.g. "COMMIT_RANGE='47ba2c3...ee50c9e'
- {sys.argv[0]}"). Defaults to current merge base when neither
- prev-commits nor the environment variable is set.
- """)
-
- parser.add_argument("--prev-commits", "-p", required=False, help="The previous n commits to check")
-
- return parser.parse_args()
-
-
-def report_diff(selection):
- filename = ""
- seen = False
- seenln = False
-
- print("The following changes were suspected:")
-
- for line in selection:
- if re.match(r"^diff", line):
- filename = line
- seen = False
- elif re.match(r"^@@", line):
- linenumber = line
- seenln = False
- else:
- if not seen:
- # The first time a file is seen with trailing whitespace or a tab character, we print the
- # filename (preceded by a newline).
- print("")
- print(filename)
- seen = True
- if not seenln:
- print(linenumber)
- seenln = True
- print(line)
-
-
-def get_diff(commit_range, check_only_code):
- exclude_args = [":(exclude)" + dir for dir in EXCLUDED_DIRS]
-
- if check_only_code:
- what_files = ["*.cpp", "*.h", "*.md", "*.py", "*.sh"]
- else:
- what_files = ["."]
-
- diff = check_output(["git", "diff", "-U0", commit_range, "--"] + what_files + exclude_args, text=True, encoding="utf8")
-
- return diff
-
-
-def main():
- args = parse_args()
-
- if not os.getenv("COMMIT_RANGE"):
- if args.prev_commits:
- commit_range = "HEAD~" + args.prev_commits + "...HEAD"
- else:
- # This assumes that the target branch of the pull request will be master.
- merge_base = check_output(["git", "merge-base", "HEAD", "master"], text=True, encoding="utf8").rstrip("\n")
- commit_range = merge_base + "..HEAD"
- else:
- commit_range = os.getenv("COMMIT_RANGE")
- if commit_range == "SKIP_EMPTY_NOT_A_PR":
- sys.exit(0)
-
- whitespace_selection = []
- tab_selection = []
-
- # Check if trailing whitespace was found in the diff.
- for line in get_diff(commit_range, check_only_code=False).splitlines():
- if re.match(r"^(diff --git|\@@|^\+.*\s+$)", line):
- whitespace_selection.append(line)
-
- whitespace_additions = [i for i in whitespace_selection if i.startswith("+")]
-
- # Check if tab characters were found in the diff.
- for line in get_diff(commit_range, check_only_code=True).splitlines():
- if re.match(r"^(diff --git|\@@|^\+.*\t)", line):
- tab_selection.append(line)
-
- tab_additions = [i for i in tab_selection if i.startswith("+")]
-
- ret = 0
-
- if len(whitespace_additions) > 0:
- print("This diff appears to have added new lines with trailing whitespace.")
- report_diff(whitespace_selection)
- ret = 1
-
- if len(tab_additions) > 0:
- print("This diff appears to have added new lines with tab characters instead of spaces.")
- report_diff(tab_selection)
- ret = 1
-
- sys.exit(ret)
-
-
-if __name__ == "__main__":
- main()
diff --git a/test/lint/test_runner/src/main.rs b/test/lint/test_runner/src/main.rs
index b97e822484..f9df576239 100644
--- a/test/lint/test_runner/src/main.rs
+++ b/test/lint/test_runner/src/main.rs
@@ -95,6 +95,85 @@ fs:: namespace, which has unsafe filesystem functions marked as deleted.
}
}
+/// Return the pathspecs for whitespace related excludes
+fn get_pathspecs_exclude_whitespace() -> Vec<String> {
+ let mut list = get_pathspecs_exclude_subtrees();
+ list.extend(
+ [
+ // Permanent excludes
+ "*.patch",
+ "src/qt/locale",
+ "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",
+ "src/qt/res/src/*.svg",
+ "test/functional/test_framework/crypto/ellswift_decode_test_vectors.csv",
+ "test/functional/test_framework/crypto/xswiftec_inv_test_vectors.csv",
+ "contrib/qos/tc.sh",
+ "contrib/verify-commits/gpg.sh",
+ "src/univalue/include/univalue_escapes.h",
+ "src/univalue/test/object.cpp",
+ "test/lint/git-subtree-check.sh",
+ ]
+ .iter()
+ .map(|s| format!(":(exclude){}", s)),
+ );
+ list
+}
+
+fn lint_trailing_whitespace() -> LintResult {
+ let trailing_space = git()
+ .args(["grep", "-I", "--line-number", "\\s$", "--"])
+ .args(get_pathspecs_exclude_whitespace())
+ .status()
+ .expect("command error")
+ .success();
+ if trailing_space {
+ Err(r#"
+^^^
+Trailing whitespace is problematic, because git may warn about it, or editors may remove it by
+default, forcing developers in the future to either undo the changes manually or spend time on
+review.
+
+Thus, it is best to remove the trailing space now.
+
+Please add any false positives, such as subtrees, Windows-related files, patch files, or externally
+sourced files to the exclude list.
+ "#
+ .to_string())
+ } else {
+ Ok(())
+ }
+}
+
+fn lint_tabs_whitespace() -> LintResult {
+ let tabs = git()
+ .args(["grep", "-I", "--line-number", "--perl-regexp", "^\\t", "--"])
+ .args(["*.cpp", "*.h", "*.md", "*.py", "*.sh"])
+ .args(get_pathspecs_exclude_whitespace())
+ .status()
+ .expect("command error")
+ .success();
+ if tabs {
+ Err(r#"
+^^^
+Use of tabs in this codebase is problematic, because existing code uses spaces and tabs will cause
+display issues and conflict with editor settings.
+
+Please remove the tabs.
+
+Please add any false positives, such as subtrees, or externally sourced files to the exclude list.
+ "#
+ .to_string())
+ } else {
+ Ok(())
+ }
+}
+
fn lint_includes_build_config() -> LintResult {
let config_path = "./src/config/bitcoin-config.h.in";
let include_directive = "#include <config/bitcoin-config.h>";
@@ -232,6 +311,8 @@ fn main() -> ExitCode {
let test_list: Vec<(&str, LintFn)> = vec![
("subtree check", lint_subtree),
("std::filesystem check", lint_std_filesystem),
+ ("trailing whitespace check", lint_trailing_whitespace),
+ ("no-tabs check", lint_tabs_whitespace),
("build config includes check", lint_includes_build_config),
("-help=1 documentation check", lint_doc),
("lint-*.py scripts", lint_all),