diff --git a/.abide/rubric.json b/.abide/rubric.json index 618c866d..2d29b637 100644 --- a/.abide/rubric.json +++ b/.abide/rubric.json @@ -1,7 +1,7 @@ { "version": 1, - "compiledAt": "2026-11-01T06:33:21Z", - "compiledBy": "botica 0.18.1", + "compiledAt": "2026-11-11T06:04:09Z", + "compiledBy": "botica 0.20.2", "manual": true, "sources": [ { @@ -1020,6 +1020,7 @@ "crates/botica-core/src/extension_run.rs", "crates/botica-core/src/external_config.rs", "crates/botica-core/src/external_weakened.rs", + "crates/botica-core/src/fingerprint.rs", "crates/botica-core/src/heavy_imports.rs", "crates/botica-core/src/layout.rs", "crates/botica-core/src/lib.rs", @@ -1278,6 +1279,7 @@ "crates/botica-core/src/extension_run.rs", "crates/botica-core/src/external_config.rs", "crates/botica-core/src/external_weakened.rs", + "crates/botica-core/src/fingerprint.rs", "crates/botica-core/src/heavy_imports.rs", "crates/botica-core/src/layout.rs", "crates/botica-core/src/lib.rs", diff --git a/crates/botica-cli/src/ext_cmd.rs b/crates/botica-cli/src/ext_cmd.rs index b51afaa2..ba90eb9d 100644 --- a/crates/botica-cli/src/ext_cmd.rs +++ b/crates/botica-cli/src/ext_cmd.rs @@ -89,6 +89,7 @@ fn prepare( fn run_quietly(argv: &[String], root: &std::path::Path) -> Result<(), String> { let status = Command::new(&argv[0]) .args(&argv[1..]) + .env_remove(botica_ext::HISTORY_KEY_VARIABLE) .current_dir(root) .stdin(Stdio::null()) .stdout(Stdio::null()) diff --git a/crates/botica-cli/src/lint_cmd.rs b/crates/botica-cli/src/lint_cmd.rs index adf6860f..12bdc1bd 100644 --- a/crates/botica-cli/src/lint_cmd.rs +++ b/crates/botica-cli/src/lint_cmd.rs @@ -23,6 +23,7 @@ mod editor; mod extensions; mod external; mod external_weakened; +mod fingerprints; mod git; mod import_layers; mod judged; @@ -311,7 +312,8 @@ pub fn run(args: LintArgs) -> Result { let mut setup = Setup::build(&args)?; if messages::asked(&args) { - let diagnostics = messages::findings(&args, &setup.resolution)?; + let mut diagnostics = messages::findings(&args, &setup.resolution)?; + fingerprints::stamp(format, None, fingerprints::Read::Messages, &mut diagnostics); write_report(&args, format, &setup, &diagnostics, &[])?; return Ok(exit_code(&[], &diagnostics, &args)); } @@ -361,6 +363,16 @@ pub fn run(args: LintArgs) -> Result { if let Some(diff) = &diff { diagnostics.retain(|diagnostic| diff.changes.keeps(diagnostic)); } + fingerprints::stamp( + format, + setup.prefix.as_deref(), + fingerprints::Read::Files { + sources: &sources, + root: &root, + staged: diff.as_ref().is_some_and(Diff::is_staged), + }, + &mut diagnostics, + ); write_report(&args, format, &setup, &diagnostics, &errors)?; if args.timings { timings::print_rules(setup.engine.rule_times()); @@ -428,6 +440,17 @@ fn run_judged( .diagnostics .retain(|diagnostic| diff.changes.keeps(diagnostic)); } + let root = std::env::current_dir()?; + fingerprints::stamp( + format, + setup.prefix.as_deref(), + fingerprints::Read::Files { + sources: &[], + root: &root, + staged: diff.is_some_and(Diff::is_staged), + }, + &mut judged.diagnostics, + ); for rule in &judged.left_out { eprintln!( "botica: `{rule}` is not a judgment rule of the catalog: its finding is left out" diff --git a/crates/botica-cli/src/lint_cmd/baseline.rs b/crates/botica-cli/src/lint_cmd/baseline.rs index 2d58f021..58b29d07 100644 --- a/crates/botica-cli/src/lint_cmd/baseline.rs +++ b/crates/botica-cli/src/lint_cmd/baseline.rs @@ -127,7 +127,7 @@ fn key_of(prefix: Option<&str>, diagnostic: &Diagnostic) -> Option { } /// A path seen from the directory `prefix` inside the project, from its root. -fn from_root(prefix: Option<&str>, path: &str) -> String { +pub(super) fn from_root(prefix: Option<&str>, path: &str) -> String { let mut parts: Vec<&str> = prefix .map(|prefix| prefix.split('/').collect()) .unwrap_or_default(); diff --git a/crates/botica-cli/src/lint_cmd/fingerprints.rs b/crates/botica-cli/src/lint_cmd/fingerprints.rs new file mode 100644 index 00000000..73f51cd9 --- /dev/null +++ b/crates/botica-cli/src/lint_cmd/fingerprints.rs @@ -0,0 +1,73 @@ +//! The fingerprint of each finding in the json and sarif output (ADR 0019 decision 5). + +use super::baseline::from_root; +use super::git; +use crate::output::eprintln; +use botica_core::fingerprint::FingerprintKey; +use botica_core::report::Format; +use botica_core::walk::SourceFile; +use botica_core::Diagnostic; +use std::collections::HashMap; +use std::path::Path; + +/// What the findings were read from. +pub(super) enum Read<'a> { + /// The texts that the run linted. A path that they lack is read from the index when + /// `staged`, else from `root`. + Files { + sources: &'a [SourceFile], + root: &'a Path, + staged: bool, + }, + /// Commit messages and titles: no finding has a line of a file. + Messages, +} + +/// Sets the fingerprint of each finding when the output carries it. A `BOTICA_HISTORY_KEY` that +/// is too short leaves every fingerprint out, with a warning. +pub(super) fn stamp( + format: Format, + prefix: Option<&str>, + read: Read<'_>, + diagnostics: &mut [Diagnostic], +) { + if !matches!(format, Format::Json | Format::Sarif) { + return; + } + let key = match FingerprintKey::from_env() { + Ok(key) => key, + Err(error) => { + eprintln!("botica: warning: the findings carry no fingerprint: {error}"); + return; + } + }; + match read { + Read::Messages => { + for diagnostic in diagnostics { + diagnostic.fingerprint = + Some(key.of_message(&diagnostic.rule, "", &diagnostic.message)); + } + } + Read::Files { + sources, + root, + staged, + } => { + let linted: HashMap<&str, &SourceFile> = sources + .iter() + .map(|source| (source.path.as_str(), source)) + .collect(); + key.stamp( + diagnostics, + |path| from_root(prefix, path), + |path| match linted.get(path) { + Some(source) => source.text().map(str::to_owned), + None if staged => { + git::show("", path).and_then(|bytes| String::from_utf8(bytes).ok()) + } + None => std::fs::read_to_string(root.join(path)).ok(), + }, + ); + } + } +} diff --git a/crates/botica-cli/src/lint_cmd/judged.rs b/crates/botica-cli/src/lint_cmd/judged.rs index e5a6f8e1..9cc828d0 100644 --- a/crates/botica-cli/src/lint_cmd/judged.rs +++ b/crates/botica-cli/src/lint_cmd/judged.rs @@ -193,6 +193,7 @@ fn diagnostic(found: Found, summary: &str, rule: &str, severity: Severity) -> Di end_line: region.as_ref().and_then(|r| r.end_line).unwrap_or(line), end_column: region.as_ref().and_then(|r| r.end_column).unwrap_or(column), file_level: region.is_none(), + fingerprint: None, measure: None, message: match found.note { Some(note) => format!("{text} ({note})"), diff --git a/crates/botica-cli/src/rubric/mod.rs b/crates/botica-cli/src/rubric/mod.rs index 42aab407..f85a02f1 100644 --- a/crates/botica-cli/src/rubric/mod.rs +++ b/crates/botica-cli/src/rubric/mod.rs @@ -229,6 +229,7 @@ fn candidate(path: &str, rule: &str) -> Diagnostic { end_line: 1, end_column: 1, file_level: true, + fingerprint: None, measure: None, message: String::new(), } diff --git a/crates/botica-cli/tests/fingerprint.rs b/crates/botica-cli/tests/fingerprint.rs new file mode 100644 index 00000000..08edac57 --- /dev/null +++ b/crates/botica-cli/tests/fingerprint.rs @@ -0,0 +1,288 @@ +//! The fingerprint of each finding in the json and sarif output (ADR 0019 decision 5). + +mod common; + +use common::{stderr, stdout, write}; +use serde_json::Value; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +const KEY: &str = "0123456789abcdef0123456789abcdef"; + +fn project(name: &str) -> PathBuf { + common::project_in("fingerprint", name, None) +} + +/// `botica lint` in `dir` with `BOTICA_HISTORY_KEY` set to `key`, or removed. +fn lint(dir: &Path, key: Option<&str>, args: &[&str]) -> Output { + let mut command = Command::new(env!("CARGO_BIN_EXE_botica")); + command + .arg("lint") + .args(args) + .arg("--no-astgrep") + .current_dir(dir); + match key { + Some(key) => command.env("BOTICA_HISTORY_KEY", key), + None => command.env_remove("BOTICA_HISTORY_KEY"), + }; + command.output().expect("run botica") +} + +fn json(output: &Output) -> Value { + serde_json::from_str(&stdout(output)).expect("one JSON document") +} + +/// The fingerprint of the one finding of `rule` in the json output. +fn fingerprint_of(output: &Output, rule: &str) -> String { + let document = json(output); + let found: Vec<&Value> = document["diagnostics"] + .as_array() + .expect("diagnostics") + .iter() + .filter(|d| d["rule"] == rule) + .collect(); + assert_eq!(found.len(), 1, "{document}"); + found[0]["fingerprint"] + .as_str() + .unwrap_or_else(|| panic!("no fingerprint in {document}")) + .to_owned() +} + +/// A file whose one finding, an unused suppression, is on its second line. +const UNUSED: &str = "import os\nx = 1 # botica: ignore[no-utils]\ny = 2\n"; + +#[test] +fn a_finding_that_moves_down_keeps_its_fingerprint() { + let dir = project("moves"); + write(&dir, "a.py", UNUSED); + let before = fingerprint_of( + &lint(&dir, None, &["--output-format", "json"]), + "unused-suppression", + ); + + write(&dir, "a.py", &format!("\"\"\"A module.\"\"\"\n\n{UNUSED}")); + let after = lint(&dir, None, &["--output-format", "json"]); + + assert_eq!(fingerprint_of(&after, "unused-suppression"), before); + assert_eq!(before.len(), 16); + assert!(before.chars().all(|c| c.is_ascii_hexdigit()), "{before}"); +} + +#[test] +fn an_edit_of_the_line_gives_a_new_fingerprint() { + let dir = project("edited"); + write(&dir, "a.py", UNUSED); + let before = fingerprint_of( + &lint(&dir, None, &["--output-format", "json"]), + "unused-suppression", + ); + + write( + &dir, + "a.py", + "import os\nx = 2 # botica: ignore[no-utils]\ny = 2\n", + ); + let after = lint(&dir, None, &["--output-format", "json"]); + + assert_ne!(fingerprint_of(&after, "unused-suppression"), before); +} + +#[test] +fn sarif_carries_the_same_fingerprint_as_json() { + let dir = project("sarif"); + write(&dir, "a.py", UNUSED); + + let from_json = fingerprint_of( + &lint(&dir, None, &["--output-format", "json"]), + "unused-suppression", + ); + let sarif = json(&lint(&dir, None, &["--output-format", "sarif"])); + + let result = &sarif["runs"][0]["results"][0]; + assert_eq!(result["ruleId"], "unused-suppression"); + assert_eq!(result["partialFingerprints"]["boticaFinding/v1"], from_json); +} + +#[test] +fn the_path_is_taken_from_the_root_of_the_project_whatever_the_working_directory() { + let dir = project("prefix"); + write(&dir, "botica.toml", ""); + write(&dir, "pkg/a.py", UNUSED); + + let from_root = fingerprint_of( + &lint(&dir, None, &["pkg/a.py", "--output-format", "json"]), + "unused-suppression", + ); + let from_pkg = fingerprint_of( + &lint(&dir.join("pkg"), None, &["a.py", "--output-format", "json"]), + "unused-suppression", + ); + + assert_eq!(from_pkg, from_root); +} + +#[test] +fn botica_history_key_changes_the_fingerprint() { + let dir = project("keyed"); + write(&dir, "a.py", UNUSED); + + let unkeyed = lint(&dir, None, &["--output-format", "json"]); + let keyed = lint(&dir, Some(KEY), &["--output-format", "json"]); + let empty = lint(&dir, Some(""), &["--output-format", "json"]); + + let unkeyed = fingerprint_of(&unkeyed, "unused-suppression"); + assert_ne!(fingerprint_of(&keyed, "unused-suppression"), unkeyed); + assert_eq!(fingerprint_of(&empty, "unused-suppression"), unkeyed); +} + +#[test] +fn a_short_key_leaves_the_fingerprints_out_with_a_warning_and_the_same_exit_code() { + let dir = project("short"); + write(&dir, "a.py", UNUSED); + + let output = lint(&dir, Some("short"), &["--output-format", "json"]); + + let document = json(&output); + assert_eq!(document["diagnostics"][0]["fingerprint"], Value::Null); + assert!( + stderr(&output).contains("`BOTICA_HISTORY_KEY` holds 5 bytes and needs at least 32"), + "{}", + stderr(&output) + ); + assert_eq!( + output.status.code(), + lint(&dir, None, &["--output-format", "json"]).status.code() + ); +} + +#[test] +fn the_text_output_is_unchanged() { + let dir = project("text"); + write(&dir, "a.py", UNUSED); + + let output = lint(&dir, Some("short"), &[]); + + assert!(stderr(&output).is_empty(), "{}", stderr(&output)); + assert!(!stdout(&output).contains("fingerprint")); +} + +#[test] +fn an_extension_process_does_not_inherit_the_key() { + let dir = project("extension"); + write(&dir, "a.py", "x = 1\n"); + write( + &dir, + "ext.sh", + r#"printf '%s' "${BOTICA_HISTORY_KEY-unset}" > seen.txt +while IFS= read -r line; do + case "$line" in + *'"initialize"'*) printf '%s\n' '{"jsonrpc":"2.0","id":1,"result":{"rules":[{"id":"ext-rule","languages":["python"]}]}}' ;; + *'"lint/files"'*) printf '%s\n' '{"jsonrpc":"2.0","id":2,"result":{"diagnostics":[{"path":"a.py","rule":"ext-rule","severity":"warning","start":0,"end":1,"message":"from ext"}]}}' ;; + *'"shutdown"'*) printf '%s\n' '{"jsonrpc":"2.0","id":3,"result":null}'; exit 0 ;; + esac +done +"#, + ); + + let output = lint( + &dir, + Some(KEY), + &["a.py", "--ext", "sh ext.sh", "--output-format", "json"], + ); + + assert_eq!( + std::fs::read_to_string(dir.join("seen.txt")).expect("the extension ran"), + "unset" + ); + assert_eq!(fingerprint_of(&output, "ext-rule").len(), 16); +} + +#[test] +fn a_finding_of_a_message_has_the_fingerprint_of_its_message_wherever_it_was_read() { + let dir = project("message"); + write( + &dir, + "botica.toml", + "preview = true\nselect = [\"conventional-commits\"]\n", + ); + write(&dir, "msg.txt", "Second commit\n"); + + let from_file = lint( + &dir, + None, + &["--commit-msg", "msg.txt", "--output-format", "json"], + ); + let from_title = lint( + &dir, + None, + &["--pr-title=Second commit", "--output-format", "json"], + ); + + assert_eq!( + fingerprint_of(&from_title, "conventional-commits"), + fingerprint_of(&from_file, "conventional-commits") + ); +} + +fn git(dir: &Path, args: &[&str]) { + let output = Command::new("git") + .args(["-c", "user.name=t", "-c", "user.email=t@example.invalid"]) + .args(args) + .current_dir(dir) + .output() + .expect("run git"); + assert!(output.status.success(), "git {args:?}"); +} + +/// A judge's finding of `test-through-interface` on line 2 of `tests/test_a.py`. +const JUDGED: &str = r#"{"version": "2.1.0", "runs": [{"results": [{"ruleId": "test-through-interface", "message": {"text": "m"}, "locations": [{"physicalLocation": {"artifactLocation": {"uri": "tests/test_a.py"}, "region": {"startLine": 2}}}]}]}]}"#; + +#[test] +fn a_judged_finding_of_a_staged_run_hashes_the_staged_text_and_not_the_working_tree() { + let dir = project("judged-staged"); + std::fs::remove_dir_all(dir.join(".git")).expect("remove the stand-in .git"); + git(&dir, &["init", "--quiet"]); + write( + &dir, + "botica.toml", + "preview = true\nselect = [\"test-through-interface\"]\n", + ); + write(&dir, "judge.sarif", JUDGED); + write(&dir, "tests/test_a.py", "def test_a():\n"); + git(&dir, &["add", "--all"]); + git(&dir, &["commit", "--quiet", "-m", "feat: a test"]); + write(&dir, "tests/test_a.py", "def test_a():\n assert a._x\n"); + git(&dir, &["add", "tests/test_a.py"]); + let args = [ + "--judged", + "judge.sarif", + "--staged", + "--output-format", + "json", + ]; + let clean = fingerprint_of(&lint(&dir, None, &args), "test-through-interface"); + + write(&dir, "tests/test_a.py", "def test_b():\n assert a._x\n"); + let dirty = lint(&dir, None, &args); + + assert_eq!(fingerprint_of(&dirty, "test-through-interface"), clean); +} + +#[test] +fn a_staged_run_hashes_the_staged_text_and_not_the_working_tree() { + let dir = project("staged"); + std::fs::remove_dir_all(dir.join(".git")).expect("remove the stand-in .git"); + git(&dir, &["init", "--quiet"]); + write(&dir, "a.py", "y = 0\n"); + git(&dir, &["add", "--all"]); + git(&dir, &["commit", "--quiet", "-m", "feat: a file"]); + write(&dir, "a.py", UNUSED); + git(&dir, &["add", "a.py"]); + let args = ["--staged", "--output-format", "json"]; + let clean = fingerprint_of(&lint(&dir, None, &args), "unused-suppression"); + + write(&dir, "a.py", &UNUSED.replace("import os", "import sys")); + let dirty = lint(&dir, None, &args); + + assert_eq!(fingerprint_of(&dirty, "unused-suppression"), clean); +} diff --git a/crates/botica-core/Cargo.toml b/crates/botica-core/Cargo.toml index 16321e51..262b336a 100644 --- a/crates/botica-core/Cargo.toml +++ b/crates/botica-core/Cargo.toml @@ -7,7 +7,7 @@ publish.workspace = true [features] default = [] -fs = ["dep:blake3", "dep:ignore", "dep:rayon"] +fs = ["dep:ignore", "dep:rayon"] [dependencies] aho-corasick = "1" @@ -15,7 +15,7 @@ anyhow.workspace = true ast-grep-core.workspace = true ast-grep-language.workspace = true botica-astgrep = { path = "../botica-astgrep" } -blake3 = { version = "1", optional = true } +blake3 = "1" botica-catalog = { path = "../botica-catalog" } botica-complexity = { path = "../botica-complexity" } botica-ext = { path = "../botica-ext" } diff --git a/crates/botica-core/src/agents_refs.rs b/crates/botica-core/src/agents_refs.rs index edeb7f2d..b00db3b9 100644 --- a/crates/botica-core/src/agents_refs.rs +++ b/crates/botica-core/src/agents_refs.rs @@ -182,6 +182,7 @@ pub fn check( end_line, end_column, file_level: false, + fingerprint: None, measure: None, message: format!( "`@{}` names no file or folder of the repository: fix the path, or remove \ diff --git a/crates/botica-core/src/botica_version.rs b/crates/botica-core/src/botica_version.rs index a50dc835..66b9e9bf 100644 --- a/crates/botica-core/src/botica_version.rs +++ b/crates/botica-core/src/botica_version.rs @@ -362,6 +362,7 @@ fn on_its_line( end_line, end_column, file_level: false, + fingerprint: None, measure: None, message, } diff --git a/crates/botica-core/src/commit.rs b/crates/botica-core/src/commit.rs index febc8c37..73d84545 100644 --- a/crates/botica-core/src/commit.rs +++ b/crates/botica-core/src/commit.rs @@ -58,6 +58,7 @@ fn finding( end_line, end_column, file_level: false, + fingerprint: None, measure: None, message: text, } diff --git a/crates/botica-core/src/complexity.rs b/crates/botica-core/src/complexity.rs index d35d2cfb..73413579 100644 --- a/crates/botica-core/src/complexity.rs +++ b/crates/botica-core/src/complexity.rs @@ -59,6 +59,7 @@ impl NativeRule for CognitiveComplexity { conditions and loops into functions of their own", function.name, function.complexity, self.max ), + fingerprint: None, measure: Some(Measure { name: function.name, value: function.complexity, diff --git a/crates/botica-core/src/coverage.rs b/crates/botica-core/src/coverage.rs index ec341efa..3a135e70 100644 --- a/crates/botica-core/src/coverage.rs +++ b/crates/botica-core/src/coverage.rs @@ -93,6 +93,7 @@ fn finding(doc: &Document<'_>, span: &Range, message: String) -> Diagnost end_line, end_column, file_level: false, + fingerprint: None, measure: None, message, } diff --git a/crates/botica-core/src/diagnostic.rs b/crates/botica-core/src/diagnostic.rs index 2d703c49..2abdeb4f 100644 --- a/crates/botica-core/src/diagnostic.rs +++ b/crates/botica-core/src/diagnostic.rs @@ -57,6 +57,10 @@ pub struct Diagnostic { #[serde(default)] pub file_level: bool, pub message: String, + /// What follows the finding from one run to the next when its line moves (ADR 0019 decision + /// 5), set by `FingerprintKey::stamp` once the findings of a run are final. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub fingerprint: Option, /// What a rule measured in a function, which the baseline compares with the value it holds. #[serde(default, skip_serializing_if = "Option::is_none")] pub measure: Option, @@ -85,6 +89,7 @@ impl Diagnostic { end_column: 1, file_level: true, message, + fingerprint: None, measure: None, } } diff --git a/crates/botica-core/src/eager_test.rs b/crates/botica-core/src/eager_test.rs index be404cb4..7220adca 100644 --- a/crates/botica-core/src/eager_test.rs +++ b/crates/botica-core/src/eager_test.rs @@ -417,6 +417,7 @@ impl NativeRule for EagerTest { listed.join(", "), self.max ), + fingerprint: None, measure: None, }); } diff --git a/crates/botica-core/src/engine.rs b/crates/botica-core/src/engine.rs index fc0f7fa2..c61114df 100644 --- a/crates/botica-core/src/engine.rs +++ b/crates/botica-core/src/engine.rs @@ -346,6 +346,7 @@ impl Engine { end_line, end_column, file_level: false, + fingerprint: None, measure: None, message: hit.message, }); diff --git a/crates/botica-core/src/env_schema/mod.rs b/crates/botica-core/src/env_schema/mod.rs index b26fe01f..ec62f9f1 100644 --- a/crates/botica-core/src/env_schema/mod.rs +++ b/crates/botica-core/src/env_schema/mod.rs @@ -95,6 +95,7 @@ fn finding( end_line, end_column, file_level: false, + fingerprint: None, measure: None, message, } diff --git a/crates/botica-core/src/extension_run.rs b/crates/botica-core/src/extension_run.rs index 2980692c..7a78c869 100644 --- a/crates/botica-core/src/extension_run.rs +++ b/crates/botica-core/src/extension_run.rs @@ -153,6 +153,7 @@ fn diagnostic(wire: WireDiagnostic, texts: &HashMap<&str, &str>) -> Diagnostic { end_line, end_column, file_level: false, + fingerprint: None, measure: None, message: wire.message, } diff --git a/crates/botica-core/src/external_weakened.rs b/crates/botica-core/src/external_weakened.rs index 16ad5c41..be36d168 100644 --- a/crates/botica-core/src/external_weakened.rs +++ b/crates/botica-core/src/external_weakened.rs @@ -201,6 +201,7 @@ fn on_a_line( end_line, end_column, file_level: false, + fingerprint: None, measure: None, message, } diff --git a/crates/botica-core/src/fingerprint.rs b/crates/botica-core/src/fingerprint.rs new file mode 100644 index 00000000..1f9a2a8a --- /dev/null +++ b/crates/botica-core/src/fingerprint.rs @@ -0,0 +1,138 @@ +//! The fingerprint of a finding (ADR 0019 decision 5): a keyed hash of its rule, its path and the +//! text around it, with no line number, so that a finding keeps it when the lines above it move. + +use crate::diagnostic::Diagnostic; +use std::collections::HashMap; +use std::fmt; + +/// The environment variable whose value keys the hash. +pub const KEY_VARIABLE: &str = botica_ext::HISTORY_KEY_VARIABLE; + +const CONTEXT: &str = "botica history fingerprint v1"; + +/// The shortest value of `BOTICA_HISTORY_KEY` that is accepted, in bytes. +pub const MIN_KEY_LEN: usize = 32; + +/// A value of `BOTICA_HISTORY_KEY` too short to keep a fingerprint from being guessed offline. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ShortKey { + pub len: usize, +} + +impl fmt::Display for ShortKey { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "`{KEY_VARIABLE}` holds {} bytes and needs at least {MIN_KEY_LEN}", + self.len + ) + } +} + +impl std::error::Error for ShortKey {} + +/// The key of the hash, derived from the value of `BOTICA_HISTORY_KEY`. +#[derive(Clone)] +pub struct FingerprintKey { + key: [u8; 32], +} + +impl FingerprintKey { + /// The key derived from `value`; with no value, or an empty one, from the empty string. + pub fn new(value: Option<&str>) -> Result { + let value = value.unwrap_or(""); + if !value.is_empty() && value.len() < MIN_KEY_LEN { + return Err(ShortKey { len: value.len() }); + } + Ok(Self { + key: blake3::derive_key(CONTEXT, value.as_bytes()), + }) + } + + /// The key that `BOTICA_HISTORY_KEY` gives this process. + pub fn from_env() -> Result { + Self::new(std::env::var(KEY_VARIABLE).ok().as_deref()) + } + + /// The fingerprint of a finding of `slug` at line `line` (1-based) of `text`: the line and + /// its nearest non-blank neighbors stand for where it is. A line past the end of the text + /// counts as a finding about the whole file. + pub fn of_line(&self, slug: &str, path: &str, text: &str, line: usize) -> String { + let lines: Vec<&str> = text.lines().collect(); + self.of_line_in(slug, path, &lines, line) + } + + /// The fingerprint of a finding about a whole file: its three lines are empty. + pub fn of_file(&self, slug: &str, path: &str) -> String { + self.hash(&[slug, path, "", "", ""]) + } + + /// The fingerprint of a finding with no line text (about a whole change, or about a message): + /// its message stands for the three lines. + pub fn of_message(&self, slug: &str, path: &str, message: &str) -> String { + self.hash(&[slug, path, &normalized(message)]) + } + + /// Sets the fingerprint of each finding. `project_path` gives the path of a finding from the + /// root of the project, and `text_of` the text that the run linted at the path of a finding, + /// which is read once for each path. + pub fn stamp( + &self, + diagnostics: &mut [Diagnostic], + project_path: impl Fn(&str) -> String, + mut text_of: impl FnMut(&str) -> Option, + ) { + let mut lines_of: HashMap>> = HashMap::new(); + for diagnostic in diagnostics { + let slug = diagnostic.rule.as_str(); + let fingerprint = if diagnostic.path.is_empty() { + self.of_message(slug, "", &diagnostic.message) + } else { + let path = project_path(&diagnostic.path); + let lines = (!diagnostic.file_level).then(|| { + lines_of.entry(diagnostic.path.clone()).or_insert_with(|| { + text_of(&diagnostic.path) + .map(|text| text.lines().map(str::to_owned).collect()) + }) + }); + match lines { + Some(Some(lines)) => self.of_line_in(slug, &path, lines, diagnostic.line), + _ => self.of_file(slug, &path), + } + }; + diagnostic.fingerprint = Some(fingerprint); + } + } + + fn of_line_in(&self, slug: &str, path: &str, lines: &[impl AsRef], line: usize) -> String { + let Some(index) = line.checked_sub(1).filter(|&index| index < lines.len()) else { + return self.of_file(slug, path); + }; + let non_blank = |line: &&_| !AsRef::::as_ref(*line).trim().is_empty(); + let above = lines[..index].iter().rev().find(non_blank); + let below = lines[index + 1..].iter().find(non_blank); + self.hash(&[ + slug, + path, + &normalized(above.map_or("", AsRef::as_ref)), + &normalized(lines[index].as_ref()), + &normalized(below.map_or("", AsRef::as_ref)), + ]) + } + + /// Each field preceded by its length in bytes as a 64-bit little-endian integer; the first + /// 16 hex digits of the keyed hash. + fn hash(&self, fields: &[&str]) -> String { + let mut hasher = blake3::Hasher::new_keyed(&self.key); + for field in fields { + hasher.update(&(field.len() as u64).to_le_bytes()); + hasher.update(field.as_bytes()); + } + hasher.finalize().to_hex()[..16].to_owned() + } +} + +/// Trimmed, with each run of whitespace turned into one space. +fn normalized(text: &str) -> String { + text.split_whitespace().collect::>().join(" ") +} diff --git a/crates/botica-core/src/heavy_imports.rs b/crates/botica-core/src/heavy_imports.rs index 70d85dbc..240a0502 100644 --- a/crates/botica-core/src/heavy_imports.rs +++ b/crates/botica-core/src/heavy_imports.rs @@ -126,6 +126,7 @@ fn finding( end_line, end_column, file_level: false, + fingerprint: None, measure: None, message: format!( "`{name}` is loaded when this module is imported, so every importer waits for it: \ diff --git a/crates/botica-core/src/import_layers/mod.rs b/crates/botica-core/src/import_layers/mod.rs index 6f3e6a08..b96dc397 100644 --- a/crates/botica-core/src/import_layers/mod.rs +++ b/crates/botica-core/src/import_layers/mod.rs @@ -91,6 +91,7 @@ pub fn check(layers: &[String], sources: &[Source<'_>], severity: Severity) -> V end_line, end_column, file_level: false, + fingerprint: None, measure: None, message: message(&module.name, &chain, &breach), }); diff --git a/crates/botica-core/src/lib.rs b/crates/botica-core/src/lib.rs index 0abaf052..cf0c69d6 100644 --- a/crates/botica-core/src/lib.rs +++ b/crates/botica-core/src/lib.rs @@ -13,6 +13,7 @@ pub mod env_schema; mod extension_run; pub mod external_config; pub mod external_weakened; +pub mod fingerprint; pub mod heavy_imports; pub mod import_layers; mod layout; diff --git a/crates/botica-core/src/recipe_conflict.rs b/crates/botica-core/src/recipe_conflict.rs index 0f8c3e21..864ae461 100644 --- a/crates/botica-core/src/recipe_conflict.rs +++ b/crates/botica-core/src/recipe_conflict.rs @@ -99,6 +99,7 @@ fn on_a_line( end_line, end_column, file_level: false, + fingerprint: None, measure: None, message, } diff --git a/crates/botica-core/src/report/mod.rs b/crates/botica-core/src/report/mod.rs index ddbfe9a6..40b37f23 100644 --- a/crates/botica-core/src/report/mod.rs +++ b/crates/botica-core/src/report/mod.rs @@ -175,6 +175,7 @@ struct JsonDiagnostic<'a> { path: &'a str, range: Option, message: &'a str, + fingerprint: Option<&'a str>, } #[derive(Serialize)] @@ -226,6 +227,7 @@ impl Serialize for Diagnostics<'_> { }, }), message: &d.message, + fingerprint: d.fingerprint.as_deref(), } })) } diff --git a/crates/botica-core/src/report/sarif.rs b/crates/botica-core/src/report/sarif.rs index cb014fd8..07dce648 100644 --- a/crates/botica-core/src/report/sarif.rs +++ b/crates/botica-core/src/report/sarif.rs @@ -72,6 +72,16 @@ struct SarifResult<'a> { level: &'static str, message: Text<'a>, locations: [Location; 1], + #[serde(skip_serializing_if = "Option::is_none")] + partial_fingerprints: Option>, +} + +/// The fingerprint of ADR 0019 decision 5, under a name that carries its version, as SARIF asks +/// (3.27.17). +#[derive(Serialize)] +struct PartialFingerprints<'a> { + #[serde(rename = "boticaFinding/v1")] + finding: &'a str, } #[derive(Serialize)] @@ -209,6 +219,10 @@ pub(super) fn write(report: &Report<'_>, out: &mut impl Write) -> io::Result<()> }), }, }], + partial_fingerprints: d + .fingerprint + .as_deref() + .map(|finding| PartialFingerprints { finding }), }) .collect(); let document = Document { diff --git a/crates/botica-core/src/route_auth.rs b/crates/botica-core/src/route_auth.rs index ed0775de..9323d463 100644 --- a/crates/botica-core/src/route_auth.rs +++ b/crates/botica-core/src/route_auth.rs @@ -228,6 +228,7 @@ impl NativeRule for RouteAuth { end_line, end_column, file_level: false, + fingerprint: None, measure: None, message: format!( "The route `{name}` gets no caller from a dependency that verifies a credential: \ diff --git a/crates/botica-core/src/suppress/report.rs b/crates/botica-core/src/suppress/report.rs index 70734d9e..c3e57b00 100644 --- a/crates/botica-core/src/suppress/report.rs +++ b/crates/botica-core/src/suppress/report.rs @@ -39,6 +39,7 @@ impl Policy { end_line, end_column, file_level: false, + fingerprint: None, measure: None, message, } diff --git a/crates/botica-core/tests/common/mod.rs b/crates/botica-core/tests/common/mod.rs index fc4ad02f..9a969ead 100644 --- a/crates/botica-core/tests/common/mod.rs +++ b/crates/botica-core/tests/common/mod.rs @@ -30,6 +30,7 @@ impl NativeRule for Flag { end_line, end_column, file_level: false, + fingerprint: None, measure: None, message: "flagged".to_owned(), }); @@ -61,6 +62,7 @@ impl NativeRule for WholeFile { end_line: 1, end_column: 1, file_level: true, + fingerprint: None, measure: None, message: "too big".to_owned(), }); @@ -97,6 +99,7 @@ impl NativeRule for Multi { end_line, end_column, file_level: false, + fingerprint: None, measure: None, message: "spans lines".to_owned(), }); diff --git a/crates/botica-core/tests/diff.rs b/crates/botica-core/tests/diff.rs index b67f7f29..2539a803 100644 --- a/crates/botica-core/tests/diff.rs +++ b/crates/botica-core/tests/diff.rs @@ -46,6 +46,7 @@ fn finding(path: &str, line: usize, end_line: usize) -> Diagnostic { end_line, end_column: 1, file_level: false, + fingerprint: None, measure: None, message: String::new(), } diff --git a/crates/botica-core/tests/fingerprint.rs b/crates/botica-core/tests/fingerprint.rs new file mode 100644 index 00000000..a1d97244 --- /dev/null +++ b/crates/botica-core/tests/fingerprint.rs @@ -0,0 +1,189 @@ +use botica_core::fingerprint::{FingerprintKey, ShortKey}; +use botica_core::{Diagnostic, Severity}; + +const KEY: &str = "0123456789abcdef0123456789abcdef"; + +fn unkeyed() -> FingerprintKey { + FingerprintKey::new(None).expect("no key is a valid key") +} + +fn finding(path: &str, rule: &str, line: usize) -> Diagnostic { + Diagnostic { + path: path.to_owned(), + rule: rule.to_owned(), + severity: Severity::Warning, + start: 0, + end: 0, + line, + column: 1, + end_line: line, + end_column: 1, + file_level: false, + message: "a message".to_owned(), + fingerprint: None, + measure: None, + } +} + +// The expected values come from the reference implementation of the `blake3` package of Python: +// the key is `blake3(value, derive_key_context="botica history fingerprint v1")`, and each field is +// hashed after its length as a 64-bit little-endian integer. + +#[test] +fn a_finding_in_a_line_hashes_the_line_and_its_nearest_non_blank_neighbors() { + let text = "def main():\n\n print(x)\n \n return 1\n"; + + let fingerprint = unkeyed().of_line("no-print", "src/a.py", text, 3); + + assert_eq!(fingerprint, "fc6c37b1e6a10db3"); +} + +#[test] +fn the_key_of_botica_history_key_changes_every_fingerprint() { + let text = "def main():\n print(x)\n return 1\n"; + let key = FingerprintKey::new(Some(KEY)).expect("a key of 32 bytes"); + + assert_eq!( + key.of_line("no-print", "src/a.py", text, 2), + "2f92438af2df38b3" + ); + assert_eq!( + FingerprintKey::new(Some("")).map(|key| key.of_file("r", "p")), + Ok(unkeyed().of_file("r", "p")) + ); +} + +#[test] +fn a_finding_about_a_whole_file_has_three_empty_lines() { + assert_eq!( + unkeyed().of_file("file-size-guard", "big.py"), + "3859838336ed4b85" + ); +} + +#[test] +fn a_finding_with_no_line_text_hashes_its_normalized_message() { + assert_eq!( + unkeyed().of_message("conventional-commits", "", " the subject\thas no type\n"), + "7d68e2a43cb6636a" + ); +} + +#[test] +fn a_key_shorter_than_32_bytes_is_refused() { + assert_eq!( + FingerprintKey::new(Some("short")).err(), + Some(ShortKey { len: 5 }) + ); +} + +#[test] +fn the_fingerprint_does_not_change_when_the_finding_moves_down() { + let before = "import os\n\ndef main():\n print(x)\n return 1\n"; + let after = "import os\nimport sys\n\n\ndef main():\n print(x)\n return 1\n"; + let mut found = [finding("src/a.py", "no-print", 4)]; + let mut moved = [finding("src/a.py", "no-print", 6)]; + + unkeyed().stamp(&mut found, str::to_owned, |_| Some(before.to_owned())); + unkeyed().stamp(&mut moved, str::to_owned, |_| Some(after.to_owned())); + + assert!(found[0].fingerprint.is_some()); + assert_eq!(found[0].fingerprint, moved[0].fingerprint); +} + +#[test] +fn whitespace_inside_and_around_the_lines_does_not_change_it() { + let plain = "a = 1\nprint(x)\nb = 2\n"; + let spaced = " a = 1\t\n\tprint(x) \nb = 2\n"; + + assert_eq!( + unkeyed().of_line("no-print", "a.py", plain, 2), + unkeyed().of_line("no-print", "a.py", spaced, 2) + ); +} + +#[test] +fn two_equal_lines_of_a_file_are_told_apart_by_their_neighbors() { + let text = "def a():\n print(x)\n\ndef b():\n print(x)\n"; + + assert_ne!( + unkeyed().of_line("no-print", "a.py", text, 2), + unkeyed().of_line("no-print", "a.py", text, 5) + ); +} + +#[test] +fn an_edit_of_a_neighbor_the_rule_or_the_path_starts_a_new_fingerprint() { + let text = "a = 1\nprint(x)\nb = 2\n"; + let first = unkeyed().of_line("no-print", "a.py", text, 2); + + assert_ne!( + first, + unkeyed().of_line("no-print", "a.py", "a = 2\nprint(x)\nb = 2\n", 2) + ); + assert_ne!(first, unkeyed().of_line("other-rule", "a.py", text, 2)); + assert_ne!(first, unkeyed().of_line("no-print", "b.py", text, 2)); +} + +#[test] +fn the_length_of_each_field_keeps_the_fields_apart() { + assert_ne!(unkeyed().of_file("ab", "c"), unkeyed().of_file("a", "bc")); +} + +#[test] +fn stamp_hashes_the_path_from_the_root_of_the_project_and_reads_each_file_once() { + let mut reads = 0; + let mut findings = [ + finding("a.py", "no-print", 2), + finding("a.py", "no-print", 3), + Diagnostic::file_level("a.py", "file-size-guard", Severity::Error, "big".to_owned()), + finding("", "a-change-rule", 0), + ]; + let text = "x = 1\nprint(x)\nprint(y)\n"; + + unkeyed().stamp( + &mut findings, + |path| format!("pkg/{path}"), + |_| { + reads += 1; + Some(text.to_owned()) + }, + ); + + assert_eq!(reads, 1); + let fingerprints: Vec<_> = findings + .iter() + .map(|finding| finding.fingerprint.clone().expect("each finding has one")) + .collect(); + assert_eq!( + fingerprints, + [ + unkeyed().of_line("no-print", "pkg/a.py", text, 2), + unkeyed().of_line("no-print", "pkg/a.py", text, 3), + unkeyed().of_file("file-size-guard", "pkg/a.py"), + unkeyed().of_message("a-change-rule", "", "a message"), + ] + ); +} + +#[test] +fn a_line_outside_the_text_counts_as_a_finding_about_the_whole_file() { + let text = "x = 1\nprint(x)\n"; + let whole = unkeyed().of_file("no-print", "a.py"); + + assert_eq!(unkeyed().of_line("no-print", "a.py", text, 0), whole); + assert_eq!(unkeyed().of_line("no-print", "a.py", text, 3), whole); + assert_ne!(unkeyed().of_line("no-print", "a.py", text, 2), whole); +} + +#[test] +fn stamp_falls_back_to_the_whole_file_when_the_text_cannot_be_read() { + let mut findings = [finding("gone.py", "no-print", 2)]; + + unkeyed().stamp(&mut findings, str::to_owned, |_| None); + + assert_eq!( + findings[0].fingerprint.as_deref(), + Some(unkeyed().of_file("no-print", "gone.py").as_str()) + ); +} diff --git a/crates/botica-core/tests/judgment_suppressions.rs b/crates/botica-core/tests/judgment_suppressions.rs index 65cf5dc2..b666039a 100644 --- a/crates/botica-core/tests/judgment_suppressions.rs +++ b/crates/botica-core/tests/judgment_suppressions.rs @@ -73,6 +73,7 @@ fn judge_finding(rule: &str) -> Diagnostic { end_line: 2, end_column: 1, file_level: false, + fingerprint: None, measure: None, message: "judged".to_owned(), } diff --git a/crates/botica-core/tests/line_project_suppressions.rs b/crates/botica-core/tests/line_project_suppressions.rs index 43359156..0074caa2 100644 --- a/crates/botica-core/tests/line_project_suppressions.rs +++ b/crates/botica-core/tests/line_project_suppressions.rs @@ -30,6 +30,7 @@ fn weakened(path: &str, text: &str, value: &str) -> Diagnostic { end_line, end_column, file_level: false, + fingerprint: None, measure: None, message: "weakened".to_owned(), } diff --git a/crates/botica-core/tests/postprocess.rs b/crates/botica-core/tests/postprocess.rs index e4cca8b3..0fa1a21f 100644 --- a/crates/botica-core/tests/postprocess.rs +++ b/crates/botica-core/tests/postprocess.rs @@ -13,6 +13,7 @@ fn found(path: &str, rule: &str, start: usize) -> Diagnostic { end_line: 1, end_column: start + 2, file_level: false, + fingerprint: None, measure: None, message: String::new(), } diff --git a/crates/botica-core/tests/report.rs b/crates/botica-core/tests/report.rs index ed04fb56..423f8a6e 100644 --- a/crates/botica-core/tests/report.rs +++ b/crates/botica-core/tests/report.rs @@ -14,6 +14,7 @@ fn in_code(path: &str, rule: &str, severity: Severity, message: &str) -> Diagnos end_line: 2, end_column: 9, file_level: false, + fingerprint: None, measure: None, message: message.to_owned(), } @@ -31,6 +32,7 @@ fn whole_file(path: &str, rule: &str, severity: Severity, message: &str) -> Diag end_line: 1, end_column: 1, file_level: true, + fingerprint: None, measure: None, message: message.to_owned(), } @@ -145,12 +147,15 @@ fn markdown_is_a_table_of_the_findings_then_the_failures_of_the_run() { #[test] fn sarif_lists_the_rules_locates_each_result_and_reports_the_errors_as_notifications() { let diagnostics = [ - in_code( - "src/my file.py", - "log-no-print", - Severity::Warning, - "no print", - ), + Diagnostic { + fingerprint: Some("0123456789abcdef".to_owned()), + ..in_code( + "src/my file.py", + "log-no-print", + Severity::Warning, + "no print", + ) + }, whole_file("big.py", "file-size-guard", Severity::Error, "too large"), ]; @@ -190,7 +195,8 @@ fn sarif_lists_the_rules_locates_each_result_and_reports_the_errors_as_notificat "endLine": 2, "endColumn": 9, "byteOffset": 10, "byteLength": 8 } - }}] + }}], + "partialFingerprints": {"boticaFinding/v1": "0123456789abcdef"} }, { "ruleId": "file-size-guard", @@ -284,7 +290,10 @@ fn github_says_a_read_error_with_the_file_in_the_message() { #[test] fn json_carries_the_schema_version_the_diagnostics_and_the_errors_apart() { let diagnostics = [ - in_code("src/a.py", "log-no-print", Severity::Warning, "no print"), + Diagnostic { + fingerprint: Some("0123456789abcdef".to_owned()), + ..in_code("src/a.py", "log-no-print", Severity::Warning, "no print") + }, whole_file("big.py", "file-size-guard", Severity::Error, "too large"), ]; @@ -306,7 +315,8 @@ fn json_carries_the_schema_version_the_diagnostics_and_the_errors_apart() { "start": {"offset": 10, "line": 2, "column": 1}, "end": {"offset": 18, "line": 2, "column": 9} }, - "message": "no print" + "message": "no print", + "fingerprint": "0123456789abcdef" }, { "rule": "file-size-guard", @@ -315,7 +325,8 @@ fn json_carries_the_schema_version_the_diagnostics_and_the_errors_apart() { "severity": "error", "path": "big.py", "range": null, - "message": "too large" + "message": "too large", + "fingerprint": null } ], "errors": [ diff --git a/crates/botica-ext/src/lib.rs b/crates/botica-ext/src/lib.rs index 528c9689..a44ad029 100644 --- a/crates/botica-ext/src/lib.rs +++ b/crates/botica-ext/src/lib.rs @@ -25,6 +25,10 @@ const REQUEST_DEADLINE: Duration = Duration::from_secs(60); /// `lint/files` in place of the two above. pub const DEADLINE_VARIABLE: &str = "BOTICA_EXTENSION_TIMEOUT"; +/// The variable that keys the fingerprints of the history (ADR 0019 decision 5). An extension +/// process does not inherit it. +pub const HISTORY_KEY_VARIABLE: &str = "BOTICA_HISTORY_KEY"; + #[derive(Debug, Serialize)] pub struct FileIn<'a> { pub path: &'a str, @@ -168,7 +172,7 @@ impl ExtensionHost { ) -> Result { let (program, args) = command.split_first().context("empty extension command")?; let mut process = Command::new(program); - process.args(args); + process.args(args).env_remove(HISTORY_KEY_VARIABLE); if let Some(dir) = dir { process.current_dir(dir); } diff --git a/crates/botica-lsp/tests/common/mod.rs b/crates/botica-lsp/tests/common/mod.rs index cb728b2c..effdbbab 100644 --- a/crates/botica-lsp/tests/common/mod.rs +++ b/crates/botica-lsp/tests/common/mod.rs @@ -134,6 +134,7 @@ pub fn finding(rule: &str, message: &str) -> Diagnostic { end_line: 1, end_column: 2, file_level: false, + fingerprint: None, measure: None, message: message.to_owned(), } diff --git a/crates/botica-lsp/tests/protocol.rs b/crates/botica-lsp/tests/protocol.rs index 1ddbef88..f8682fa7 100644 --- a/crates/botica-lsp/tests/protocol.rs +++ b/crates/botica-lsp/tests/protocol.rs @@ -42,6 +42,7 @@ fn finding(line: usize, column: usize, end_column: usize, severity: Severity) -> end_line: line, end_column, file_level: false, + fingerprint: None, measure: None, message: "BTSTRUC002 (no-utils): no".to_owned(), } diff --git a/website/docs/guide/lint.md b/website/docs/guide/lint.md index 6f0e36a6..c9ad7660 100644 --- a/website/docs/guide/lint.md +++ b/website/docs/guide/lint.md @@ -17,7 +17,8 @@ botica lint # the current directory botica lint src tests/a.py # directories to walk, or files (as prek passes them) botica lint --select file-size-guard,no-utils botica lint --output-format json # text (default), json, sarif, github or markdown; json and sarif carry the - # code and the standard of the rule of each finding + # code and the standard of the rule of each finding, and its + # fingerprint (ADR 0019), which BOTICA_HISTORY_KEY keys botica lint --judged judge.sarif # the findings of a judge, filtered by the configuration and comments; # the file is SARIF or the report of `abide check --json` botica lint --diff origin/main # the files changed since the merge base, and only the findings on