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
54 changes: 54 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,60 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
every `ModuleInfo`. Existing owned APIs remain compatible, and detector facts
are prepared before modules become immutable.

- **BREAKING: CI-facing formats now emit repository-root-relative paths when
`--root` is a subdirectory.** `codeclimate`, `review-github`, and
`review-gitlab` addressed files relative to `--root`, while
`github-annotations` already rebased onto the git toplevel. CI platforms
address files from the repository root, so GitLab's Code Quality widget
matched nothing and every inline review discussion was rejected when the
analyzed project lived in a package subdirectory. All CI formats now share
one namespace, detected via the git toplevel. Consumers that post-process
these paths themselves (prepending the offset in a wrapper script) should
drop that step or pass `--report-path-prefix ''` to restore the old output.
Single-package repositories, where `--root` is the toplevel, are unaffected.

- **`--annotations-path-prefix` is now `--report-path-prefix`.** It governs
every CI-facing format rather than only the GitHub-native ones. The old name
keeps working as an alias. An explicit empty value disables rebasing.

### Fixed

- **`--diff-file` no longer silently discards every source-anchored finding
when `--root` is a subdirectory.** `git diff` names paths relative to the
repository toplevel, but findings were keyed relative to `--root` before the
lookup, so in a monorepo package the two namespaces never met: every
source-anchored finding was dropped and the run reported a clean diff, exit
0, with nothing on stderr. A unified diff does not declare its own base, and
both conventions are real (`git diff --relative` writes `--root`-relative
paths), so fallow now resolves the base from the diff's paths themselves:
whichever candidate directory they actually name files under wins. Both
conventions work; when `--root` is the repository toplevel they coincide and
output is byte-identical to before.

- **`--diff-file` now scopes inline review comments, not only analysis results.**
That filter was gated on `FALLOW_DIFF_FILE`, so the flag rendered every comment
while appearing to have applied the diff.

- **A `--diff-file` whose paths name no file under the repository toplevel or
the analysis root now warns.** This class of mismatch previously produced a
plausible-looking empty report rather than an error. A diff whose paths name
real files under *both* candidates is likewise reported as ambiguous rather
than silently guessed, since existence alone cannot place it. In both cases the
diff is discarded and findings are reported at full scope: a path that cannot be
expressed in the diff's namespace is retained, never silently dropped.

- **Renamed files keep their `old_path` in `review-gitlab`.** The rename map is
keyed in the diff's namespace, but the lookup used the rendered path, so below
the repository toplevel (or under any `--report-path-prefix`) it missed and
`old_path` silently fell back to `new_path` -- telling GitLab a moved file had
not moved, and getting the discussion rejected.

- **`--report-path-prefix` no longer decides which inline review comments
survive the diff filter.** The review and sticky-summary filters keyed issues
by their rendered path, so a custom or empty prefix silently stopped matching
the diff and dropped every comment. They now key by the analysis-root-relative
path and the diff's own base; the prefix only affects how paths are rendered.

## [3.3.0] - 2026-07-09

### Added
Expand Down
4 changes: 2 additions & 2 deletions crates/api/src/duplication_filters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@

use std::path::{Path, PathBuf};

use fallow_output::{DiffIndex, relative_to_diff_path};
use fallow_output::DiffIndex;
use fallow_types::duplicates::{CloneInstance, DuplicationReport};

