refactor: the inventory gate was right about both of them - #55
Merged
Conversation
Greenhouse `decisions/0213`'s gate flagged two new public members with no consumer but their own tests, and neither was a false positive. `RollbackContracts::unresolved()` was public and nothing outside its own tests called it: a caller wants the FINDINGS — what a report prints and what an agent can act on — and a second public shape for the same answer is a second thing to keep true. It is private now, and the tests assert through `findings()`. `EffectProfile::rollbackOperation()` looked unwired for a subtler reason: the gate counts a use only when the touching file also NAMES the member's owner, and this class reached the profile through `$operation->effects?->` without ever naming `EffectProfile`. The rule is a good one — a bare method name proves nothing — so the fix is to say what is being read: the profile is resolved to a typed local, checked with `instanceof`, and the class is imported. It reads better too; the double null-safe chain was asking the same question twice.
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
1 participant
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.
Greenhouse
decisions/0213's gate flagged two new public members with no consumer but their own tests, and neither was a false positive.RollbackContracts::unresolved()was public and nothing outside its own tests called it. A caller wants the FINDINGS — what a report prints and what an agent can act on — and a second public shape for the same answer is a second thing to keep true. Private now; the tests assert throughfindings().EffectProfile::rollbackOperation()looked unwired for a subtler reason: the gate counts a use only when the touching file also names the member's owner, and this class reached the profile through$operation->effects?->without ever namingEffectProfile. That rule is a good one — a bare method name proves nothing — so the fix is to say what is being read: the profile is resolved to a typed local, checked withinstanceof, and the class is imported. It reads better too; the double null-safe chain was asking the same question twice.207 tests green, phpstan clean, cs-fixer clean, docblock gate clean. The greenhouse inventory gate is back to «no creció» without re-recording anything.