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

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 <!–meta-tag:bot-skip–> 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]

2026-01-05

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.

  0diff --git a/test/lint/test_runner/src/main.rs b/test/lint/test_runner/src/main.rs
  1index 171c98072a..acd36ac2dd 100644
  2--- a/test/lint/test_runner/src/main.rs
  3+++ b/test/lint/test_runner/src/main.rs
  4@@ -2,18 +2,27 @@
  5 // Distributed under the MIT software license, see the accompanying
  6 // file COPYING or https://opensource.org/license/mit/.
  7 
  8+mod lint_cpp;
  9+mod lint_docs;
 10+mod lint_py;
 11+mod lint_repo_hygiene;
 12+mod lint_text_format;
 13+mod util;
 14+
 15 use std::env;
 16-use std::fs::{self, File};
 17-use std::io::{ErrorKind, Read, Seek, SeekFrom};
 18-use std::path::PathBuf;
 19-use std::process::{Command, ExitCode, Stdio};
 20+use std::fs;
 21+use std::process::{Command, ExitCode};
 22 
 23-/// A possible error returned by any of the linters.
 24-///
 25-/// The error string should explain the failure type and list all violations.
 26-type LintError = String;
 27-type LintResult = Result<(), LintError>;
 28-type LintFn = fn() -> LintResult;
 29+use lint_cpp::{
 30+    lint_boost_assert, lint_includes_build_config, lint_rpc_assert, lint_std_filesystem,
 31+};
 32+use lint_docs::{lint_doc_args, lint_doc_release_note_snippets, lint_markdown};
 33+use lint_py::lint_py_lint;
 34+use lint_repo_hygiene::{lint_scripted_diff, lint_subtree};
 35+use lint_text_format::{
 36+    lint_commit_msg, lint_tabs_whitespace, lint_trailing_newline, lint_trailing_whitespace,
 37+};
 38+use util::{check_output, commit_range, get_git_root, git, LintFn, LintResult};
 39 
 40 struct Linter {
 41     pub description: &'static str,
 42@@ -26,7 +35,7 @@ fn get_linter_list() -> Vec<&'static Linter> {
 43         &Linter {
 44             description: "Check that all command line arguments are documented.",
 45             name: "doc",
 46-            lint_fn: lint_doc
 47+            lint_fn: lint_doc_args
 48         },
 49         &Linter {
 50             description: "Check that no symbol from bitcoin-build-config.h is used without the header being included",
 51@@ -162,7 +171,7 @@ fn parse_lint_args(args: &[String]) -> Vec<&'static Linter> {
 52 /// Lint functions should use this command, so that only files tracked by git are considered and
 53 /// temporary and untracked files are ignored. For example, instead of 'grep', 'git grep' should be
 54 /// used.
 55-fn git() -> Command {
 56+pub fn git() -> Command {
 57     let mut git = Command::new("git");
 58     git.arg("--no-pager");
 59     git
 60@@ -170,7 +179,7 @@ fn git() -> Command {
 61 
 62 /// Return stdout on success and a LintError on failure, when invalid UTF8 was detected or the
 63 /// command did not succeed.
 64-fn check_output(cmd: &mut std::process::Command) -> Result<String, LintError> {
 65+pub fn check_output(cmd: &mut Command) -> Result<String, LintError> {
 66     let out = cmd.output().expect("command error");
 67     if !out.status.success() {
 68         return Err(String::from_utf8_lossy(&out.stderr).to_string());
 69@@ -184,12 +193,12 @@ fn check_output(cmd: &mut std::process::Command) -> Result<String, LintError> {
 70 }
 71 
 72 /// Return the git root as utf8, or panic
 73-fn get_git_root() -> PathBuf {
 74+pub fn get_git_root() -> PathBuf {
 75     PathBuf::from(check_output(git().args(["rev-parse", "--show-toplevel"])).unwrap())
 76 }
 77 
 78 /// Return the commit range, or panic
 79-fn commit_range() -> String {
 80+pub fn commit_range() -> String {
 81     // Use the env var, if set. E.g. COMMIT_RANGE='HEAD~n..HEAD' for the last 'n' commits.
 82     env::var("COMMIT_RANGE").unwrap_or_else(|_| {
 83         // Otherwise, assume that a merge commit exists. This merge commit is assumed
 84@@ -204,7 +213,7 @@ fn commit_range() -> String {
 85 }
 86 
 87 /// Return all subtree paths
 88-fn get_subtrees() -> Vec<&'static str> {
 89+pub fn get_subtrees() -> Vec<&'static str> {
 90     // Keep in sync with [test/lint/README.md#git-subtree-checksh]
 91     vec![
 92         "src/crc32c",
 93@@ -217,7 +226,7 @@ fn get_subtrees() -> Vec<&'static str> {
 94 }
 95 
 96 /// Return the pathspecs to exclude by default
 97-fn get_pathspecs_default_excludes() -> Vec<String> {
 98+pub fn get_pathspecs_default_excludes() -> Vec<String> {
 99     get_subtrees()
100         .iter()
101         .chain(&[
102@@ -227,7 +236,7 @@ fn get_pathspecs_default_excludes() -> Vec<String> {
103         .collect()
104 }
105 
106-fn lint_subtree() -> LintResult {
107+pub fn lint_subtree() -> LintResult {
108     // This only checks that the trees are pure subtrees, it is not doing a full
109     // check with -r to not have to fetch all the remotes.
110     let mut good = true;
111@@ -245,7 +254,7 @@ fn lint_subtree() -> LintResult {
112     }
113 }
114 
115-fn lint_scripted_diff() -> LintResult {
116+pub fn lint_scripted_diff() -> LintResult {
117     if Command::new("test/lint/commit-script-check.sh")
118         .arg(commit_range())
119         .status()
120@@ -258,7 +267,7 @@ fn lint_scripted_diff() -> LintResult {
121     }
122 }
123 
124-fn lint_commit_msg() -> LintResult {
125+pub fn lint_commit_msg() -> LintResult {
126     let mut good = true;
127     let commit_hashes = check_output(git().args([
128         "-c",
129@@ -293,7 +302,7 @@ fn lint_commit_msg() -> LintResult {
130     }
131 }
132 
133-fn lint_py_lint() -> LintResult {
134+pub fn lint_py_lint() -> LintResult {
135     let bin_name = "ruff";
136     let checks = format!(
137         "--select={}",
138@@ -353,16 +362,14 @@ fn lint_py_lint() -> LintResult {
139         Ok(status) if status.success() => Ok(()),
140         Ok(_) => Err(format!("`{bin_name}` found errors!")),
141         Err(e) if e.kind() == ErrorKind::NotFound => {
142-            println!(
143-                "`{bin_name}` was not found in $PATH, skipping those checks."
144-            );
145+            println!("`{bin_name}` was not found in $PATH, skipping those checks.");
146             Ok(())
147         }
148         Err(e) => Err(format!("Error running `{bin_name}`: {e}")),
149     }
150 }
151 
152-fn lint_std_filesystem() -> LintResult {
153+pub fn lint_std_filesystem() -> LintResult {
154     let found = git()
155         .args([
156             "grep",
157@@ -390,7 +397,7 @@ fs:: namespace, which has unsafe filesystem functions marked as deleted.
158     }
159 }
160 
161-fn lint_rpc_assert() -> LintResult {
162+pub fn lint_rpc_assert() -> LintResult {
163     let found = git()
164         .args([
165             "grep",
166@@ -420,7 +427,7 @@ checks should be used over assert. See: src/util/check.h
167     }
168 }
169 
170-fn lint_boost_assert() -> LintResult {
171+pub fn lint_boost_assert() -> LintResult {
172     let found = git()
173         .args([
174             "grep",
175@@ -446,7 +453,7 @@ include of the boost/assert.hpp dependency.
176     }
177 }
178 
179-fn lint_doc_release_note_snippets() -> LintResult {
180+pub fn lint_doc_release_note_snippets() -> LintResult {
181     let non_release_notes = check_output(git().args([
182         "ls-files",
183         "--",
184@@ -497,7 +504,7 @@ fn get_pathspecs_exclude_whitespace() -> Vec<String> {
185     list
186 }
187 
188-fn lint_trailing_whitespace() -> LintResult {
189+pub fn lint_trailing_whitespace() -> LintResult {
190     let trailing_space = git()
191         .args(["grep", "-I", "--line-number", "\\s$", "--"])
192         .args(get_pathspecs_exclude_whitespace())
193@@ -522,7 +529,7 @@ sourced files to the exclude list.
194     }
195 }
196 
197-fn lint_trailing_newline() -> LintResult {
198+pub fn lint_trailing_newline() -> LintResult {
199     let files = check_output(
200         git()
201             .args([
202@@ -560,7 +567,7 @@ Please add any false positives to the exclude list.
203     }
204 }
205 
206-fn lint_tabs_whitespace() -> LintResult {
207+pub fn lint_tabs_whitespace() -> LintResult {
208     let tabs = git()
209         .args(["grep", "-I", "--line-number", "--perl-regexp", "^\\t", "--"])
210         .args(["*.cpp", "*.h", "*.md", "*.py", "*.sh"])
211@@ -584,7 +591,7 @@ Please add any false positives, such as subtrees, or externally sourced files to
212     }
213 }
214 
215-fn lint_includes_build_config() -> LintResult {
216+pub fn lint_includes_build_config() -> LintResult {
217     let config_path = "./cmake/bitcoin-build-config.h.in";
218     let defines_regex = format!(
219         r"^\s*(?!//).*({})",
220@@ -673,7 +680,7 @@ the header. Consider removing the unused include.
221     Ok(())
222 }
223 
224-fn lint_doc() -> LintResult {
225+pub fn lint_doc_args() -> LintResult {
226     if Command::new("test/lint/check-doc.py")
227         .status()
228         .expect("command error")
229@@ -685,7 +692,7 @@ fn lint_doc() -> LintResult {
230     }
231 }
232 
233-fn lint_markdown() -> LintResult {
234+pub fn lint_markdown() -> LintResult {
235     let bin_name = "mlc";
236     let mut md_ignore_paths = get_subtrees();
237     md_ignore_paths.push("./doc/README_doxygen.md");
238@@ -728,6 +735,7 @@ Markdown link errors found:
239     }
240 }
241 
242+
243 fn run_all_python_linters() -> LintResult {
244     let mut good = true;
245     let lint_dir = get_git_root().join("test/lint");

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

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

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)