pub fn filter_by_diff(report: &mut DuplicationReport, diff_index: &DiffIndex, root: &Path) {
let instance_overlaps = |instance: &CloneInstance| -> bool {
let Some(rel) = relative_to_diff_path(&instance.file, root) else {
let Some(rel) = diff_index.key_for(&instance.file, root) else {
return true;
};
let start = u64::try_from(instance.start_line).unwrap_or(u64::MAX);
Expand Down
7 changes: 4 additions & 3 deletions crates/api/src/runtime/dead_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use fallow_engine::{
};
use fallow_output::{
CHECK_SCHEMA_VERSION, CheckOutputInput, DeadCodeNextStepsInput, DiffIndex, build_check_output,
build_dead_code_next_steps, check_meta, relative_to_diff_path,
build_dead_code_next_steps, check_meta,
};
use fallow_types::output_format::OutputFormat;
use fallow_types::path_util::is_absolute_path_any_platform;
Expand Down Expand Up @@ -334,10 +334,11 @@ fn apply_dead_code_scope(

fn filter_dead_code_by_diff(results: &mut AnalysisResults, diff: &DiffIndex, root: &Path) {
let touches_file = |path: &Path| -> bool {
relative_to_diff_path(path, root).is_none_or(|rel| diff.touches_file(&rel))
diff.key_for(path, root)
.is_none_or(|rel| diff.touches_file(&rel))
};
let line_in_diff = |path: &Path, line: u32| -> bool {
relative_to_diff_path(path, root)
diff.key_for(path, root)
.is_none_or(|rel| diff.line_is_added(&rel, u64::from(line)))
};

Expand Down
3 changes: 1 addition & 2 deletions crates/api/src/runtime/feature_flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::time::Instant;
use fallow_engine::{project_config::ProjectConfig, session::AnalysisSession};
use fallow_output::{
CHECK_SCHEMA_VERSION, FeatureFlagsOutputInput, build_feature_flags_output, feature_flags_meta,
relative_to_diff_path,
};
use fallow_types::output_format::OutputFormat;
use fallow_types::results::FeatureFlag;
Expand Down Expand Up @@ -115,7 +114,7 @@ fn apply_feature_flags_scope(
}
if let Some(diff) = resolved.diff.as_ref() {
flags.retain(|flag| {
relative_to_diff_path(&flag.path, session.root())
diff.key_for(&flag.path, session.root())
.is_none_or(|rel| diff.touches_file(&rel))
});
}
Expand Down
4 changes: 2 additions & 2 deletions crates/cli/src/audit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1529,8 +1529,8 @@ fn compute_decision_surface(
// or renames); lets each decision carry a rename-durable `previous_signal_id`.
let rename_old_path = |rel: &str| -> Option<String> {
crate::report::ci::diff_filter::shared_diff_index()
.and_then(|idx| idx.old_path_for(rel))
.map(str::to_string)
.and_then(|idx| idx.old_path_for_root_relative(rel))
.map(std::borrow::Cow::into_owned)
};

// Honest per-anchor consumer count, looked up from the map precomputed before
Expand Down
21 changes: 10 additions & 11 deletions crates/cli/src/check/filtering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,26 +39,26 @@ pub use fallow_engine::changed_files::{
/// the PR caused. Mirrors the default `FALLOW_SUMMARY_SCOPE=all` behavior
/// in typed PR-comment rendering.
///
/// `relative_to_diff_path` normalizes the finding's absolute path to the
/// forward-slashed key shape `git diff` writes (`+++ b/<path>`). When the
/// path cannot be expressed relative to `root` (different drive, traversal
/// escape), the finding is RETAINED rather than silently dropped: an
/// unfilterable path is better surfaced than silently hidden.
/// `DiffIndex::key_for` normalizes the finding's absolute path to the
/// forward-slashed key shape `git diff` writes (`+++ b/<path>`), relative to
/// the diff's own base rather than to `root` — the two differ whenever the
/// analysis root sits below the repository toplevel. When the path cannot be
/// expressed relative to that base (different drive, traversal escape), the
/// finding is RETAINED rather than silently dropped: an unfilterable path is
/// better surfaced than silently hidden.
pub fn filter_results_by_diff(
results: &mut fallow_types::results::AnalysisResults,
diff_index: &crate::report::ci::diff_filter::DiffIndex,
root: &Path,
) {
use crate::report::ci::diff_filter::relative_to_diff_path;

let touches_file = |path: &Path| -> bool {
match relative_to_diff_path(path, root) {
match diff_index.key_for(path, root) {
Some(p) => diff_index.touches_file(&p),
None => true,
}
};
let line_in_diff = |path: &Path, line: u32| -> bool {
match relative_to_diff_path(path, root) {
match diff_index.key_for(path, root) {
Some(p) => diff_index
.added_lines_in(&p)
.is_some_and(|set| set.contains(&u64::from(line))),
Expand Down Expand Up @@ -256,11 +256,10 @@ pub fn retain_gate_new(
diff_index: &crate::report::ci::diff_filter::DiffIndex,
root: &Path,
) {
use crate::report::ci::diff_filter::relative_to_diff_path;
use fallow_types::results::TraceHopRole;

let line_in_diff = |path: &Path, line: u32| -> bool {
match relative_to_diff_path(path, root) {
match diff_index.key_for(path, root) {
Some(p) => diff_index
.added_lines_in(&p)
.is_some_and(|set| set.contains(&u64::from(line))),
Expand Down
74 changes: 70 additions & 4 deletions crates/cli/src/cli_startup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ fn global_value_options() -> &'static [&'static str] {
"--group-by",
"--file",
"--sarif-file",
"--report-path-prefix",
"--annotations-path-prefix",
"--only",
"--skip",
"--dupes-mode",
Expand Down Expand Up @@ -318,22 +320,40 @@ pub fn run_pre_dispatch_checks(
return Err(fail(code, telemetry::FailureReason::Validation));
}

if cli.annotations_path_prefix.is_some()
if cli.report_path_prefix.is_some()
&& !matches!(
output,
fallow_config::OutputFormat::GithubAnnotations
| fallow_config::OutputFormat::GithubSummary
| fallow_config::OutputFormat::CodeClimate
| fallow_config::OutputFormat::ReviewGithub
| fallow_config::OutputFormat::ReviewGitlab
)
{
let code = emit_known_failure(
"--annotations-path-prefix is only valid with --format github-annotations or github-summary",
"--report-path-prefix is only valid with --format github-annotations, \
github-summary, codeclimate, review-github, or review-gitlab",
2,
output,
telemetry::FailureReason::Validation,
);
return Err(fail(code, telemetry::FailureReason::Validation));
}
report::github::set_annotations_path_prefix(cli.annotations_path_prefix.clone());
report::github::set_report_path_prefix(cli.report_path_prefix.clone());
// `init_report_prefix` shells out to `git rev-parse --show-toplevel`. Only
// the formats that read the resolved global (`report_prefix()`) need it:
// codeclimate applies it at the wire boundary, and review-{github,gitlab}
// apply it in the renderer, which has no `root` to re-derive it from. The
// github-native formats compute their rebase from `root` directly, so skip
// the probe for every other format.
if matches!(
output,
fallow_config::OutputFormat::CodeClimate
| fallow_config::OutputFormat::ReviewGithub
| fallow_config::OutputFormat::ReviewGitlab
) {
report::github::init_report_prefix(root);
}

parse_cli_tolerance(cli, output)
.map_err(|code| fail(code, telemetry::FailureReason::Validation))
Expand Down Expand Up @@ -464,6 +484,47 @@ fn parse_cli_tolerance(
})
}

/// Directories a supplied unified diff's paths might be relative to, most
/// preferred first.
///
/// `git diff` writes paths relative to the repository toplevel, while
/// `git diff --relative` writes them relative to the invoking directory. Both
/// reach fallow through `--diff-file` / `--diff-stdin`, and a unified diff does
/// not say which one it is, so the caller offers both and the paths decide (see
/// `choose_diff_base`). The two coincide for a single-package repo, which is why
/// keying against `--root` alone went unnoticed until `--root` addressed a
/// package inside a monorepo.
///
/// The toplevel is only used to measure how far `root` sits below it; the
/// returned base is that many components popped off `root` itself, so it keeps
/// `root`'s spelling. Finding paths are built from `root`, and a canonicalized
/// base would fail to prefix them wherever the two disagree (`/tmp` vs
/// `/private/tmp` on macOS).
fn diff_base_candidates(root: &Path) -> Vec<PathBuf> {
let Some(toplevel) = git_toplevel_base(root) else {
return vec![root.to_path_buf()];
};
if toplevel == root {
return vec![root.to_path_buf()];
}
vec![toplevel, root.to_path_buf()]
}

/// `root` with its offset below the git toplevel popped off, preserving
/// `root`'s spelling. `None` outside a git repo.
fn git_toplevel_base(root: &Path) -> Option<PathBuf> {
let toplevel = crate::base_worktree::git_toplevel(root)?;
let canonical_root = dunce::canonicalize(root).unwrap_or_else(|_| root.to_path_buf());
let offset = canonical_root.strip_prefix(&toplevel).ok()?;
let mut base = root.to_path_buf();
for _ in offset.components() {
if !base.pop() {
return None;
}
}
Some(base)
}

fn init_cli_diff_filter(
cli: &Cli,
root: &Path,
Expand All @@ -488,7 +549,12 @@ fn init_cli_diff_filter(
diff_source,
Some(report::ci::diff_filter::DiffSource::EnvVar(_)) | None
);
let _ = report::ci::diff_filter::init_shared_diff(diff_source.as_ref(), suppress_warnings);
let _ = report::ci::diff_filter::init_shared_diff(
diff_source.as_ref(),
root,
&diff_base_candidates(root),
suppress_warnings,
);
Ok(())
}

Expand Down
9 changes: 8 additions & 1 deletion crates/cli/src/combined/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -946,10 +946,17 @@ fn build_combined_codeclimate(
dupes: Option<&DupesResult>,
health: Option<&HealthResult>,
) -> serde_json::Value {
let all_issues = build_combined_codeclimate_issues(check, dupes, health);
let mut all_issues = build_combined_codeclimate_issues(check, dupes, health);
// Rebase at the wire boundary only: the same issues feed the sticky summary,
// whose diff filter matches on analysis-root-relative paths.
crate::report::codeclimate::rebase_codeclimate_paths(&mut all_issues);
codeclimate_issues_to_value(&all_issues)
}

/// Analysis-root-relative CodeClimate issues for every enabled analysis.
///
/// Deliberately un-rebased: the sticky summary filters these against the diff,
/// and the presentation prefix belongs at the wire boundary.
fn build_combined_codeclimate_issues(
check: Option<&CheckResult>,
dupes: Option<&DupesResult>,
Expand Down
4 changes: 1 addition & 3 deletions crates/cli/src/dupes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,8 @@ fn filter_by_diff(
diff_index: &crate::report::ci::diff_filter::DiffIndex,
root: &std::path::Path,
) {
use crate::report::ci::diff_filter::relative_to_diff_path;

let instance_overlaps = |instance: &fallow_types::duplicates::CloneInstance| -> bool {
let Some(rel) = relative_to_diff_path(&instance.file, root) else {
let Some(rel) = diff_index.key_for(&instance.file, root) else {
return true;
};
let start = u64::try_from(instance.start_line).unwrap_or(u64::MAX);
Expand Down
23 changes: 15 additions & 8 deletions crates/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,14 +360,21 @@ struct Cli {
#[arg(short = 'o', long, global = true, value_name = "PATH")]
output_file: Option<PathBuf>,

/// Prefix prepended to every `file=` path in `--format github-annotations`
/// output. GitHub resolves annotation paths against the repository root,
/// so when the analyzed project lives in a subdirectory (e.g.
/// `packages/app/`), paths need that offset. fallow detects the offset via
/// the git toplevel automatically; this flag overrides the detection.
/// Valid only with the GitHub-native formats.
#[arg(long = "annotations-path-prefix", global = true, value_name = "PREFIX")]
annotations_path_prefix: Option<String>,
/// Prefix prepended to every path in the CI-facing formats
/// (`github-annotations`, `github-summary`, `codeclimate`,
/// `review-github`, `review-gitlab`). CI platforms address files by
/// repository-root-relative path, so when the analyzed project lives in a
/// subdirectory (e.g. `packages/app/`), paths need that offset. fallow
/// detects the offset via the git toplevel automatically; this flag
/// overrides the detection. Pass an empty string to disable rebasing and
/// emit paths relative to `--root`.
#[arg(
long = "report-path-prefix",
visible_alias = "annotations-path-prefix",
global = true,
value_name = "PREFIX"
)]
report_path_prefix: Option<String>,

/// Fail if issue count increased beyond tolerance compared to a regression baseline.
#[arg(long, global = true)]
Expand Down
Loading
Loading