Allow processing class constants #37

Merged
merged 6 commits into from Feb 22, 2017

Projects

None yet

2 participants

@weierophinney
Member
weierophinney commented Feb 17, 2017 edited

This patch updates the Constant processor to allow processing class constants, including the ::class pseudo-constant; additionally, it ensures that both the Token and Constant processors can apply to Config keys as well as the values by calling their enableKeyProcessing() methods.

To allow this to work, I've changed the visibility of Token::doProcess() from private to protected, to allow overriding the method within the Constant class. I've also added the new test case ZendTest\Config\Processor\Constant to cover the behavior.

Essentially, this allows the following to work (assuming the provided classes and constants exist):

{
    "Acme\\Component::CONFIG_KEY": {
        "host": "Acme\\Component::CONFIG_HOST",
        "dependencies": {
	    "Acme\\Foo::class": "Acme\\FooFactory::class",
        }
    }
}

Via:

$config = new Config(Factory::fromFile('config.json'), true);
$processor = new Processor\Constant();
$processor->enableKeyProcessing();
$processor->process($config);
@weierophinney weierophinney Allow processing class constants
This patch updates the `Constant` processor to allow processing class
constants, including the `::class` pseudo-constant.

To allow this to work, I've changed the visibility of
`Token::doProcess()` from `private` to `protected`, to allow
overriding the method within the `Constant` class. I've also added two
new tests in the new test case `ZendTest\Config\Processor\Constant` to
cover the behavior.
138c940
@weierophinney weierophinney Allow constant and token value subsitutions in keys
This patch updates the `Token` processor, and, by extension, the
`Constant` processor, to allow substituting tokens/constants found in
*key* values. This enables the following:

```json
{
    "Acme\\Component::CONFIG_KEY": {
        "dependencies": {
            "Acme\\Middleware::class": "Acme\\MiddlewareFactory::class"
        }
    }
}
```
4a7c77d
@Ocramius
Member

I really don't like the fact that a string may now possibly change meaning with a version upgrade.

This should be mentioned as BC Break because of that.

Still, feature is awesome and worth pursuing.

src/Processor/Constant.php
+ }
+
+ // Handle ::class notation
+ if (! preg_match('/::class$/', $value)) {
@Ocramius
Ocramius Feb 17, 2017 Member

::class must be case-insensitive (test also needs to be added for that)

@weierophinney
weierophinney Feb 17, 2017 Member

Forgot about that; will update with a data provider.

src/Processor/Constant.php
+ return $class;
+ }
+
+ return parent::doProcess($value, $replacements);
@Ocramius
Ocramius Feb 17, 2017 Member

This makes the string just become... a string again. I think this counts as a silent failure, and should be avoided.

@weierophinney
weierophinney Feb 17, 2017 Member

I decided to do some testing locally and see if it was possible to get to this point in the code in any meaningful way, and, well, it's not. If you were to try and define a constant named Foo::class, for instance, PHP tells you "Class contasnts cannot be defined or redefined", even if the class does not exist. Which means that once we get to this point, there's no way $replacements could even contain a value for it. As such, this should likely just be return $value — is that what you're suggesting, @Ocramius ?

@Ocramius Ocramius added the BC Break label Feb 17, 2017
@weierophinney
Member

This should be mentioned as BC Break because of that.

I'll update the class to accept a flag for enabling the behavior instead, and have that disabled by default; that way we can introduce the feature without requiring a major version bump.

weierophinney added some commits Feb 17, 2017
@weierophinney weierophinney Make key processing opt-in
Prevents BC break, as the original behavior, which does not process
keys, is the default.

Call `$processor->enableKeyProcessing()` to enable it.
53e616b
@weierophinney weierophinney Return the value verbatim if it does not resolve to a class 0e450f9
@weierophinney weierophinney Document new constant features
- `Constant` processor now also recognizes class and `::class` constants.
- `Constant` and `Token` processors now allow optionally processing keys.
1955e33
@weierophinney weierophinney removed the BC Break label Feb 17, 2017
@weierophinney
Member

Removed BC break flag, as I've made the key processing opt-in at this time.

@weierophinney weierophinney added this to the 3.1.0 milestone Feb 17, 2017
+As of version 3.1.0, you can also tell the `Constant` processor to process keys:
+
+```php
+$processor->enableKeyProcessing();
@Ocramius
Ocramius Feb 17, 2017 Member

A constructor argument would be preferrable

@weierophinney
Member
@weierophinney weierophinney Allow enabling key processing via constructor arguments
Added a new boolean argument to each of the `Token` and `Constant`
processors, `$enableKeyProcessing`. Defaults to `false`, but when set to
`true`, will enable key processing in each.

This patch also adds type casting for optional arguments that are
expected to be of known types.
0921839

Added constructor arguments and documented them. Kept the method call as well, as the additional constructor argument is non-intuitive if that is the only behavior you are changing on the instance.

@weierophinney weierophinney merged commit 0921839 into zendframework:master Feb 22, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.2%) to 84.272%
Details
@weierophinney weierophinney added a commit that referenced this pull request Feb 22, 2017
@weierophinney weierophinney Added CHANGELOG for #37 776bbf5
@weierophinney weierophinney added a commit that referenced this pull request Feb 22, 2017
@weierophinney weierophinney Merge branch 'hotfix/class-constants'
Close #37
ff11a3d
@weierophinney weierophinney added a commit that referenced this pull request Feb 22, 2017
@weierophinney weierophinney Merge branch 'hotfix/class-constants' into develop
Forward port #37

Conflicts:
	CHANGELOG.md
69eab1c
@weierophinney weierophinney deleted the weierophinney:hotfix/class-constants branch Feb 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment