test: [move-only] Move lint functions into modules #34098

maflcko commented at 11:21 AM on December 18, 2025: member

The single, large main.rs file is fine, but at some point it becomes harder to read.

So reduce the size by pulling functions out into modules.

This can be reviewed with the git option: --color-moved=dimmed-zebra

DrahtBot added the label Tests on Dec 18, 2025

DrahtBot commented at 11:21 AM on December 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/34098.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	l0rinc, sedited
Concept ACK	rkrux

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

git-subtree-checksh -> git-subtree-check.sh [missing dot before "sh" in reference to the script name]

maflcko commented at 9:37 AM on December 22, 2025: member

IIRC @davidgumberg had a similar patch a year or two ago, but moved each function into a separate file. This patch keeps a grouping into topics. Maybe @davidgumberg can nack or ack this one?

rkrux commented at 2:23 PM on December 23, 2025: contributor

Concept ACK fafdeb8 on an initial glance, will take a deeper look.

The context based code separation seems fine to me, also makes the Rust code in the linter slightly more inviting to me, who has limited experience with it.

DrahtBot added the label Needs rebase on Jan 4, 2026

lint: [move-only] Move util functions to util.rs faf40c2f84

lint: [move-only] Move text related lints to text_format.rs fad09e77db

lint: [move-only] Move docs related lints to lint_docs.rs

Also, rename lint_doc to lint_doc_args.

fa3e48e3fd

lint: [move-only] Move cpp related lints to lint_cpp.rs fab0cfa987

lint: [move-only] Move repo related lints to lint_repo_hygiene.rs

Also, run cargo fmt on main.rs

fa392c31e7

lint: [move-only] Move python related lints to lint_py.rs fa578d9434

maflcko force-pushed on Jan 5, 2026

DrahtBot removed the label Needs rebase on Jan 5, 2026

l0rinc commented at 5:14 PM on January 5, 2026: contributor

Inlined the changes back to see the diff, besides some whitespace, it basically only contained exposing the functions as public and renaming lint_doc to lint_doc_args.

<details> <summary>Details</summary>

diff --git a/test/lint/test_runner/src/main.rs b/test/lint/test_runner/src/main.rs
index 171c98072a..acd36ac2dd 100644
--- a/test/lint/test_runner/src/main.rs
+++ b/test/lint/test_runner/src/main.rs
@@ -2,18 +2,27 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or https://opensource.org/license/mit/.
 
+mod lint_cpp;
+mod lint_docs;
+mod lint_py;
+mod lint_repo_hygiene;
+mod lint_text_format;
+mod util;
+
 use std::env;
-use std::fs::{self, File};
-use std::io::{ErrorKind, Read, Seek, SeekFrom};
-use std::path::PathBuf;
-use std::process::{Command, ExitCode, Stdio};
+use std::fs;
+use std::process::{Command, ExitCode};
 
