You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2022/06/18 04:56:51 UTC

[GitHub] [netbeans] junichi11 opened a new pull request, #4240: [NETBEANS-5599] PHP 8.1 Support: First class callable syntax

junichi11 opened a new pull request, #4240:
URL: https://github.com/apache/netbeans/pull/4240

   - https://issues.apache.org/jira/browse/NETBEANS-5599
   - https://wiki.php.net/rfc/first_class_callable_syntax
   
   #### Part 1
   - Fix the parser
   - Fix `PHP81UnhandledError`
   - Add unit tests
   
   PHP 8.1
   
   ![nb-php81-first-class-callable-syntax-parser](https://user-images.githubusercontent.com/738383/174423232-2c7c7bcc-2384-41d9-ba55-75b14bd25be8.png)
   
   PHP 8.0
   
   ![nb-php81-first-class-callable-syntax-parser-php80](https://user-images.githubusercontent.com/738383/174423238-7dc8398b-650d-4a60-be96-8099a05c2d1e.png)
   
   #### Part 2
   - Fix the formatter
   - Add unit tests
   
   #### Part 3
   - Fix the GotoDeclaration and MarkOccurences
   - Add unit tests
   
   #### Part 4
   - Add the HintError to check incorrect first-class callable syntax
   - Add unit tests
   
   ![nb-php81-first-class-callable-syntax-fatal-errors](https://user-images.githubusercontent.com/738383/174423287-86cbd816-1167-4512-8536-6847d5526868.png)
   
   #### Part 5
   - Fix the code completion
   - Don't add callable(...) cc items by default
   - Add unit tests
   
   ![nb-php81-first-class-callable-syntax-cc-options](https://user-images.githubusercontent.com/738383/174423320-7108e024-0c04-4d37-9302-a976cafc80df.png)
   
   ![nb-php81-first-class-callable-syntax-cc](https://user-images.githubusercontent.com/738383/174423374-2c2fdc78-a397-4461-ac79-bf6f61210f31.png)
   
   
   


-- 
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

[GitHub] [netbeans] tmysik commented on a diff in pull request #4240: [NETBEANS-5599] PHP 8.1 Support: First class callable syntax

Posted by GitBox <gi...@apache.org>.
tmysik commented on code in PR #4240:
URL: https://github.com/apache/netbeans/pull/4240#discussion_r901120303


##########
php/php.editor/src/org/netbeans/modules/php/editor/completion/PHPCompletionItem.java:
##########
@@ -134,10 +134,11 @@ public abstract class PHPCompletionItem implements CompletionProposal {
     private QualifiedNameKind generateAs;
     private static ScheduledExecutorService service = Executors.newSingleThreadScheduledExecutor();
     private static final Cache<FileObject, PhpLanguageProperties> PROPERTIES_CACHE
-            = new Cache<>(new WeakHashMap<FileObject, PhpLanguageProperties>());
+            = new Cache<>(new WeakHashMap<>());
     private final boolean isPlatform;
     private final boolean isDeprecated;
     private PhpVersion phpVersion;
+    private static Boolean ADD_FIRST_CLASS_CALLABLE = null; // for unit tests

Review Comment:
   I think this field should be `volatile`, no? Or is always read/written in just one thread? If so, please document it. Thank you.
   



-- 
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

[GitHub] [netbeans] junichi11 commented on a diff in pull request #4240: [NETBEANS-5599] PHP 8.1 Support: First class callable syntax

Posted by GitBox <gi...@apache.org>.
junichi11 commented on code in PR #4240:
URL: https://github.com/apache/netbeans/pull/4240#discussion_r901126097


##########
php/php.editor/src/org/netbeans/modules/php/editor/completion/PHPCompletionItem.java:
##########
@@ -134,10 +134,11 @@ public abstract class PHPCompletionItem implements CompletionProposal {
     private QualifiedNameKind generateAs;
     private static ScheduledExecutorService service = Executors.newSingleThreadScheduledExecutor();
     private static final Cache<FileObject, PhpLanguageProperties> PROPERTIES_CACHE
-            = new Cache<>(new WeakHashMap<FileObject, PhpLanguageProperties>());
+            = new Cache<>(new WeakHashMap<>());
     private final boolean isPlatform;
     private final boolean isDeprecated;
     private PhpVersion phpVersion;
+    private static Boolean ADD_FIRST_CLASS_CALLABLE = null; // for unit tests

Review Comment:
   Fixed https://github.com/apache/netbeans/compare/7069e4d2f2b90e385bc61681e1b8e4b37770bb24..34af956e74b9b92809cefdfd1126005d8ba35697



-- 
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

[GitHub] [netbeans] junichi11 commented on a diff in pull request #4240: [NETBEANS-5599] PHP 8.1 Support: First class callable syntax

Posted by GitBox <gi...@apache.org>.
junichi11 commented on code in PR #4240:
URL: https://github.com/apache/netbeans/pull/4240#discussion_r901125544


##########
php/php.editor/src/org/netbeans/modules/php/editor/completion/PHPCompletionItem.java:
##########
@@ -770,6 +808,13 @@ public List<String> getInsertParams() {
 
         @Override
         public String getSortText() {
+            if (isFirstClassCallable) {
+                // put first-class callable syntax on the last position
+                // e.g.
+                // strlen($length)
+                // strlen(...)
+                return getName() + "99"; // NOI18N

Review Comment:
   e.g. 
   ```php
   function func($param1, $param2 = 2, $param3 = 3) {
   }
   // cc items
   // func($param1) "func1"
   // func($param1, $param2) "func2"
   // func($param1, $param2, $pram3) "func3"
   ```
   if we use "100" instead of "99", the order is...
   func(...) func100
   func($param1) "func1"
   func($param1, $param2) "func2"
   func($param1, $param2, $pram3) "func3"
   
   if we use "99"...
   func($param1) "func1"
   func($param1, $param2) "func2"
   func($param1, $param2, $pram3) "func3"
   func(...) func99
   
   I think that the priority of `func(...)` is low. So, I select "99".



-- 
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

[GitHub] [netbeans] junichi11 commented on a diff in pull request #4240: [NETBEANS-5599] PHP 8.1 Support: First class callable syntax

Posted by GitBox <gi...@apache.org>.
junichi11 commented on code in PR #4240:
URL: https://github.com/apache/netbeans/pull/4240#discussion_r901123485


##########
php/php.editor/src/org/netbeans/modules/php/editor/completion/PHPCompletionItem.java:
##########
@@ -134,10 +134,11 @@ public abstract class PHPCompletionItem implements CompletionProposal {
     private QualifiedNameKind generateAs;
     private static ScheduledExecutorService service = Executors.newSingleThreadScheduledExecutor();
     private static final Cache<FileObject, PhpLanguageProperties> PROPERTIES_CACHE
-            = new Cache<>(new WeakHashMap<FileObject, PhpLanguageProperties>());
+            = new Cache<>(new WeakHashMap<>());
     private final boolean isPlatform;
     private final boolean isDeprecated;
     private PhpVersion phpVersion;
+    private static Boolean ADD_FIRST_CLASS_CALLABLE = null; // for unit tests

Review Comment:
   Will fix it. Thank you!



-- 
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

[GitHub] [netbeans] junichi11 commented on pull request #4240: [NETBEANS-5599] PHP 8.1 Support: First class callable syntax

Posted by GitBox <gi...@apache.org>.
junichi11 commented on PR #4240:
URL: https://github.com/apache/netbeans/pull/4240#issuecomment-1159762236

   @tmysik Fixed: https://github.com/apache/netbeans/compare/7069e4d2f2b90e385bc61681e1b8e4b37770bb24..34af956e74b9b92809cefdfd1126005d8ba35697
   
   Thank you for your review!


-- 
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

[GitHub] [netbeans] junichi11 commented on pull request #4240: [NETBEANS-5599] PHP 8.1 Support: First class callable syntax

Posted by GitBox <gi...@apache.org>.
junichi11 commented on PR #4240:
URL: https://github.com/apache/netbeans/pull/4240#issuecomment-1159735155

   @tmysik Could you please have a look at this when you have time? If there is no problem, let's merge 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

[GitHub] [netbeans] tmysik commented on pull request #4240: [NETBEANS-5599] PHP 8.1 Support: First class callable syntax

Posted by GitBox <gi...@apache.org>.
tmysik commented on PR #4240:
URL: https://github.com/apache/netbeans/pull/4240#issuecomment-1159754316

   @junichi11 Junichi, please, let me know what you think about my nitpick comments.
   
   The change looks good to me, I am ready to merge. Thanks again for your great work!


-- 
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

[GitHub] [netbeans] junichi11 commented on a diff in pull request #4240: [NETBEANS-5599] PHP 8.1 Support: First class callable syntax

Posted by GitBox <gi...@apache.org>.
junichi11 commented on code in PR #4240:
URL: https://github.com/apache/netbeans/pull/4240#discussion_r901125544


##########
php/php.editor/src/org/netbeans/modules/php/editor/completion/PHPCompletionItem.java:
##########
@@ -770,6 +808,13 @@ public List<String> getInsertParams() {
 
         @Override
         public String getSortText() {
+            if (isFirstClassCallable) {
+                // put first-class callable syntax on the last position
+                // e.g.
+                // strlen($length)
+                // strlen(...)
+                return getName() + "99"; // NOI18N

Review Comment:
   e.g. 
   ```php
   function func($param1, $param2 = 2, $param3 = 3) {
   }
   // cc items
   // func($param1) "func1"
   // func($param1, $param2) "func2"
   // func($param1, $param2, $pram3) "func3"
   ```
   if we use "100" instead of "99", the order is...
   func(...) func100
   func($param1) "func1"
   func($param1, $param2) "func2"
   func($param1, $param2, $pram3) "func3"
   
   if we use "99"...
   func($param1) "func1"
   func($param1, $param2) "func2"
   func($param1, $param2, $pram3) "func3"
   func(...) func99
   
   I think that the priority of `func(...)` is low. So, I selected "99".



-- 
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

[GitHub] [netbeans] junichi11 commented on pull request #4240: [NETBEANS-5599] PHP 8.1 Support: First class callable syntax

Posted by GitBox <gi...@apache.org>.
junichi11 commented on PR #4240:
URL: https://github.com/apache/netbeans/pull/4240#issuecomment-1159363652

   Probably, once this is merged, PHP 8.1 support is complete :)


-- 
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

[GitHub] [netbeans] tmysik commented on a diff in pull request #4240: [NETBEANS-5599] PHP 8.1 Support: First class callable syntax

Posted by GitBox <gi...@apache.org>.
tmysik commented on code in PR #4240:
URL: https://github.com/apache/netbeans/pull/4240#discussion_r901125684


##########
php/php.editor/src/org/netbeans/modules/php/editor/completion/PHPCompletionItem.java:
##########
@@ -770,6 +808,13 @@ public List<String> getInsertParams() {
 
         @Override
         public String getSortText() {
+            if (isFirstClassCallable) {
+                // put first-class callable syntax on the last position
+                // e.g.
+                // strlen($length)
+                // strlen(...)
+                return getName() + "99"; // NOI18N

Review Comment:
   I see now, 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

[GitHub] [netbeans] tmysik merged pull request #4240: [NETBEANS-5599] PHP 8.1 Support: First class callable syntax

Posted by GitBox <gi...@apache.org>.
tmysik merged PR #4240:
URL: https://github.com/apache/netbeans/pull/4240


-- 
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

[GitHub] [netbeans] tmysik commented on a diff in pull request #4240: [NETBEANS-5599] PHP 8.1 Support: First class callable syntax

Posted by GitBox <gi...@apache.org>.
tmysik commented on code in PR #4240:
URL: https://github.com/apache/netbeans/pull/4240#discussion_r901120765


##########
php/php.editor/src/org/netbeans/modules/php/editor/completion/PHPCompletionItem.java:
##########
@@ -770,6 +808,13 @@ public List<String> getInsertParams() {
 
         @Override
         public String getSortText() {
+            if (isFirstClassCallable) {
+                // put first-class callable syntax on the last position
+                // e.g.
+                // strlen($length)
+                // strlen(...)
+                return getName() + "99"; // NOI18N

Review Comment:
   `99`? 🤔 



-- 
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