Add withImport() method to RectorConfigBuilder

[carlos-granados]

The RectorConfig class has a function, import() that allow you to import another configuration file but this was never exposed in the RectorConfigBuilder class. This PR adds a withImport() function to the builder class that allows you to import other configuration files while using it

[samsonasik]

There is already withSets() for it :), you can do:

->withSets([
      __DIR__ . '/my/rector.php',
])

[carlos-granados]

@samsonasik yes, but the use case would be, for example, having different configurations for local and CI, with a base common configuration. And we want to import whole configuration, not just sets or rules

[carlos-granados]

I believe all these test failures are unrelated to my PR

[samsonasik]

the sets works like that, just like import, we had unique sets verification also :).

rector-src/src/Configuration/RectorConfigBuilder.php

Lines 219 to 268 in f1c5104

	$uniqueSets = array_unique($this->sets);
	if ($this->isWithPhpLevelUsed && $this->isWithPhpSetsUsed) {
	throw new InvalidConfigurationException(sprintf(
	'Your config uses "withPhp*()" and "withPhpLevel()" methods at the same time.%sPick one of them to avoid rule conflicts.',
	PHP_EOL
	));
	}
	if (in_array(SetList::TYPE_DECLARATION, $uniqueSets, true) && $this->isTypeCoverageLevelUsed === true) {
	throw new InvalidConfigurationException(sprintf(
	'Your config already enables type declarations set.%sRemove "->withTypeCoverageLevel()" as it only duplicates it, or remove type declaration set.',
	PHP_EOL
	));
	}
	if (in_array(
	SetList::TYPE_DECLARATION_DOCBLOCKS,
	$uniqueSets,
	true
	) && $this->isTypeCoverageDocblockLevelUsed === true) {
	throw new InvalidConfigurationException(sprintf(
	'Your config already enables type declarations set.%sRemove "->withTypeCoverageDocblockLevel()" as it only duplicates it, or remove type declaration set.',
	PHP_EOL
	));
	}
	if (in_array(SetList::DEAD_CODE, $uniqueSets, true) && $this->isDeadCodeLevelUsed === true) {
	throw new InvalidConfigurationException(sprintf(
	'Your config already enables dead code set.%sRemove "->withDeadCodeLevel()" as it only duplicates it, or remove dead code set.',
	PHP_EOL
	));
	}
	if (in_array(SetList::CODE_QUALITY, $uniqueSets, true) && $this->isCodeQualityLevelUsed === true) {
	throw new InvalidConfigurationException(sprintf(
	'Your config already enables code quality set.%sRemove "->withCodeQualityLevel()" as it only duplicates it, or remove code quality set.',
	PHP_EOL
	));
	}
	if (in_array(SetList::CODING_STYLE, $uniqueSets, true) && $this->isCodingStyleLevelUsed === true) {
	throw new InvalidConfigurationException(sprintf(
	'Your config already enables coding style set.%sRemove "->withCodingStyleLevel()" as it only duplicates it, or remove coding style set.',
	PHP_EOL
	));
	}
	if ($uniqueSets !== []) {
	$rectorConfig->sets($uniqueSets);

Have a 2 way to import sets will make buggy code and possible future unwanted reported issue :)

[carlos-granados]

@samsonasik but this is not just about sets, this is what I am trying to say. To give you one example:

• Locally I don't want to use too much machine resources, so I want to set withParallel(maxNumberOfProcess: 2)
• In CI I want to use max machine resources, so I want to set withParallel(maxNumberOfProcess:8)
• Both in local and CI I want to use the same set of rules

So I would have three config files:
rector_base.php which sets the rules to be applied
rector.php which sets the parallel config for local
rector_ci.php which sets the parallel config for CI

Both rector.php and rector_ci.php import rector_base.php

This cannot be done with the existing code unless I repeat the set of rules both in rector.php and rector_ci.php

This is just one example but I can think of many more where this would be useful.

I ask you to please reconsider and re-open the PR as I think it is a valid concern

[carlos-granados]

@samsonasik also, the capability to import other config files already exists through the import() function in the RectorConfig class, and it works and is not buggy. This PR only allows access to that capability through the RectorConfigBuilder

[samsonasik]

The sets() is call the $rectorConfig->import() internally, so they are doing the same.

The RectorConfigBuilder can be used as "root caller" config, and you can use configurable normal RectorConfig static config in separate config that you called, which if else inside there is fine :)

[TomasVotruba]

@carlos-granados Indeed, we use "sets" as a synonym to "imports". It was leftover from Symfony naming, a DI container we used in the past. I prefer to stick with one naming here as sets are major killer feature of Rector.

[carlos-granados]

@TomasVotruba @samsonasik thanks for the explanation, I did not realise that the withSets function actually called import internally. But I hope that you realise that, even if it is the historical usage in Rector, it is not obvious, natural or evident that you need to call withSets if all you want to do is import another config file. I believe that any developer who is not familiar with Rector internals would be just as confused as I was. Just saying

[TomasVotruba]

Reading the #7837 (comment), instead of creating multiple configs, which is necessary if there is format like XML, YML etc., you can make use of PHP itself in single config:

$processCount = ...;

// ...
->withParallel($processCount);

It's also more readable than config juggling to change sole value. I've seen other devs using same logic to configure temp directory based on env 👍