diff --git a/test/lint/lint-whitespace.py b/test/lint/lint-whitespace.py deleted file mode 100755 index f5e4a776d0ab5..0000000000000 --- 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 b97e822484bf7..e22e047e4b19c 100644 --- a/test/lint/test_runner/src/main.rs +++ b/test/lint/test_runner/src/main.rs @@ -14,7 +14,9 @@ type LintFn = fn() -> LintResult; /// Return the git command fn git() -> Command { - Command::new("git") + let mut git = Command::new("git"); + git.arg("--no-pager"); + git } /// Return stdout @@ -95,6 +97,85 @@ fs:: namespace, which has unsafe filesystem functions marked as deleted. } } +/// Return the pathspecs for whitespace related excludes +fn get_pathspecs_exclude_whitespace() -> Vec { + 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 "; @@ -232,6 +313,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),