Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression with parens in 3.0.0-prerelease.7 #234

Closed
alexander-akait opened this issue Nov 12, 2018 · 5 comments
Closed

regression with parens in 3.0.0-prerelease.7 #234

alexander-akait opened this issue Nov 12, 2018 · 5 comments
Labels
bug RELEASE-READY Waiting to release

Comments

@alexander-akait
Copy link
Collaborator

Input:

$var = (new foo)[0];
$var = new foo[0];

Difference ast for first and second (for first identifier, for second classreference)

@alexander-akait
Copy link
Collaborator Author

Maybe related #233

@alexander-akait alexander-akait added bug high-pri High Priority bug labels Nov 12, 2018
@ichiriac
Copy link
Member

Hi @evilebottnawi,

Not sure it's a bug as it's not the same expression. First takes the offset 0 from the new created object, second creates an object from the offset 0 of the constant foo (this should not be a valid syntax).

$ php -r "\$var = new foo[0];"
PHP Parse error:  syntax error, unexpected '[' in Command line code on line 1

For the first the AST is :

        New {
          "arguments": Array [],
          "kind": "new",
          "what": OffsetLookup {
            "kind": "offsetlookup",
            "offset": Number {
              "kind": "number",
              "value": "0",
            },
            "what": Identifier {
              "kind": "identifier",
              "name": ClassReference {
                "kind": "classreference",
                "name": "foo",
                "resolution": "uqn",
              },
            },
          }

The what is expected to be a ClassReference directly from read_namespace_name - here the DEBUG output :

      Line 2 : T_VARIABLE>$a< @-->at engine.parse [as parseEval] (/.../php-parser/src/index.js:126:22)
      Line 2 : '='>=< @-->at parser.read_simple_variable [as read_reference_variable] (/.../php-parser/src/parser/variable.js:289:23)
      Line 2 : '('>(< @-->at parser.read_expr_item [as read_expr] (/.../php-parser/src/parser/expr.js:12:19)
      Line 2 : T_NEW>new< @-->at parser.read_expr_item [as read_expr] (/.../php-parser/src/parser/expr.js:12:19)
      Line 2 : T_STRING>foo< @-->at parser.read_new_expr [as read_expr_item] (/.../php-parser/src/parser/expr.js:212:21)
      Line 2 : ')'>)< @-->at parser.read_list [as read_namespace_name] (/.../php-parser/src/parser/namespace.js:71:24)
      Line 2 : '['>[< @-->at parser.read_expr_item [as read_expr] (/.../php-parser/src/parser/expr.js:12:19)
      Line 2 : T_LNUMBER>0< @-->at parser.read_dereferencable [as handleDereferencable] (/home/ioan/Documents/glayzzle/php-parser/src/parser/expr.js:524:21)
      Line 2 : ']'>]< @-->at parser.read_scalar [as read_expr_item] (/.../php-parser/src/parser/expr.js:432:19)
      Line 2 : ';'>;< @-->at parser.read_dereferencable [as handleDereferencable] (/.../php-parser/src/parser/expr.js:524:21)
      Line 3 : the end of file (EOF)>< @-->at parser.expectEndOfStatement [as read_statement] (/.../php-parser/src/parser/statement.js:378:14)

As the debug is not easy, I've added a vscode configuration to run step by step the test/debug.js file (debug with jest fails).

The reason of the bug is that read_class_name_reference is defined in several places : namespace.js and expr.js

@ichiriac
Copy link
Member

bug introduced here : #220

ichiriac added a commit that referenced this issue Nov 18, 2018
ichiriac added a commit that referenced this issue Nov 18, 2018
ichiriac added a commit that referenced this issue Nov 18, 2018
@ichiriac
Copy link
Member

Here the bug is fixed on both cases, but means that the previous bug with instanceof might also have problems as it should be exactly the same definition.

Before closing it, we should add more tests over new & instanceof keywords because I'm almost sure that's it not yet correct.

@ichiriac ichiriac added help wanted RELEASE-READY Waiting to release and removed high-pri High Priority bug help wanted labels Nov 18, 2018
@ichiriac
Copy link
Member

ichiriac commented Jan 7, 2019

released in 3.0.0-prerelease.8

@ichiriac ichiriac closed this as completed Jan 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug RELEASE-READY Waiting to release
Projects
None yet
Development

No branches or pull requests

2 participants