Skip to content

fix: Improve CI/CD reliability and add static analysis - #18

Merged
Strobotti merged 14 commits into
masterfrom
improve/ci-cd-workflows
Sep 5, 2026
Merged

Strobotti merged 14 commits into
masterfrom
improve/ci-cd-workflows

Conversation

@Strobotti

@Strobotti Strobotti commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Summary

A set of improvements to the GitHub Actions workflows and source code
quality, addressing several reliability issues in the CI and release
pipelines and introducing PHPStan static analysis.

Changes

Release pipeline (release.yaml)

  • Add fetch-depth: 0 to the checkout step — semantic-release needs
    the full git history to correctly calculate the next version number.
    Without it, a shallow clone can produce wrong or missing version bumps.
  • Replace php-actions/composer with the standard shivammathur/setup-php
    • explicit cache + composer install pattern, consistent with ci.yaml.
  • Add explicit permissions: contents/issues/pull-requests: write,
    required for the git plugin to commit back to master.
  • Install @semantic-release/changelog and @semantic-release/git via
    extra_plugins and wire them into .releaserc.json. Previously release
    notes existed only in GitHub Releases; CHANGELOG.md will now be
    written and committed to the repo on every release.
  • Remove the coverage badge generation step — it was a fragile side-effect
    that could block releases on gh-pages push failures (moved to ci.yaml).

CI pipeline (ci.yaml)

  • Move the coverage badge generation out of release.yaml into a
    dedicated coverage-badge job. The badge only runs on pushes to
    master (not PRs), depends on unit-tests passing, and can no longer
    block a release if the gh-pages push fails.
  • Add a static-analysis job running PHPStan on PHP 8.3, in parallel
    with unit-tests (both depend on dependency-validation). The job
    uses composer.lock as the cache key and composer install for a
    fully reproducible, pinned vendor state.
  • Add a comment explaining the PHP_CS_FIXER_IGNORE_ENV flag.

Static analysis (phpstan.neon, composer.json, Makefile)

  • Add phpstan/phpstan ^2.2 as a dev dependency.
  • Add phpstan.neon targeting src/ at level 6.
  • Add a phpstan target to the Makefile.

Source fixes (src/)

All findings from PHPStan level 6 resolved — no suppressions or
baselines used:

  • Uninitialized typed string properties given default '' values
  • openssl_pkey_get_public() and openssl_pkey_get_details() return
    values guarded against false
  • json_encode() and base64_decode() return values guarded against
    false; Base64UrlConverter::decode() now throws on failure rather
    than silently passing false as a string
  • __toString() return types tightened to string
  • is_array() guards added before iterating json_decode() results
  • Missing bool type hint added to $strict parameter in
    Base64UrlConverterInterface::decode()

Testing

Existing test suite is unchanged and should continue to pass across the
full PHP 7.4–8.5 matrix.

…dge step

- Add fetch-depth: 0 to checkout so semantic-release has full git
  history to correctly determine the next version
- Replace php-actions/composer with the same composer install pattern
  used in ci.yaml for consistency; use proper cache key per PHP version
- Remove the coverage badge generation step from the release pipeline;
  it was a fragile side-effect that could block releases on gh-pages
  push failures (will be moved to ci.yaml)
- Add explicit extensions and xdebug coverage config to setup-php
The badge step was previously in release.yaml, where a push failure
to gh-pages could block a release entirely. It now lives as a dedicated
'coverage-badge' job in ci.yaml that:
- only runs on pushes to master (not PRs), preventing concurrent
  gh-pages write conflicts
- depends on unit-tests passing, so the badge always reflects a
  green build
- uses the same composer install pattern as the rest of ci.yaml
Previously release notes only existed in GitHub Releases; CHANGELOG.md
was never written to the repo. This adds:
- @semantic-release/changelog: writes/updates CHANGELOG.md on each release
- @semantic-release/git: commits CHANGELOG.md back to master with a
  [skip ci] message to avoid triggering another release run

Also adds explicit `permissions: contents/issues/pull-requests: write`
to the release job, required for the git plugin to push back to the
branch, and installs the extra plugins via cycjimmy extra_plugins.
- Add phpstan/phpstan ^1.12 as a dev dependency (^1.x supports PHP 7.2+,
  keeping compatibility with the library's minimum PHP 7.3 requirement)
- Add phpstan.neon config targeting src/ at level 6
- Add 'phpstan' target to Makefile
- Add 'static-analysis' job to ci.yaml that runs in parallel with
  unit-tests (both depend on dependency-validation), using a single
  PHP 8.3 runner — no need to matrix static analysis across versions
- KeyInterface: tighten __toString() return to string (was bool|string)
- AbstractKey: initialise $kty and $alg to '' to satisfy string return
  types; cast json_encode() result in __toString(); add is_array() guard
  before iterating json_decode() result in createFromJSON()
- Rsa: initialise $n and $e to ''; add is_array() guard in createFromJSON()
- Base64UrlConverterInterface: add bool type hint to $strict parameter
- Base64UrlConverter: throw on false return from base64_decode() instead
  of silently returning false as string
- KeyConverter: extract base64_decode() result into a variable and throw
  on false before passing to BigInteger
- KeyFactory: add null-checks on openssl_pkey_get_public() and
  openssl_pkey_get_details() return values; guard json_encode() result
- KeySet: fix __toString() return type; use ?? '' for nullable getKeyId()
  array key; add return type annotation to jsonSerialize()
- KeySetFactory: guard json_encode($keyData) result before passing to
  createFromJson()
- composer.lock: update to include phpstan/phpstan ^1.12
Upgrade phpstan/phpstan from ^1.12 to ^2.2 and fix all 8 reported errors:

- AbstractKey::createFromJSON: remove new static() by making $prototype
  required (non-nullable); callers must always supply a concrete instance,
  eliminating the unsafe instantiation of an abstract class
- Rsa::createFromJSON: narrow return type from KeyInterface to self;
  always construct a new self() prototype when none is provided, removing
  the last new static() call; add @var self cast after parent call so
  PHPStan tracks the concrete type through the loop
- KeyFactory::createFromPem: add @PARAM array<string, mixed> to $options
- KeySet: add @implements IteratorAggregate<int, KeyInterface> on the
  class; add @return ArrayIterator<int, KeyInterface> to getIterator();
  remove the unused $keyFactory property, constructor injection, and
  setter (KeySet never used KeyFactory directly — that was KeySetFactory's
  responsibility)
- composer.lock: update for phpstan/phpstan ^2.2
…ency-validation

The php-actions/composer@v6 action fails with a git safe.directory
ownership error when run in GitHub Actions. Replace it with the same
composer install pattern used by all other jobs in this workflow
(shivammathur/setup-php + explicit cache dir + composer install).

Also update composer.lock to resolve phpstan/phpstan to 2.2.12,
matching the ^2.2 constraint added in the previous commit.
…ibility

The previous lock file was generated by the composer:2 Docker image
which runs PHP 8.4 internally. This caused it to resolve symfony/string
to v8.1.2 which requires PHP >=8.4.1, breaking composer install on the
PHP 8.3 runner in the dependency-validation job.

Regenerated on php:8.3-cli, which downgrades symfony/string to v7.4.15
(a transitive dependency of friendsofphp/php-cs-fixer), making the lock
file installable on all supported PHP versions in the matrix.
PHP's LSP enforcement requires that overriding methods use covariant
return types. Returning `self` (which resolves to `Rsa`) is not
compatible with the parent's declared return type of `KeyInterface`.
Changing to `static` satisfies the constraint since `static` is the
only narrowing return type PHP recognises as covariant in overrides.
…JSON

When no prototype is provided, instantiate the concrete subclass via
new static() instead of throwing InvalidArgumentException. This allows
subclasses (and test mocks) to call createFromJSON without supplying an
explicit prototype, while keeping the method safe since static resolves
to the concrete type at call time, never to the abstract AbstractKey.
- AbstractKey::createFromJSON: add @return static docblock and
  @var static annotation on $instance (clone), suppress the
  new.static PHPStan warning with inline // @PHPStan-Ignore new.static
- Rsa::createFromJSON: change declared return type from `static`
  (PHP 8.0+ only) back to `self` for PHP 7.3/7.4 compatibility,
  add @return static docblock, change @var self to @var static
  so PHPStan accepts the covariant return without error
- Update README.md and CONTRIBUTING.md: replace PHP 7.3 with PHP 7.4
  as the minimum version requirement, consistent with the PHP 7.3
  support drop merged in master (ebfb030)
- Regenerate composer.lock on PHP 8.3 after rebase to ensure it
  reflects the current platform and dependency resolution
@Strobotti
Strobotti force-pushed the improve/ci-cd-workflows branch from 1dde1f9 to bcd3ffd Compare September 5, 2026 10:33
…ic-analysis job

- Change cache key hash from composer.json to composer.lock so the cache
  is invalidated only when locked dependencies actually change
- Replace `composer update` with `composer install` to install the exact
  versions recorded in composer.lock, matching the other jobs
@Strobotti
Strobotti merged commit 34b06af into master Sep 5, 2026
20 checks passed
@Strobotti
Strobotti deleted the improve/ci-cd-workflows branch September 5, 2026 12:11
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