Skip to content

Apply Rector Safe Modernization Rules

Priority: 🟠 P1 - HIGH (Quick Win) Status: Planning Related Analysis: phpstan-rector-analysis-overview.md

Problem Statement

Rector has identified 53 files with modernization opportunities. Many of these are safe, automated improvements that will:

  • Add missing type hints
  • Improve code quality
  • Fix PHPStan errors
  • Modernize PHP usage

Identified Safe Rules

✅ Safe to Apply Automatically

1. AddArrowFunctionReturnTypeRector

What: Adds return types to arrow functions Example:

// Before
$mapper = fn($item) => $item->getId();

// After
$mapper = fn($item): int => $item->getId();

Impact: ~10 files Risk: Very Low - type inference is reliable

2. ClosureReturnTypeRector

What: Adds return types to closures Example:

// Before
$filter = function($bean) {
    return $bean->isActive();
};

// After
$filter = function($bean): bool {
    return $bean->isActive();
};

Impact: ~15 files Risk: Very Low

3. CompleteReturnDocblockFromToManyRector

What: Adds @return Collection<int, Entity> to getters Example:

// Before
public function getMarkets(): Collection
{
    return $this->markets;
}

// After
/**
 * @return Collection<int, Market>
 */
public function getMarkets(): Collection
{
    return $this->markets;
}

Impact: Multiple entities Risk: Very Low Note: Covered in add-doctrine-collection-generics.md

4. AddAnnotationToRepositoryRector

What: Adds repository type annotations Example:

// Before
class AffiliateProgramRepository extends ServiceEntityRepository
{
}

// After
/**
 * @extends ServiceEntityRepository<AffiliateProgram>
 */
class AffiliateProgramRepository extends ServiceEntityRepository
{
}

Impact: All repositories Risk: Very Low

5. Namespace Import Optimizations

What: Converts FQCN to imports Example:

// Before
private \DateTimeImmutable $createdAt;

// After
use DateTimeImmutable;
private DateTimeImmutable $createdAt;

Impact: Multiple entities Risk: None - cosmetic only

6. AddArrowFunctionReturnTypeRector

What: Adds return types to arrow functions Impact: Service layer with arrow functions Risk: Very Low

⚠️ Requires Manual Review

1. RemoveUnusedPublicMethodParameterRector

What: Removes unused parameters from public methods Risk: HIGH

  • Could break public API contracts
  • Could break interface implementations
  • Could break event listeners

Recommendation: Review each case individually Files Affected:

  • EntityListener/RoasterCrawlConfigListener.php
  • Serializer/CircularReferenceHandler.php
  • Serializer/MaxDepthHandler.php
  • Service/Crawler/Persistance/CoffeeBeanPersister.php

2. TypedPropertyFromCreateMockAssignRector

What: Types properties from mock creation in tests Risk: Low - test code only Recommendation: Apply but verify tests pass

❌ Known Issues

  • config/services.php - Rector internal error
  • Skip this file for now

Implementation Plan

Step 1: Backup

git status  # Ensure clean
git checkout -b type-safety/rector-modernization

Step 2: Run Rector with Safe Rules Only

Create temporary rector config to run only safe rules:

// rector-safe.php
use Rector\Config\RectorConfig;
use Rector\TypeDeclaration\Rector\ArrowFunction\AddArrowFunctionReturnTypeRector;
use Rector\TypeDeclaration\Rector\Closure\ClosureReturnTypeRector;
use Rector\Doctrine\CodeQuality\Rector\Class_\CompleteReturnDocblockFromToManyRector;
use Rector\Doctrine\CodeQuality\Rector\Class_\AddAnnotationToRepositoryRector;

return RectorConfig::configure()
    ->withPaths([
        __DIR__ . '/src',
        __DIR__ . '/tests',
    ])
    ->withSkip([
        __DIR__ . '/config/services.php',
    ])
    ->withRules([
        AddArrowFunctionReturnTypeRector::class,
        ClosureReturnTypeRector::class,
        CompleteReturnDocblockFromToManyRector::class,
        AddAnnotationToRepositoryRector::class,
    ]);

Step 3: Dry Run

vendor/bin/rector process --config=rector-safe.php --dry-run

Step 4: Review Changes

  • Check diff for unexpected changes
  • Verify types look correct
  • Ensure no breaking changes

Step 5: Apply

vendor/bin/rector process --config=rector-safe.php

Step 6: Run Quality Tools

make phpstan  # Should have fewer errors
make test     # Should all pass
make phpcs    # Should pass

Step 7: Commit

git add .
git commit -m "Apply safe Rector modernization rules

Applied rules:
- AddArrowFunctionReturnTypeRector
- ClosureReturnTypeRector
- CompleteReturnDocblockFromToManyRector
- AddAnnotationToRepositoryRector

Improvements:
- Added return types to closures and arrow functions
- Added Collection generic types to entities
- Added repository type annotations
- Improved type safety and IDE support

All tests passing, no functional changes."

Phase 2: Manual Review Rules (Optional)

Step 1: Review RemoveUnusedPublicMethodParameterRector

For each affected file, determine:

  1. Is this method part of a public API?
  2. Does it implement an interface?
  3. Is it an event listener/subscriber?
  4. Is it a Doctrine lifecycle callback?

If YES to any: Keep the parameter (may be required by contract) If NO to all: Safe to remove

Step 2: Apply Selectively

Manually remove unused parameters only where safe.

Expected Results

PHPStan Improvements

  • ~20-30 fewer errors from missing return types
  • ~69 fewer errors from Collection generics
  • ~10 fewer errors from repository annotations

Total: ~100 fewer errors (635 → ~535)

Code Quality

  • Better IDE autocomplete
  • Improved type safety
  • More self-documenting code
  • Easier refactoring

Testing Strategy

Automated

  1. PHPStan: Should have fewer errors
  2. Unit Tests: Should all pass
  3. Integration Tests: Should all pass
  4. PHPCS: Should pass

Manual

  1. Check IDE autocomplete improvements
  2. Verify no runtime errors
  3. Test critical paths manually

Success Criteria

  • [ ] All safe Rector rules applied
  • [ ] PHPStan error count reduced by ~100
  • [ ] All tests passing
  • [ ] No new runtime errors
  • [ ] Code quality improved
  • [ ] Changes committed

Risk Assessment

Safe Rules: Very Low Risk

  • Type hints are inferred, not guessed
  • Docblocks don't affect runtime
  • Changes are conservative
  • Rector is battle-tested

Unsafe Rules: High Risk

  • Could break APIs
  • Requires careful review
  • Should be done manually

Mitigation:

  • Apply safe rules first
  • Dry run and review
  • Run full test suite
  • Can revert if needed

Estimated Effort

Safe Rules:

  • Setup and dry run: 30 min
  • Review changes: 1 hour
  • Apply and test: 1 hour
  • Total: 2.5 hours

Manual Review Rules:

  • Review each case: 2-3 hours
  • Apply selectively: 1 hour
  • Total: 3-4 hours

Recommended: Do safe rules first, manual rules later if needed.

Dependencies

  • Should be done AFTER add-doctrine-collection-generics.md (included in safe rules)
  • Can be done BEFORE entity refactoring
  • Independent of complexity refactoring

Follow-up Tasks

  1. Add Rector to CI/CD pipeline
  2. Configure Rector to run on pre-commit hook
  3. Update coding standards to match Rector rules
  4. Consider enabling more Rector rules gradually

Files to Review Manually

Based on Rector output:

Event Listeners (Keep Parameters)

  • EntityListener/RoasterCrawlConfigListener.php
    • Doctrine lifecycle callbacks require specific signature

Serializers (Keep Parameters)

  • Serializer/CircularReferenceHandler.php
  • Serializer/MaxDepthHandler.php
    • Symfony Serializer interface requirements

Services (Review)

  • Service/Crawler/Persistance/CoffeeBeanPersister.php:64
    • Check if parameter is part of interface

Notes

  • This is a quick win - automated improvements
  • Safe to apply in isolation
  • Provides foundation for future type safety work
  • Makes codebase more modern and maintainable
  • Can be done incrementally (safe rules first)
  • add-doctrine-collection-generics.md - Overlaps with this
  • fix-uninitialized-properties.md - Complementary type work
  • add-array-type-hints-*.md - Next steps after this