Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand references #156

Closed
wants to merge 37 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
db62a7b
Refactor: split merge plan class
vjik Dec 7, 2023
2b55f90
Improve psalm
vjik Dec 7, 2023
6dc1132
Expand variables
vjik Dec 7, 2023
9b09bf0
Apply Rector changes (CI)
vjik Dec 7, 2023
ca77914
improve
vjik Dec 7, 2023
62e38a4
Merge remote-tracking branch 'origin/fix-nested-groups' into fix-nest…
vjik Dec 7, 2023
14c184e
Merge remote-tracking branch 'origin/master' into fix-nested-groups
vjik Dec 9, 2023
a8cbfcb
fix test
vjik Dec 9, 2023
b51f2c5
Merge remote-tracking branch 'origin/master' into fix-nested-groups
vjik Dec 10, 2023
1c7e2af
rename
vjik Dec 10, 2023
5601267
test
vjik Dec 10, 2023
41566f6
dead code
vjik Dec 10, 2023
213f7c4
rename
vjik Dec 10, 2023
54690d7
improve
vjik Dec 10, 2023
feb7489
improve
vjik Dec 10, 2023
f111e78
fix psalm
vjik Dec 10, 2023
894778d
changelog
vjik Dec 10, 2023
3e5429e
rename
vjik Dec 10, 2023
ddbcc1b
add test
vjik Dec 11, 2023
3a3a022
Update CHANGELOG.md
vjik Dec 19, 2023
ea76db0
Update src/Composer/MergePlanCollector.php
vjik Dec 19, 2023
d5cce0a
tmp
vjik Dec 22, 2023
0291adf
Merge remote-tracking branch 'origin/master' into fix-nested-groups
vjik Dec 22, 2023
e1f2a3f
tmp
vjik Dec 22, 2023
f07acd7
improve
vjik Dec 23, 2023
3130a4d
change
vjik Dec 23, 2023
7e934f8
improve
vjik Dec 23, 2023
f3876ce
improve
vjik Dec 23, 2023
a12abfe
Apply fixes from StyleCI
StyleCIBot Dec 23, 2023
9bfc9b1
remove unused code
vjik Dec 23, 2023
de89472
delete unnecessary test
vjik Dec 23, 2023
67f2389
test
vjik Dec 23, 2023
dbec55e
Apply fixes from StyleCI
StyleCIBot Dec 23, 2023
3dcd4c8
remove unused code
vjik Dec 23, 2023
877b9ee
Merge remote-tracking branch 'origin/fix-nested-groups' into fix-nest…
vjik Dec 23, 2023
25f676b
fix psalm
vjik Dec 23, 2023
68d3ad5
improve test
vjik Dec 23, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- New #155: Add ability to specify recursion depth for recursive modifier (@vjik)
- Enh #157: Remove unnecessary code in `PackagesListBuilder` (@vjik)
- Bug #153: Do not throw "Duplicate key…" exception when using nested groups (@vjik)

## 1.4.0 November 17, 2023

Expand Down
180 changes: 180 additions & 0 deletions src/Composer/MergePlanCollector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
<?php

declare(strict_types=1);

namespace Yiisoft\Config\Composer;

use ErrorException;
use Yiisoft\Config\MergePlan;
use Yiisoft\Config\Options;

use function in_array;

