[PHP] Use IDL namer in PHP generator #8842
Open
+392
−128
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Most languages are not affected by this change. In PHP, some names such as Bool cannot be used because it is a reserved keyword for to the bool data type. The field `keywords_casing` in the configs enables checking all characters in lowercase against every keyword. This should be safe as flatbuffers does not allow non-ASCII characters in its grammar.
github-actions
bot
added
golang
python
java
c++
javascript
rust
typescript
codegen
Collaborator
|
Thanks @razvanalex -- I still have these on my list to review and get merged. Doing all this on volunteer time but I'm still eager to keep things rolling forward! |
Contributor
Author
|
I really appreciate your time! I'm also doing this in my free time, this is why it took me so long with this PR. I am still working on finalizing (i.e., specifically with testing and keep rebasing the code) the other three PRs to improve the status for PHP flatbuffers. The only concern I have now is that the changes depend on previous ones (hence stacked PRs), and opening new PRs will falsely trigger the github-action labeler to tag with other languages (such as in this PR). |
Collaborator
|
don't sweat the labeler -- itll correct as we chew through these. |
This refactors the code from google#8202, specifically the part related to using the NamerPhp.
Add a test case to verify the PHP code generator's handling of FlatBuffers type names that conflict with PHP's case-insensitive reserved keywords. The previous changes updated `idl_gen_php.cpp` to use `CASE_INSENSITIVE` keyword checking. This test ensures that when a FlatBuffers schema defines a type (e.g., `table Bool`) with a name that is a PHP keyword, the generated PHP code correctly escapes the identifier (e.g., `class Bool_`) to prevent syntax errors.
razvanalex
force-pushed
the
php-idl-namer
branch
from
aaa7f1f to
95fb8ff
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refactors the PHP IDL generator to use
IdlNamer. This continues the work partially addressed in #7491 (which appears stale).I noticed that tables using PHP keywords as names (e.g.,
Bool,String) would yield errors in the generated code because keywords cannot be used as class names in PHP, regardless of casing (see PHP docs). It would complain with an error similar to:To address this, this PR utilizes the
CASE_INSENSITIVEkeyword check introduced in #8420. I have enabled the option for the PHP generator and added a new test case (tests/php_keyword_test.fbs) to verify that keywords (e.g.,Bool) are correctly escaped (e.g.,Bool_).This is a stacked PR on top of #8420.