-/// A possible error returned by any of the linters.
-///
-/// The error string should explain the failure type and list all violations.
-type LintError = String;
-type LintResult = Result<(), LintError>;
-type LintFn = fn() -> LintResult;
+use lint_cpp::{
+    lint_boost_assert, lint_includes_build_config, lint_rpc_assert, lint_std_filesystem,
+};
+use lint_docs::{lint_doc_args, lint_doc_release_note_snippets, lint_markdown};
+use lint_py::lint_py_lint;
+use lint_repo_hygiene::{lint_scripted_diff, lint_subtree};
+use lint_text_format::{
+    lint_commit_msg, lint_tabs_whitespace, lint_trailing_newline, lint_trailing_whitespace,
+};
+use util::{check_output, commit_range, get_git_root, git, LintFn, LintResult};
 
 struct Linter {
     pub description: &'static str,
@@ -26,7 +35,7 @@ fn get_linter_list() -> Vec<&'static Linter> {
         &Linter {
             description: "Check that all command line arguments are documented.",
             name: "doc",
-            lint_fn: lint_doc
+            lint_fn: lint_doc_args
         },
         &Linter {
             description: "Check that no symbol from bitcoin-build-config.h is used without the header being included",
@@ -162,7 +171,7 @@ fn parse_lint_args(args: &[String]) -> Vec<&'static Linter> {
 /// Lint functions should use this command, so that only files tracked by git are considered and
 /// temporary and untracked files are ignored. For example, instead of 'grep', 'git grep' should be
 /// used.
-fn git() -> Command {
+pub fn git() -> Command {
     let mut git = Command::new("git");
     git.arg("--no-pager");
     git
@@ -170,7 +179,7 @@ fn git() -> Command {
 
 /// Return stdout on success and a LintError on failure, when invalid UTF8 was detected or the
 /// command did not succeed.
-fn check_output(cmd: &mut std::process::Command) -> Result<String, LintError> {
+pub fn check_output(cmd: &mut Command) -> Result<String, LintError> {
     let out = cmd.output().expect("command error");
     if !out.status.success() {
         return Err(String::from_utf8_lossy(&out.stderr).to_string());
@@ -184,12 +193,12 @@ fn check_output(cmd: &mut std::process::Command) -> Result<String, LintError> {
 }
 
 /// Return the git root as utf8, or panic
-fn get_git_root() -> PathBuf {
+pub fn get_git_root() -> PathBuf {
     PathBuf::from(check_output(git().args(["rev-parse", "--show-toplevel"])).unwrap())
 }
 
 /// Return the commit range, or panic
-fn commit_range() -> String {
+pub fn commit_range() -> String {
     // Use the env var, if set. E.g. COMMIT_RANGE='HEAD~n..HEAD' for the last 'n' commits.
     env::var("COMMIT_RANGE").unwrap_or_else(|_| {
         // Otherwise, assume that a merge commit exists. This merge commit is assumed
@@ -204,7 +213,7 @@ fn commit_range() -> String {
 }
 
 /// Return all subtree paths
-fn get_subtrees() -> Vec<&'static str> {
+pub fn get_subtrees() -> Vec<&'static str> {
     // Keep in sync with [test/lint/README.md#git-subtree-checksh]
     vec![
         "src/crc32c",
@@ -217,7 +226,7 @@ fn get_subtrees() -> Vec<&'static str> {
 }
 
 /// Return the pathspecs to exclude by default
-fn get_pathspecs_default_excludes() -> Vec<String> {
+pub fn get_pathspecs_default_excludes() -> Vec<String> {
     get_subtrees()
         .iter()
         .chain(&[
@@ -227,7 +236,7 @@ fn get_pathspecs_default_excludes() -> Vec<String> {
         .collect()
 }
 
-fn lint_subtree() -> LintResult {
+pub fn lint_subtree() -> LintResult {
     // This only checks that the trees are pure subtrees, it is not doing a full
     // check with -r to not have to fetch all the remotes.
     let mut good = true;
@@ -245,7 +254,7 @@ fn lint_subtree() -> LintResult {
     }
 }
 
-fn lint_scripted_diff() -> LintResult {
+pub fn lint_scripted_diff() -> LintResult {
     if Command::new("test/lint/commit-script-check.sh")
         .arg(commit_range())
         .status()
@@ -258,7 +267,7 @@ fn lint_scripted_diff() -> LintResult {
     }
 }
 
-fn lint_commit_msg() -> LintResult {
+pub fn lint_commit_msg() -> LintResult {
     let mut good = true;
     let commit_hashes = check_output(git().args([
         "-c",
@@ -293,7 +302,7 @@ fn lint_commit_msg() -> LintResult {
     }
 }
 
-fn lint_py_lint() -> LintResult {
+pub fn lint_py_lint() -> LintResult {
     let bin_name = "ruff";
     let checks = format!(
         "--select={}",
@@ -353,16 +362,14 @@ fn lint_py_lint() -> LintResult {
         Ok(status) if status.success() => Ok(()),
         Ok(_) => Err(format!("`{bin_name}` found errors!")),
         Err(e) if e.kind() == ErrorKind::NotFound => {
-            println!(
-                "`{bin_name}` was not found in $PATH, skipping those checks."
-            );
+            println!("`{bin_name}` was not found in $PATH, skipping those checks.");
             Ok(())
         }
         Err(e) => Err(format!("Error running `{bin_name}`: {e}")),
     }
 }
 
-fn lint_std_filesystem() -> LintResult {
+pub fn lint_std_filesystem() -> LintResult {
     let found = git()
         .args([
             "grep",
@@ -390,7 +397,7 @@ fs:: namespace, which has unsafe filesystem functions marked as deleted.
     }
 }
 
-fn lint_rpc_assert() -> LintResult {
+pub fn lint_rpc_assert() -> LintResult {
     let found = git()
         .args([
             "grep",
@@ -420,7 +427,7 @@ checks should be used over assert. See: src/util/check.h
     }
 }
 
-fn lint_boost_assert() -> LintResult {
+pub fn lint_boost_assert() -> LintResult {
     let found = git()
         .args([
             "grep",
@@ -446,7 +453,7 @@ include of the boost/assert.hpp dependency.
     }
 }
 
-fn lint_doc_release_note_snippets() -> LintResult {
+pub fn lint_doc_release_note_snippets() -> LintResult {
     let non_release_notes = check_output(git().args([
         "ls-files",
         "--",
@@ -497,7 +504,7 @@ fn get_pathspecs_exclude_whitespace() -> Vec<String> {
     list
 }
 
-fn lint_trailing_whitespace() -> LintResult {
+pub fn lint_trailing_whitespace() -> LintResult {
     let trailing_space = git()
         .args(["grep", "-I", "--line-number", "\\s$", "--"])
         .args(get_pathspecs_exclude_whitespace())
@@ -522,7 +529,7 @@ sourced files to the exclude list.
     }
 }
 
-fn lint_trailing_newline() -> LintResult {
+pub fn lint_trailing_newline() -> LintResult {
     let files = check_output(
         git()
             .args([
@@ -560,7 +567,7 @@ Please add any false positives to the exclude list.
     }
 }
 
-fn lint_tabs_whitespace() -> LintResult {
+pub fn lint_tabs_whitespace() -> LintResult {
     let tabs = git()
         .args(["grep", "-I", "--line-number", "--perl-regexp", "^\\t", "--"])
         .args(["*.cpp", "*.h", "*.md", "*.py", "*.sh"])
@@ -584,7 +591,7 @@ Please add any false positives, such as subtrees, or externally sourced files to
     }
 }
 
-fn lint_includes_build_config() -> LintResult {
+pub fn lint_includes_build_config() -> LintResult {
     let config_path = "./cmake/bitcoin-build-config.h.in";
     let defines_regex = format!(
         r"^\s*(?!//).*({})",
@@ -673,7 +680,7 @@ the header. Consider removing the unused include.
     Ok(())
 }
 
-fn lint_doc() -> LintResult {
+pub fn lint_doc_args() -> LintResult {
     if Command::new("test/lint/check-doc.py")
         .status()
         .expect("command error")
@@ -685,7 +692,7 @@ fn lint_doc() -> LintResult {
     }
 }
 
-fn lint_markdown() -> LintResult {
+pub fn lint_markdown() -> LintResult {
     let bin_name = "mlc";
     let mut md_ignore_paths = get_subtrees();
     md_ignore_paths.push("./doc/README_doxygen.md");
@@ -728,6 +735,7 @@ Markdown link errors found:
     }
 }
 
+
 fn run_all_python_linters() -> LintResult {
     let mut good = true;
     let lint_dir = get_git_root().join("test/lint");

</details>

Lightly tested code review ACK fa578d9434fdb090d27c7b5598dcd7f0ff0965cc

DrahtBot requested review from rkrux on Jan 5, 2026

maflcko removed review request from rkrux on Jan 27, 2026

maflcko requested review from rkrux on Jan 27, 2026

maflcko requested review from willcl-ark on Jan 28, 2026

sedited approved

sedited commented at 1:54 PM on January 28, 2026: contributor

ACK fa578d9434fdb090d27c7b5598dcd7f0ff0965cc

sedited merged this on Jan 28, 2026

sedited closed this on Jan 28, 2026

maflcko deleted the branch on Jan 28, 2026