Folio rename decl/rename tests never exercise a non-contact-length leaf - #196
Merged
mikebronner merged 2 commits intoJun 16, 2026
Merged
Conversation
…-length leaf Every Folio decl/rename test seeds a `contact` leaf (7 chars), so they all re-read a span that coincides with the `NAME_START`/`NAME_END` constants — a span-computation bug for a different name length would still pass green. Add two tests driving the production decl logic with a 9-char `dashboard` leaf, asserting the *literal* columns the page body dictates: - `decl_range_at_returns_the_page_name_span_for_a_non_contact_length_leaf` asserts the prepare_rename range covers columns 12..21. - `collect_targets_rewrites_a_non_contact_length_leaf_declaration` asserts the EditTarget's start_column == 12 and end_column == 21. Neither references NAME_START/NAME_END, so a span regression for another name length now fails red instead of passing by coincidence. seed_page already derives the body from the leaf (PR #190), so no helper change is needed. Fixes #191
mikebronner
marked this pull request as ready for review
June 16, 2026 18:08
There was a problem hiding this comment.
✅ Approved
Review Summary
- Test-only PR (+86 −0): two new
#[tokio::test]s inlaravel-lsp/src/tests/folio_rename.rsthat drive the Folio decl/rename logic against a 9-chardashboardleaf instead of the 7-charcontactleaf every other test uses — closing the span-length blind spot #191 flagged. - AC #1 — met (by existing state, noted divergence). The AC anticipated
seed_pagebeing updated to derive the body from the leaf; that update already landed in PR #190 (closing #188).seed_pagewritesformat!("<?php name('{leaf}'); ?>")(folio_rename.rs:91) from the leaf at:88, so no change was needed here — the new tests confirm it producesname('dashboard')for adashboardleaf, and the pre-existingcontact/admin.contacttests stay green. Intent satisfied; flagging the divergence for the record. - AC #2 — met.
decl_range_at_returns_the_page_name_span_for_a_non_contact_length_leafasserts the returnedRangeis startcharacter: 12, endcharacter: 21as hardcoded literals, notNAME_START/NAME_END. Column math checks out: in<?php name('dashboard'); ?>the 9-char content sits at cols 12..=20, quote-excluded span 12..21. - AC #3 — met.
collect_targets_rewrites_a_non_contact_length_leaf_declarationassertst.start_column == 12andt.end_column == 21(literals), plustargets.len() == 1,file_path,line == 0, andnew_text == "home". Call convention matches the siblingcollect_targets_rewrites_the_page_name_declaration. - AC #4 — met. CI green (LSP — test, fmt, clippy passed), and the review lenses independently ran
cargo test -p laravel-lsp folio_rename— all 14 tests pass, including the two new ones.
The tests are honest regression guards: because they assert the literal 12/21 columns rather than the contact-sized constants, a span-computation bug for any other name length would fail red — exactly the gap #191 set out to close.
📋 Non-blocking follow-ups
- None.
Ready for @mikebronner to merge.
mikebronner
deleted the
chore/191-folio-rename-decl-tests-non-contact-leaf
branch
June 16, 2026 18:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements #191 — closes the span-length blind spot in the Folio rename test suite.
Every test that drives the production decl/rename logic seeded a
contactleaf (7 chars), so the re-readname('...')span always coincided with theNAME_START/NAME_ENDconstants. A span-computation regression for a different name length would have passed green. This adds two tests that drive the same production functions against a 9-chardashboardleaf and assert the literal columns the page body dictates.Changes
decl_range_at_returns_the_page_name_span_for_a_non_contact_length_leaf— drivesdecl_range_atwith adashboardleaf, asserting the prepare_renameRangecovers columns12..21(quote-excluded), not the constants.collect_targets_rewrites_a_non_contact_length_leaf_declaration— drivescollect_route_declaration_targetswith the same leaf, assertingstart_column == 12andend_column == 21on the returnedEditTarget.seed_page: it already derives the body'sname('<leaf>'), filename, and uri from the leaf (merged PR seed_page still hardcodes contact-specific page content and uri that don't track route_name #190 / issue seed_page still hardcodes contact-specific page content and uri that don't track route_name #188), so it produces a consistentdashboardfixture for any length and all existingcontact-seeding tests pass unchanged.Acceptance Criteria
seed_pagewrites<?php name('<leaf>'); ?>derived from the actual leaf segment, so body and route-index key stay consistent for any leaf length; existingcontact/admin.contacttests pass unchanged. (Already satisfied by PR seed_page still hardcodes contact-specific page content and uri that don't track route_name #190 — verified, no change needed.)#[tokio::test]drivesdecl_range_atwith a non-contact-length leaf (dashboard, 9 chars), asserting the returnedRangecovers columns12..21, NOT theNAME_START/NAME_ENDconstants.#[tokio::test]drivescollect_route_declaration_targetswith the same leaf, assertingstart_column == 12andend_column == 21on the returnedEditTarget.cargo test -p laravel-lsp folio_renamepasses with all tests green (14 tests: 2 new + 12 pre-existing).Test Plan
cargo test -p laravel-lsp folio_rename— 14 passed, 0 failed.cargo fmt --check— clean.cargo clippy --tests -p laravel-lsp— no new lints.Fixes #191