Add more debugging code to the rules execution pathways (#51)

.github/workflows/release.yaml

@@ -41,7 +41,7 @@ jobs:
         env:
           HORTON_RELEASE: ${{ github.event.inputs.release_tag }}
 
-      - uses: actions/upload-artifact@v3
+      - uses: actions/upload-artifact@v4
         with:
           name: ${{ matrix.target }}
           path: target/${{ matrix.target }}/release/trunk-toolbox
@@ -55,7 +55,7 @@ jobs:
       - uses: actions/checkout@v4
 
       - id: download
-        uses: actions/download-artifact@v3
+        uses: actions/download-artifact@v4
         with:
           path: build
 

.github/workflows/upgrade.yaml

@@ -13,7 +13,7 @@ jobs:
       pull-requests: write
     steps:
       - name: Checkout
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
       - name: Trunk Upgrade
         uses: trunk-io/trunk-action/upgrade@v1
         with:

.trunk/trunk.yaml

@@ -30,8 +30,6 @@ lint:
           run: ${workspace}/toolbox-latest --upstream=${upstream-ref} --cache-dir=${cachedir} --results=${tmpfile} ${target}
           output: sarif
           batch: true
-          cache_results: false
-          cache_ttl: 6h
           success_codes: [0]
           read_output_from: tmp_file
           disable_upstream: false

src/main.rs

@@ -186,10 +186,10 @@ fn init_default_logger() {
         )))
         .build();
 
-    // Build the log4rs configuration
+    // Build the log4rs configuration - log only errors to stdout by default
     let config = log4rs::Config::builder()
         .appender(Appender::builder().build("stdout", Box::new(stdout)))
-        .build(Root::builder().appender("stdout").build(LevelFilter::Debug))
+        .build(Root::builder().appender("stdout").build(LevelFilter::Error))
         .expect("Failed to build log4rs configuration");
 
     log4rs::init_config(config).unwrap();
