Document the Span → TextRange boundary contract and defer exact identifier spans in ddlint-design.md

[coderabbitai]

Background

Raised during review of PR #248 (implement-unused-variable-diagnostics).

The design document docs/ddlint-design.md currently states that rule-local Symbol::span() values are coarse provenance (pointing to the enclosing rule or literal site). This is correct for the current implementation. However, nothing in the document makes explicit:

• that Span → TextRange conversion is the linter's responsibility, not the semantic model's;
• that the coarse semantics of Symbol::span() must not be silently changed when exact identifier locations become desirable;
• that exact identifier-token spans are a deferred milestone, not an oversight.

Without this written contract, the author of the next correctness rule may either duplicate the local conversion glue or quietly change Symbol::span() to point at the identifier token, breaking the coarse-provenance contract relied on by maps and debugging tooling.

Concern (Doggylump & Telefono)

Write the contract down before the next rule is started. One short addition to the design document is cheaper than a post-hoc migration.

Recommended change

Add a subsection to the relevant section of docs/ddlint-design.md (the semantic-model or diagnostic-rendering section) covering:

1. Conversion ownership: the linter crate owns Span → rowan::TextRange conversion. The semantic model stores Span as coarse provenance; it does not vend TextRange.
2. Stability of Symbol::span(): the existing span() accessor returns the coarse declaration or literal site. Its meaning must not change. If exact identifier-token location is required, a separate field (e.g. name_span or identifier_span) must be added additively.
3. Deferred milestone: exact identifier-token spans are a planned but deferred semantic-model enhancement. Rules written before that milestone use coarse spans; rules written after may opt in to the precise field without breaking earlier rules.

Example wording (illustrative, not prescriptive)

Diagnostic range conversion. The semantic model stores all span information as Span (byte-offset pairs). Converting a Span to a rowan::TextRange for use in a LintDiagnostic is the linter's responsibility and is performed by the shared helper in src/linter/support/span.rs.
Symbol::span() returns coarse provenance — the enclosing rule or literal site — and its semantics are stable. When precise identifier-token location is required, add an additive name_span field to the relevant semantic record rather than redefining span().
Exact identifier-token spans are a deferred milestone; see roadmap item TBD.

Acceptance criteria

• docs/ddlint-design.md contains an explicit statement that Span → TextRange conversion belongs to the linter.
• The document states that Symbol::span() semantics are stable and must not be silently changed.
• The document acknowledges exact identifier spans as a deferred milestone.
• The roadmap references the new milestone (or an existing entry is updated).

Related

• PR Implement unused-variable diagnostics (UnusedVariableRule) #248 — the gap this issue closes.
• Infrastructure issue: centralise Span → TextRange helper (see companion issue).
• Future issue: add exact identifier-token spans to semantic records.
• Raised by: @leynos (Logisphere crew review, PR Implement unused-variable diagnostics (UnusedVariableRule) #248)

[leynos]

@CodeRabbit plan

[coderabbitai]

Coding Plan

Summary

• Add a focused subsection (§3.2.2) to the design document codifying the conversion boundary, stability of Symbol::span(), and the additive pattern for precise spans
• Register exact identifier-token spans as a deferred but planned milestone in Phase 4 of the roadmap
• Correct the file path reference from the ticket's src/linter/support/span.rs to the actual location src/linter/span_utils.rs
• No code changes are required; this is purely a documentation exercise to prevent contract drift before the next correctness rule is written

Implementation Steps

Document the Boundary Contract and Roadmap Milestone

This phase adds the missing contract documentation to the design document and registers exact identifier-token spans as a deferred milestone in the roadmap. Both changes are purely documentation; no code modifications are required.

Task 1: Add Span → TextRange Boundary Contract Subsection to ddlint-design.md

Insert a new subsection ### 3.2.2. Span → TextRange boundary contract between line 764 (end of §3.2.1) and line 765 (start of §3.3) in docs/ddlint-design.md.

• Insert the new heading ### 3.2.2. Span → TextRange boundary contract after the blank line at line 764 and before ### 3.3. Initial lint rule catalog
• Open with a paragraph establishing conversion ownership: the semantic model stores Span (byte-offset pairs); converting to rowan::TextRange for LintDiagnostic is the linter's responsibility via the shared helper in src/linter/span_utils.rs
• State explicitly that Symbol::span() returns coarse provenance (enclosing rule or literal site) and its semantics are stable; do not silently redefine it
• Specify the additive pattern: when precise identifier-token location is required, add a new field (e.g. name_span or identifier_span) to the relevant semantic record rather than altering span()
• Conclude with a forward reference to the deferred milestone in the roadmap (docs/roadmap.md §4.1.3)

Task 2: Add Exact Identifier-Token Span Milestone to roadmap.md

Insert a new milestone entry 4.1.3 in Phase 4 of docs/roadmap.md, renumbering existing items 4.1.3 and 4.1.4 to 4.1.4 and 4.1.5 respectively.

• Insert the new entry under ### 4.1. Correctness rules, immediately after 4.1.2 (unused-variable diagnostics)
• Use the established format: - [ ] 4.1.3. Add exact identifier-token \name_span` field to semantic records.`
• Include a See clause pointing to the new design-doc section: See docs/ddlint-design.md §3.2.2.
• Optionally add sub-bullets noting that this field is additive and does not replace span()
• Renumber the existing 4.1.3 (shadowed-variable) to 4.1.4 and existing 4.1.4 (recursive-negation) to 4.1.5
• Update any Requires clauses in the file that reference the renumbered items

🤖 Prompt for AI agents

This phase is purely documentation — write no production code whatsoever. Edit
two Markdown files only.

**File 1: `docs/ddlint-design.md`**

Insert a new subsection immediately after line 764 (the blank line closing
§3.2.1) and before the existing `### 3.3. Initial lint rule catalog` heading:

- Add the heading `### 3.2.2. Span → TextRange boundary contract`
- Write an opening paragraph that establishes conversion ownership:
  - The semantic model stores `Span` (byte-offset pairs)
- Converting to `rowan::TextRange` for `LintDiagnostic` is the linter's
responsibility, performed via the shared helper in `src/linter/span_utils.rs`
- State explicitly that `Symbol::span()` returns coarse provenance (enclosing
rule or literal site) and that its semantics are stable — do not silently
redefine it
- Specify the additive pattern: when a precise identifier-token location is
required, add a new field (e.g. `name_span` or `identifier_span`) to the
relevant semantic record rather than altering `span()`
- Close with a forward reference to the deferred roadmap milestone:
`docs/roadmap.md §4.1.3`

**File 2: `docs/roadmap.md`**

Under `### 4.1. Correctness rules`, immediately after the `4.1.2`
(`unused-variable`) entry:

- Insert a new entry using the established GFM checkbox format: `- [ ] 4.1.3.
Add exact identifier-token \`name_span\` field to semantic records.`
- Append a `See` clause: `See docs/ddlint-design.md §3.2.2.`
- Optionally add sub-bullets clarifying that this field is additive and does not
replace `span()`
- Renumber the existing `4.1.3` (shadowed-variable) → `4.1.4` and the existing
`4.1.4` (recursive-negation) → `4.1.5`
- Scan the entire file for any `Requires` clauses referencing the old `4.1.3` or
`4.1.4` identifiers and update them to their new numbers

Research

The codebase is a Differential Datalog linter with a CST-first architecture. The design document (docs/ddlint-design.md) contains detailed sections on parsing, the rule ecosystem, diagnostics, and testing. Section 3.2.1 covers the semantic model for symbol tables and already discusses span provenance at a high level. The roadmap (docs/roadmap.md) follows a phased structure with dotted numbering and GFM checkboxes. The actual span conversion helper resides at src/linter/span_utils.rs (note: the ticket incorrectly references src/linter/support/span.rs). Symbol::span() returns &Span (a byte-offset Range<usize>), and LintDiagnostic stores rowan::TextRange.

---

🚀 Next Steps

🤖 All AI agent prompts combined

This phase is purely documentation — write no production code whatsoever. Edit
two Markdown files only.

**File 1: `docs/ddlint-design.md`**

Insert a new subsection immediately after line 764 (the blank line closing
§3.2.1) and before the existing `### 3.3. Initial lint rule catalog` heading:

- Add the heading `### 3.2.2. Span → TextRange boundary contract`
- Write an opening paragraph that establishes conversion ownership:
  - The semantic model stores `Span` (byte-offset pairs)
- Converting to `rowan::TextRange` for `LintDiagnostic` is the linter's
responsibility, performed via the shared helper in `src/linter/span_utils.rs`
- State explicitly that `Symbol::span()` returns coarse provenance (enclosing
rule or literal site) and that its semantics are stable — do not silently
redefine it
- Specify the additive pattern: when a precise identifier-token location is
required, add a new field (e.g. `name_span` or `identifier_span`) to the
relevant semantic record rather than altering `span()`
- Close with a forward reference to the deferred roadmap milestone:
`docs/roadmap.md §4.1.3`

**File 2: `docs/roadmap.md`**

Under `### 4.1. Correctness rules`, immediately after the `4.1.2`
(`unused-variable`) entry:

- Insert a new entry using the established GFM checkbox format: `- [ ] 4.1.3.
Add exact identifier-token \`name_span\` field to semantic records.`
- Append a `See` clause: `See docs/ddlint-design.md §3.2.2.`
- Optionally add sub-bullets clarifying that this field is additive and does not
replace `span()`
- Renumber the existing `4.1.3` (shadowed-variable) → `4.1.4` and the existing
`4.1.4` (recursive-negation) → `4.1.5`
- Scan the entire file for any `Requires` clauses referencing the old `4.1.3` or
`4.1.4` identifiers and update them to their new numbers

💡 Iterate on the plan with: @coderabbitai <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!