Skip to content

Add PHPStan level 8 + PHP-CS-Fixer; fix surfaced issues (roadmap §7)#18

Merged
mmucklo merged 1 commit into
masterfrom
tooling-phpstan-csfixer
Apr 13, 2026
Merged

Add PHPStan level 8 + PHP-CS-Fixer; fix surfaced issues (roadmap §7)#18
mmucklo merged 1 commit into
masterfrom
tooling-phpstan-csfixer

Conversation

@mmucklo

@mmucklo mmucklo commented Apr 13, 2026

Copy link
Copy Markdown
Owner

Addresses ROADMAP §7 (first pass — phpstan + cs-fixer).

Tooling added

  • phpstan/phpstan ^2.1 as dev dep. Config at `phpstan.neon.dist`: level 8, scans `src/` and `tests/`.
  • friendsofphp/php-cs-fixer ^3.95 as dev dep. Config at `.php-cs-fixer.dist.php`:
    • `@PSR12`
    • `declare_strict_types`
    • `no_unused_imports`
    • `ordered_imports`
    • `single_quote`
    • `trailing_comma_in_multiline`
  • Both wired into `.github/workflows/ci.yml` on the PHP 8.3 matrix leg. phpstan uses `--error-format=github` for inline PR annotations. cs-fixer runs `--dry-run --diff`.
  • `.gitignore`: `.php-cs-fixer.cache`.

Issues phpstan surfaced (and fixes)

1. Missing value types on array properties/returns (8 errors)

Added `@var array<K, V>` on all static rule tables, caches, and data-provider return types.

2. `preg_replace` can return null (5 errors)

PHP's `preg_replace` returns `string|null` — `null` on regex engine error. We control all patterns so null is effectively impossible in practice, but the type system correctly flags it. Fall back to the input string via `?? $string` at all three call sites:

```php

  • self::$pluralCache[$string] = preg_replace($pattern, $result, $string);
  • self::$pluralCache[$string] = preg_replace($pattern, $result, $string) ?? $string;
    ```

Issues cs-fixer surfaced (and fixes)

  • Double-quoted literals with regex backreferences (`"$1zes"`) converted to single-quoted (`'$1zes'`). PHP doesn't interpolate `$1` since a digit can't start an identifier, so behavior is identical; single-quoted is idiomatic for non-interpolated strings.
  • One whitespace fix around a fat arrow.

Test plan

  • 117 tests / 118 assertions pass on PHP 8.1 and 8.3.
  • phpstan level 8: No errors.
  • cs-fixer dry-run: clean.
  • CI green on PHP 8.1–8.4 + new phpstan/cs-fixer steps on 8.3.

Deferred

Roadmap §7 also calls for `infection` (mutation testing) and `phpbench` (benchmarks). Keeping those out of this PR to avoid ballooning scope — can land as separate PRs.

🤖 Generated with Claude Code

Roadmap item #7 (first pass — phpstan + cs-fixer).

Tooling:
- phpstan/phpstan ^2.1 (dev dep), configured at level 8 in
  phpstan.neon.dist scanning src/ and tests/.
- friendsofphp/php-cs-fixer ^3.95 (dev dep), configured in
  .php-cs-fixer.dist.php with @psr12, declare_strict_types,
  no_unused_imports, ordered_imports, single_quote, and
  trailing_comma_in_multiline.
- Both wired into .github/workflows/ci.yml on the PHP 8.3 matrix
  leg (phpstan uses --error-format=github for inline PR annotations).
- .gitignore: .php-cs-fixer.cache.

Code fixes from phpstan level 8:
- @var array<K,V> PHPDoc on all static rule tables and caches
  (plural, singular, irregular, uncountable, pluralCache, singularCache).
- preg_replace returns string|null when the regex engine errors. We
  control all patterns so null is effectively impossible, but the
  type system doesn't know that — fall back to the input string via
  ?? $string. Three call sites in pluralize + singularize.
- @return array<...> PHPDoc on the three data providers.

Code fixes from cs-fixer:
- Double-quoted string literals with regex backreferences ("$1zes")
  converted to single-quoted ('$1zes'). PHP doesn't interpolate $1
  since a digit can't start an identifier, so behavior is identical;
  single-quoted is the idiomatic form for non-interpolated strings.
- Whitespace around one long fat-arrow.

117 tests / 118 assertions still pass on PHP 8.1 and 8.3.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Apr 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.07%. Comparing base (a5bba46) to head (43b5fe2).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master      #18   +/-   ##
=========================================
  Coverage     98.07%   98.07%           
  Complexity       27       27           
=========================================
  Files             1        1           
  Lines            52       52           
=========================================
  Hits             51       51           
  Misses            1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mmucklo mmucklo merged commit 307b3ff into master Apr 13, 2026
7 checks passed
@mmucklo mmucklo deleted the tooling-phpstan-csfixer branch April 13, 2026 07:16
mmucklo added a commit that referenced this pull request Apr 13, 2026
Convert the §5a "Open design questions" into resolved decisions so
implementation can start without a round-trip:

- Locale is an abstract class holding rule tables as protected
  instance state, with a concrete regex-rule engine shared across
  subclasses. Bare Locale interface deferred to a later revision if
  needed for exotic morphologies.
- Rule-table visibility: protected (not private — subclasses seed
  them; not public — we moved away from mutable shared state in 2.0).
  Defaults come from protected const class constants on subclasses.
- Caching: per-instance, not global. Extension methods mutate
  instance state and invalidate the instance cache.
- Default locale: the static API always uses En. No global
  setDefaultLocale — avoids action-at-a-distance.
- Instance API: new Inflect(Locale|string $locale = 'en').
  Inflect::registerLocale(name, LocaleOrClassString) for third
  parties. Resolution is lazy.
- Back-compat: static methods keep signatures; internally delegate
  to a lazily-initialized shared En instance. Proxy extension
  methods mutate that shared instance.

Also reflect ship state:
- §6 docs: largely shipped in #19.
- §7 tooling: phpstan + cs-fixer shipped in #18; infection/phpbench
  deferred.
- §8: v2.0.0 tagged 2026-04-13.
- Phasing: v2.1 now merges items 5 + 5a (they share the same API
  surface); v2.2 becomes "add a non-English locale"; v3.x remains
  the conditional Path B split.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mmucklo added a commit that referenced this pull request Apr 16, 2026
* Roadmap: lock §5a locale design; reflect shipped §6/§7/§8 work

Convert the §5a "Open design questions" into resolved decisions so
implementation can start without a round-trip:

- Locale is an abstract class holding rule tables as protected
  instance state, with a concrete regex-rule engine shared across
  subclasses. Bare Locale interface deferred to a later revision if
  needed for exotic morphologies.
- Rule-table visibility: protected (not private — subclasses seed
  them; not public — we moved away from mutable shared state in 2.0).
  Defaults come from protected const class constants on subclasses.
- Caching: per-instance, not global. Extension methods mutate
  instance state and invalidate the instance cache.
- Default locale: the static API always uses En. No global
  setDefaultLocale — avoids action-at-a-distance.
- Instance API: new Inflect(Locale|string $locale = 'en').
  Inflect::registerLocale(name, LocaleOrClassString) for third
  parties. Resolution is lazy.
- Back-compat: static methods keep signatures; internally delegate
  to a lazily-initialized shared En instance. Proxy extension
  methods mutate that shared instance.

Also reflect ship state:
- §6 docs: largely shipped in #19.
- §7 tooling: phpstan + cs-fixer shipped in #18; infection/phpbench
  deferred.
- §8: v2.0.0 tagged 2026-04-13.
- Phasing: v2.1 now merges items 5 + 5a (they share the same API
  surface); v2.2 becomes "add a non-English locale"; v3.x remains
  the conditional Path B split.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Roadmap: add "Beyond v3.x" section with four strategic directions (#21)

Captures the design space past the Path B package split so future
decisions sit against an explicit menu rather than getting invented
ad hoc:

- §9 CLDR plural categories — lifts the English-binary assumption,
  rides on ext-intl / Unicode CLDR. One new method, locales delegate
  category resolution to MessageFormatter/NumberFormatter.
- §10 Morphology expansion — verb conjugation, indefinite articles,
  ordinals, case/gender. Scope creep; would change the product's
  identity.
- §11 Locale data quality — test corpora (Wiktionary/UniMorph) with
  CI accuracy metrics; optional ML fallback via ONNX/FFI.
- §12 Ecosystem — Symfony/Laravel bridges, composer-plugin locale
  discovery, benchmark-as-identity against Doctrine/Symfony.

Headline recommendation: §9 if we pick one — scoped, ext-intl-based,
doesn't change the library's identity but makes the current product
genuinely multilingual.

Explicitly framed as "not commitments — captured so the decision
space is explicit when we get there."

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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