Clear the 482 inherited phpcs findings in lib/ - #2099
Closed
rubenvdlinde wants to merge 1 commit into
Closed
rubenvdlinde wants to merge 1 commit into
rubenvdlinde wants to merge 1 commit into
Conversation
Every one of these was already on development and reproduces against the programme base 778e220. Three categories, all fixed in the code rather than suppressed or baselined. 346 RequireNamedParameters. Rewritten by resolving each callee's real signature from the AST, never from the parameter name a reader would guess. The 5 parent::__construct() calls into OCP\AppFramework\Controller were resolved by parsing the vendored stub, because reflection cannot autoload it. PHPStan and Psalm are the oracle here: a named argument aimed at a name that does not exist is a fatal at runtime and phpcs passes it. 116 DisallowInlineIf. Each ternary is hoisted into an if/else over a named local, or into a small private helper where the same shape repeated. No behaviour moved across a try boundary that could throw. 20 others: ImplicitTrue, two lowercase inline comments and one line over 150 characters. One of these was nearly a behaviour change. FileMigrationSource::read() built its records from the mapping's own kind, not from the kind the caller asked for, and hoisting the expression made the two easy to confuse. Two tests now pin it: the existing suite could not tell them apart because every case passed the same value on both sides. Verified: phpcs 482 errors to 0, PHPStan clean, Psalm clean, 3687 tests pass. phpmd is unchanged at 123, which is the next PR. Refs the round 2 parity programme.
rubenvdlinde
requested review from
WilcoLouwerse,
bbrands02 and
rjzondervan
as code owners
September 19, 2026 15:34
Member
|
PR was superceded with fixes from #2065 |
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
2 participants
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.
What this does
integriq carried 482 phpcs errors on
development. This clears all of them in the code. No baseline was regenerated, no suppression was added, andphpcs.xmlis untouched.They are ours: eleven PRs from this programme merged them to
developmentbefore the integration branch existed, and every one reproduces against the programme base778e2208.Three categories
346
RequireNamedParameters. The sniff only fires on calls into our own code:$this->,self::/static::/parent::, andnew OurClass(). Each one was rewritten by resolving the callee's real signature out of the AST, never from the parameter name a reader would guess. Fiveparent::__construct()calls intoOCP\AppFramework\Controllerneeded the vendored stub parsed, because reflection cannot autoload it.This is the category that bites. A named argument aimed at a name that does not exist is a fatal at runtime, and phpcs passes it happily. PHPStan and Psalm are the oracle, and both are clean.
116
DisallowInlineIf. Each ternary is hoisted into an if/else over a named local, or into a small private helper where the same shape repeated (requestOf,arrayOrEmpty,outcomeOf,describe). Nothing moved across a try boundary that could throw.20 others. 17
ImplicitTrue, two inline comments starting lowercase, one line over 150 characters.One near miss, now pinned
FileMigrationSource::read()builds its records from the mapping's kind, not from the$kindthe caller asked to read. Hoisting($mapping->getKind() ?: 'row')out of the call made the two trivially easy to swap, and the existing suite could not tell them apart: every test passed'case'on both sides.Two tests now separate them. Mutation checked: swapping the source of the value reddens
assertSame('besluit', $records[0]->getKind()), not a setup line.Verified locally
composer phpcs: 482 errors to 0, exit 0vendor/bin/phpstan: no errorsvendor/bin/psalm: no errorsvendor/bin/phpunit: 3687 tests, 12606 assertions, all passcomposer phpmd: 123, unchanged from base. That pile is the next PR.Inherited findings left alone
phpmd stays at 123 and
check:schema-l10nstays over its baseline. Both get their own PR rather than riding along here.🤖 Generated with Claude Code