Skip to content
This repository was archived by the owner on Jan 29, 2020. It is now read-only.

Allow arrays as constant values in generated constants #94

Closed
wants to merge 5 commits into from

Conversation

basz
Copy link
Contributor

@basz basz commented Sep 28, 2016

const FOO = array('bar'=>'foobar');

This is a valid const, but can't be generated

@basz basz changed the base branch from master to develop September 28, 2016 12:49
@Ocramius
Copy link
Member

@basz can you add a test case for it?

@Ocramius Ocramius added the bug label Sep 28, 2016
@Ocramius Ocramius added this to the 3.0.5 milestone Sep 28, 2016
@basz
Copy link
Contributor Author

basz commented Sep 29, 2016

@Ocramius update by author

$propertyGenerator = new PropertyGenerator('FOO', $generator);
$propertyGenerator->setConst(true);

$this->assertInternalType('string', $propertyGenerator->generate());
Copy link
Member

Choose a reason for hiding this comment

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

Your data provider should probably also give the expected string outcome.

[new PropertyValueGenerator('bar', PropertyValueGenerator::TYPE_STRING)],
[new PropertyValueGenerator(null, PropertyValueGenerator::TYPE_NULL)],
[new PropertyValueGenerator(null, PropertyValueGenerator::TYPE_NULL)],
[new PropertyValueGenerator('PHP_EOL', PropertyValueGenerator::TYPE_CONSTANT)],
Copy link
Member

Choose a reason for hiding this comment

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

What about nested values? Such as ['foo' => ['bar' => 'baz']]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about them? They aren't asserted by the generator afaik...

@Ocramius Ocramius self-assigned this Oct 24, 2016
@Ocramius Ocramius changed the title Fix/const valid types Allow arrays as constant values in generated constants Oct 24, 2016
Ocramius added a commit that referenced this pull request Oct 24, 2016
@Ocramius Ocramius closed this in 8856a32 Oct 24, 2016
Ocramius added a commit that referenced this pull request Oct 24, 2016
Ocramius added a commit that referenced this pull request Oct 24, 2016
@Ocramius
Copy link
Member

@basz merged, thanks!

master: 8856a32
develop: 4606aad

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants