From 467622555c7fffe05882d53e295a41fe7f199558 Mon Sep 17 00:00:00 2001
From: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Date: Thu, 12 Dec 2024 19:47:19 +0100
Subject: [PATCH] lint: output-only - Avoid repeated arrows, trim

- No empty line separating errors and arrows ("^^^"). Keeping them together signals they are related.
- No empty line separating error message and linter failure line (not completely empty, it contains several spaces left over from Rust multi-line literal).
- Keep the linter description on the same line as the failure line, otherwise it looks like it's a description for the following step.
---
 test/lint/test_runner/src/main.rs | 37 +++++++++++++++----------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/test/lint/test_runner/src/main.rs b/test/lint/test_runner/src/main.rs
index ebfc3d051e90bf..8479fd2d64af9f 100644
--- a/test/lint/test_runner/src/main.rs
+++ b/test/lint/test_runner/src/main.rs
@@ -294,10 +294,10 @@ fn lint_std_filesystem() -> LintResult {
         .success();
     if found {
         Err(r#"
-^^^
 Direct use of std::filesystem may be dangerous and buggy. Please include <util/fs.h> and use the
 fs:: namespace, which has unsafe filesystem functions marked as deleted.
             "#
+        .trim()
         .to_string())
     } else {
         Ok(())
@@ -322,12 +322,12 @@ fn lint_rpc_assert() -> LintResult {
         .success();
     if found {
         Err(r#"
-^^^
 CHECK_NONFATAL(condition) or NONFATAL_UNREACHABLE should be used instead of assert for RPC code.
 
 Aborting the whole process is undesirable for RPC code. So nonfatal
 checks should be used over assert. See: src/util/check.h
             "#
+        .trim()
         .to_string())
     } else {
         Ok(())
@@ -350,10 +350,10 @@ fn lint_boost_assert() -> LintResult {
         .success();
     if found {
         Err(r#"
-^^^
 BOOST_ASSERT must be replaced with Assert, BOOST_REQUIRE, or BOOST_CHECK to avoid an unnecessary
 include of the boost/assert.hpp dependency.
             "#
+        .trim()
         .to_string())
     } else {
         Ok(())
@@ -370,17 +370,15 @@ fn lint_doc_release_note_snippets() -> LintResult {
     if non_release_notes.is_empty() {
         Ok(())
     } else {
-        Err(format!(
-            r#"
-{}
-^^^
+        println!("{non_release_notes}");
+        Err(r#"
 Release note snippets and other docs must be put into the doc/ folder directly.
 
 The doc/release-notes/ folder is for archived release notes of previous releases only. Snippets are
 expected to follow the naming "/doc/release-notes-<PR number>.md".
-            "#,
-            non_release_notes
-        ))
+            "#
+        .trim()
+        .to_string())
     }
 }
 
@@ -423,7 +421,6 @@ fn lint_trailing_whitespace() -> LintResult {
         .success();
     if trailing_space {
         Err(r#"
-^^^
 Trailing whitespace (including Windows line endings [CR LF]) 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.
@@ -433,6 +430,7 @@ 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.
             "#
+        .trim()
         .to_string())
     } else {
         Ok(())
@@ -449,7 +447,6 @@ fn lint_tabs_whitespace() -> LintResult {
         .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.
 
@@ -457,6 +454,7 @@ Please remove the tabs.
 
 Please add any false positives, such as subtrees, or externally sourced files to the exclude list.
             "#
+        .trim()
         .to_string())
     } else {
         Ok(())
@@ -531,7 +529,6 @@ fn lint_includes_build_config() -> LintResult {
     if missing {
         return Err(format!(
             r#"
-^^^
 One or more files use a symbol declared in the bitcoin-build-config.h header. However, they are not
 including the header. This is problematic, because the header may or may not be indirectly
 included. If the indirect include were to be intentionally or accidentally removed, the build could
@@ -547,12 +544,13 @@ include again.
 #include <bitcoin-build-config.h> // IWYU pragma: keep
             "#,
             defines_regex
-        ));
+        )
+        .trim()
+        .to_string());
     }
     let redundant = print_affected_files(false);
     if redundant {
         return Err(r#"
-^^^
 None of the files use a symbol declared in the bitcoin-build-config.h header. However, they are including
 the header. Consider removing the unused include.
             "#
@@ -605,7 +603,9 @@ Markdown link errors found:
 {}
                 "#,
                 stderr
-            ))
+            )
+            .trim()
+            .to_string())
         }
         Err(e) if e.kind() == ErrorKind::NotFound => {
             println!("`mlc` was not found in $PATH, skipping markdown lint check.");
@@ -657,10 +657,9 @@ fn main() -> ExitCode {
         env::set_current_dir(&git_root).unwrap();
         if let Err(err) = (linter.lint_fn)() {
             println!(
-                "{err}\n^---- ⚠️ Failure generated from lint check '{}'!",
-                linter.name
+                "^^^\n{err}\n^---- ⚠️ Failure generated from lint check '{}' ({})!\n\n",
+                linter.name, linter.description,
             );
-            println!("{}\n\n", linter.description);
             test_failed = true;
         }
     }