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

broken ast for $$$$$ #248

Closed
alexander-akait opened this issue Jan 18, 2019 · 3 comments
Closed

broken ast for $$$$$ #248

alexander-akait opened this issue Jan 18, 2019 · 3 comments
Assignees
Labels
AST bug high-pri High Priority bug

Comments

@alexander-akait
Copy link
Collaborator

alexander-akait commented Jan 18, 2019

Input:

get_class($var)::$$$$$property;

Two problem:

  • leading comments property 😕
Variable {
        kind: 'variable',
        leadingComments:
         Location {
           source: '$$$$property',
           start: Position { line: 3, column: 18, offset: 25 },
           end: Position { line: 3, column: 30, offset: 37 } },
        name:
         Variable {
           kind: 'variable',
           leadingComments:
            Location { source: '$$$property', start: [Position], end: [Position] },
           name:
            Variable {
              kind: 'variable',
              loc: [Location],
              name: [Variable],
              byref: false,
              curly: false },
           byref: false,
           curly: false },
        byref: false,
        curly: false }
  • invalid count of variable nodes (now is 4), but should be 5

Simple example:

get_class($var)::$$property;

Output from php parser:

array(
    0: Stmt_Expression(
        expr: Expr_StaticPropertyFetch(
            class: Expr_FuncCall(
                name: Name(
                    parts: array(
                        0: get_class
                    )
                )
                args: array(
                    0: Arg(
                        value: Expr_Variable(
                            name: var
                        )
                        byRef: false
                        unpack: false
                    )
                )
            )
            name: Expr_Variable(
                name: Expr_Variable(
                    name: Expr_Variable(
                        name: Expr_Variable(
                            name: property
                        )
                    )
                )
            )
        )
    )
)
@ichiriac
Copy link
Member

related to https://github.com/glayzzle/php-parser/blob/master/src/parser/variable.js#L162

In principle this should also be afected : $foo::${$bar} and, should also be a problem on ->

I'll investigate

ichiriac pushed a commit that referenced this issue Jan 22, 2019
ichiriac pushed a commit that referenced this issue Jan 22, 2019
@ichiriac
Copy link
Member

It's fixed, variable names can be variables. also the ${ it's now a variable with the curly = true property.

For the implementation of the offset on StaticLookup or PropertyLookup, you can have :

  • an identifier $foo::bar
  • a variable (curly = false) $foo::$bar (the variable name can be a variable...)
  • a variable (curly = true) $foo::${bar} (the variable name can be an expression...)
  • an Encapsed : $foo::bar_$baz
  • any other case meaning it's an expression like $foo::{expr($bar)}

@ichiriac
Copy link
Member

Ok, after working on #128 I've introduced an simplification, the $foo->{bar} is now wrapped by a Literal node, containing the expression on the value property

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AST bug high-pri High Priority bug
Projects
None yet
Development

No branches or pull requests

2 participants