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

Feature: Add ability to define declare statements #169

Merged
merged 7 commits into from Oct 5, 2019
Merged

Feature: Add ability to define declare statements #169

merged 7 commits into from Oct 5, 2019

Conversation

icanhazstring
Copy link
Contributor

@icanhazstring icanhazstring commented Jun 17, 2019

This will add the ability to configure declare() statements
at the top of generated files.

I took the implementation suggestion from @GaryJones in #102 and changes it a bit.
I changed the behavior to be only able to generate valid declare statements at all.
Also I added static factories to generate the possible directives.

Should solve #102


One problem tho. Since declare is a reserved keyword. The I decided to go with Declare_ as the class representation. But the codesniffer doesn't like this. Any suggestions? Renamed to DeclareStatement as suggested by @webimpress

This adds the ability to configure declare() statements
at the top of generated files. It is only possible to
generate valid declare statements as stated in php documentation
https://www.php.net/manual/en/control-structures.declare.php

Solves #102
@michalbundyra
Copy link
Member

@icanhazstring I would go with DeclareStatement

@michalbundyra
Copy link
Member

@icanhazstring maybe I missed something but with that implementation is not possible to add declare like:

declare(strict_types=1,ticks=1);

but it's valid. It can have even all three declarations - strict_types, ticks and encodning.

@michalbundyra
Copy link
Member

@icanhazstring
Ok, nvm. I see, you are doing it on separate statements. It's fine. Maybe even better :)

@icanhazstring
Copy link
Contributor Author

icanhazstring commented Jun 17, 2019

@webimpress wanted to write this. Yes ;)
The other way around would be also possible, but I guess this is a matter of taste. I would have to change the FileGenerator to only have a single DeclareStatement. Maybe then we could remove the logic inside setDeclares to check if the declare is already given. Maybe some better SoC? Don't know ;)

src/DeclareStatement.php Outdated Show resolved Hide resolved
src/DeclareStatement.php Outdated Show resolved Hide resolved
src/DeclareStatement.php Outdated Show resolved Hide resolved
src/DeclareStatement.php Outdated Show resolved Hide resolved
src/Generator/FileGenerator.php Outdated Show resolved Hide resolved
src/Generator/FileGenerator.php Outdated Show resolved Hide resolved
test/Generator/FileGeneratorTest.php Outdated Show resolved Hide resolved
src/DeclareStatement.php Outdated Show resolved Hide resolved
src/DeclareStatement.php Outdated Show resolved Hide resolved
@michalbundyra michalbundyra added this to the 3.4.0 milestone Aug 28, 2019
@michalbundyra michalbundyra changed the base branch from master to develop October 5, 2019 22:37
michalbundyra added a commit that referenced this pull request Oct 5, 2019
Feature: Add ability to define declare statements
michalbundyra added a commit that referenced this pull request Oct 5, 2019
michalbundyra added a commit that referenced this pull request Oct 5, 2019
@michalbundyra michalbundyra merged commit 6918dee into zendframework:develop Oct 5, 2019
@michalbundyra
Copy link
Member

Thanks, @icanhazstring!

@icanhazstring icanhazstring deleted the feature/declare branch October 10, 2019 11:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants