You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@thrift.apache.org by "sveneld (via GitHub)" <gi...@apache.org> on 2026/05/26 19:53:51 UTC
[PR] THRIFT-1209: Regression test for PHP-reserved words as Thrift enum names [thrift]
sveneld opened a new pull request, #3532:
URL: https://github.com/apache/thrift/pull/3532
<!-- Explain the changes in the pull request below: -->
Resolves [THRIFT-1209](https://issues.apache.org/jira/browse/THRIFT-1209).
The original bug: an IDL `enum { GLOBAL = 1, ... }` produced PHP code (`class Foo { const GLOBAL = 1; }`) that failed to parse on older PHP. PHP 7.1+ permits reserved identifiers as class constants (except `class`), so the historical breakage is gone — but there is no test pinning that behaviour. This adds one:
- `lib/php/test/Resources/ThriftTest.thrift` gains `enum ReservedKeywordEnum { GLOBAL, STATIC, LIST, RETURN }` (PHP-only test thrift, won't affect cross-tests).
- New `lib/php/test/Unit/Compiler/ReservedKeywordEnumTest.php` references `\Basic\TestValidators\ReservedKeywordEnum::GLOBAL` etc., forcing the generated stub to be parsed by PHP. If the generator ever regresses and emits unparseable code for these names, phpunit will surface it.
- [x] [Apache JIRA](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket: THRIFT-1209
- [x] Title follows `THRIFT-NNNN: …`
- [x] Single squashed commit
- [x] No breaking change — test-only
- [ ] `[skip ci]` (n/a — test addition)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-1209: Regression test for PHP-reserved words as Thrift enum names [thrift]
Posted by "sveneld (via GitHub)" <gi...@apache.org>.
sveneld commented on PR #3532:
URL: https://github.com/apache/thrift/pull/3532#issuecomment-4548337635
Closing — the test was misaimed. THRIFT-1209 was about the legacy non-namespaced PHP generator output (`final class metastore_HiveObjectType { const GLOBAL = 1; ... }`) failing on older PHP. That was resolved by two independent refactors: (1) the PHP minimum bump to 8.1 (THRIFT-5956 / PHP 7.1+ allows reserved words as class constants), and (2) the namespaced PHP generator output. Adding a regression test today tests the current behaviour rather than the original bug. Will note that on the JIRA.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-1209: Regression test for PHP-reserved words as Thrift enum names [thrift]
Posted by "sveneld (via GitHub)" <gi...@apache.org>.
sveneld closed pull request #3532: THRIFT-1209: Regression test for PHP-reserved words as Thrift enum names
URL: https://github.com/apache/thrift/pull/3532
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org