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 2021/01/05 00:19:12 UTC

[GitHub] [netbeans] junichi11 opened a new pull request #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

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


   https://issues.apache.org/jira/browse/NETBEANS-4443
   
   - https://wiki.php.net/rfc/attributes_v2
   - https://wiki.php.net/rfc/shorter_attribute_syntax
   - https://wiki.php.net/rfc/shorter_attribute_syntax_change
   
   #### Part 1
   
   - Fix the parser and lexers
   - Add/Fix unit tests for the parser and the lexers
   - Fix the `PHP80UnhandledError`
   
   ##### PHP 8.0
   
   ![nb-php80-attribute-syntax-01](https://user-images.githubusercontent.com/738383/103592511-9b769400-4f36-11eb-9ca6-dbb7bdfc9fe7.png)
   ![nb-php80-attribute-syntax-02](https://user-images.githubusercontent.com/738383/103592533-a03b4800-4f36-11eb-97b5-8c31c5965890.png)
   
   ##### PHP 7.4
   
   ![nb-php80-attribute-syntax-01-php74](https://user-images.githubusercontent.com/738383/103592569-b517db80-4f36-11eb-826d-34956ee636ec.png)
   ![nb-php80-attribute-syntax-02-php74](https://user-images.githubusercontent.com/738383/103592573-b8ab6280-4f36-11eb-9cab-b984a4070c1a.png)
   
   
   #### Part 2
   
   - Fix the formatting feature
   
   ![nb-php80-attribute-syntax-formatting-indent](https://user-images.githubusercontent.com/738383/103592581-bea14380-4f36-11eb-9cda-1ed048360af9.gif)
   
   
   #### Part 3
   
   - Fix the braces matching feature
   
   ![nb-php80-attribute-syntax-brace-matching](https://user-images.githubusercontent.com/738383/103592592-c82aab80-4f36-11eb-9f47-8aac00cb88b2.gif)
   


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

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 #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

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


   @tmysik 
   > The patch is huge because the change is huge. So, again, hard to review carefully :) However, we have plenty of tests so we should be safe.
   
   Yes, I think so :)
   
   Thank you for reviewing this, Tomas!
   
   Merging.


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

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 change in pull request #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

Posted by GitBox <gi...@apache.org>.
tmysik commented on a change in pull request #2640:
URL: https://github.com/apache/netbeans/pull/2640#discussion_r553194747



##########
File path: php/php.editor/tools/ASTPHP5Parser.cup
##########
@@ -382,6 +409,7 @@ terminal T_POW_EQUAL;
 terminal T_ELLIPSIS;
 terminal T_COALESCE;
 terminal T_COALESCE_EQUAL;
+terminal T_ATTRIBUTE;

Review comment:
       I see.
   




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

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] KacerCZ commented on pull request #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

Posted by GitBox <gi...@apache.org>.
KacerCZ commented on pull request #2640:
URL: https://github.com/apache/netbeans/pull/2640#issuecomment-755736946


   **Thank you for your work on this big feature.**
   
   I tested typing, brace matching, different formatting options and it worked as expected.
   
   The only thing that worked differently than I expected was blank lines (more their lack of) between fields with attributes.
   Box for Formatting > PHP > Blank Lines > Group Fields with PHP Doc is checked.
   I expected that fields with attribute will be treated as fields with PHPDoc and will have blank line between them.
   The same applies to constants
   
   Expected result:
   ```php
   class Person {
   
       #[Required]
       public string $name;
   
       #[Required]
       public string $login;
   
       /**
        * ISO code of country
        */
       public string $country;
       public string $street;
       public string $city;
   ```
   
   Actual result:
   ```php
   class Person {
   
       #[Required]
       public string $name;
       #[Required]
       public string $login;
   
       /**
        * ISO code of country
        */
       public string $country;
       public string $street;
       public string $city;
   ```


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

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 change in pull request #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

Posted by GitBox <gi...@apache.org>.
tmysik commented on a change in pull request #2640:
URL: https://github.com/apache/netbeans/pull/2640#discussion_r553194947



##########
File path: php/php.editor/tools/ASTPHP5Parser.cup
##########
@@ -2419,6 +2547,13 @@ is_variadic:isVariadic expr_without_variable:var
     RESULT = paramsList;
 :}
 
+| error:expr
+{:
+    List paramsList = new LinkedList();
+    paramsList.add(new ASTErrorExpression(exprleft, exprright));
+    RESULT = paramsList;
+:}
+

Review comment:
       Great.
   




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

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 change in pull request #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

Posted by GitBox <gi...@apache.org>.
junichi11 commented on a change in pull request #2640:
URL: https://github.com/apache/netbeans/pull/2640#discussion_r551650459



##########
File path: php/php.editor/tools/ASTPHP5Parser.cup
##########
@@ -2445,6 +2580,12 @@ is_variadic:isVariadic expr_without_variable:var
     paramsList.add(var_ref);
     RESULT = paramsList;
 :}
+
+| non_empty_function_call_parameter_list:paramsList T_COMMA error:expr
+{:
+    paramsList.add(new ASTErrorExpression(exprleft, exprright));
+    RESULT = paramsList;
+:}

Review comment:
       The same as the above.




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

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 change in pull request #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

Posted by GitBox <gi...@apache.org>.
junichi11 commented on a change in pull request #2640:
URL: https://github.com/apache/netbeans/pull/2640#discussion_r551646445



##########
File path: php/php.editor/tools/ASTPHP5Parser.cup
##########
@@ -382,6 +409,7 @@ terminal T_POW_EQUAL;
 terminal T_ELLIPSIS;
 terminal T_COALESCE;
 terminal T_COALESCE_EQUAL;
+terminal T_ATTRIBUTE;

Review comment:
       To avoid the incompatible api change, always add new one to the last position.




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

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 change in pull request #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

Posted by GitBox <gi...@apache.org>.
junichi11 commented on a change in pull request #2640:
URL: https://github.com/apache/netbeans/pull/2640#discussion_r551650262



##########
File path: php/php.editor/tools/ASTPHP5Parser.cup
##########
@@ -2419,6 +2547,13 @@ is_variadic:isVariadic expr_without_variable:var
     RESULT = paramsList;
 :}
 
+| error:expr
+{:
+    List paramsList = new LinkedList();
+    paramsList.add(new ASTErrorExpression(exprleft, exprright));
+    RESULT = paramsList;
+:}
+

Review comment:
       To avoid `ASTError`, add `ASTErrorExpression`. 
   e.g. 
   If we don't add `ASTErrorExpression`, the following (whole statement)is recognized as `ASTError`. In such case, some features(e.g. formatting, CC) may not work correctly. Plus, we have to sanitize the code and parse it again.
   ```
   #[A(1, self::)]
   class Test {}
   ```




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

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 #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

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


   @junichi11 
   
   I can only repeat @KacerCZ comment above - thanks a lot, Junichi! This is a lot of work and I cannot image that anyone else than you could do it. Grealy done!
   


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

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 #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

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


   @junichi11 
   
   Some checks are still not finished - feel free to merge it or I will do it later. 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.

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 change in pull request #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

Posted by GitBox <gi...@apache.org>.
junichi11 commented on a change in pull request #2640:
URL: https://github.com/apache/netbeans/pull/2640#discussion_r551654897



##########
File path: php/php.editor/test/unit/data/testfiles/formatting/broken/issue197074_04.php.formatted
##########
@@ -21,25 +21,28 @@ class companyAddressesActions extends myFrontModuleActions {
         }
 
         if ($this->getPage()->getModuleAction() == 'supplier/editCompanyAddress')
-            $this->getRequest ()->setParameter ('cancel_url', $this->getHelper ()main/checkout')
+        $this->getRequest ()->setParameter ('cancel_url', $this->getHelper ()main/checkout')

Review comment:
       The cause of this change is I added `ASTErrroExpression` to the parameter (grammar file).
   Original issue is https://bz.apache.org/netbeans/show_bug.cgi?id=197074
   
   formatting behavior is changed, but original code has syntax error and the purpose of the original issue is to prevent AssertionError. So, I suppose there is no problem.




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

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 merged pull request #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

Posted by GitBox <gi...@apache.org>.
junichi11 merged pull request #2640:
URL: https://github.com/apache/netbeans/pull/2640


   


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

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 change in pull request #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

Posted by GitBox <gi...@apache.org>.
junichi11 commented on a change in pull request #2640:
URL: https://github.com/apache/netbeans/pull/2640#discussion_r551651408



##########
File path: php/php.editor/tools/ASTPHP5Parser.cup
##########
@@ -192,6 +192,28 @@ parser code {:
         return new ClassName(varleft, propertyListright, dispatch);
     }
 
+    Statement createAttributedStatement(Statement statement, List<Attribute> attributes) {
+        Statement attributedStatement = statement;
+        if (statement instanceof FunctionDeclaration) {
+            attributedStatement = FunctionDeclaration.create((FunctionDeclaration) statement, attributes);
+        } else if (statement instanceof ClassDeclaration) {
+            attributedStatement = ClassDeclaration.create((ClassDeclaration) statement, attributes);
+        } else if (statement instanceof InterfaceDeclaration) {
+            attributedStatement = InterfaceDeclaration.create((InterfaceDeclaration) statement, attributes);
+        } else if (statement instanceof TraitDeclaration) {
+            attributedStatement = TraitDeclaration.create((TraitDeclaration) statement, attributes);
+        } else if (statement instanceof FieldsDeclaration) {
+            attributedStatement = FieldsDeclaration.create((FieldsDeclaration) statement, attributes);
+        } else if (statement instanceof ConstantDeclaration) {
+            attributedStatement = ConstantDeclaration.create((ConstantDeclaration) statement, attributes);
+        } else if (statement instanceof MethodDeclaration) {
+            attributedStatement = MethodDeclaration.create((MethodDeclaration) statement, attributes);
+        } else {
+            assert false;
+        }
+        return attributedStatement;
+    }

Review comment:
       I've added the static create methods to each node to add attributes (recreate the node with attributes).




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

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 #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

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


   I have to improve/fix other features of Attribute Syntax (e.g. CC, model). However, I'm going to fix other syntaxes before that.


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

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 change in pull request #2640: [NETBEANS-4443] PHP 8.0 Support: Attribute Syntax

Posted by GitBox <gi...@apache.org>.
tmysik commented on a change in pull request #2640:
URL: https://github.com/apache/netbeans/pull/2640#discussion_r553195411



##########
File path: php/php.editor/test/unit/data/testfiles/formatting/broken/issue197074_04.php.formatted
##########
@@ -21,25 +21,28 @@ class companyAddressesActions extends myFrontModuleActions {
         }
 
         if ($this->getPage()->getModuleAction() == 'supplier/editCompanyAddress')
-            $this->getRequest ()->setParameter ('cancel_url', $this->getHelper ()main/checkout')
+        $this->getRequest ()->setParameter ('cancel_url', $this->getHelper ()main/checkout')

Review comment:
       Agreed.
   




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

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