# ADR 0018: destinations — the module at 0% coverage was dead, and every internal link with it

- **Status:** Accepted
- **Date:** 2026-08-16
- **Review item:** the open item left by ADR 0017, *"`destinations.rs`
  remains at 0% coverage and has no floor; it needs tests before it needs a
  floor"*
- **Supersedes:** nothing
- **Related:** ADR 0017 (declared-versus-delivered; this is the same class of
  defect, found by following its open item), ADR 0012 (refusing dangerous
  actions rather than performing them), ADR 0004 / ADR 0006 (deep-resolve
  destroying the references that identify things)

## Context

ADR 0017 recorded `destinations.rs` at 0% line coverage. The obvious reading
is "an untested module". The real one is worse.

**Nothing called it.** The module was `pub use`d from `lib.rs` and referenced
from nowhere else in the workspace:

```
$ grep -rn 'destinations::|GoToAction|DestinationKind' --include='*.rs' .
./pdf-document/src/lib.rs:23:pub use destinations::{Destination, GoToAction};
```

0% coverage was not a gap in the tests. It was the *symptom* of dead code,
and the thing it was supposed to do was not being done by anything else.

### Internal links did nothing

`PdfAnnotation::from_dict` read a link's target as:

```rust
dest: dict.get_name("Dest").map(|s| s.to_string()),
```

A **name** `/Dest`, and nothing else. Not `/Dest [4 0 R /Fit]` (an explicit
array), and not `/A << /S /GoTo /D ... >>` — which is how internal links are
written in practically every real document.

The corpus already contained one, in `annotations/links.pdf`, and had since
Phase 6:

```
11: << /Type /Annot /Subtype /Link /Rect [72 660 272 680]
      /P 3 0 R /A << /S /GoTo /D [4 0 R /Fit] >> >>
```

Probing it before any change:

```
Link { uri: Some("https://example.org/docs"), dest: None }  -> OpenUri(...)
Link { uri: None, dest: None }                              -> action=None
```

`action=None`. Clicking that link did nothing. No error, no warning — the
viewer received no action and correctly performed none. A link to nowhere
and a link the reader cannot parse are indistinguishable from the outside,
which is exactly the failure mode ADR 0017 is about, one layer up.

The viewer was *already wired* for this: `PdfAction::GoToPage` exists in
`interaction.rs`, is matched in `test_host.rs`, and was **never constructed
by anything**. A complete delivery path with nothing at the source end.

### Why no test caught it

The corpus had one internal link and no test asserted where it went.
`annotation_roundtrip.rs` and `corpus.rs` checked that link annotations
*parsed* and that their rectangles were right. Both passed. The assertion
that was missing is the one that names a page number.

## Decision

### Make the module real, and call it

`destinations.rs` now parses all three forms a destination is legally
written in (§12.3.2): an explicit array, a name, a byte string, and the
`/D`-wrapped dictionary a name tree leaf uses. `Destination::from_object`
accepts any of them.

`XYZ` carries `Option<f64>` per component, because **null is meaningful and
only meaningful there**: it means "leave this unchanged". Collapsing null to
`0.0` is not a harmless default — it scrolls to the origin, and for `zoom`
it requests 0% magnification. A zoom of literal `0` means the same as null
(Table 151) and is normalised to `None`, so a viewer cannot render at zero.

An unrecognised fit degrades to `/Fit` rather than failing: losing the
framing is recoverable, losing the page is not.

### Resolve names where the catalogue is in reach

`PdfDocument` gained `lookup_named_destination`, `resolve_destination` and
`named_destinations`. Both spellings are searched, because both are current:
the `/Names /Dests` **name tree** (§7.9.6, walked through `/Kids`) and the
pre-1.2 `/Root /Dests` **dictionary**. Supporting only the modern one loses
every link in an older file.

Lookup uses a deliberate `resolve_shallow`, not `resolve`. Deep-resolving a
destination array replaces `[4 0 R /Fit]` with the page *dictionary* and
destroys the only thing identifying the target — the same defect that once
emptied every AcroForm (ADR 0006) and every annotation reference (ADR 0004).
Mutation M8 below confirms this is load-bearing, not superstition.

Destinations are resolved in `page_annotations`, so annotations reach the
viewer already carrying a page index. An unresolvable one is **left
unresolved**, never defaulted to page 0: silently landing on the first page
is the worst outcome, because it looks like the link worked.

### Refuse by verb

