test(checker): pin rename invalidation for transitive callers - #147
Conversation
- Replace Project::prev_return_slots with a has_prev_emit bool; the full RType vector was cloned every emit and only tested for presence. - compute_fixpoint_scope now calls with_transitive_callers instead of re-implementing its reverse-caller closure loop. - The shared test helper check_with_scope delegates to the public Checker::check_with_scope instead of replaying collect plus a manual fixpoint. - Drop the unused rationale field from Verdict in rule_evidence.rs; code and verdict are the asserted fields. - Drop the cfg(test) is_package_library_file helper and its single assertion, which re-tested ry-workspace classification. - Rename infer_schema_call's unused span parameter to _span. - State the current constraint in the filter-builder comment instead of the move history.
#90 flagged force_full_recollection for clearing collected_files before update_file could move old function names into invalidated_fns. #81 closed as a harness race; #93 deleted the function. The invalidation #90 asked for already lives in update_file from #58. The rename path had no regression test. Add one. A rename drops the old name from the new function table, so only the retained name seeds the reverse call graph. The test asserts a transitive caller matches a cold check. Reverting the invalidation locally fails it with the stale RY040 #90 describes.
|
Warning Review limit reachedNext included review available in 15 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
-
Reviewed the full diff of
crates/ry-checker/tests/project.rs— one new integration test,renamed_function_matches_cold_for_transitive_callers, pinning the rename-invalidation path from #90. -
Verified the test is real coverage, not theatre. I mutation-checked it locally: deleting the
invalidated_fns.extend(...)block inProject::update_file(crates/ry-checker/src/project.rs:196-200) makes the test fail with exactly the staleness #90 describes —top.RkeepsRY040 cannot apply arithmetic op to `character` and `integer`after the rename while the cold check has none. With the invalidation restored, the full--test projectsuite passes 22/22. -
Confirmed the coverage claim. No pre-existing regression test exercises the rename path; the closest sibling,
removed_function_matches_cold, covers removal with a direct caller only. The new test's transitive-caller chain (leaf.R→middle.R→top.R) subsumes the direct case, and the precondition assertion pins the pre-rename RY040 so the test can't silently degrade into comparing two empty result sets. -
Checked the test against file conventions. It follows the established
*_matches_coldbefore/update/incremental/cold pattern, including theArc::new(parse(...))update idiom and the doc comment citing the issue. No CHANGELOG entry needed for a test-only change.
openai-compatible/glm-5.3 ｜ 𝕏
…tion-invalidation
1 participant
Summary
#90 asked for a fix in
force_full_recollection: it clearedcollected_filesbeforeupdate_filecould move old function names intoinvalidated_fns. #90 also said to check #81 first.#81 closed as a harness race, not a checker defect. #93 then deleted
force_full_recollection. The invalidation #90 asked for already lives inupdate_fileandremove_file, added in #58: each extendsinvalidated_fnsfrom the removed pass-1 entry before the entry leaves the map.So this PR is test-only. The rename path had no regression test. I reverted the invalidation locally and ran the new test: it failed with the exact staleness #90 describes. The transitive caller kept
RY040 cannot apply arithmetic op to `character` and `integer`after the rename. With the invalidation restored, it passes.Changes
crates/ry-checker/tests/project.rs: addrenamed_function_matches_cold_for_transitive_callers.leaf.Rdefineshelper,middle.Rwraps it,top.Rcalls the wrapper. An update renameshelpertorenamed. The old name is absent from the new function table, so only the retained name seeds the reverse call graph. The test asserts the incremental result matches a cold check.Verification
cargo fmt -p ry-checkercargo clippy -p ry-checker --all-targets -- -D warningscargo test -p ry-checker— all 17 suites greenStacked on #142. Merge after it.
#90 item status
ry-analysisSetOpenFileversion comparisonry-analysis.ry-analysissnapshotHashMaporderingry-analysis.project.rsrecollection clearing ordercached_path_precedencetestbinary_source_precedencenow exercises the cache branch.p36_contractdrain loop stops at first timeoutcrates/ry-lsp/tests/protocol_contract.rs.check-ledger.pyper-key comparisoncompare_summarycompares each summary map against itsCounter.ry-cliforward resolved degraded scopesry checkprints them viaprint_degraded();dump-typesprints one line per scope.WorkspaceContext.degraded_scopeshas no consumer in the check path.Every item is resolved or moot.
Closes #90