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

Feature request: Array property extending doesn't work when property has default value #2228

Closed
jrfnl opened this issue Nov 13, 2018 · 3 comments

Comments

@jrfnl
Copy link
Contributor

jrfnl commented Nov 13, 2018

PR #2154 added support for array properties being extended, however it appears that this doesn't work when an array property already has a default value set.

If we take the original example of the Generic.PHP.ForbiddenFunctions sniff, it all works correctly if you overrule the forbiddenFunctions property in a standard and then extend it from a project-based ruleset.

However, if you want to extend the default values of the forbiddenFunctions property:

public $forbiddenFunctions = [
     'sizeof' => 'count',
     'delete' => 'unset',
];

either via a standard or a project ruleset:

<rule ref="Generic.PHP.ForbiddenFunctions">
	<properties>
		<property name="forbiddenFunctions" type="array" extend="true">
			<element key="foo" value="fixedFoo"/>
			<element key="bar" value="fixedBar"/>
		</property>
	</properties>
</rule>

You end up with:

public $forbiddenFunctions = [
    'foo' => 'fixedFoo',
    'bar' => 'fixedBar',
];

instead of the expected:

public $forbiddenFunctions = [
    'sizeof' => 'count',
    'delete' => 'unset',
    'foo' => 'fixedFoo',
    'bar' => 'fixedBar',
];

@Majkl578 Any thoughts on this ?

Related #2153

jrfnl added a commit to Yoast/yoastcs that referenced this issue Nov 13, 2018
The new array property `extend`-ing as added in PHPCS 3.4.0 appears to have a bug which doesn't allow it to work with default values of array properties, so until this bug is fixed, the default value of the `TestDoubles` `doubles_path` property also needs to be set in the standard.

For more details, see the upstream bug report:  squizlabs/PHP_CodeSniffer#2228
@gsherwood
Copy link
Member

This is an interesting use case, but not what the feature was designed to do, or how I documented it. I don't think this should be considered a bug - it just a feature that still doesn't exist despite the extends feature being added.

@jrfnl
Copy link
Contributor Author

jrfnl commented Nov 14, 2018

Let's call it a feature request in that case. Without having read the (not yet published as not yet released) changelog, intuitively I, at least, would expect this to work as described above.

@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 2, 2023

Closing as replaced by PHPCSStandards/PHP_CodeSniffer#15

@jrfnl jrfnl closed this as not planned Won't fix, can't repro, duplicate, stale Dec 2, 2023
@github-project-automation github-project-automation bot moved this to Ready for Release in PHPCS v3 Development Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Ready for Release
Development

No branches or pull requests

2 participants