`GoToAction::from_dict` requires `/S` to be `GoTo`. The previous version
ignored `/S` entirely and read `/D` from whatever it was given, so a
`/GoToR` (another *file*), a `/Launch` (a program) or a `/JavaScript`
carrying a `/D` was reported as an in-document page jump. Same policy as ADR
0012: refuse by name, do not silently perform.

## Verification

Every claim checked by running it.

**Before:** `action=None` for the corpus's only internal link.
**After:** `action=Some(GoToPage { page_index: 1 })`.

**Mutation testing — seven mutations, all killed:**

| # | Mutation | Result |
|---|---|---|
| M1 | `/S` verb check removed | **1 fails** — `/Launch` became `GoToPage { page_index: 1 }` |
| M2 | null in `XYZ` read as `0.0` | **1 fails** |
| M3 | zoom `0` not normalised | **1 fails** |
| M4 | name tree `/Kids` not descended | **2 fail** |
| M5 | legacy `/Root /Dests` fallback removed | **1 fails** |
| M6 | unresolvable name defaults to page 0 | **1 fails** |
| M7 | `/Dest` on the annotation ignored | **4 fail** |
| M8 | `resolve_shallow` → deep `resolve` | **5 fail** |

**M1 was a false negative on the first attempt, and that is worth
recording.** The first run reported it surviving. It had not survived — the
Python replacement string omitted an interleaved comment, so the patch never
applied and I measured the *unmutated* build. Re-run with an `assert old in
s` guard, it failed as it should. A mutation harness that does not verify
the mutation was applied reports "test is weak" when the truth is "the test
never ran against changed code", and the conclusion — deleting a real
security check as untested — would have been actively harmful. Every
mutation here now asserts its own application.

**Coverage:** `destinations.rs` **0% → 98.65%**; total 83.42% → **83.86%**.
Floors added for `destinations.rs` (94) and `annotations.rs` (70), and the
new floor verified to fail when raised above measurement.

**Suite:** `TEST_TARGET=pdf ./tools/test-rust-clean.sh` → **724 passed, 0
failed** (was 695; +29).

## Merge criteria

- [x] `destinations.rs` is called by production code, not only exported
- [x] Explicit array, name, string and `/D`-wrapped forms all parse
- [x] All eight fit kinds parse to distinct values, asserted individually
- [x] `null` in `XYZ` reads as "unchanged", not `0.0`
- [x] Zoom `0` normalised to "unchanged"; coordinate `0` kept as `0.0`
- [x] Named destinations resolved through a nested `/Names /Dests` tree
- [x] Legacy `/Root /Dests` dictionary resolved
- [x] `/GoToR`, `/Launch`, `/JavaScript` refused by verb even when carrying
      a `/D`
- [x] An unresolvable destination never becomes page 0
- [x] A cyclic name tree terminates
- [x] Corpus-wide invariant: no link resolves past the end of its document
- [x] Five `destinations/` fixtures generated by `generate.py`
- [x] Seven mutations, each verified to apply and each killed
- [x] Coverage floor added for `destinations.rs`, verified to fail when
      breached
- [x] `TEST_TARGET=pdf ./tools/test-rust-clean.sh` green (724)
- [x] `cargo fmt --check` and `clippy -D warnings` clean

## Consequences

**Positive.** Internal navigation works: table-of-contents links,
cross-references and outline targets now reach the viewer as page indices.
The `GoToPage` path that existed end-to-end but was never fed is now fed.
A dead module is either alive or gone.

**Negative.** `AnnotationType::Link` changed shape — `dest: Option<String>`
became `destination: Option<Destination>` — and `AnnotationAction` gained
`GoToDestination`. Both are public. The old field could not express an
explicit destination, so keeping it would have meant keeping the bug.

`AnnotationAction` also lost its `Eq` derive, because a destination carries
`f64` coordinates. Deriving `Eq` would have forced those coordinates to be
dropped or rounded, which is the wrong trade for a comparison nothing needs.

**Risk.** `resolve_destination` is called for every link annotation on every
`page_annotations`, adding a name-tree walk per named link. The tree is
traversed linearly rather than binary-searched: correctness first, and these
trees are small. If a document with thousands of named links ever shows up
in a profile, the ordered `/Limits` make a binary search straightforward.

**Out of scope, deliberately:** `/GoToR` remote navigation (parsed and
refused, not followed — acting on it means opening another file, which is a
host decision under rule 5), outline `/Outlines` trees, `/OpenAction` on the
catalogue, and structure-destination `/SD` preference. Each is a separate
feature with its own risk, not a detail of this one.