Let FinderOptions own its path invariants

[PiotrRogulski]

Follow-up to the normalization thread on #61.

FinderOptions makes rootPath and analysisRootPath absolute and normalized,
so nothing else in the process normalizes them. Ciach.run rejects an analysis
root that doesn't contain the scanned one, before it starts the analysis server.

That check is the part that isn't only tidying. It lived only in
bin/ciach.dart, so a library caller could pass an analysis root that doesn't
contain the scanned path and get references counted from the wrong tree, with
nothing to say so.

One normalization, one rule

Prompted by Komoszek's review:

• bin/ciach.dart builds the FinderOptions before it validates, so its checks,
  --verbose and the run all read the same paths. ResolvedOptions no longer
  normalizes separately — absoluteRootPath and absoluteAnalysisRootPath are
  gone.
• analysisRootContains in lib/src/paths.dart holds the containment rule. The
  CLI and run both call it: the CLI so a typo gets a usage message and exit 2
  instead of the catch-all's stack trace, run for every other caller.

--verbose now reports what the run uses, relative input included:

$ cd /tmp/pathdep && ciach pkgs/core --analysis-root . -v
[  0.0s]   path: /tmp/pathdep/pkgs/core (command line)
[  0.0s]   analysis-root: /tmp/pathdep (command line)

Why run and not the constructor

The constructor only normalizes; validation sits where the roots are used. It
also throws rather than asserting, which matters more than it looks: asserts only
run under --enable-asserts, which dart test sets and nothing else does.

$ dart run probe.dart                   # asserts off — the default
BAD: constructed with an analysis root beside the scanned one
$ dart run --enable-asserts probe.dart
Unhandled exception: Failed assertion: … 'must contain'

An assert would have passed its own tests while doing nothing for dart run ciach, for a compiled binary, or for the library consumer it exists to protect.

The const cost

The constructor is no longer const — p.absolute isn't a constant expression.
Nothing in the repo constructs FinderOptions as const; it is exported from
package:ciach/ciach.dart, so this is a public break, pre-1.0.

Testing

2 new tests, 278 passing; dart analyze and dart format clean repo-wide. They
cover relative paths coming back absolute and normalized, and a run with an
analysis root beside the scanned one throwing. An analysis root equal to the
scanned one is already covered by the existing run-level test.

Because the CLI's validation moved below the SDK lookup, I re-ran each path by
hand: the containment error, a missing analysis root, a missing scan path,
--verbose, and a working run.

$ ciach pkgs/core --analysis-root pkgs/app
The analysis root must contain the analyzed path: …/pkgs/app does not contain …/pkgs/core.   # exit 2
$ ciach pkgs/core --analysis-root /nope
Analysis root does not exist: /nope                                                          # exit 2

Worth noting for a separate change: assert(concurrency > 0) in the same
constructor is inert for the same reason, and I left it alone rather than widen
this PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SYSJaEbjNiw5KAMXty4Cok

[Komoszek]

Now the normalization and validation of absolutne root path happens twice when using CLI. Maybe we could just remove the validation from the ciach.dart? Also, verbose displays options from the ResolvedOptions not from the actual FinderOptions and those might not point to the same values. I belive we should unify those options

[PiotrRogulski]

Both correct. Details I checked, then what I'd do about each.

The duplicate containment check. Only containment is duplicated — the CLI's existence check has no counterpart in run. Deleting the CLI's check outright costs a lot of polish: a typo'd --analysis-root would fall through to the catch-all in bin/ciach.dart, which prints Failed to analyze: $e and the stack trace, for what is a plain usage error.

The two checks also aren't quite the same thing. The CLI's is for the person typing the command; run's is for any caller, including the library consumer this PR is about. What's genuinely duplicated is the rule, not the enforcement. So I'd rather extract the predicate and have both call it than drop either:

bool analysisRootContains(String analysisRoot, String rootPath) =>
    p.equals(analysisRoot, rootPath) || p.isWithin(analysisRoot, rootPath);

One definition, CLI keeps its message and exit 2, run keeps the backstop.

verbose vs the real options. You're right, and it's the more interesting one. describeSettings reads resolved.absoluteRootPath / absoluteAnalysisRootPath, which is a second normalize(absolute(…)) independent of the one FinderOptions now does. They agree today because both call the same two functions on the same input — nothing enforces that.

The smallest fix that actually unifies: have bin/ciach.dart build FinderOptions first, use its rootPath/analysisRootPath for its own checks and for --verbose, then hand it to Ciach. ResolvedOptions loses its absolute getters, and there is exactly one normalization in the process. That reorders the flow in _run a little, since describeSettings currently runs before the options are built.

Both are follow-on changes to a PR that is green and already a bit wider than the thread that spawned it. @PiotrRogulski — happy to fold either or both in here, or take them as the next PR; your call.

---

Generated by Claude Code

[PiotrRogulski]

Both done in 326d27a.

The rule has one definition. analysisRootContains in lib/src/paths.dart; the CLI and Ciach.run both call it. The CLI keeps enforcing it so a typo still gets the usage message and exit 2 rather than the catch-all's stack trace, and run keeps it for every other caller.

One normalization per process. bin/ciach.dart now builds the FinderOptions before it validates, and its own checks, --verbose and the run all read that object's paths. ResolvedOptions.absoluteRootPath and absoluteAnalysisRootPath are gone — there was nothing left for them to do.

So --verbose reports what the run actually uses, including for relative input:

$ cd /tmp/pathdep && ciach pkgs/core --analysis-root . -v
[  0.0s]   path: /tmp/pathdep/pkgs/core (command line)
[  0.0s]   analysis-root: /tmp/pathdep (command line)

Re-checked all of it after the reorder, since the validation moved below the SDK lookup: containment error, missing analysis root, missing scan path, --verbose, and a working run. 278 tests, analyze and format clean.

---

Generated by Claude Code