Repository navigation
Assorted small clean-ups - #170
Merged
Merged
Conversation
- Rename FqcnRemover::removeRqcn() to removeFqcn() (typo). - Remove StringUtils::endsWith(): unused in src, subtly wrong for an empty needle, and PHP 8 provides str_ends_with. - Register 'remove' as a real alias of remove-baseline-results. The documented 'sarb remove' only worked via Symfony's unambiguous abbreviation matching, so adding any future remove-* command would have silently broken every user's CI. - list-static-analysis-tools printed copy/paste commands that fail: both were missing the mandatory baseline-file argument. - --clean-up now picks its random sample from the issues that matched the baseline, as its help text says, rather than from all the latest issues (which included the new ones just reported). - Set the console application name and version: sarb --version printed 'Console Tool'. - InvalidOutputFormatterException now extends SarbException (the error reporter assumes all exceptions do). - Exclude value objects and exceptions from the service container sweep, so a newly type hinted value object fails at container compile time rather than at runtime. - Add an architecture rule: Domain must not depend on Symfony (it is currently clean; this stops erosion). - Fix the PhpstanJsonResultsParser docblock which claimed PHPStan's general errors are ignored (SARB stops with an error). Co-authored-by: Dave Liddament <dave.liddament@gmail.com>
DaveLiddament
force-pushed
the
chore/small-cleanups
branch
from
November 6, 2026 14:45
b0620ca to
a2d04e4
Compare
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.
A collection of small fixes from a codebase review — none change matching/baselining behaviour:
sarb removeis now a real alias ofremove-baseline-results. The documented short form only worked via Symfony's unambiguous-abbreviation matching, so adding any futureremove-*command would have silently broken every user's CI.list-static-analysis-toolsprinted commands that fail — both the create and remove examples were missing the mandatorybaseline-fileargument (verified: copy/pasting them yieldsNot enough arguments).--clean-upsampled from the wrong set — its help says "issues in the baseline to fix" but it sampled from all latest issues, including the brand-new ones just reported above it. Now samples from input-minus-pruned via a newPrunedResults::getBaseLinedResults().sarb --versionprinted "Console Tool" — the application now has a name and a version (viaComposer\InstalledVersions).FqcnRemover::removeRqcn()→removeFqcn()(long-standing typo).StringUtils::endsWith()removed — unused in src, subtly wrong for an empty needle, and PHP 8 hasstr_ends_with. (startsWithkept for now — it has many diff-parser callers and PR Fix git diff handling for non-default git configurations #162 touches those files.)InvalidOutputFormatterExceptionnow extendsSarbException—ErrorReporter's catch-all comment assumes every exception does; this was the only Domain exception outside the hierarchy.Domain\Commonvalue objects are excluded from the autowiring sweep, so a newly type-hinted value object fails at compile time instead of at runtime.Conflicts with open PRs
FqcnRemovercall site in the PHPStan parser and adds its ownPrunedResultsTest— whichever merges second needs a small rebase (I'm happy to do it).ErrorReportercatch arm near code this PR doesn't touch — should merge cleanly.Full
composer cipasses (100% coverage, PHPStan max incl. the new phpat rule, cs-fixer).