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
59 changes: 56 additions & 3 deletions crates/cli/src/cli_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
//! re-rendering from a saved envelope is a recorded follow-up. Dispatch is on
//! the envelope's `kind` field, so any envelope produced by `--format json`
//! (dead-code, dupes, health, audit, security, or the bare combined run)
//! renders byte-identically to the direct `--format` run.
//! renders byte-identically to the direct `--format` run. The `fallow fix`
//! envelope carries no `kind`; it is detected by its top-level fields and
//! rendered via [`EnvelopeKind::Fix`].

use std::path::Path;
use std::process::ExitCode;
Expand Down Expand Up @@ -76,6 +78,13 @@ fn envelope_kind(
output: OutputFormat,
) -> Result<EnvelopeKind, ExitCode> {
let Some(kind) = envelope.get("kind").and_then(serde_json::Value::as_str) else {
// The `fallow fix --format json` envelope is the only kind-less document
// fallow emits (crates/output/src/fix.rs: no top-level `kind`). Resolve it
// by field detection so `report --from <fix-results.json>` renders the fix
// job summary natively; genuinely unrecognized documents keep erroring.
if is_fix_envelope(envelope) {
return Ok(EnvelopeKind::Fix);
}
return Err(crate::emit_known_failure(
&format!(
"{} is not a fallow results envelope (missing top-level `kind`); \
Expand All @@ -102,8 +111,8 @@ fn envelope_kind(
}

/// Map the `--format json` root `kind` onto the renderer dispatch. The fix
/// envelope has no `kind` field, so fix output is not re-renderable here (use
/// `fallow fix --format github-summary` directly).
/// envelope has no `kind` field; it is resolved separately via
/// [`is_fix_envelope`] field detection (see [`envelope_kind`]).
fn parse_envelope_kind(kind: &str) -> Option<EnvelopeKind> {
match kind {
"dead-code" => Some(EnvelopeKind::DeadCode),
Expand All @@ -116,6 +125,20 @@ fn parse_envelope_kind(kind: &str) -> Option<EnvelopeKind> {
}
}

/// Recognize a kind-less `fallow fix --format json` envelope by its stable
/// top-level keys. The fix root always carries both a `fixes` array and a
/// numeric `total_fixed` (see `crates/output/src/fix.rs::FixJsonOutput`); no
/// other fallow envelope is kind-less, so the two keys together are an
/// unambiguous signal.
fn is_fix_envelope(envelope: &serde_json::Value) -> bool {
envelope
.get("fixes")
.is_some_and(serde_json::Value::is_array)
&& envelope
.get("total_fixed")
.is_some_and(serde_json::Value::is_number)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -145,4 +168,34 @@ mod tests {
assert_eq!(parse_envelope_kind("feature-flags"), None);
assert_eq!(parse_envelope_kind(""), None);
}

#[test]
fn is_fix_envelope_detects_kindless_fix_document() {
let fix = serde_json::json!({
"dry_run": false,
"total_fixed": 3,
"skipped": 0,
"fixes": [{ "type": "remove_export", "applied": true }],
});
assert!(is_fix_envelope(&fix));
}

#[test]
fn is_fix_envelope_rejects_other_kindless_documents() {
// A dead-code envelope stripped of its `kind` must NOT masquerade as
// fix: it has neither `fixes` nor `total_fixed`.
assert!(!is_fix_envelope(&serde_json::json!({
"total_issues": 4,
"unused_files": [{ "path": "src/a.ts" }],
})));
// `fixes` alone (no `total_fixed`) is not enough.
assert!(!is_fix_envelope(&serde_json::json!({ "fixes": [] })));
// `total_fixed` alone (no `fixes` array) is not enough.
assert!(!is_fix_envelope(&serde_json::json!({ "total_fixed": 0 })));
// A `fixes` value that is not an array is rejected.
assert!(!is_fix_envelope(&serde_json::json!({
"fixes": "nope",
"total_fixed": 0,
})));
}
}
8 changes: 8 additions & 0 deletions crates/cli/src/report/github_annotations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ pub enum EnvelopeKind {
Audit,
Combined,
Security,
/// The `fallow fix --format json` envelope. It carries no top-level `kind`
/// field (see `crates/output/src/fix.rs`), so `fallow report --from`
/// detects it by its stable top-level keys rather than a `kind` string.
Fix,
}

/// Render and print the annotation stream for one envelope, resolving the
Expand Down Expand Up @@ -84,6 +88,10 @@ fn collect_annotations(
collect_value_section(envelope, "health", &mut out, collect_health);
collect_value_section(envelope, "dupes", &mut out, collect_dupes);
}
// The bundled action no-ops annotations for the fix command
// (`action/scripts/annotate.sh` skips fix), so the native renderer
// matches by emitting nothing.
EnvelopeKind::Fix => {}
}
out
}
Expand Down
7 changes: 5 additions & 2 deletions crates/cli/src/report/github_summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,10 @@ pub fn print_summary(kind: EnvelopeKind, envelope: &Value, root: &Path) -> ExitC
ExitCode::SUCCESS
}

/// Render the fix envelope's job summary (the fix envelope has no `kind`
/// field, so it does not route through [`EnvelopeKind`]).
/// Render the fix envelope's job summary directly from `fallow fix`. The fix
/// envelope has no `kind` field; `fallow report --from` reaches the same
/// renderer via [`EnvelopeKind::Fix`] (resolved by field detection), while the
/// live `fallow fix` command calls this entry point.
pub fn print_fix_summary(envelope: &Value) -> ExitCode {
outln!("{}", render_fix_summary(envelope));
ExitCode::SUCCESS
Expand All @@ -87,6 +89,7 @@ pub fn render_summary(kind: EnvelopeKind, envelope: &Value, links: &LinkContext)
EnvelopeKind::Audit => render_audit_summary(envelope),
EnvelopeKind::Security => render_security_summary(envelope),
EnvelopeKind::Combined => render_combined_summary(envelope, links),
EnvelopeKind::Fix => render_fix_summary(envelope),
}
}

Expand Down
64 changes: 64 additions & 0 deletions crates/cli/tests/github_format_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,7 @@ fn report_from_round_trip_parity() {
(EnvelopeKind::Audit, audit_envelope()),
(EnvelopeKind::Security, security_envelope()),
(EnvelopeKind::Combined, combined_envelope()),
(EnvelopeKind::Fix, fix_envelope()),
] {
let direct = render_annotations(kind, &envelope, &plain_options());
let serialized = serde_json::to_string_pretty(&envelope).expect("serialize");
Expand Down Expand Up @@ -650,6 +651,68 @@ fn github_summary_fix_snapshot() {
insta::assert_snapshot!("github_summary_fix", rendered);
}

/// `report --from <fix-results.json> --format github-summary` routes the
/// kind-less fix envelope through `EnvelopeKind::Fix`, which must reach the
/// same renderer as the live `fallow fix` command (`render_fix_summary`) and
/// carry `summary-fix.jq`'s sections.
#[test]
fn render_summary_fix_mirrors_render_fix_summary() {
let env = fix_envelope();
let via_dispatch = render_summary(EnvelopeKind::Fix, &env, &LinkContext::default());
let direct = fallow_cli::report::github_summary::render_fix_summary(&env);
assert_eq!(
via_dispatch, direct,
"EnvelopeKind::Fix must reach render_fix_summary"
);
// The summary-fix.jq sections: heading, headline, count table, details.
assert!(
via_dispatch.starts_with("## Fallow - Auto-fix"),
"{via_dispatch}"
);
assert!(
via_dispatch.contains("**Dry run**: would apply **3 fixes**"),
"{via_dispatch}"
);
assert!(
via_dispatch.contains("skipped 1 file(s) that changed since analysis"),
"{via_dispatch}"
);
assert!(
via_dispatch.contains(
"kept exports in 2 file(s) where consumers may be hidden from static analysis"
),
"{via_dispatch}"
);
assert!(
via_dispatch.contains("| Export removals | 2 |"),
"{via_dispatch}"
);
assert!(
via_dispatch.contains("| Dependency removals | 1 |"),
"{via_dispatch}"
);
assert!(
via_dispatch.contains("<summary>View details</summary>"),
"{via_dispatch}"
);
assert!(
via_dispatch.contains("- `src/api/client.ts:42` - `unusedFn`"),
"{via_dispatch}"
);
assert!(
via_dispatch.contains("- `left-pad` from dependencies in `package.json`"),
"{via_dispatch}"
);
}

/// The bundled action no-ops annotations for the fix command, so the native
/// `EnvelopeKind::Fix` annotation renderer must emit nothing.
#[test]
fn render_annotations_fix_is_empty() {
let rendered = render_annotations(EnvelopeKind::Fix, &fix_envelope(), &plain_options());
assert_eq!(rendered, "", "fix annotations must be empty: {rendered}");
}

#[test]
fn github_summary_combined_snapshot() {
// Populated link context exercises the blob-URL branch of the dupes file
Expand All @@ -674,6 +737,7 @@ fn summary_has_no_em_dashes() {
(EnvelopeKind::Audit, audit_envelope()),
(EnvelopeKind::Security, security_envelope()),
(EnvelopeKind::Combined, combined_envelope()),
(EnvelopeKind::Fix, fix_envelope()),
] {
let summary = render_summary(kind, &envelope, &LinkContext::default());
assert!(!summary.contains('\u{2014}'), "em dash in {kind:?} summary");
Expand Down
Loading