/**
* @internal
*
* @psalm-import-type FileType from MergePlan
* @psalm-import-type MergePlanType from MergePlan
*/
final class MergePlanCollector
{
private const PACKAGES_ORDER = [
Options::VENDOR_OVERRIDE_PACKAGE_NAME => 1,
Options::ROOT_PACKAGE_NAME => 2,
];

/**
* @psalm-var array<string, array<string, list<FileType>>>
*/
private array $mergePlan = [];

/**
* @psalm-var array<string,true|null>
*/
private array $processedGroups = [];

/**
* Adds an item to the merge plan.
*
* @param array|string $file The config file.
* @param string $package The package name.
* @param string $group The group name.
*
* @psalm-param array{0:string,1:string}|string $file
*/
public function add(array|string $file, string $package, string $group): void
{
$this->mergePlan[$group][$package][] = $file;
}

/**
* Adds a multiple items to the merge plan.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Adds a multiple items to the merge plan.
* Adds multiple items to the merge plan.

*
* @param array $files The config files.
* @param string $package The package name.
* @param string $group The group name.
*
* @psalm-param list<string|array{0:string,1:string}> $files
*/
public function addMultiple(
array $files,
string $package,
string $group,
): void {
$this->mergePlan[$group][$package] = $files;
}

/**
* Add empty group if it doesn't exist.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Add empty group if it doesn't exist.
* Add an empty group if it doesn't exist.

*
* @param string $group The group name.
*/
public function addGroup(string $group): void
{
if (!isset($this->mergePlan[$group])) {
$this->mergePlan[$group] = [];
}
}

/**
* Returns the merge plan as an array.
*
* @psalm-return MergePlanType
*/
public function asArray(): array
{
$groups = [];
foreach ($this->mergePlan as $group => $packages) {
$groups[$group] = $this->expandVariablesInPackages($packages);
}

$environments = [];
foreach ($groups as $packages) {
foreach ($packages as $files) {
foreach ($files as $file) {
if (is_array($file)) {
$environments[$file[0]] = true;

Check warning on line 96 in src/Composer/MergePlanCollector.php

View workflow job for this annotation

GitHub Actions / mutation / PHP 8.2-ubuntu-latest

Escaped Mutant for Mutator "TrueValue": --- Original +++ New @@ @@ foreach ($packages as $files) { foreach ($files as $file) { if (is_array($file)) { - $environments[$file[0]] = true; + $environments[$file[0]] = false; } } }
}
}
}
}

return [
'groups' => $groups,
'environments' => array_keys($environments),
];
}

/**
* @throws ErrorException
*
* @psalm-param array<string, list<FileType>> $packages
* @psalm-return array<string, list<FileType>>
*/
private function expandVariablesInPackages(array $packages, ?string $targetGroup = null): array
{
if ($targetGroup !== null) {
if (!isset($this->mergePlan[$targetGroup])) {
throw new ErrorException(
sprintf('The "%s" configuration group does not exist.', $targetGroup),
severity: E_USER_ERROR
);
}

if (isset($this->processedGroups[$targetGroup])) {
throw new ErrorException('Circular references in configuration.', severity: E_USER_ERROR);
}
$this->processedGroups[$targetGroup] = true;
$groupPackages = $this->expandVariablesInPackages($this->mergePlan[$targetGroup]);
$this->processedGroups[$targetGroup] = null;

$variable = '$' . $targetGroup;
foreach ($groupPackages as $groupPackage => $groupItems) {
$packageItems = $packages[$groupPackage] ?? [];
$packages[$groupPackage] = in_array($variable, $packageItems, true)
? $this->replaceVariableToFiles($packageItems, $variable, $groupItems)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
? $this->replaceVariableToFiles($packageItems, $variable, $groupItems)
? $this->replaceVariableWithFiles($packageItems, $variable, $groupItems)

: array_merge($packageItems, $groupItems);
}
foreach ($packages as $package => $items) {
$packages[$package] = array_values(
array_filter(
$items,
static fn($item) => $item !== $variable,
)
);
}
uksort(
$packages,
static fn(string $a, string $b) => (self::PACKAGES_ORDER[$a] ?? 0) <=> (self::PACKAGES_ORDER[$b] ?? 0),

Check warning on line 148 in src/Composer/MergePlanCollector.php

View workflow job for this annotation

GitHub Actions / mutation / PHP 8.2-ubuntu-latest

Escaped Mutant for Mutator "DecrementInteger": --- Original +++ New @@ @@ foreach ($packages as $package => $items) { $packages[$package] = array_values(array_filter($items, static fn($item) => $item !== $variable)); } - uksort($packages, static fn(string $a, string $b) => (self::PACKAGES_ORDER[$a] ?? 0) <=> (self::PACKAGES_ORDER[$b] ?? 0)); + uksort($packages, static fn(string $a, string $b) => (self::PACKAGES_ORDER[$a] ?? -1) <=> (self::PACKAGES_ORDER[$b] ?? 0)); } foreach ($packages as $items) { foreach ($items as $item) {
);
}

foreach ($packages as $items) {
foreach ($items as $item) {
if (!is_array($item) && Options::isVariable($item)) {
return $this->expandVariablesInPackages($packages, substr($item, 1));
}
}
}

return $packages;
}

/**
* @psalm-param list<FileType> $items
* @psalm-param list<FileType> $files
* @psalm-return list<FileType>
*/
private function replaceVariableToFiles(array $items, string $variable, array $files): array
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private function replaceVariableToFiles(array $items, string $variable, array $files): array
private function replaceVariableWithFiles(array $items, string $variable, array $files): array

{
$result = [];
foreach ($items as $item) {
if ($item === $variable) {
$result = array_merge($result, $files);
} else {
$result[] = $item;
}
}
return $result;
}
}
41 changes: 8 additions & 33 deletions src/Composer/MergePlanProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use Composer\Composer;
use Composer\Package\PackageInterface;
use Composer\Util\Filesystem;
use Yiisoft\Config\MergePlan;
use Yiisoft\Config\Options;
use Yiisoft\VarDumper\VarDumper;

Expand All @@ -24,15 +23,15 @@
*/
final class MergePlanProcess
{
private MergePlan $mergePlan;
private MergePlanCollector $mergePlanCollector;
private ProcessHelper $helper;

/**
* @param Composer $composer The composer instance.
*/
public function __construct(Composer $composer)
{
$this->mergePlan = new MergePlan();
$this->mergePlanCollector = new MergePlanCollector();
$this->helper = new ProcessHelper($composer);

if (!$this->helper->shouldBuildMergePlan()) {
Expand All @@ -43,7 +42,6 @@
$this->addPackagesConfigsToMergePlan(true);

$this->addRootPackageConfigToMergePlan();
$this->addEnvironmentsConfigsToMergePlan();

$this->updateMergePlan();
}
Expand All @@ -57,32 +55,32 @@
$packageName = $isVendorOverrideLayer ? Options::VENDOR_OVERRIDE_PACKAGE_NAME : $name;

foreach ($this->helper->getPackageConfig($package) as $group => $files) {
$this->mergePlan->addGroup($group);
$this->mergePlanCollector->addGroup($group);

foreach ((array) $files as $file) {
$isOptional = false;

if (Options::isOptional($file)) {
$isOptional = true;
$file = substr($file, 1);

Check warning on line 65 in src/Composer/MergePlanProcess.php

View workflow job for this annotation

GitHub Actions / mutation / PHP 8.2-ubuntu-latest

Escaped Mutant for Mutator "DecrementInteger": --- Original +++ New @@ @@ $isOptional = false; if (Options::isOptional($file)) { $isOptional = true; - $file = substr($file, 1); + $file = substr($file, 0); } if (Options::isVariable($file)) { $this->mergePlanCollector->add($file, $packageName, $group);

Check warning on line 65 in src/Composer/MergePlanProcess.php

View workflow job for this annotation

GitHub Actions / mutation / PHP 8.2-ubuntu-latest

Escaped Mutant for Mutator "IncrementInteger": --- Original +++ New @@ @@ $isOptional = false; if (Options::isOptional($file)) { $isOptional = true; - $file = substr($file, 1); + $file = substr($file, 2); } if (Options::isVariable($file)) { $this->mergePlanCollector->add($file, $packageName, $group);

Check warning on line 65 in src/Composer/MergePlanProcess.php

View workflow job for this annotation

GitHub Actions / mutation / PHP 8.2-ubuntu-latest

Escaped Mutant for Mutator "UnwrapSubstr": --- Original +++ New @@ @@ $isOptional = false; if (Options::isOptional($file)) { $isOptional = true; - $file = substr($file, 1); + $file = $file; } if (Options::isVariable($file)) { $this->mergePlanCollector->add($file, $packageName, $group);
}

if (Options::isVariable($file)) {
$this->mergePlan->add($file, $packageName, $group);
$this->mergePlanCollector->add($file, $packageName, $group);
continue;

Check warning on line 70 in src/Composer/MergePlanProcess.php

View workflow job for this annotation

GitHub Actions / mutation / PHP 8.2-ubuntu-latest

Escaped Mutant for Mutator "Continue_": --- Original +++ New @@ @@ } if (Options::isVariable($file)) { $this->mergePlanCollector->add($file, $packageName, $group); - continue; + break; } $absoluteFilePath = $this->helper->getAbsolutePackageFilePath($package, $options, $file); if (Options::containsWildcard($file)) {
}

$absoluteFilePath = $this->helper->getAbsolutePackageFilePath($package, $options, $file);

if (Options::containsWildcard($file)) {

Check warning on line 75 in src/Composer/MergePlanProcess.php

View workflow job for this annotation

GitHub Actions / mutation / PHP 8.2-ubuntu-latest

Escaped Mutant for Mutator "IfNegation": --- Original +++ New @@ @@ continue; } $absoluteFilePath = $this->helper->getAbsolutePackageFilePath($package, $options, $file); - if (Options::containsWildcard($file)) { + if (!Options::containsWildcard($file)) { $matches = glob($absoluteFilePath); if (empty($matches)) { continue;
$matches = glob($absoluteFilePath);

if (empty($matches)) {
continue;

Check warning on line 79 in src/Composer/MergePlanProcess.php

View workflow job for this annotation

GitHub Actions / mutation / PHP 8.2-ubuntu-latest

Escaped Mutant for Mutator "Continue_": --- Original +++ New @@ @@ if (Options::containsWildcard($file)) { $matches = glob($absoluteFilePath); if (empty($matches)) { - continue; + break; } foreach ($matches as $match) { $this->mergePlanCollector->add($this->normalizePackageFilePath($package, $match, $isVendorOverrideLayer), $packageName, $group);
}

foreach ($matches as $match) {
$this->mergePlan->add(
$this->mergePlanCollector->add(
$this->normalizePackageFilePath($package, $match, $isVendorOverrideLayer),
$packageName,
$group,
Expand All @@ -96,7 +94,7 @@
continue;
}

$this->mergePlan->add(
$this->mergePlanCollector->add(
$this->normalizePackageFilePath($package, $absoluteFilePath, $isVendorOverrideLayer),
$packageName,
$group,
Expand All @@ -109,40 +107,17 @@
private function addRootPackageConfigToMergePlan(): void
{
foreach ($this->helper->getRootPackageConfig() as $group => $files) {
$this->mergePlan->addMultiple(
$this->mergePlanCollector->addMultiple(
(array) $files,
Options::ROOT_PACKAGE_NAME,
$group,
);
}
}

private function addEnvironmentsConfigsToMergePlan(): void
{
foreach ($this->helper->getEnvironmentConfig() as $environment => $groups) {
if ($environment === Options::DEFAULT_ENVIRONMENT) {
continue;
}

if (empty($groups)) {
$this->mergePlan->addEnvironmentWithoutConfigs($environment);
continue;
}

foreach ($groups as $group => $files) {
$this->mergePlan->addMultiple(
(array) $files,
Options::ROOT_PACKAGE_NAME,
$group,
$environment,
);
}
}
}

private function updateMergePlan(): void
{
$mergePlan = $this->mergePlan->toArray();
$mergePlan = $this->mergePlanCollector->asArray();
ksort($mergePlan);

$filePath = $this->helper->getPaths()->absolute(
Expand Down
13 changes: 0 additions & 13 deletions src/Composer/ProcessHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -190,19 +190,6 @@ public function getRootPackageConfig(): array
return (array) ($this->rootPackageExtra['config-plugin'] ?? []);
}

/**
* Returns the environment configuration.
*
* @return array The environment configuration.
*
* @psalm-return array<string, array<string, string|string[]>>
* @psalm-suppress MixedReturnTypeCoercion
*/
public function getEnvironmentConfig(): array
{
return (array) ($this->rootPackageExtra['config-plugin-environments'] ?? []);
}

/**
* Returns the config paths instance.
*
Expand Down
44 changes: 4 additions & 40 deletions src/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
use function restore_error_handler;
use function set_error_handler;
use function sprintf;
use function substr;

/**
* Config takes merge plan prepared by {@see \Yiisoft\Config\Composer\EventHandler}
Expand Down Expand Up @@ -39,17 +38,14 @@ final class Config implements ConfigInterface
*/
public function __construct(
ConfigPaths $paths,
string $environment = null,
?string $environment = null,
array $modifiers = [],
private ?string $paramsGroup = 'params',
string $mergePlanFile = Options::DEFAULT_MERGE_PLAN_FILE,
) {
$environment = empty($environment) ? Options::DEFAULT_ENVIRONMENT : $environment;
$mergePlan = new MergePlan($paths->absolute($mergePlanFile));

/** @psalm-suppress UnresolvableInclude, MixedArgument */
$mergePlan = new MergePlan(require $paths->absolute($mergePlanFile));

if (!$mergePlan->hasEnvironment($environment)) {
if ($environment !== null && !$mergePlan->hasEnvironment($environment)) {
$this->throwException(sprintf('The "%s" configuration environment does not exist.', $environment));
}

Expand Down Expand Up @@ -117,46 +113,14 @@ private function buildGroup(string $group): void
$this->build[$group] = [];

foreach ($this->filesExtractor->extract($group) as $file => $context) {
if (Options::isVariable($file)) {
$variable = $this->prepareVariable($file, $group);
$array = $this->get($variable);
} else {
$array = $this->buildFile($file);
}

$this->build[$group] = $this->merger->merge(
$context,
$this->build[$group],
$array,
$this->buildFile($file),
);
}
}

/**
* Checks the configuration variable and returns its name.
*
* @param string $variable The variable.
* @param string $group The group name.
*
* @throws ErrorException If the variable name is not valid.
*
* @return string The variable name.
*/
private function prepareVariable(string $variable, string $group): string
{
$name = substr($variable, 1);

if ($name === $group) {
$this->throwException(sprintf(
'The variable "%s" must not be located inside the "%s" config group.',
"$variable",
"$name",
));
}

return $name;
}

/**
* Builds the configuration from the file.
*
Expand Down
Loading
Loading