search

zendframework / zend-code

BSD 3-Clause "New" or "Revised" License
1.68k stars 78 forks source link

Feature: Add ability to define declare statements #169

Closed icanhazstring closed 5 years ago

icanhazstring commented 5 years ago

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

michalbundyra commented 5 years ago

@icanhazstring I would go with DeclareStatement

michalbundyra commented 5 years ago

@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 commented 5 years ago

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

icanhazstring commented 5 years ago

@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 ;)

michalbundyra commented 5 years ago

Thanks, @icanhazstring!