You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "junichi11 (via GitHub)" <gi...@apache.org> on 2023/11/27 05:43:24 UTC
[PR] PHP 8.3 Support: Dynamic class constant fetch #6701 [netbeans]
junichi11 opened a new pull request, #6752:
URL: https://github.com/apache/netbeans/pull/6752
- https://github.com/apache/netbeans/issues/6701
- https://wiki.php.net/rfc/dynamic_class_constant_fetch
### PHP 8.3 Support: Add PHP 8.3 to the PhpVersion
- https://wiki.php.net/rfc#php_83
- https://wiki.php.net/todo/php83
### PHP 8.3 Support: Dynamic class constant fetch (Part 1)
- Fix the grammar file (parser)
- Add the `PHP83UnhandledError`
- Add/Fix unit tests for the parser
Example:
```php
class Example {
const CONSTANT = 'constant';
}
$constant = 'CONSTANT';
echo Example::{$constant};
```
PHP 8.3:
PHP 8.2:
### PHP 8.3 Support: Dynamic class constant fetch (Part 2)
- Fix the `SemanticAnalysis`
- Fix the mark occurrences and the go to declaration features
- Add unit tests
### PHP 8.3 Support: Dynamic class constant fetch (Part 3)
- Fix the formatter
- Add the option of the around spaces for the scope resolution operator (`::`)
- Don't add spaces within braces (`{}`) e.g. {$example}
- Add unit tests
### PHP 8.3 Support: Dynamic class constant fetch (Part 4)
- Fix the code completion feature
- Add unit tests for the code completion and brace matching features
### PHP 8.3 Support: Dynamic class constant fetch (Part 5)
- Fix the `IntroduceSuggestion` and `PHP55UnhandledError`
- Add unit tests
--
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@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] PHP 8.3 Support: Dynamic class constant fetch #6701 [netbeans]
Posted by "tmysik (via GitHub)" <gi...@apache.org>.
tmysik commented on code in PR #6752:
URL: https://github.com/apache/netbeans/pull/6752#discussion_r1406317313
##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/PHP55UnhandledError.java:
##########
@@ -124,11 +124,15 @@ public void visit(StaticConstantAccess node) {
if (CancelSupport.getDefault().isCancelled()) {
return;
}
- Identifier constant = node.getConstantName();
- if (constant != null) {
- String constantName = constant.getName();
- if ("class".equals(constantName.toLowerCase())) { //NOI18N
- createError(constant);
+ if (node.isDynamicName()) {
+ super.visit(node);
+ } else {
+ Identifier constant = node.getConstantName();
+ if (constant != null) {
+ String constantName = constant.getName();
+ if ("class".equals(constantName.toLowerCase())) { //NOI18N
Review Comment:
Nitpick: I believe we should use some locale in `toLowerCase()` call.
--
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@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] PHP 8.3 Support: Dynamic class constant fetch #6701 [netbeans]
Posted by "tmysik (via GitHub)" <gi...@apache.org>.
tmysik commented on code in PR #6752:
URL: https://github.com/apache/netbeans/pull/6752#discussion_r1406280616
##########
php/php.editor/src/org/netbeans/modules/php/editor/parser/astnodes/StaticConstantAccess.java:
##########
@@ -54,10 +62,24 @@ public Identifier getConstantName() {
while (expression instanceof ExpressionArrayAccess) {
expression = ((ExpressionArrayAccess) expression).getExpression();
}
+ // can use dynamic constant names since PHP 8.3
+ // e.g. Foo::{$bar};
+ assert expression != null;
+ if (isDynamicName) {
+ // XXX create an identifier to avoid NPE, can improve this?
Review Comment:
`XXX` - do we want to keep ii in sources?
--
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@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] PHP 8.3 Support: Dynamic class constant fetch #6701 [netbeans]
Posted by "tmysik (via GitHub)" <gi...@apache.org>.
tmysik commented on code in PR #6752:
URL: https://github.com/apache/netbeans/pull/6752#discussion_r1406284183
##########
php/php.editor/src/org/netbeans/modules/php/editor/parser/astnodes/StaticConstantAccess.java:
##########
@@ -54,10 +62,24 @@ public Identifier getConstantName() {
while (expression instanceof ExpressionArrayAccess) {
expression = ((ExpressionArrayAccess) expression).getExpression();
}
+ // can use dynamic constant names since PHP 8.3
+ // e.g. Foo::{$bar};
+ assert expression != null;
+ if (isDynamicName) {
+ // XXX create an identifier to avoid NPE, can improve this?
+ if (expression instanceof ReflectionVariable) {
+ expression = ((ReflectionVariable) expression).getName();
+ }
+ expression = new Identifier(constant.getStartOffset(), constant.getEndOffset(), "{" + expression.toString() + "}"); // NOI18N
+ }
assert expression instanceof Identifier;
Review Comment:
Perhaps a bit better?
`assert expression instanceof Identifier : "Expected Identifier but got: " + expression.getClass().getName();`
--
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@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] PHP 8.3 Support: Dynamic class constant fetch #6701 [netbeans]
Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 commented on code in PR #6752:
URL: https://github.com/apache/netbeans/pull/6752#discussion_r1406852978
##########
php/php.editor/src/org/netbeans/modules/php/editor/parser/astnodes/StaticConstantAccess.java:
##########
@@ -54,10 +62,24 @@ public Identifier getConstantName() {
while (expression instanceof ExpressionArrayAccess) {
expression = ((ExpressionArrayAccess) expression).getExpression();
}
+ // can use dynamic constant names since PHP 8.3
+ // e.g. Foo::{$bar};
+ assert expression != null;
+ if (isDynamicName) {
+ // XXX create an identifier to avoid NPE, can improve this?
+ if (expression instanceof ReflectionVariable) {
+ expression = ((ReflectionVariable) expression).getName();
+ }
+ expression = new Identifier(constant.getStartOffset(), constant.getEndOffset(), "{" + expression.toString() + "}"); // NOI18N
+ }
assert expression instanceof Identifier;
Review Comment:
Yes, I'll change to it :) Thanks!
--
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@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] PHP 8.3 Support: Dynamic class constant fetch #6701 [netbeans]
Posted by "tmysik (via GitHub)" <gi...@apache.org>.
tmysik commented on PR #6752:
URL: https://github.com/apache/netbeans/pull/6752#issuecomment-1828945465
@junichi11 Nothing to thank for, really. NetBeans users have to thank you, Junichi. Without you, there would not be any up-to-date PHP support in NetBeans at all! 👍
--
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@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] PHP 8.3 Support: Dynamic class constant fetch #6701 [netbeans]
Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 commented on PR #6752:
URL: https://github.com/apache/netbeans/pull/6752#issuecomment-1827819316
@tmysik Could you please have a look at each commit?
--
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@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] PHP 8.3 Support: Dynamic class constant fetch #6701 [netbeans]
Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 commented on PR #6752:
URL: https://github.com/apache/netbeans/pull/6752#issuecomment-1828797756
@tmysik Fixed : https://github.com/apache/netbeans/compare/e53b9278c63011edf23dc592d66827e77d98edbe..7e705639b19ce57a0fe0984cdc8e91ac2f4272bc
Thank you for your review and your time :)
--
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@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] PHP 8.3 Support: Dynamic class constant fetch #6701 [netbeans]
Posted by "tmysik (via GitHub)" <gi...@apache.org>.
tmysik commented on code in PR #6752:
URL: https://github.com/apache/netbeans/pull/6752#discussion_r1406312854
##########
php/php.editor/test/unit/src/org/netbeans/modules/php/editor/PHPBracesMatcherTest.java:
##########
@@ -522,7 +522,7 @@ public void testAttributeSyntax_05b() throws Exception {
+ " #[Class1(5)]\n"
+ " public function method~(#[Class1(6)] $param1, #[Class1('foo', 'bar', 7)] int $pram2@)^ {}\n"
+ "\n"
- + "@}^"
+ + "}"
Review Comment:
Why these changes exactly?
--
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@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] PHP 8.3 Support: Dynamic class constant fetch #6701 [netbeans]
Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 commented on code in PR #6752:
URL: https://github.com/apache/netbeans/pull/6752#discussion_r1406851399
##########
php/php.editor/src/org/netbeans/modules/php/editor/parser/astnodes/StaticConstantAccess.java:
##########
@@ -54,10 +62,24 @@ public Identifier getConstantName() {
while (expression instanceof ExpressionArrayAccess) {
expression = ((ExpressionArrayAccess) expression).getExpression();
}
+ // can use dynamic constant names since PHP 8.3
+ // e.g. Foo::{$bar};
+ assert expression != null;
+ if (isDynamicName) {
+ // XXX create an identifier to avoid NPE, can improve this?
Review Comment:
Will remove `XXX`. Thanks!
--
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@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] PHP 8.3 Support: Dynamic class constant fetch #6701 [netbeans]
Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 commented on code in PR #6752:
URL: https://github.com/apache/netbeans/pull/6752#discussion_r1406862268
##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/PHP55UnhandledError.java:
##########
@@ -124,11 +124,15 @@ public void visit(StaticConstantAccess node) {
if (CancelSupport.getDefault().isCancelled()) {
return;
}
- Identifier constant = node.getConstantName();
- if (constant != null) {
- String constantName = constant.getName();
- if ("class".equals(constantName.toLowerCase())) { //NOI18N
- createError(constant);
+ if (node.isDynamicName()) {
+ super.visit(node);
+ } else {
+ Identifier constant = node.getConstantName();
+ if (constant != null) {
+ String constantName = constant.getName();
+ if ("class".equals(constantName.toLowerCase())) { //NOI18N
Review Comment:
Yes, I'll fix it. Thanks!
--
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@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] PHP 8.3 Support: Dynamic class constant fetch #6701 [netbeans]
Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 commented on code in PR #6752:
URL: https://github.com/apache/netbeans/pull/6752#discussion_r1406861890
##########
php/php.editor/test/unit/src/org/netbeans/modules/php/editor/PHPBracesMatcherTest.java:
##########
@@ -522,7 +522,7 @@ public void testAttributeSyntax_05b() throws Exception {
+ " #[Class1(5)]\n"
+ " public function method~(#[Class1(6)] $param1, #[Class1('foo', 'bar', 7)] int $pram2@)^ {}\n"
+ "\n"
- + "@}^"
+ + "}"
Review Comment:
Maybe, I forgot to remove them when I copied and pasted the previous test.
`@^~` are used to get positions. (e.g. `^` is caret position)
1. get positions using the `indexOf()` method
2. replace all `@^~` to empty string (`""`)
So, the extra markers did not affect unit tests.
--
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@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] PHP 8.3 Support: Dynamic class constant fetch #6701 [netbeans]
Posted by "tmysik (via GitHub)" <gi...@apache.org>.
tmysik commented on code in PR #6752:
URL: https://github.com/apache/netbeans/pull/6752#discussion_r1406985121
##########
php/php.editor/test/unit/src/org/netbeans/modules/php/editor/PHPBracesMatcherTest.java:
##########
@@ -522,7 +522,7 @@ public void testAttributeSyntax_05b() throws Exception {
+ " #[Class1(5)]\n"
+ " public function method~(#[Class1(6)] $param1, #[Class1('foo', 'bar', 7)] int $pram2@)^ {}\n"
+ "\n"
- + "@}^"
+ + "}"
Review Comment:
I see, thanks for the explanation.
--
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@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] PHP 8.3 Support: Dynamic class constant fetch #6701 [netbeans]
Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 merged PR #6752:
URL: https://github.com/apache/netbeans/pull/6752
--
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@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists