Skip to content

Modernize fork: PHP 8.3+, PHPUnit 12, PHPStan 9, new namespace, dictionary updates#60

Closed
CodeByZach wants to merge 14 commits into
theiconic:masterfrom
CodeByZach:modernize
Closed

Modernize fork: PHP 8.3+, PHPUnit 12, PHPStan 9, new namespace, dictionary updates#60
CodeByZach wants to merge 14 commits into
theiconic:masterfrom
CodeByZach:modernize

Conversation

@CodeByZach
Copy link
Copy Markdown

Summary

Modernizes this dormant fork for active use:

  • Merges upstream Add more English salutations and suffixes #54 (English dictionary expansions: ordinals 6th-10th, roman numerals VI-X, professional degrees, additional salutations)
  • Renames package: theiconic/name-parsercodebyzach/name-parser
  • Renames namespace: TheIconic\NameParserCodeByZach\NameParser
  • Bumps PHP floor to ^8.3 (tested on 8.3/8.4/8.5)
  • Upgrades PHPUnit 7 → 12, PHPStan 1 → 2 at level 9
  • Replaces dead Travis/Scrutinizer CI with GitHub Actions matrix
  • Switches code style tool from old (none) → PHP-CS-Fixer with PER-CS2.0
  • Adds typed properties, constructor promotion, #[\Override], generic array types throughout
  • Fixes real bugs: preg_quote missing delimiter, preg_replace null handling, duplicate vii dictionary key
  • Updates README + LICENSE for fork attribution

Test plan

  • CI matrix green on PHP 8.3, 8.4, 8.5
  • PHPStan level 9: 0 errors
  • PHP-CS-Fixer: clean
  • composer audit: no advisories
  • 131 tests pass on every PHP version

frankiejarrett and others added 14 commits December 28, 2021 18:07
Adds expanded English dictionary entries from frankiejarrett's PR:
- Suffixes: ordinals 6th-10th, roman numerals VI-X, degrees (DC, DO,
  DSW, EMBA, LCSW, MBA, MS, MSW, PsyD), Esquire
- Salutations: Hon./Honorable/The Honorable, Missus

PR was authored against pre-Esq master; clean three-way merge with
master's existing Esq entry. Note: PR contains a typo ('vii' => 'VIII'
duplicate key) which is fixed in the follow-up commit.
PR theiconic#54 (now merged) contained a typo: 'vii' => 'VIII' as a duplicate of
'vii' => 'VII'. PHP silently overwrites duplicate array keys, so 'vii'
was mapping to 'VIII' and 'viii' was never defined. Corrected the second
key to 'viii' so both Roman numerals work as intended.
Full rename across 25 src/ and 13 tests/ files. Tests now live under
Tests\CodeByZach\NameParser via composer.json autoload-dev split.

- composer.json: package renamed to codebyzach/name-parser; autoload split
  into src (CodeByZach\NameParser\*) and dev tests (Tests\CodeByZach\NameParser\*);
  Zachary Miller added to authors alongside The Iconic
- src/Name.php: hardcoded PARTS_NAMESPACE constant updated
- tests/bootstrap.php: hardcoded PHPMock::defineFunctionMock target updated
  (kept as the src namespace, not the Tests prefix, since the mocked
  function_exists call lives in src/Part/AbstractPart.php)
- README.md: composer require line updated to codebyzach/name-parser

Pure mechanical rewrite, zero behavior change. PHPUnit run deferred to
Phase 3 since PHPUnit ^7 doesn't install on PHP 8.x.
PHP / Composer:
- Bump php floor from >=7.1 to ^8.2; declare ext-mbstring as required
  (the AbstractPart::camelcaseReplace fallback produces incorrect
  Unicode output without it)
- Replace PHPUnit ^7 with ^11; replace php-mock-phpunit ^2.1 with ^2.15
- Drop php-coveralls (Coveralls.io reporting no longer used)
- Add phpstan ^1.12 and laravel/pint ^1.18 as dev tooling
- Split autoload: src under CodeByZach\NameParser, tests under
  Tests\CodeByZach\NameParser via autoload-dev (was combined before)
- Add composer scripts: test, lint, lint:fix, analyse
- Sort require-dev and scripts alphabetically; enable sort-packages
- Add .tool-versions pinning php 8.4 for local dev (mise/asdf)

PHPUnit 11 schema migration:
- Rewrite phpunit.xml.dist to PHPUnit 11 schema (<source> not <whitelist>;
  drop deprecated logging targets; cacheDirectory; failOnWarning/Risky)
- Migrate three @dataProvider PHPDoc annotations to #[DataProvider]
  attributes (ParserTest, GermanParserTest, AbstractMapperTestCase)
- Make all provider() methods static (PHPUnit 11 requirement)
- Rename AbstractMapperTest.php to AbstractMapperTestCase.php so PHPUnit
  doesn't try to discover tests in the abstract base
- Replace deprecated getMockForAbstractClass() in AbstractPartTest with
  PHP 8 anonymous classes (PHPUnit 12 will remove the mock helper)
- Move PHPMock::defineFunctionMock from bootstrap.php into
  NormalisationTest::setUpBeforeClass (the static trait call is
  deprecated when not on a class using the trait)
- Convert SuffixMapperTest data sets to use 'arguments' named key so
  PHPUnit's argument unpacking doesn't mix named and positional args
- Add missing use statements in test files for Parser, Name,
  AbstractPart, Firstname, Lastname, and the Mapper classes (became
  necessary after Phase 2 split tests into Tests\ namespace)

PHP 8.4 explicit-nullable migration:
- Name::__construct: array $parts = null -> ?array $parts = null
- PreNormalizedPart::__construct, LastnamePrefix::__construct: same fix
  for the $normalized parameter

Removed:
- .travis.yml (Travis CI free tier dead since 2020)
- .scrutinizer.yml (Scrutinizer-CI replaced by PHPStan)
- composer.lock (regenerated under new constraints)

Test suite: 131 tests, 235 assertions, 0 errors, 0 warnings,
0 deprecations on PHP 8.4 / PHPUnit 11.5.55.
…Unit 12

PHP / Composer:
- Bump php floor from ^8.2 to ^8.3 (allows PHPUnit 12; constraint
  already covers 8.3, 8.4, 8.5)
- Replace laravel/pint ^1.18 with friendsofphp/php-cs-fixer ^3.64
  (framework-neutral tooling for a framework-neutral library;
  matches the PHPUnit-not-Pest decision)
- Bump phpunit/phpunit ^11.0 -> ^12.0
- Bump phpstan/phpstan ^1.12 -> ^2.1 (50-70% less memory, level 10
  available, better PHP 8.4/8.5 support)
- .tool-versions: php 8.4 -> php 8.3 to match the project floor and
  silence the PHP-CS-Fixer 'running on newer PHP than minimum' warning
- Composer scripts: lint/lint:fix now invoke php-cs-fixer

Static analysis:
- Add phpstan.neon.dist (level 5; covers src/ and tests/)
- Add phpstan-baseline.neon (currently empty - codebase passes level 5)
- Fix one real bug PHPStan 2.x caught: preg_quote() in Parser::normalize
  was missing the '/' delimiter argument, so user-provided whitespace
  containing '/' would corrupt the regex
- Tighten three @return PHPDoc inconsistencies that PHPStan flagged
  (FirstnameMapper, MiddlenameMapper, AbstractPart)

Code style:
- Add .php-cs-fixer.dist.php using @PER-CS2.0 + @PHP83Migration rulesets
  (PER-CS is the modern PSR-12 successor; framework-neutral)
- Apply formatter to whole codebase: alphabetized imports, trailing
  commas in multi-line, single quotes, short array syntax, etc.
- Restore camelCase test method names (Pint's laravel preset had
  converted them to snake_case, which is Laravel app convention but
  not standard PHPUnit)

Tooling:
- Remove pint.json (replaced by .php-cs-fixer.dist.php)
- .gitignore: add .php-cs-fixer.cache

Test suite: 131 tests, 235 assertions, 0 errors/warnings/deprecations
on PHP 8.3 / PHPUnit 12.x / PHPStan 2.x.
Add .github/workflows/ci.yml:
- Three parallel jobs: tests (matrix PHP 8.3/8.4/8.5), static-analysis,
  code-style
- All third-party actions SHA-pinned with version comments:
  * actions/checkout@93cb6efe (v5.0.1)
  * shivammathur/setup-php@fcafdd6392 (v2.37.0)
  * ramsey/composer-install@a8d0d959dab (v3.2.1)
