Skip to content

Make the quality gates pass again - #669

Merged
Spomky merged 2 commits into
4.2.xfrom
fix/qa-tooling
Aug 6, 2026
Merged

Make the quality gates pass again#669
Spomky merged 2 commits into
4.2.xfrom
fix/qa-tooling

Conversation

@Spomky

@Spomky Spomky commented Aug 6, 2026

Copy link
Copy Markdown
Member

Target branch: 4.2.x

  • It is a Bug fix
  • It is a New feature
  • It is related to dependencies

Includes:

  • Breaks BC
  • Deprecations

ECS and PHPStan had both drifted away from the tool versions in ghcr.io/spomky-labs/phpqa. This makes the three gates green again on 4.2.x.

Easy Coding Standard

ECS did not run at all: the configuration imported SetList::PHPUNIT and SetList::STRICT, two sets that no longer exist, and aborted before analysing anything.

  • Both imports are dropped. The three fixers STRICT used to bring (DeclareStrictTypes, StrictComparison, StrictParam) are already registered explicitly, and ECS now considers the PHPUnit set unsafe to run outside of Rector.
  • OrderedImportsFixer was registered without configuration, which silently discarded the class, function, const order that CLEAN_CODE had already set. That is the order the code base follows, so ECS reported a diff on 160 untouched files. It is now configured explicitly.
  • The remaining drift is fixed: single line comment spacing, stray trailing commas in the #[AsCommand] attributes, and the closing parenthesis of the multiline ones.

One fix had to be written by hand. On Chacha20Poly1305KeyEncryptionTest, the fixer rewrote

$tag[0] = $tag[0] ^ "\xff";   // into
$tag[0] ^= "\xff";

which PHP rejects with Cannot use assign-op operators with string offsets. Both byte flips now go through substr_replace(); the flipped byte is unchanged.

PHPStan

The ergebnis rule set is removed from the configuration, as asked. It accounted for 777 of the baseline entries, so the code base never followed it. phpstan-beberlei-assert goes with it: beberlei/assert is not used anywhere in src/.

The rest came from type declarations tightened by brick/math, Symfony and PHPStan since the baseline was written. They are fixed rather than baselined:

  • BigInteger, RSAKey, Math, AbstractECDH and KeyConverter reject the empty string before BigInteger::fromBase(), which now expects a non-empty-string.
  • Curve::generate(), JWKFactory::createOctKey() and RSACrypt keep their lengths positive, as random_bytes() and randomBits() now require.
  • crit and key_ops are checked to be lists of strings. Both are fed to array_diff() and implode(), which a nested array would have broken.
  • KeyConverter requires the key entry it reads, and KeyConverter\RSAKey no longer indexes an array with the result of array_search(), which can be false.

Two real defects surfaced on the way:

  • JsonConverter::decode() passed JSON_UNESCAPED_SLASHES and JSON_UNESCAPED_UNICODE to json_decode(). Both are encoding flags and were simply ignored.
  • JWEBuilder had a ?? null on a property that is always set.

The baseline is regenerated so that it describes what is actually left, without the entries that no longer match anything.

Result

Before After
ECS did not run no errors
PHPStan 50 errors no errors
Deptrac 0 violations 0 violations
PHPUnit 38 failures the same 38 failures

The 38 remaining failures are pre-existing, all in the bundle configuration tests, and untouched by this PR.

Note for reviewers: two caches survive a branch switch and will give you stale results. var/cache/test holds the test kernel container, and tmp-phpqa/phpstan holds the PHPStan result cache. Clear both when comparing branches.

Spomky added 2 commits August 6, 2026 22:22
The configuration imported SetList::PHPUNIT and SetList::STRICT, two sets
that no longer exist, so ECS aborted before analysing anything. Both are
dropped: the three fixers STRICT used to bring (DeclareStrictTypes,
StrictComparison, StrictParam) are already registered explicitly, and ECS
now considers the PHPUnit set unsafe to run outside of Rector.

The configuration also registered OrderedImportsFixer without any
configuration, which silently discarded the "class, function, const" order
that the CLEAN_CODE set had already set. That is the order the code base
actually follows, so ECS reported a diff on 160 untouched files. It is now
configured explicitly.

The remaining drift is fixed: single line comment spacing, stray trailing
commas in the #[AsCommand] attributes and the closing parenthesis of the
multiline ones.

Two byte flips in Chacha20Poly1305KeyEncryptionTest are rewritten with
substr_replace(). The fixer turned "$tag[0] = $tag[0] ^ ..." into
"$tag[0] ^= ...", which PHP rejects on a string offset with "Cannot use
assign-op operators with string offsets". The flipped byte is unchanged.
The ergebnis rule set is removed from the configuration, as asked. It was
producing 777 of the baseline entries, so the code base never followed it.
The phpstan-beberlei-assert extension goes with it: beberlei/assert is not
used anywhere in src/.

The rest of the errors came from the type declarations that brick/math,
Symfony and PHPStan itself have tightened since the baseline was written.
They are fixed rather than baselined:

- BigInteger, RSAKey, Math, AbstractECDH and KeyConverter now reject the
  empty string before handing it to BigInteger::fromBase(), which expects
  a non-empty-string.
- Curve::generate(), JWKFactory::createOctKey() and RSACrypt keep their
  lengths positive, as random_bytes() and randomBits() now require.
- "crit" and "key_ops" are checked to be lists of strings. Both are used
  with array_diff() and implode(), which a nested array would have broken.
- KeyConverter requires the "key" entry it reads, and KeyConverter\RSAKey
  no longer indexes an array with the result of array_search(), which can
  be false.

Two real defects surfaced on the way:

- JsonConverter::decode() passed JSON_UNESCAPED_SLASHES and
  JSON_UNESCAPED_UNICODE to json_decode(). Both are encoding flags and
  were ignored.
- JWEBuilder had a "?? null" on a property that is always set.

The baseline is regenerated so that it describes what is left, without
the entries that no longer match anything.
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