Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,54 @@ The format follows [Keep a Changelog](https://keepachangelog.com/).

---

## [Unreleased] — `--limit` selects the newest commits; `--since` works

### Changed

- **`--limit N` now analyzes the N NEWEST commits** (the `git log -n`
convention) instead of the N oldest. Commits are still delivered to the
analyzers oldest-first, so tick, burndown and identity semantics are
unchanged — only the selected window moves. This is a deliberate
divergence from the retired Go reference, which selected the oldest
window (measured: `--limit 3` on kubernetes emitted `2c4b3a562ce3`, its
first commits). `specs/frds/FRD-newest-commit-window.md`.
- **`--since` with a cutoff in the middle of the history now works.** It
previously planned a non-zero commit count, loaded zero (the oldest-first
loader stopped at the first too-old commit) and aborted with
`expected N commits, got 0: EOF`, empty stdout and exit 1. A cutoff newer
than every commit still yields the empty report with exit 0.
- **`--since` now accepts a git revision** (`v1.2.0`, `HEAD~100`, a SHA):
that commit's author time becomes the cutoff. Duration forms now honor the
`CODEFANG_NOW` test clock, and the CLI's duplicate time parser was deleted in
favour of the single `cf-gitlib` one.
- **A bad `--since` value says so**: `Error: cannot resolve --since "<value>"`
instead of the generic `command dispatch is blocked` placeholder (same exit
code, still no stdout).

### Fixed

- `site/guide/data-analytics.md` told users to run `--since 6m` for "the last
6 months"; `m` is minutes, so that meant 6 minutes. Docs now state the
supported units (`h`/`m`/`s`) and give `4380h`.
- `cf_gitlib::testutil` documented a `testutil` feature that did not exist, so
no other crate could build a deterministic fixture repo. The feature exists.
- `make lint` and `make deadcode` are implemented — `AGENTS.md` mandated both
gates, but neither target existed (and `cargo clippy` was failing outright on
deny-by-default lints in `cf-reportutil`, `cf-gojson` and `cf-goyaml`). Both
are clean now.
- Removed the never-read `InitState.rule_index` field in `cf-uast` (it pinned a
HashMap of rule names per parser) and cleared the workspace clippy backlog.

### Added

- `crates/cf-commands/src/handlers.rs`: `history_window_tests` — window
contract (newest-N, empty/partial/full `--since`, `--limit` inside a window,
revision spec, injected clock, unresolvable spec).
- `bins/codefang/tests/history_window.rs`: CLI-level e2e driving the real
binary over a real git repo, including the `--head` ≡ `--limit 1` invariant.

---

## [Unreleased] — Repo hygiene & race fix

### Fixed
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,19 @@ install: submodules ## Build and install codefang + uast onto PATH (~/.cargo/bin
test: submodules ## Run the workspace test suite
$(CARGO) test --workspace

.PHONY: lint
lint: submodules ## Clippy across the workspace; every warning is an error
$(CARGO) clippy --workspace --all-targets -- -D warnings

.PHONY: deadcode
# rustc's `dead_code` lint reports unreachable PRIVATE items (a `pub` item in a
# library crate always looks reachable to it, so a clean run means "no private
# code is unreachable", not "no exported function is unused"). A pub-API
# reachability audit needs external tooling (cargo-deadstats-class) which is
# deliberately not vendored here.
deadcode: submodules ## Fail the build on any `dead_code` finding
RUSTFLAGS="-D dead_code" $(CARGO) check --workspace --all-targets

.PHONY: clean
clean: ## Remove build artifacts
$(CARGO) clean
5 changes: 5 additions & 0 deletions bins/codefang/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,8 @@ git2 = { workspace = true }
# thin shell that bootstraps malloc tunables + the --profile watchdog, then calls
# cf_commands::run(argv).
cf-commands = { path = "../../crates/cf-commands" }

[dev-dependencies]
# Deterministic commit fixtures for the CLI-level window tests (feature-gated:
# compiled for tests only, never in a shipped binary).
cf-gitlib = { path = "../../crates/cf-gitlib", features = ["testutil"] }
235 changes: 235 additions & 0 deletions bins/codefang/tests/history_window.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,235 @@
//! End-to-end coverage of the `--limit` / `--since` commit window, driven
//! through the **real binary** over a real on-disk git repository.
//!
//! The window contract (see `specs/frds/FRD-newest-commit-window.md`):
//! `--limit N` takes the N NEWEST commits, `--since C` takes every commit with
//! author time >= C, and both are delivered to the analyzers oldest-first.
//! Reports are asserted in machine formats only (`ndjson` / `json`), which are
//! the byte-stable formats the golden harness also treats as binding.

use cf_gitlib::testutil::{commit_files, init_repo, TestRepo};
use cf_gitlib::Hash;

/// Path to the freshly built `codefang` binary under test.
const BIN: &str = env!("CARGO_BIN_EXE_codefang");
/// Base instant of the fixture history: 2021-01-01T00:00:00Z.
const T0: i64 = 1_609_459_200;
/// One day between fixture commits, so every cutoff lands between two commits.
const DAY: i64 = 86_400;
/// Fixture length: five linear commits; `hashes[0]` is the root.
const N_COMMITS: usize = 5;
/// Cutoff naming the 3rd commit's instant; commits 1-2 fall outside it.
const CUTOFF_3RD: &str = "2021-01-03T00:00:00Z";
/// A newer-than-HEAD cutoff: excludes the whole history.
const CUTOFF_FUTURE: &str = "2030-01-01";

/// The ndjson key that carries a commit hash, and the byte offset of the hash
/// value behind it (`"hash":"` is 8 bytes, a SHA-1 is 40).
const NEEDLE_HASH: &str = "\"hash\":\"";

/// Runs the binary with a deterministic environment; state is disabled so the
/// run is self-contained (same discipline as the golden captures).
fn codefang(args: &[&str]) -> std::process::Output {
Command::new(BIN)
.args(["run", "--checkpoint=false", "--resume=false", "--no-cache"])
.args(args)
.env("TZ", "UTC")
.env("NO_COLOR", "1")
.env("LANG", "C")
.env("LC_ALL", "C")
.output()
.expect("run the codefang binary")
}

use std::process::Command;

/// Builds the five-commit fixture; returns it with its hashes, oldest-first.
fn fixture() -> (TestRepo, Vec<Hash>) {
let test = init_repo().expect("init fixture repo");
let mut hashes = Vec::new();
for i in 0..N_COMMITS {
let h = commit_files(
&test,
&format!("commit {i}"),
T0 + i64::try_from(i).expect("index fits i64") * DAY,
&[("f.txt", format!("content v{i}\n").as_bytes())],
)
.expect("fixture commit");
hashes.push(h);
}
(test, hashes)
}

/// The commit hashes of a `burndown --format ndjson` run, in emitted order,
/// together with `(exit code, stdout, stderr)`.
fn ndjson_window(path: &str, extra: &[&str]) -> (i32, Vec<String>, String, String) {
let out = codefang(
&[
&[
"-a",
"history/burndown",
"--workers",
"1",
"--format",
"ndjson",
],
extra,
&[path],
]
.concat(),
);
let stdout = String::from_utf8_lossy(&out.stdout).into_owned();
let hashes = stdout
.lines()
.filter_map(|line| {
line.find(NEEDLE_HASH)
.map(|i| line[i + 8..i + 48].to_owned())
})
.collect();
(
out.status.code().unwrap_or(-1),
hashes,
stdout,
String::from_utf8_lossy(&out.stderr).into_owned(),
)
}

/// `--limit N` selects the N NEWEST commits, delivered oldest-first.
#[test]
fn limit_selects_the_newest_commits() {
let (test, hashes) = fixture();
let path = test.path().to_str().expect("utf-8 path");
let (rc, got, stdout, stderr) = ndjson_window(path, &["--limit", "2"]);
assert_eq!(rc, 0, "run failed: {stderr}");
assert_eq!(
got,
vec![hashes[3].to_string(), hashes[4].to_string()],
"--limit 2 must take the two NEWEST commits oldest-first, got {got:?} in {stdout}"
);
}

/// The user-visible commit count follows `--limit` and grows with it.
#[test]
fn limit_bound_shows_in_the_report() {
let (test, _hashes) = fixture();
let path = test.path().to_str().expect("utf-8 path");
for (limit, want) in [("1", 1), ("2", 2), ("5", 5)] {
let out = codefang(&[
"-a",
"history/devs",
"--limit",
limit,
"--format",
"json",
path,
]);
assert!(out.status.success(), "run failed for --limit {limit}");
let stdout = String::from_utf8_lossy(&out.stdout).into_owned();
let needle = format!(r#""total_commits":{want}"#);
assert!(
stdout.contains(&needle),
"--limit {limit} must report {want} commits ({needle}):\n{stdout}"
);
}
}

/// A `--since` cutoff in the MIDDLE of the history yields exactly that window.
/// This is the case the reference-parity loader aborted on with empty stdout.
#[test]
fn since_partial_window_succeeds_with_the_window() {
let (test, hashes) = fixture();
let path = test.path().to_str().expect("utf-8 path");
let (rc, got, _stdout, stderr) = ndjson_window(path, &["--since", CUTOFF_3RD]);
assert_eq!(
rc, 0,
"a partial --since window must exit 0, stderr: {stderr}"
);
assert_eq!(
got,
vec![
hashes[2].to_string(),
hashes[3].to_string(),
hashes[4].to_string()
],
"--since must analyze exactly the in-window commits"
);
}

/// `--since` also accepts a git revision.
#[test]
fn since_accepts_a_revision() {
let (test, hashes) = fixture();
let path = test.path().to_str().expect("utf-8 path");
let (rc, got, _stdout, stderr) = ndjson_window(path, &["--since", "HEAD~2"]);
assert_eq!(rc, 0, "run failed: {stderr}");
assert_eq!(
got.first().map(String::as_str),
Some(hashes[2].to_string().as_str()),
"`--since HEAD~2` must start the window at that commit"
);
assert_eq!(got.len(), 3, "window must hold HEAD~2..HEAD");
}

/// A cutoff newer than every commit analyzes nothing but still succeeds.
#[test]
fn since_after_head_is_empty_and_succeeds() {
let (test, _hashes) = fixture();
let path = test.path().to_str().expect("utf-8 path");
let (rc, got, _stdout, stderr) = ndjson_window(path, &["--since", CUTOFF_FUTURE]);
assert_eq!(rc, 0, "an empty window is not an error: {stderr}");
assert!(got.is_empty(), "no commit is in window, got {got:?}");
}

/// An unresolvable `--since` aborts with empty stdout, exit 1, and a message
/// that names the flag rather than the internal dispatch placeholder.
#[test]
fn unresolvable_since_fails_with_a_named_diagnostic() {
let (test, _hashes) = fixture();
let path = test.path().to_str().expect("utf-8 path");
let (rc, _got, stdout, stderr) = ndjson_window(path, &["--since", "not-a-time-or-ref"]);
assert_eq!(rc, 1, "an unresolvable --since must exit 1");
assert!(stdout.is_empty(), "stdout must stay empty, got {stdout}");
assert!(
stderr.contains("cannot resolve --since"),
"the error must name the flag, stderr: {stderr}"
);
assert!(
!stderr.contains("dispatch is blocked"),
"the failure must not be misattributed to the port, stderr: {stderr}"
);
}

/// `--head` means "exactly HEAD", so it must equal `--limit 1` (which now
/// selects HEAD) and must not be widened by a larger `--limit`.
#[test]
fn head_overrides_limit() {
let (test, _hashes) = fixture();
let path = test.path().to_str().expect("utf-8 path");
let report = |extra: &[&str]| {
let out = codefang(
&[
&["-a", "history/burndown", "--format", "json"],
extra,
&[path],
]
.concat(),
);
assert!(
out.status.success(),
"run failed: {:?}",
String::from_utf8_lossy(&out.stderr)
);
out.stdout
};
let head = report(&["--head"]);
let newest_one = report(&["--limit", "1"]);
assert_eq!(
head, newest_one,
"--limit 1 must select exactly HEAD, so both reports must be identical bytes"
);
assert_eq!(
head,
report(&["--head", "--limit", "3"]),
"--head must ignore --limit"
);
}
6 changes: 5 additions & 1 deletion bins/uast/src/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,11 @@ fn show_tree_sitter(files: &[String], language: &str) -> Result<(), String> {
if files.is_empty() {
return Err("no input files provided".to_string());
}
for filename in files {
// The reference loops over every input file, but every path below returns,
// so only the first file is ever reached while grammar wiring is pending in
// cf-uast (DESIGN §5). Stated as an explicit first-file bail instead of a
// loop that cannot iterate twice; the sentinel strings are unchanged.
if let Some(filename) = files.first() {
let _ = std::fs::read(filename)
.map_err(|e| format!("failed to process {filename}: failed to read file: {e}"))?;
// Language resolution mirrors mapping.go: an unknown/empty language is an
Expand Down
6 changes: 3 additions & 3 deletions crates/cf-anomaly/src/detect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,10 @@ pub fn sort_external_results(anomalies: &mut [ExternalAnomaly], summaries: &mut
go_sort_slice(anomalies, |a, b| a.z_score.abs() > b.z_score.abs());

go_sort_slice(summaries, |a, b| {
if a.source != b.source {
a.source < b.source
} else {
if a.source == b.source {
a.dimension < b.dimension
} else {
a.source < b.source
}
});
}
Expand Down
2 changes: 2 additions & 0 deletions crates/cf-clones/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,8 @@ mod tests {
assert_eq!(clone_message(16), MSG_HIGH_CLONES);
}

// Exact on purpose: the zero-denominator guard returns 0.0 exactly.
#[allow(clippy::float_cmp)]
#[test]
fn clone_ratio_zero_when_either_is_zero() {
assert_eq!(compute_clone_ratio(0, 10), 0.0);
Expand Down
5 changes: 5 additions & 0 deletions crates/cf-clones/src/report_section.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,9 @@ mod tests {
f
}

// Exact comparisons on purpose: `compute_score` maps its clamp endpoints
// (0.0 / 1.0) exactly, so an epsilon compare would hide a real regression.
#[allow(clippy::float_cmp)]
#[test]
fn compute_score_clamps_and_inverts() {
assert_eq!(compute_score(1.5), 0.0);
Expand All @@ -499,6 +502,8 @@ mod tests {
assert_eq!(issues[0].severity, "Poor"); // similarity 1.0 >= 0.8
}

// Exact by construction: clone_ratio 0.0 maps to the score 1.0 endpoint.
#[allow(clippy::float_cmp)]
#[test]
fn empty_report_uses_default_status_and_full_score() {
let a = Analyzer::new();
Expand Down
2 changes: 1 addition & 1 deletion crates/cf-cohesion/src/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ mod tests {
Some(&ReportValue::Str("No functions found".into()))
);
// The empty result has no analyzer_name / functions keys.
assert!(report.get("functions").is_none());
assert_eq!(report.get("functions"), None);
}

#[test]
Expand Down
3 changes: 3 additions & 0 deletions crates/cf-commands/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,6 @@ tempfile = "3"
# tempdirs (history-filter oracle tests) — write access comes straight from
# git2, the same libgit2 vendoring cf-gitlib is pinned to.
git2 = { workspace = true }
# Deterministic commit fixtures for the history-window tests (feature-gated, so
# it is compiled for tests only).
cf-gitlib = { path = "../cf-gitlib", features = ["testutil"] }
Loading
Loading