@@ -212,6 +212,7 @@ fn main() {
         }
     } else {
         init_default_logger();
+        debug!("using default built-in logging setup - no log4rs.yaml found");
     }
 
     match run() {

src/rules/if_change_then_change.rs

@@ -1,6 +1,6 @@
 use crate::run::Run;
 use anyhow::Context;
-use log::trace;
+use log::{debug, trace};
 use std::collections::{HashMap, HashSet};
 use std::fs::File;
 use std::io::{BufRead, BufReader};
@@ -52,6 +52,8 @@ lazy_static::lazy_static! {
 pub fn find_ictc_blocks(path: &PathBuf) -> anyhow::Result<Vec<IctcBlock>> {
     let mut blocks: Vec<IctcBlock> = Vec::new();
 
+    trace!("scanning contents of {}", path.display());
+
     let in_file = File::open(path).with_context(|| format!("failed to open: {:#?}", path))?;
     let in_buf = BufReader::new(in_file);
 
@@ -116,9 +118,15 @@ pub fn ictc(run: &Run, upstream: &str) -> anyhow::Result<Vec<diagnostic::Diagnos
     let config = &run.config.ifchange;
 
     if !config.enabled {
+        trace!("'ifchange' is disabled");
         return Ok(vec![]);
     }
 
+    debug!(
+        "scanning {} files for if_change_then_change",
+        run.paths.len()
+    );
+
     // Build up list of files that actually have a ifchange block - this way we can avoid
     // processing git modified chunks if none are present
     let all_blocks: Vec<_> = run

src/rules/never_edit.rs

@@ -4,6 +4,8 @@ use crate::run::Run;
 use glob::glob;
 use glob_match::glob_match;
 
+use log::debug;
+use log::trace;
 use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
 
 use crate::diagnostic;
@@ -23,6 +25,7 @@ pub fn never_edit(run: &Run, upstream: &str) -> anyhow::Result<Vec<diagnostic::D
     let config = &run.config.neveredit;
 
     if !config.enabled {
+        trace!("'neveredit' is disabled");
         return Ok(vec![]);
     }
 
@@ -31,6 +34,7 @@ pub fn never_edit(run: &Run, upstream: &str) -> anyhow::Result<Vec<diagnostic::D
     // We only emit config issues for the current run (not the upstream) so we can guarantee
     // that config issues get reported and not conceiled by HTL
     if config.paths.is_empty() && !run.is_upstream() {
+        trace!("'neveredit' no protected paths configured");
         diagnostics.push(diagnostic::Diagnostic {
             path: run.config_path.clone(),
             range: None,
@@ -44,6 +48,7 @@ pub fn never_edit(run: &Run, upstream: &str) -> anyhow::Result<Vec<diagnostic::D
 
     // We only report diagnostic issues for config when not running as upstream
     if !run.is_upstream() {
+        debug!("verifying protected paths are valid and exist");
         for glob_path in &config.paths {
             let mut matches_something = false;
             match glob(glob_path) {
@@ -102,6 +107,11 @@ pub fn never_edit(run: &Run, upstream: &str) -> anyhow::Result<Vec<diagnostic::D
         return Ok(diagnostics);
     }
 
+    debug!(
+        "tool configured for {} protected files",
+        protected_files.len()
+    );
+
     let modified = git::modified_since(upstream, None)?;
 
     for protected_file in &protected_files {
@@ -136,7 +146,7 @@ pub fn never_edit(run: &Run, upstream: &str) -> anyhow::Result<Vec<diagnostic::D
         path: "".to_string(),
         range: None,
         severity: diagnostic::Severity::Note,
-        code: "toolbox-perf".to_string(),
+        code: "toolbox-never-edit-perf".to_string(),
         message: format!("{:?} protected files checked", protected_files.len()),
         replacements: None,
     });

src/rules/pls_no_land.rs

@@ -4,6 +4,7 @@ extern crate regex;
 use crate::diagnostic;
 use crate::run::Run;
 use anyhow::Context;
+use log::{debug, trace};
 use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
 use regex::Regex;
 use std::fs::File;
@@ -37,9 +38,13 @@ pub fn pls_no_land(run: &Run) -> anyhow::Result<Vec<diagnostic::Diagnostic>> {
 
     // Avoid opening the file if neither are enabled.
     if !dnl_config.enabled && !todo_config.enabled {
+        trace!("'donotland' is disabled");
+        trace!("'todo' is disabled");
         return Ok(vec![]);
     }
 
+    debug!("scanning {} files for pls_no_land", run.paths.len());
+
     // Scan files in parallel
     let results: Result<Vec<_>, _> = run
         .paths
@@ -57,11 +62,12 @@ fn pls_no_land_impl(path: &PathBuf, run: &Run) -> anyhow::Result<Vec<diagnostic:
     let config: &crate::config::Conf = &run.config;
 
     if is_binary_file(path).unwrap_or(true) {
-        log::debug!("Ignoring binary file {}", path.display());
+        debug!("ignoring binary file {}", path.display());
         return Ok(vec![]);
     }
 
     if is_ignored_file(path) {
+        debug!("ignoring ignored file {}", path.display());
         return Ok(vec![]);
     }
 
@@ -75,6 +81,8 @@ fn pls_no_land_impl(path: &PathBuf, run: &Run) -> anyhow::Result<Vec<diagnostic:
         return Ok(vec![]);
     }
 
+    trace!("scanning contents of {}", path.display());
+
     let first_line_view = String::from_utf8(first_line)
         .with_context(|| format!("could not read first line of {:#?}", path))?;
     let lines_view = in_buf

tests/integration_testing.rs

@@ -19,6 +19,7 @@ pub struct HortonOutput {
 }
 
 impl HortonOutput {
+    #[allow(dead_code)]
     pub fn runs(&self) -> Vec<Run> {
         let sarif: Sarif = match serde_json::from_str(&self.results) {
             Ok(s) => s,
@@ -28,6 +29,7 @@ impl HortonOutput {
         sarif.runs
     }
 
+    #[allow(dead_code)]
     pub fn has_result(&self, rule_id: &str, message: &str, file: Option<&str>) -> bool {
         // Iterate over the runs and results to find the matching code and message
         for run in self.runs() {
@@ -63,6 +65,7 @@ impl HortonOutput {
         false
     }
 
+    #[allow(dead_code)]
     pub fn has_result_with_rule_id(&self, rule_id: &str) -> bool {
         // Iterate over the runs and results to find the matching code and message
         for run in self.runs() {
@@ -195,6 +198,7 @@ impl TestRepo {
         assert!(output.status.success(), "Git commit failed");
     }
 
+    #[allow(dead_code)]
     pub fn run_horton(&self) -> anyhow::Result<HortonOutput> {
         self.run_horton_with("HEAD", "sarif", true)
     }
@@ -216,7 +220,6 @@ impl TestRepo {
             horton::git::modified_since(upstream_ref, Some(self.dir.path()))?.paths;
         let files: Vec<String> = modified_paths.keys().map(|key| key.to_string()).collect();
 
-        cmd.env("RUST_LOG", "debug");
         cmd.arg("--upstream")
             .arg(upstream_ref)
             .current_dir(self.dir.path());
@@ -227,7 +230,7 @@ impl TestRepo {
 
         let tmpfile_path = if write_results_to_file {
             // create a temporary file
-            let mut tmpfile = NamedTempFile::new()?;
+            let tmpfile = NamedTempFile::new()?;
             let path = tmpfile.path().to_str().unwrap().to_string();
             cmd.arg("--results").arg(tmpfile.path().to_str().unwrap());
             path

tests/output_format_test.rs

@@ -2,33 +2,24 @@
 extern crate regex;
 
 use serde_json::Error;
-use serde_sarif::sarif::{Run, Sarif};
+use serde_sarif::sarif::Sarif;
 use spectral::prelude::*;
-use std::fs;
-use std::path::Path;
 
-use regex::Regex;
 mod integration_testing;
 use integration_testing::TestRepo;
 
 #[test]
 fn default_sarif() -> anyhow::Result<()> {
     let test_repo = TestRepo::make()?;
 
-    let expected_file = Path::new(file!()).parent().unwrap().join("output.sarif");
-    let expected_sarif = fs::read_to_string(expected_file)?;
-
     test_repo.write(
         "alpha.foo",
         "lorem ipsum dolor\ndo-NOT-lAnD\nDONOTLAND sit amet\n".as_bytes(),
     );
     test_repo.git_add_all()?;
     let horton = test_repo.run_horton_with("HEAD", "sarif", false)?;
 
-    let re = Regex::new(r"\d+\.\d+ms").unwrap();
-    let normalized_output = re.replace(&horton.stdout, "ms");
-
-    let sarif: Result<Sarif, Error> = serde_json::from_str(&normalized_output);
+    let sarif: Result<Sarif, Error> = serde_json::from_str(&horton.stdout);
     assert_that(&sarif.is_ok()).is_true();
     assert_that(&sarif.unwrap().runs).has_length(1);