From a4accaa36c7874f592f037a0ea73dd167876bdc0 Mon Sep 17 00:00:00 2001 From: "Eirik S. Morland" Date: Wed, 1 Apr 2026 07:46:11 +0200 Subject: [PATCH 1/9] Make it possible for a rule to override back to default --- src/Config.php | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/src/Config.php b/src/Config.php index 59ba38e..56628dc 100644 --- a/src/Config.php +++ b/src/Config.php @@ -598,33 +598,17 @@ protected function mergeConfig(Config $other, $extends_name, $parent) protected function mergeConfigFromConfigObject(\stdClass $config, \stdClass $other) : array { $affected = []; - $default_config = $this->getDefaultConfig(); foreach ($other as $key => $value) { - // If the value corresponds to the default config, we don't need to - // set it. - if (isset($default_config->{$key}) && $default_config->{$key} === $value) { + // If the config already has this value, no change is needed. + if (isset($config->{$key}) && $config->{$key} === $value) { continue; } - // This special case is because the default config is a stdclass, + // This special case is because the config value is a stdclass, // and that will not pass the strict equal test. So let's just // loosen it up a bit for this specific case. - if ($key === 'bundled_packages' && $default_config->{$key} == $value) { + if ($key === 'bundled_packages' && $config->{$key} == $value) { continue; } - // If our option is set, but not set to the default, let's not merge - // it. - if (isset($default_config->{$key}) && isset($config->{$key})) { - // Special case for bundled packages again. - if ($key === 'bundled_packages') { - if ($config->{$key} != $default_config->{$key}) { - continue; - } - } else { - if ($config->{$key} !== $default_config->{$key}) { - continue; - } - } - } $config->{$key} = $value; $affected[$key] = $value; } From 147d9add09497503fab3ee4fbeada92e3efdc3f1 Mon Sep 17 00:00:00 2001 From: "Eirik S. Morland" Date: Wed, 1 Apr 2026 07:57:15 +0200 Subject: [PATCH 2/9] Fix tests --- src/Config.php | 42 ++++++++++++++++++++++++---------- tests/RuleObjectConfigTest.php | 34 +++++++++++++++++++++++++-- 2 files changed, 62 insertions(+), 14 deletions(-) diff --git a/src/Config.php b/src/Config.php index 56628dc..4ebc3f1 100644 --- a/src/Config.php +++ b/src/Config.php @@ -550,7 +550,7 @@ public function getConfigForRuleObject(\stdClass $rule_object) return $this; } $new_config = clone $this->config; - $this->mergeConfigFromConfigObject($new_config, $rule_object->config); + $this->mergeConfigFromConfigObject($new_config, $rule_object->config, true); return self::createFromViolinistConfig($new_config); } @@ -572,7 +572,7 @@ public function getConfigForPackage(string $package_name) : self continue; } // Then merge the config for this rule. - $this->mergeConfigFromConfigObject($new_config, $rule->config); + $this->mergeConfigFromConfigObject($new_config, $rule->config, true); } return self::createFromViolinistConfig($new_config); } @@ -595,19 +595,37 @@ protected function mergeConfig(Config $other, $extends_name, $parent) } } - protected function mergeConfigFromConfigObject(\stdClass $config, \stdClass $other) : array + protected function mergeConfigFromConfigObject(\stdClass $config, \stdClass $other, bool $force = false) : array { $affected = []; + $default_config = $this->getDefaultConfig(); foreach ($other as $key => $value) { - // If the config already has this value, no change is needed. - if (isset($config->{$key}) && $config->{$key} === $value) { - continue; - } - // This special case is because the config value is a stdclass, - // and that will not pass the strict equal test. So let's just - // loosen it up a bit for this specific case. - if ($key === 'bundled_packages' && $config->{$key} == $value) { - continue; + if (!$force) { + // If the value corresponds to the default config, we don't need to + // set it. + if (isset($default_config->{$key}) && $default_config->{$key} === $value) { + continue; + } + // This special case is because the default config is a stdclass, + // and that will not pass the strict equal test. So let's just + // loosen it up a bit for this specific case. + if ($key === 'bundled_packages' && $default_config->{$key} == $value) { + continue; + } + // If our option is set, but not set to the default, let's not merge + // it. + if (isset($default_config->{$key}) && isset($config->{$key})) { + // Special case for bundled packages again. + if ($key === 'bundled_packages') { + if ($config->{$key} != $default_config->{$key}) { + continue; + } + } else { + if ($config->{$key} !== $default_config->{$key}) { + continue; + } + } + } } $config->{$key} = $value; $affected[$key] = $value; diff --git a/tests/RuleObjectConfigTest.php b/tests/RuleObjectConfigTest.php index dc6894e..8f55670 100644 --- a/tests/RuleObjectConfigTest.php +++ b/tests/RuleObjectConfigTest.php @@ -32,7 +32,7 @@ public function testRuleWithoutConfigReturnsOriginalConfig() self::assertSame($config, $result); } - public function testRuleDoesNotOverrideExistingConfig() + public function testRuleDoesOverrideExistingConfig() { $config = Config::createFromViolinistConfig((object) [ 'update_dev_dependencies' => 0, @@ -47,6 +47,36 @@ public function testRuleDoesNotOverrideExistingConfig() self::assertNotSame($config, $config_from_rule); self::assertFalse($config->shouldUpdateDevDependencies()); - self::assertFalse($config_from_rule->shouldUpdateDevDependencies()); + self::assertTrue($config_from_rule->shouldUpdateDevDependencies()); + } + + public function testRuleCanOverrideBackToDefault() + { + $config_data = (object) [ + 'security_updates_only' => 1, + 'rules' => [ + (object) [ + 'name' => 'Always update these', + 'matchRules' => [ + (object) ['type' => 'names', 'values' => ['vendor/package-a', 'vendor/package-b']], + ], + 'config' => (object) [ + 'security_updates_only' => 0, + ], + ], + ], + ]; + $config = Config::createFromViolinistConfig($config_data); + + self::assertTrue($config->shouldOnlyUpdateSecurityUpdates()); + + $config_for_package_a = $config->getConfigForPackage('vendor/package-a'); + self::assertFalse($config_for_package_a->shouldOnlyUpdateSecurityUpdates()); + + $config_for_package_b = $config->getConfigForPackage('vendor/package-b'); + self::assertFalse($config_for_package_b->shouldOnlyUpdateSecurityUpdates()); + + $config_for_other = $config->getConfigForPackage('vendor/package-c'); + self::assertTrue($config_for_other->shouldOnlyUpdateSecurityUpdates()); } } From 486a30459742ed21889394f079a86fab47176113 Mon Sep 17 00:00:00 2001 From: "Eirik S. Morland" Date: Wed, 1 Apr 2026 08:02:19 +0200 Subject: [PATCH 3/9] Redo without override --- src/Config.php | 76 +++++++++++++++++++++------------- tests/RuleObjectConfigTest.php | 22 +++++++--- 2 files changed, 64 insertions(+), 34 deletions(-) diff --git a/src/Config.php b/src/Config.php index 4ebc3f1..c14ced3 100644 --- a/src/Config.php +++ b/src/Config.php @@ -550,7 +550,7 @@ public function getConfigForRuleObject(\stdClass $rule_object) return $this; } $new_config = clone $this->config; - $this->mergeConfigFromConfigObject($new_config, $rule_object->config, true); + $this->mergeConfigFromConfigObject($new_config, $rule_object->config); return self::createFromViolinistConfig($new_config); } @@ -563,6 +563,22 @@ public function getConfigForPackage(string $package_name) : self return $this; } $new_config = clone $this->config; + $default_config = $this->getDefaultConfig(); + // Determine which keys are explicitly set in the global config (non-default). + // Rules cannot override these, but rules can override each other. + $globally_set_keys = []; + foreach ($default_config as $key => $default_value) { + if (!isset($new_config->{$key})) { + continue; + } + if ($key === 'bundled_packages') { + if ($new_config->{$key} != $default_value) { + $globally_set_keys[$key] = true; + } + } elseif ($new_config->{$key} !== $default_value) { + $globally_set_keys[$key] = true; + } + } foreach ($this->config->rules as $rule) { if (empty($rule->config)) { continue; @@ -571,8 +587,14 @@ public function getConfigForPackage(string $package_name) : self if (!$matches) { continue; } - // Then merge the config for this rule. - $this->mergeConfigFromConfigObject($new_config, $rule->config, true); + // Apply rule config. Rules cannot override the global config, but + // later rules can override earlier rules. + foreach ($rule->config as $key => $value) { + if (isset($globally_set_keys[$key])) { + continue; + } + $new_config->{$key} = $value; + } } return self::createFromViolinistConfig($new_config); } @@ -595,35 +617,33 @@ protected function mergeConfig(Config $other, $extends_name, $parent) } } - protected function mergeConfigFromConfigObject(\stdClass $config, \stdClass $other, bool $force = false) : array + protected function mergeConfigFromConfigObject(\stdClass $config, \stdClass $other) : array { $affected = []; $default_config = $this->getDefaultConfig(); foreach ($other as $key => $value) { - if (!$force) { - // If the value corresponds to the default config, we don't need to - // set it. - if (isset($default_config->{$key}) && $default_config->{$key} === $value) { - continue; - } - // This special case is because the default config is a stdclass, - // and that will not pass the strict equal test. So let's just - // loosen it up a bit for this specific case. - if ($key === 'bundled_packages' && $default_config->{$key} == $value) { - continue; - } - // If our option is set, but not set to the default, let's not merge - // it. - if (isset($default_config->{$key}) && isset($config->{$key})) { - // Special case for bundled packages again. - if ($key === 'bundled_packages') { - if ($config->{$key} != $default_config->{$key}) { - continue; - } - } else { - if ($config->{$key} !== $default_config->{$key}) { - continue; - } + // If the value corresponds to the default config, we don't need to + // set it. + if (isset($default_config->{$key}) && $default_config->{$key} === $value) { + continue; + } + // This special case is because the default config is a stdclass, + // and that will not pass the strict equal test. So let's just + // loosen it up a bit for this specific case. + if ($key === 'bundled_packages' && $default_config->{$key} == $value) { + continue; + } + // If our option is set, but not set to the default, let's not merge + // it. + if (isset($default_config->{$key}) && isset($config->{$key})) { + // Special case for bundled packages again. + if ($key === 'bundled_packages') { + if ($config->{$key} != $default_config->{$key}) { + continue; + } + } else { + if ($config->{$key} !== $default_config->{$key}) { + continue; } } } diff --git a/tests/RuleObjectConfigTest.php b/tests/RuleObjectConfigTest.php index 8f55670..085eb35 100644 --- a/tests/RuleObjectConfigTest.php +++ b/tests/RuleObjectConfigTest.php @@ -32,7 +32,7 @@ public function testRuleWithoutConfigReturnsOriginalConfig() self::assertSame($config, $result); } - public function testRuleDoesOverrideExistingConfig() + public function testRuleDoesNotOverrideExistingConfig() { $config = Config::createFromViolinistConfig((object) [ 'update_dev_dependencies' => 0, @@ -47,14 +47,26 @@ public function testRuleDoesOverrideExistingConfig() self::assertNotSame($config, $config_from_rule); self::assertFalse($config->shouldUpdateDevDependencies()); - self::assertTrue($config_from_rule->shouldUpdateDevDependencies()); + self::assertFalse($config_from_rule->shouldUpdateDevDependencies()); } - public function testRuleCanOverrideBackToDefault() + public function testLaterRuleOverridesEarlierRuleForSamePackage() { + // A catch-all rule sets security_updates_only for all matched packages. + // A more specific rule after it can override back to the default for + // specific packages, since rules can override each other (but not the + // global config). $config_data = (object) [ - 'security_updates_only' => 1, 'rules' => [ + (object) [ + 'name' => 'Security only for vendor packages', + 'matchRules' => [ + (object) ['type' => 'names', 'values' => ['vendor/*']], + ], + 'config' => (object) [ + 'security_updates_only' => 1, + ], + ], (object) [ 'name' => 'Always update these', 'matchRules' => [ @@ -68,8 +80,6 @@ public function testRuleCanOverrideBackToDefault() ]; $config = Config::createFromViolinistConfig($config_data); - self::assertTrue($config->shouldOnlyUpdateSecurityUpdates()); - $config_for_package_a = $config->getConfigForPackage('vendor/package-a'); self::assertFalse($config_for_package_a->shouldOnlyUpdateSecurityUpdates()); From e5f6835ed650ddb3794371eca99bc8b6dd80dd6a Mon Sep 17 00:00:00 2001 From: "Eirik S. Morland" Date: Sat, 4 Apr 2026 21:07:05 +0200 Subject: [PATCH 4/9] Coverage --- src/Config.php | 3 -- tests/RuleObjectConfigTest.php | 60 ++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/Config.php b/src/Config.php index c14ced3..e200123 100644 --- a/src/Config.php +++ b/src/Config.php @@ -568,9 +568,6 @@ public function getConfigForPackage(string $package_name) : self // Rules cannot override these, but rules can override each other. $globally_set_keys = []; foreach ($default_config as $key => $default_value) { - if (!isset($new_config->{$key})) { - continue; - } if ($key === 'bundled_packages') { if ($new_config->{$key} != $default_value) { $globally_set_keys[$key] = true; diff --git a/tests/RuleObjectConfigTest.php b/tests/RuleObjectConfigTest.php index 085eb35..c1d4db5 100644 --- a/tests/RuleObjectConfigTest.php +++ b/tests/RuleObjectConfigTest.php @@ -89,4 +89,64 @@ public function testLaterRuleOverridesEarlierRuleForSamePackage() $config_for_other = $config->getConfigForPackage('vendor/package-c'); self::assertTrue($config_for_other->shouldOnlyUpdateSecurityUpdates()); } + + public function testRuleDoesNotOverrideGlobalConfigForPackage() + { + $config_data = (object) [ + 'security_updates_only' => 1, + 'rules' => [ + (object) [ + 'name' => 'Try to override security_updates_only', + 'matchRules' => [ + (object) ['type' => 'names', 'values' => ['vendor/*']], + ], + 'config' => (object) [ + 'security_updates_only' => 0, + ], + ], + ], + ]; + $config = Config::createFromViolinistConfig($config_data); + + $config_for_package = $config->getConfigForPackage('vendor/package-a'); + self::assertTrue($config_for_package->shouldOnlyUpdateSecurityUpdates()); + } + + public function testRuleDoesNotOverrideGlobalBundledPackages() + { + $config_data = (object) [ + 'bundled_packages' => (object) ['psr/log' => ['symfony/console']], + 'rules' => [ + (object) [ + 'name' => 'Try to override bundled_packages', + 'matchRules' => [ + (object) ['type' => 'names', 'values' => ['psr/*']], + ], + 'config' => (object) [ + 'bundled_packages' => (object) ['psr/log' => ['other/package']], + ], + ], + ], + ]; + $config = Config::createFromViolinistConfig($config_data); + + $config_for_package = $config->getConfigForPackage('psr/log'); + self::assertEquals(['symfony/console'], $config_for_package->getBundledPackagesForPackage('psr/log')); + } + + public function testRuleObjectDoesNotOverrideGlobalBundledPackages() + { + $config_data = (object) [ + 'bundled_packages' => (object) ['psr/log' => ['symfony/console']], + ]; + $config = Config::createFromViolinistConfig($config_data); + $rule = (object) [ + 'config' => (object) [ + 'bundled_packages' => (object) ['psr/log' => ['other/package']], + ], + ]; + + $config_from_rule = $config->getConfigForRuleObject($rule); + self::assertEquals(['symfony/console'], $config_from_rule->getBundledPackagesForPackage('psr/log')); + } } From 9e8c5985782e1973033b784e3e6bcd0c888d1ba2 Mon Sep 17 00:00:00 2001 From: "Eirik S. Morland" Date: Tue, 7 Apr 2026 08:37:59 +0200 Subject: [PATCH 5/9] Updates --- src/Config.php | 57 ++++++++++----------- tests/RuleObjectConfigTest.php | 93 +++++++++++++++++++++++++++++++--- 2 files changed, 112 insertions(+), 38 deletions(-) diff --git a/src/Config.php b/src/Config.php index e200123..7609deb 100644 --- a/src/Config.php +++ b/src/Config.php @@ -544,56 +544,49 @@ public function getCommitMessageConvention() public function getConfigForRuleObject(\stdClass $rule_object) { - // @todo: This is a bit duplication from below actually. Specifically - // the method ::getConfigForPackage if (empty($rule_object->config)) { return $this; } - $new_config = clone $this->config; - $this->mergeConfigFromConfigObject($new_config, $rule_object->config); - return self::createFromViolinistConfig($new_config); + $clone = clone $this; + $clone->config = clone $this->config; + $affected = $this->mergeConfigFromConfigObject($clone->config, $rule_object->config); + foreach ($affected as $key => $value) { + $clone->configOptionsSet[$key] = true; + } + return $clone; } public function getConfigForPackage(string $package_name) : self { - // @todo: Consider de-duplicating with the method above - // (::getConfigForRuleObject). $rules = $this->getRules(); if (empty($rules)) { return $this; } $new_config = clone $this->config; - $default_config = $this->getDefaultConfig(); - // Determine which keys are explicitly set in the global config (non-default). - // Rules cannot override these, but rules can override each other. - $globally_set_keys = []; - foreach ($default_config as $key => $default_value) { - if ($key === 'bundled_packages') { - if ($new_config->{$key} != $default_value) { - $globally_set_keys[$key] = true; - } - } elseif ($new_config->{$key} !== $default_value) { - $globally_set_keys[$key] = true; - } - } - foreach ($this->config->rules as $rule) { + $applied_keys = []; + foreach ($rules as $rule) { if (empty($rule->config)) { continue; } - $matches = $this->getMatcherFactory()->hasMatches($rule, $package_name); - if (!$matches) { + if (!$this->getMatcherFactory()->hasMatches($rule, $package_name)) { continue; } - // Apply rule config. Rules cannot override the global config, but - // later rules can override earlier rules. + // Apply rule config. Later rules override earlier rules and global + // config (last-wins, same as Renovate's packageRules). foreach ($rule->config as $key => $value) { - if (isset($globally_set_keys[$key])) { - continue; - } $new_config->{$key} = $value; + $applied_keys[$key] = true; } } - return self::createFromViolinistConfig($new_config); + if (empty($applied_keys)) { + return $this; + } + $clone = clone $this; + $clone->config = $new_config; + foreach ($applied_keys as $key => $_) { + $clone->configOptionsSet[$key] = true; + } + return $clone; } public function getRules() : array @@ -608,8 +601,12 @@ protected function mergeConfig(Config $other, $extends_name, $parent) { $keys_and_values_affected = $this->mergeConfigFromConfigObject($this->getConfig(), $other->getConfig()); $this->extendsStorage->addExtendItems($other->getExtendsStorage()->getExtendItems()); + foreach ($this->getDefaultConfig() as $key => $value) { + if ($other->hasConfigForKey($key)) { + $this->configOptionsSet[$key] = true; + } + } foreach ($keys_and_values_affected as $key => $value) { - $this->configOptionsSet[$key] = true; $this->extendsStorage->addExtendItem(new ExtendsChainItem($extends_name, $key, $value)); } } diff --git a/tests/RuleObjectConfigTest.php b/tests/RuleObjectConfigTest.php index c1d4db5..50e1b80 100644 --- a/tests/RuleObjectConfigTest.php +++ b/tests/RuleObjectConfigTest.php @@ -54,8 +54,7 @@ public function testLaterRuleOverridesEarlierRuleForSamePackage() { // A catch-all rule sets security_updates_only for all matched packages. // A more specific rule after it can override back to the default for - // specific packages, since rules can override each other (but not the - // global config). + // specific packages, since rules can override each other. $config_data = (object) [ 'rules' => [ (object) [ @@ -90,13 +89,13 @@ public function testLaterRuleOverridesEarlierRuleForSamePackage() self::assertTrue($config_for_other->shouldOnlyUpdateSecurityUpdates()); } - public function testRuleDoesNotOverrideGlobalConfigForPackage() + public function testRuleOverridesGlobalConfigForPackage() { $config_data = (object) [ 'security_updates_only' => 1, 'rules' => [ (object) [ - 'name' => 'Try to override security_updates_only', + 'name' => 'Override security_updates_only', 'matchRules' => [ (object) ['type' => 'names', 'values' => ['vendor/*']], ], @@ -109,16 +108,38 @@ public function testRuleDoesNotOverrideGlobalConfigForPackage() $config = Config::createFromViolinistConfig($config_data); $config_for_package = $config->getConfigForPackage('vendor/package-a'); - self::assertTrue($config_for_package->shouldOnlyUpdateSecurityUpdates()); + self::assertFalse($config_for_package->shouldOnlyUpdateSecurityUpdates()); } - public function testRuleDoesNotOverrideGlobalBundledPackages() + public function testRuleOverridesExplicitGlobalValueForPackage() + { + $config_data = (object) [ + 'update_dev_dependencies' => 1, + 'rules' => [ + (object) [ + 'name' => 'Override update_dev_dependencies', + 'matchRules' => [ + (object) ['type' => 'names', 'values' => ['vendor/*']], + ], + 'config' => (object) [ + 'update_dev_dependencies' => 0, + ], + ], + ], + ]; + $config = Config::createFromViolinistConfig($config_data); + + $config_for_package = $config->getConfigForPackage('vendor/package-a'); + self::assertFalse($config_for_package->shouldUpdateDevDependencies()); + } + + public function testRuleOverridesGlobalBundledPackages() { $config_data = (object) [ 'bundled_packages' => (object) ['psr/log' => ['symfony/console']], 'rules' => [ (object) [ - 'name' => 'Try to override bundled_packages', + 'name' => 'Override bundled_packages', 'matchRules' => [ (object) ['type' => 'names', 'values' => ['psr/*']], ], @@ -131,7 +152,63 @@ public function testRuleDoesNotOverrideGlobalBundledPackages() $config = Config::createFromViolinistConfig($config_data); $config_for_package = $config->getConfigForPackage('psr/log'); - self::assertEquals(['symfony/console'], $config_for_package->getBundledPackagesForPackage('psr/log')); + self::assertEquals(['other/package'], $config_for_package->getBundledPackagesForPackage('psr/log')); + } + + public function testRuleOverridesGlobalSecurityUpdatesOnlyForSpecificPackage() + { + $config_data = (object) [ + 'security_updates_only' => 1, + 'rules' => [ + (object) [ + 'name' => 'PSR Log', + 'matchRules' => [ + (object) ['type' => 'names', 'values' => ['psr/log']], + ], + 'config' => (object) [ + 'security_updates_only' => 0, + ], + ], + ], + ]; + $config = Config::createFromViolinistConfig($config_data); + + $config_for_package = $config->getConfigForPackage('psr/log'); + self::assertFalse($config_for_package->shouldOnlyUpdateSecurityUpdates()); + } + + public function testGetConfigForPackageDoesNotMarkUnsetOptionsAsSet() + { + // automerge_method is explicitly set to 'squash'; automerge_method_security + // is intentionally left unset so it should fall back to automerge_method. + $config_data = (object) [ + 'automerge_method' => 'squash', + 'rules' => [ + (object) [ + 'name' => 'Enable automerge for vendor packages', + 'matchRules' => [ + (object) ['type' => 'names', 'values' => ['vendor/*']], + ], + 'config' => (object) [ + 'automerge' => 1, + ], + ], + ], + ]; + $config = Config::createFromViolinistConfig($config_data); + + // Sanity: on the global config, automerge_method_security is unset, so + // getAutomergeMethod(true) must fall back to automerge_method ('squash'). + self::assertFalse($config->hasConfigForKey('automerge_method_security')); + self::assertEquals('squash', $config->getAutomergeMethod(true)); + + $config_for_package = $config->getConfigForPackage('vendor/package-a'); + + // After getConfigForPackage(), automerge_method_security was still never + // explicitly configured, so hasConfigForKey() must still return false and + // getAutomergeMethod(true) must still fall back to 'squash'. + self::assertFalse($config_for_package->hasConfigForKey('automerge_method_security')); + self::assertEquals('squash', $config_for_package->getAutomergeMethod(true)); } public function testRuleObjectDoesNotOverrideGlobalBundledPackages() From 554e417ce3e21b059b226dce0508f8b187da0349 Mon Sep 17 00:00:00 2001 From: "Eirik S. Morland" Date: Tue, 7 Apr 2026 14:54:07 +0200 Subject: [PATCH 6/9] Tweaks --- src/Config.php | 7 +++++++ tests/RuleObjectConfigTest.php | 28 +++++++++++++++++++++------- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/Config.php b/src/Config.php index 7609deb..1f4a774 100644 --- a/src/Config.php +++ b/src/Config.php @@ -550,6 +550,13 @@ public function getConfigForRuleObject(\stdClass $rule_object) $clone = clone $this; $clone->config = clone $this->config; $affected = $this->mergeConfigFromConfigObject($clone->config, $rule_object->config); + // bundled_packages is protected from override in mergeConfigFromConfigObject + // (which is correct for config inheritance), but rules should always be able + // to override it, consistent with getConfigForPackage(). + if (isset($rule_object->config->bundled_packages)) { + $clone->config->bundled_packages = $rule_object->config->bundled_packages; + $affected['bundled_packages'] = $rule_object->config->bundled_packages; + } foreach ($affected as $key => $value) { $clone->configOptionsSet[$key] = true; } diff --git a/tests/RuleObjectConfigTest.php b/tests/RuleObjectConfigTest.php index 50e1b80..fac4c56 100644 --- a/tests/RuleObjectConfigTest.php +++ b/tests/RuleObjectConfigTest.php @@ -175,6 +175,8 @@ public function testRuleOverridesGlobalSecurityUpdatesOnlyForSpecificPackage() $config_for_package = $config->getConfigForPackage('psr/log'); self::assertFalse($config_for_package->shouldOnlyUpdateSecurityUpdates()); + self::assertTrue($config->shouldOnlyUpdateSecurityUpdates()); + self::assertTrue($config->getConfigForPackage('not/exist')->shouldOnlyUpdateSecurityUpdates()); } public function testGetConfigForPackageDoesNotMarkUnsetOptionsAsSet() @@ -211,19 +213,31 @@ public function testGetConfigForPackageDoesNotMarkUnsetOptionsAsSet() self::assertEquals('squash', $config_for_package->getAutomergeMethod(true)); } - public function testRuleObjectDoesNotOverrideGlobalBundledPackages() + public function testGetConfigForRuleObjectAndForPackageProduceSameBundledPackages() { $config_data = (object) [ 'bundled_packages' => (object) ['psr/log' => ['symfony/console']], - ]; - $config = Config::createFromViolinistConfig($config_data); - $rule = (object) [ - 'config' => (object) [ - 'bundled_packages' => (object) ['psr/log' => ['other/package']], + 'rules' => [ + (object) [ + 'name' => 'Override bundled_packages', + 'matchRules' => [ + (object) ['type' => 'names', 'values' => ['psr/*']], + ], + 'config' => (object) [ + 'bundled_packages' => (object) ['psr/log' => ['other/package']], + ], + ], ], ]; + $config = Config::createFromViolinistConfig($config_data); + $rule = $config_data->rules[0]; + $config_for_package = $config->getConfigForPackage('psr/log'); $config_from_rule = $config->getConfigForRuleObject($rule); - self::assertEquals(['symfony/console'], $config_from_rule->getBundledPackagesForPackage('psr/log')); + + self::assertEquals( + $config_for_package->getBundledPackagesForPackage('psr/log'), + $config_from_rule->getBundledPackagesForPackage('psr/log') + ); } } From b87e5dd679f2dc0a942a9ff0bce6bb5810b457ba Mon Sep 17 00:00:00 2001 From: "Eirik S. Morland" Date: Tue, 7 Apr 2026 15:02:27 +0200 Subject: [PATCH 7/9] Simplify --- src/Config.php | 34 +++++----------------------------- tests/RuleObjectConfigTest.php | 4 ++-- 2 files changed, 7 insertions(+), 31 deletions(-) diff --git a/src/Config.php b/src/Config.php index 1f4a774..f009cf6 100644 --- a/src/Config.php +++ b/src/Config.php @@ -549,15 +549,8 @@ public function getConfigForRuleObject(\stdClass $rule_object) } $clone = clone $this; $clone->config = clone $this->config; - $affected = $this->mergeConfigFromConfigObject($clone->config, $rule_object->config); - // bundled_packages is protected from override in mergeConfigFromConfigObject - // (which is correct for config inheritance), but rules should always be able - // to override it, consistent with getConfigForPackage(). - if (isset($rule_object->config->bundled_packages)) { - $clone->config->bundled_packages = $rule_object->config->bundled_packages; - $affected['bundled_packages'] = $rule_object->config->bundled_packages; - } - foreach ($affected as $key => $value) { + foreach ($rule_object->config as $key => $value) { + $clone->config->{$key} = $value; $clone->configOptionsSet[$key] = true; } return $clone; @@ -569,31 +562,14 @@ public function getConfigForPackage(string $package_name) : self if (empty($rules)) { return $this; } - $new_config = clone $this->config; - $applied_keys = []; + $result = $this; foreach ($rules as $rule) { - if (empty($rule->config)) { - continue; - } if (!$this->getMatcherFactory()->hasMatches($rule, $package_name)) { continue; } - // Apply rule config. Later rules override earlier rules and global - // config (last-wins, same as Renovate's packageRules). - foreach ($rule->config as $key => $value) { - $new_config->{$key} = $value; - $applied_keys[$key] = true; - } - } - if (empty($applied_keys)) { - return $this; + $result = $result->getConfigForRuleObject($rule); } - $clone = clone $this; - $clone->config = $new_config; - foreach ($applied_keys as $key => $_) { - $clone->configOptionsSet[$key] = true; - } - return $clone; + return $result; } public function getRules() : array diff --git a/tests/RuleObjectConfigTest.php b/tests/RuleObjectConfigTest.php index fac4c56..71deaff 100644 --- a/tests/RuleObjectConfigTest.php +++ b/tests/RuleObjectConfigTest.php @@ -32,7 +32,7 @@ public function testRuleWithoutConfigReturnsOriginalConfig() self::assertSame($config, $result); } - public function testRuleDoesNotOverrideExistingConfig() + public function testRuleOverridesExistingConfig() { $config = Config::createFromViolinistConfig((object) [ 'update_dev_dependencies' => 0, @@ -47,7 +47,7 @@ public function testRuleDoesNotOverrideExistingConfig() self::assertNotSame($config, $config_from_rule); self::assertFalse($config->shouldUpdateDevDependencies()); - self::assertFalse($config_from_rule->shouldUpdateDevDependencies()); + self::assertTrue($config_from_rule->shouldUpdateDevDependencies()); } public function testLaterRuleOverridesEarlierRuleForSamePackage() From 01810d0acb16f50b729d7b2d36bf57f746f7fe26 Mon Sep 17 00:00:00 2001 From: "Eirik S. Morland" Date: Tue, 7 Apr 2026 21:02:09 +0200 Subject: [PATCH 8/9] Review fixes --- src/Config.php | 23 ++++++++++++++++------- tests/RuleObjectConfigTest.php | 15 +++++++++++++++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/Config.php b/src/Config.php index f009cf6..d575b3e 100644 --- a/src/Config.php +++ b/src/Config.php @@ -230,14 +230,24 @@ public static function createFromViolinistConfigJsonString(string $data) public function setConfig($config) { + if (!$config instanceof \stdClass) { + return; + } + $config = $this->normalizeConfigKeys($config); foreach ($this->getDefaultConfig() as $key => $value) { if (isset($config->{$key})) { $this->config->{$key} = $config->{$key}; $this->configOptionsSet[$key] = true; } } - // Also make sure to set the block list config from the deprecated part. - // Plus alternative spelling from allow list. + if (!empty($config->rules)) { + $this->config->rules = $config->rules; + } + } + + private function normalizeConfigKeys(\stdClass $config) : \stdClass + { + $config = clone $config; $renamed_and_aliased = [ 'blacklist' => 'blocklist', 'block_list' => 'blocklist', @@ -245,12 +255,11 @@ public function setConfig($config) ]; foreach ($renamed_and_aliased as $not_real => $real) { if (isset($config->{$not_real})) { - $this->config->{$real} = $config->{$not_real}; + $config->{$real} = $config->{$not_real}; + unset($config->{$not_real}); } } - if (!empty($config->rules)) { - $this->config->rules = $config->rules; - } + return $config; } public function getComposerOutdatedFlag() : string @@ -549,7 +558,7 @@ public function getConfigForRuleObject(\stdClass $rule_object) } $clone = clone $this; $clone->config = clone $this->config; - foreach ($rule_object->config as $key => $value) { + foreach ($this->normalizeConfigKeys($rule_object->config) as $key => $value) { $clone->config->{$key} = $value; $clone->configOptionsSet[$key] = true; } diff --git a/tests/RuleObjectConfigTest.php b/tests/RuleObjectConfigTest.php index 71deaff..a77854a 100644 --- a/tests/RuleObjectConfigTest.php +++ b/tests/RuleObjectConfigTest.php @@ -7,6 +7,21 @@ class RuleObjectConfigTest extends TestCase { + public function testRuleWithDeprecatedAliasKeyIsNormalized() + { + $config = new Config(); + $rule = (object) [ + 'config' => (object) [ + 'blacklist' => ['vendor/package-a'], + ], + ]; + + $config_from_rule = $config->getConfigForRuleObject($rule); + + self::assertEquals(['vendor/package-a'], $config_from_rule->getBlockList()); + self::assertTrue($config_from_rule->hasConfigForKey('blocklist')); + } + public function testGetConfigForRuleObjectReturnsRuleConfig() { $config_data = json_decode(file_get_contents(__DIR__ . '/fixtures/violinist-test-drupal-config.json')); From 92f31fa74f75be0239d0ee9d77867a16c158a1e0 Mon Sep 17 00:00:00 2001 From: "Eirik S. Morland" Date: Tue, 7 Apr 2026 21:59:42 +0200 Subject: [PATCH 9/9] Review fixes --- src/Config.php | 16 ++++- src/ExtendsStorage.php | 7 +++ tests/RuleObjectConfigTest.php | 58 +++++++++++++++++++ tests/VersionThree/OverrideVisibilityTest.php | 25 ++++++++ 4 files changed, 104 insertions(+), 2 deletions(-) diff --git a/src/Config.php b/src/Config.php index d575b3e..98f0e5d 100644 --- a/src/Config.php +++ b/src/Config.php @@ -558,9 +558,15 @@ public function getConfigForRuleObject(\stdClass $rule_object) } $clone = clone $this; $clone->config = clone $this->config; - foreach ($this->normalizeConfigKeys($rule_object->config) as $key => $value) { - $clone->config->{$key} = $value; + $clone->extendsStorage = clone $this->extendsStorage; + $normalized = $this->normalizeConfigKeys($rule_object->config); + foreach ($this->getDefaultConfig() as $key => $value) { + if (!isset($normalized->{$key})) { + continue; + } + $clone->config->{$key} = $normalized->{$key}; $clone->configOptionsSet[$key] = true; + $clone->extendsStorage->removeItemsForKey($key); } return $clone; } @@ -578,6 +584,12 @@ public function getConfigForPackage(string $package_name) : self } $result = $result->getConfigForRuleObject($rule); } + if ($result === $this) { + $clone = clone $this; + $clone->config = clone $this->config; + $clone->extendsStorage = clone $this->extendsStorage; + return $clone; + } return $result; } diff --git a/src/ExtendsStorage.php b/src/ExtendsStorage.php index 5cb9001..a01acb7 100644 --- a/src/ExtendsStorage.php +++ b/src/ExtendsStorage.php @@ -24,4 +24,11 @@ public function addExtendItem(ExtendsChainItem $item) { $this->items[$item->getName()][$item->getKey()] = $item; } + + public function removeItemsForKey(string $key) + { + foreach ($this->items as $extend_name => $items) { + unset($this->items[$extend_name][$key]); + } + } } diff --git a/tests/RuleObjectConfigTest.php b/tests/RuleObjectConfigTest.php index a77854a..d8311c0 100644 --- a/tests/RuleObjectConfigTest.php +++ b/tests/RuleObjectConfigTest.php @@ -255,4 +255,62 @@ public function testGetConfigForRuleObjectAndForPackageProduceSameBundledPackage $config_from_rule->getBundledPackagesForPackage('psr/log') ); } + + public function testRuleConfigWithUnknownKeyIsIgnored() + { + $config = new Config(); + $rule = (object) [ + 'config' => (object) [ + 'this_key_does_not_exist' => 'some_value', + 'security_updates_only' => 1, + ], + ]; + $config_from_rule = $config->getConfigForRuleObject($rule); + self::assertFalse($config_from_rule->hasConfigForKey('this_key_does_not_exist')); + // Known keys are still applied. + self::assertTrue($config_from_rule->shouldOnlyUpdateSecurityUpdates()); + } + + public function testRuleConfigWithNestedRulesIsNotPropagated() + { + $config = new Config(); + $rule = (object) [ + 'config' => (object) [ + 'security_updates_only' => 1, + 'rules' => [ + (object) [ + 'config' => (object) ['automerge' => 1], + ], + ], + ], + ]; + $config_from_rule = $config->getConfigForRuleObject($rule); + // Nested rules must not leak into the package-specific config. + self::assertEmpty($config_from_rule->getRules()); + // Other keys in the rule config are still applied. + self::assertTrue($config_from_rule->shouldOnlyUpdateSecurityUpdates()); + } + + public function testGetConfigForPackageReturnsIndependentConfigWhenNoRuleMatches() + { + $config = Config::createFromViolinistConfig((object) [ + 'security_updates_only' => 0, + 'rules' => [ + (object) [ + 'matchRules' => [ + (object) ['type' => 'names', 'values' => ['vendor/specific-package']], + ], + 'config' => (object) ['security_updates_only' => 1], + ], + ], + ]); + + $config_for_unmatched = $config->getConfigForPackage('vendor/other-package'); + + // Must be an independent object, not the global config instance. + self::assertNotSame($config, $config_for_unmatched); + // Mutating the returned config must not affect the global config. + $config_for_unmatched->setConfig((object) ['security_updates_only' => 1]); + self::assertFalse($config->shouldOnlyUpdateSecurityUpdates()); + } } diff --git a/tests/VersionThree/OverrideVisibilityTest.php b/tests/VersionThree/OverrideVisibilityTest.php index c689d76..c84c2a6 100644 --- a/tests/VersionThree/OverrideVisibilityTest.php +++ b/tests/VersionThree/OverrideVisibilityTest.php @@ -26,4 +26,29 @@ public function testInvalidExtendName() self::expectException(\RuntimeException::class); $readable = $config->getReadableChainForExtendName('invalid_key'); } + + public function testRuleOverridingExtendsKeyResetsProvenance() + { + // automerge_method = 'squash' originates from violinist-base-config.json via extends. + self::assertEquals('violinist-base-config.json', $this->config->getExtendNameForKey('automerge_method')); + + $this->config->setConfig((object) [ + 'rules' => [ + (object) [ + 'matchRules' => [ + (object) ['type' => 'names', 'values' => ['vendor/package-a']], + ], + 'config' => (object) ['automerge_method' => 'merge'], + ], + ], + ]); + + $config_for_package = $this->config->getConfigForPackage('vendor/package-a'); + + // The rule overrode automerge_method; the extend file must no longer + // be reported as its source on the package-specific config. + self::assertEquals('', $config_for_package->getExtendNameForKey('automerge_method')); + // The global config's provenance must be unaffected. + self::assertEquals('violinist-base-config.json', $this->config->getExtendNameForKey('automerge_method')); + } }