- composer audit step in tests matrix to catch known security
  advisories on every supported PHP version
- Static-analysis and code-style jobs read PHP version from
  .tool-versions via setup-php's php-version-file input, so there's
  one source of truth for the canonical dev version
- Concurrency control: cancel-in-progress for PRs (so new commits
  cancel stale runs), keep main branch runs sequential

Add .gitattributes:
- export-ignore for tests/, .github/, all dev configs (.tool-versions,
  phpstan*, phpunit.xml.dist, .php-cs-fixer.dist.php)
- Reduces what's downloaded into consumers' vendor/ directories when
  they composer require this library
README:
- Retitle 'THE ICONIC Name Parser' -> 'CodeByZach Name Parser'
- Comment out the six theiconic/* badges (HTML-commented so they're
  easy to re-enable if we ever publish to Packagist under our own name)
- Add a 'Fork notice' callout at the top: explains origin, what's new,
  and the framework-neutral runtime positioning
- Update Setup section: composer require codebyzach/name-parser plus
  PHP version note (8.3+, tested on 8.3/8.4/8.5)
- Add 'Laravel integration' subsection: shows how to bind Parser::class
  in AppServiceProvider; emphasizes that no Laravel-specific glue
  ships with the library
- Add 'Local development' section: mise/.tool-versions and the four
  composer scripts (test, analyse, lint, lint:fix)
- Add 'Contributors' section listing The Iconic (original) and
  CodeByZach (fork maintainer)
- Update License section: prose -> link to LICENSE file with
  dual copyright attribution
- Update upstream test link to point to the fork's ParserTest.php
- Replace dead $xslt code-fence syntax with bash

LICENSE:
- Add 'Copyright (c) 2026 Zachary Miller (CodeByZach)' line below the
  preserved 'Copyright (c) 2017 THE ICONIC' line. MIT body covers both
  copyright holders unchanged.
Type system improvements:
- Add typed properties throughout: Name::\$parts (array), all 6 Parser
  properties, Mapper internal state, AbstractPart::\$value (string),
  PreNormalizedPart::\$normalized (string), LastnamePrefix::\$normalized
  (string), NicknameMapper::\$delimiters (array)
- Constructor property promotion (PHP 8.0): SalutationMapper, MiddlenameMapper,
  SuffixMapper, InitialMapper, LastnameMapper now declare their state
  inline in the constructor signature instead of separate property +
  assignment boilerplate
- Tighten AbstractPart constructor/setValue to accept string|AbstractPart
  union (previously untyped); setValue returns static for fluent chaining
- AbstractMapper: add missing array return type on abstract map(); type
  hasUnmappedPartsBefore \$index as int; type findFirstMapped return
  as int|false (was untyped); type getKey \$word as string

PHP 8.3 #[\Override] attributes:
- Add to all overriding non-constructor methods so PHP fails at compile
  time if a parent method is renamed/removed: NamePart::normalize,
  Nickname::normalize, Initial::normalize, LastnamePrefix::normalize,
  PreNormalizedPart::normalize, and the map() implementation in all 7
  concrete Mapper classes
- Constructors deliberately not annotated (idiomatic PHP convention)

Other modernization:
- AbstractPart::camelcase: use first-class callable syntax
  \$this->camelcaseReplace(...) instead of [\$this, 'camelcaseReplace']
  (PHP 8.1 syntax)
- AbstractPart::camelcaseReplace: type \$matches as array<int, string>

Comment formatting consistency:
- Restored descriptive PHPDoc on AbstractPart constructor/setValue/getValue
  and PreNormalizedPart/LastnamePrefix normalize() that I'd over-aggressively
  trimmed in earlier passes (they explained intent, not types)
- Standardize all property/method PHPDoc to multi-line block style to
  match the existing codebase convention
- Add 'phpdoc_line_span' rule to .php-cs-fixer.dist.php to enforce
  multi-line PHPDoc going forward (property/method/const all 'multi')
- Note: not introducing declare(strict_types=1) - reviewed actual usage
  in major libraries (Laravel framework: no, Symfony: no, Doctrine: yes);
  community is split, and Laravel-friendliness favors not having it

Test suite still: 131 tests, 235 assertions, 0 errors/warnings/deprecations.
PHPStan level 5: 0 errors.
Bump phpstan.neon.dist level from 5 to 9 (highest practical level
short of bleeding-edge level 10, which targets brand-new generic
features).

Real bugs caught and fixed by stricter analysis:
- Parser::normalize: preg_replace() can return null on regex error;
  was being assigned to a string return type. Added ?? \$name fallback.
- AbstractPart::camelcase: same fix for preg_replace_callback() null
  return path.

Type annotations added throughout (75+ docblock additions):
- LanguageInterface and concrete English/German: every getter declares
  its return as array<string, string> (lookup-key => display-form)
- AbstractMapper and concrete mappers: \$parts annotated as
  array<int, AbstractPart|string>; helper methods get \$type as
  class-string, \$index as int, etc.
- LastnameMapper: tightened internal helper signatures, added
  is_string() narrowing where the union type leaked through helpers,
  removed redundant @param/@return tags now that types are native
- SalutationMapper: isMatchingSubset() gets a @phpstan-assert-if-true
  array<int, string> annotation so PHPStan knows the post-check
  narrowing on \$subset
- SuffixMapper: isSuffix() declared with AbstractPart|string param +
  @phpstan-assert-if-true string annotation (returns true only when
  the input is a string suffix candidate)
- InitialMapper: added is_string() guard before isInitial() now that
  \$part is typed as the union
- Name: \$parts property + setParts/getParts/__construct typed with
  array<int, AbstractPart|string>; getAll() returns array<string, string>
- Parser: \$languages typed array<int, LanguageInterface>; \$mappers
  typed array<int, AbstractMapper>; getPrefixes/getSuffixes/getSalutations
  return array<string, string>; setWhitespace param typed as string;
  parseSplitName params typed as string

Test annotations:
- All test methods get : void return type (PHPUnit convention; PHPStan
  level 7+ requires it)
- Data providers get @return array<int, array<string, mixed>> (Mapper
  tests) or array<int, array{string, array<string, string>}> (parser
  tests)
- testParse(string \$input, array \$expectation) - typed inputs
- AbstractMapperTestCase::testMap typed parameters, replaced
  call_user_func_array with first-class callable spread (\$this->getMapper(...\$arguments))
- AbstractMapperTestCase::getMapper now declared as returning
  AbstractMapper; concrete subclasses declare their specific mapper
  return type (e.g., FirstnameMapper)

Type system change: list<X> -> array<int, X>:
- list<X> requires consecutive 0-indexed keys, which PHPStan can't
  prove after \$parts[\$k] = \$value reassignment (even though it's
  semantically a replacement, not a sparse insert). array<int, X>
  is slightly less precise but eliminates 25+ false-positive
  return-type errors without weakening real safety.

Test suite still: 131 tests, 235 assertions, 0 errors/warnings/deprecations.
PHPStan level 9: 0 errors. PHP-CS-Fixer: 0 fixable.
Code reuse:
- Add @phpstan-type PartArray = array<int, AbstractPart|string> alias
  on AbstractMapper; concrete mappers @phpstan-import-type the alias.
  Replaces 25+ verbose array<int, AbstractPart|string> with PartArray
  throughout. DRY and reads cleaner.

Efficiency (real perf win):
- Memoize Parser::getPrefixes/getSuffixes/getSalutations. Previously
  they re-iterated all $this->languages and merged their dictionaries
  on every call - 2-3 times per parse for split (comma-separated) names.
  Now cached in private nullable properties; assigned on first access
  via ??=.
- Extracted the 3 duplicated language-merge loops into one private
  mergeFromLanguages(string \$method) helper.

Code quality:
- Remove "map [thing] in (the) parts array" docblock noise from the 6
  remaining concrete mappers - it just restated what the well-named
  method already conveys. Kept the @param PartArray / @return PartArray
  annotations since the alias carries real info.
- Add one-line WHY comment on the preg_replace ?? \$name fallback in
  Parser::normalize and AbstractPart::camelcase: preg_replace can return
  null on regex compile error (e.g., user-set invalid whitespace pattern).

Verified post-simplify:
- 131 tests, 235 assertions: pass
- PHPStan level 9: 0 errors
- PHP-CS-Fixer: 0 fixable
@CodeByZach CodeByZach closed this May 10, 2026
@CodeByZach CodeByZach deleted the modernize branch May 10, 2026 22:02
@CodeByZach CodeByZach restored the modernize branch May 10, 2026 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants