Skip to content

fix(phpcs): clear the 2 PHPCS errors in lib/Controller - #133

Merged
rubenvdlinde merged 1 commit into
developmentfrom
fix/phpcs-errors
Aug 17, 2026
Merged

fix(phpcs): clear the 2 PHPCS errors in lib/Controller#133
rubenvdlinde merged 1 commit into
developmentfrom
fix/phpcs-errors

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

What

Clears the 2 PHPCS errors in lib/Controller. Errors only — the warnings are left alone.

The fleet's shared quality gate maps phpcs exit 1 to success, so these have been shipping silently. Raw composer phpcs exit goes 1 → 0 on this branch.

Base is development, not this repo's default branch (main).

before after
PHPCS errors 2 0
PHPCS warnings 470 468 — see below, both were false
raw phpcs exit 1 0

The errors, and what was done

Both are PEAR.Commenting.FunctionComment.WrongStyle, in HealthController::index() and PageController::publicShare(). Both are the same shape:

	 */                                  ← the real docblock ends here
	#[PublicPage]
	#[NoCSRFRequired]
	// Liveness probe — no credential, so a ceiling and no counter.   ← this
	#[AnonRateLimit(limit: 120, period: 60)]
	public function index(): JSONResponse {

Because the // block is what immediately precedes the function, PHPCS reads that as the function comment and rejects its style — the /** */ docblock above it is never considered.

Fixed by folding the rate-limit rationale into the docblock it belongs to and keeping the attributes contiguous. No prose is deleted — every sentence is preserved verbatim in meaning, as a Rate-limit rationale: paragraph in the description.

Why the warning count drops by 2 — and why that is a correction, not suppression

The 2 removed warnings are CustomSniffs.Commenting.SpecTag.MissingMethodSpec, on exactly the 2 methods fixed here. Those warnings were false. Both methods already carried an @spec tag; the stray // block sat between the docblock and the function and hid the whole docblock from the sniff, the same mechanism that caused the error.

Evidenced three ways rather than asserted:

  • grep -c '@spec' is byte-identical before and after in both files (2 and 4). No tag was added, so no gap was papered over.
  • A warning set-diff keyed on file + sniff + message shows 0 added, and the 2 removed named individually.
  • A second, independent instrument agrees: composer lint:spec-annotations is a separate PHP script, not a PHPCS sniff, and it drops these same two methods off its missing-annotation list — with nothing added.

Blast radius

Documentation only. No executable statement is touched — 10 insertions / 7 deletions, all inside docblocks and the removed // lines. The #[AnonRateLimit] limits and periods are unchanged.

No phpcs:ignore, no <exclude>, no ruleset edit, no phpcbf/--fix. Every rule stays on.

Verification

Measured against a second pristine clone of development at the same commit (2bf1a6b), every check run in php:8.3-cli on both sides:

check:strict component base this branch
lint 0 0
lint:initial-state 0 0
lint:spec-annotations 1 1 — pre-existing, see below
lint:licenses 0 0
phpcs 1 0
phpmd 0 0
psalm 2 2 — identical 28-error set
phpstan 0 0
test:all 2 2

lint:spec-annotations stays red on both sides on its pre-existing lib/Db/* and lib/Migration/* backlog, which this branch does not touch. Its list gets strictly shorter here (the two controller methods above drop off); nothing is added.

PHPUnit compared by name, not by tally: 1575 tests / 3974 assertions / 16 errors on both sides, and the failing-test-name set is identical — 0 new, 0 fixed.

Note on the red that is already there: psalm and phpunit are red on both sides in this harness; pre-existing and identical on both sides, so parity holds. Not this branch's doing.

The shared quality gate maps `phpcs` exit 1 to success, so these have been
shipping silently. Raw `composer phpcs` exit goes 1 -> 0.

Both are PEAR.Commenting.FunctionComment.WrongStyle, and both are the same
shape: a real `/** */` docblock, then a `//` block explaining the
`#[AnonRateLimit]` choice, then the attribute, then the function. Because the
`//` block is what immediately precedes the function, PHPCS reads THAT as the
function comment and rejects its style.

Fixed by folding the rate-limit rationale into the docblock it belongs to and
keeping the attributes contiguous. No prose is deleted -- every sentence is
preserved, as a "Rate-limit rationale:" paragraph.

Side effect, deliberate and verified: this also removes 2
CustomSniffs.Commenting.SpecTag.MissingMethodSpec warnings (470 -> 468), and
drops the same two methods off `composer lint:spec-annotations`. Those reports
were false. Both methods already carried an `@spec` tag; the stray `//` block
sat between the docblock and the function and hid it. `grep -c '@SPEC'` is
identical before and after in both files -- no tag was added, and none was
suppressed. A warning set-diff confirms 0 warnings added.

Documentation only -- no executable statement is touched. Verified against a
second pristine clone of development: lint, lint:initial-state, lint:licenses,
phpmd, psalm, phpstan and phpunit are unchanged (1575 tests, identical
failing-test-name set). lint:spec-annotations stays red on its pre-existing
Db/Migration backlog, which this branch does not touch.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/launchpad @ c413177

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
format
composer ✅ 104/104
npm ✅ 525/525
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman
Playwright
Hydra gates

Quality workflow — 2026-08-17 04:12 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 2743cc9 into development Aug 17, 2026
47 of 68 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant