[DX] Make PhpDocInfoFactory explicitly required in Rector rule constructor, if needed

[TomasVotruba]

AbstractRector is quite heavy class with "handy" dependencies. At least that was the original idea :)

Now it contains ~15 services just in case they're used in child service:

rector-src/src/Rector/AbstractRector.php

Lines 86 to 114 in 5ee831f

	public function autowire(
	NodeNameResolver $nodeNameResolver,
	NodeTypeResolver $nodeTypeResolver,
	SimpleCallableNodeTraverser $simpleCallableNodeTraverser,
	NodeFactory $nodeFactory,
	PhpDocInfoFactory $phpDocInfoFactory,
	StaticTypeMapper $staticTypeMapper,
	Skipper $skipper,
	ValueResolver $valueResolver,
	BetterNodeFinder $betterNodeFinder,
	NodeComparator $nodeComparator,
	CurrentFileProvider $currentFileProvider,
	CreatedByRuleDecorator $createdByRuleDecorator,
	ChangedNodeScopeRefresher $changedNodeScopeRefresher,
	): void {
	$this->nodeNameResolver = $nodeNameResolver;
	$this->nodeTypeResolver = $nodeTypeResolver;
	$this->simpleCallableNodeTraverser = $simpleCallableNodeTraverser;
	$this->nodeFactory = $nodeFactory;
	$this->phpDocInfoFactory = $phpDocInfoFactory;
	$this->staticTypeMapper = $staticTypeMapper;
	$this->skipper = $skipper;
	$this->valueResolver = $valueResolver;
	$this->betterNodeFinder = $betterNodeFinder;
	$this->nodeComparator = $nodeComparator;
	$this->currentFileProvider = $currentFileProvider;
	$this->createdByRuleDecorator = $createdByRuleDecorator;
	$this->changedNodeScopeRefresher = $changedNodeScopeRefresher;
	}

This design doesn't seem smart anymore and only forces each rule to decide if they use global dependency or local one. It's like having Doctrine serivce in every Controller.

Instead, there should be zero global dependencies (unless used in the AbstractRector itself) and each rule should ask only what they need. Goal is to make design from ambiguous to single dependency injection, and lighten all the rules.

[samsonasik]

I think this should has soft warning:

        if (! property_exists($this, $name) && isset($this->deprecatedDependencies[$name])) {
            $this->symfonyStyle->warning(
                sprintf('pull %s property from AbstractRector is deprecated, inject via __construct() instead', $name)
            );
       }

[TomasVotruba]

Good idea 👍 Could you add it?

[samsonasik]

I wil try 👍

[samsonasik]

adding warning on __get() seems make too many verbose in console:

➜  rector-src git:(main) ✗ bin/rector process src 
   0/146 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░]   0%

pull phpDocInfoFactory property from AbstractRector on class Rector\PHPUnit\CodeQuality\Rector\Class_\AddSeeTestAnnotationRector is deprecated, inject via __construct() instead
pull phpDocInfoFactory property from AbstractRector on class Rector\PHPUnit\CodeQuality\Rector\Class_\AddSeeTestAnnotationRector is deprecated, inject via __construct() instead
pull phpDocInfoFactory property from AbstractRector on class Rector\PHPUnit\CodeQuality\Rector\Class_\AddSeeTestAnnotationRector is deprecated, inject via __construct() instead
pull phpDocInfoFactory property from AbstractRector on class Rector\PHPUnit\AnnotationsToAttributes\Rector\Class_\TicketAnnotationToAttributeRector is deprecated, inject via __construct() instead
pull phpDocInfoFactory property from AbstractRector on class Rector\PHPUnit\AnnotationsToAttributes\Rector\Class_\AnnotationWithValueToAttributeRector is deprecated, inject via __construct() instead

if next release is 0.19, then I think the __get() itself is unnecessary, then add release note instead.

[samsonasik]

Ok, message seems can be cached, see #5069