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

invalid ast for silent #247

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

invalid ast for silent #247

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

Comments

@alexander-akait
Copy link
Collaborator

alexander-akait commented Jan 18, 2019

Input:

$var = @foo() || @foo();

silent node should be before call, now it is before bin

Right output from php parser:

array(
    0: Stmt_Expression(
        expr: Expr_Assign(
            var: Expr_Variable(
                name: var
            )
            expr: Expr_BinaryOp_BooleanOr(
                left: Expr_ErrorSuppress(
                    expr: Expr_FuncCall(
                        name: Name(
                            parts: array(
                                0: foo
                            )
                        )
                        args: array(
                        )
                    )
                )
                right: Expr_ErrorSuppress(
                    expr: Expr_FuncCall(
                        name: Name(
                            parts: array(
                                0: foo
                            )
                        )
                        args: array(
                        )
                    )
                )
            )
        )
    )
)

But

$var = @(foo() || foo());

Output valid ast.

Very high priority.

@alexander-akait alexander-akait added bug high-pri High Priority bug labels Jan 18, 2019
@alexander-akait alexander-akait added this to the 3.0.0-prerelease.9 milestone Jan 18, 2019
@alexander-akait
Copy link
Collaborator Author

alexander-akait commented Jan 18, 2019

Same problem for

@$i / 0;
@($i / 0);

@alexander-akait alexander-akait changed the title invalid ast for cast invalid ast for silent Jan 18, 2019
@ichiriac
Copy link
Member

It's about node precedence, I'll take a look

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

i've started a fix on this, it resolves your case but may not work on other nodes like retif, cast or propertylookup - needs more tests but didn't had enough time.

I'll be back next week ✋

@alexander-akait
Copy link
Collaborator Author

@ichiriac thanks for amazing work, it is really help to improve our prettier plugin for PHP, we try to update to latest commit and check on regressions

@alexander-akait
Copy link
Collaborator Author

alexander-akait commented Mar 13, 2019

/cc @ichiriac friendly ping, can we solve some high priority issues (include this)? 😄

@alexander-akait
Copy link
Collaborator Author

/cc @ichiriac friendly ping again 😞 We need help with this

@ichiriac
Copy link
Member

ichiriac commented Apr 7, 2019

Hi @evilebottnawi,

$> ping ichiriac
PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.

--- 127.0.0.1 ping statistics ---
4 packets transmitted, 0 received, 100% packet loss, time 12days

Sorry dude, had a lot of work to finish, I'm back on the project this week

@ichiriac ichiriac self-assigned this Apr 7, 2019
ichiriac added a commit that referenced this issue Aug 9, 2019
@ichiriac
Copy link
Member

ichiriac commented Aug 9, 2019

I have some time this summer to fix some issues.

This bug is ready to release unless I'm missing some test. Here my tests :

  it("test silent node / bin", function() {
    shouldBeSame("@foo() || @foo();", "(@foo()) || (@foo());");
  });
  it("test silent node / div", function() {
    shouldBeSame("@$i / 0;", "@($i) / 0;");
  });
  it("test silent node / ret if", function() {
    shouldBeSame("@$i == true ? @$foo : @$bar;", "@($i) == true ? @($foo) : @($bar);");
  });
  it("test silent node / cast", function() {
    shouldBeSame("@(int)$i + 1;", "@((int)$i) + 1;");
  });
  it("test silent node / property lookup", function() {
    shouldBeSame("@$foo->bar;", "@($foo)->bar;");
  });

@ichiriac ichiriac added the RELEASE-READY Waiting to release label Aug 9, 2019
@alexander-akait
Copy link
Collaborator Author

Will be great if we fix some major bugs

@ichiriac ichiriac removed the RELEASE-READY Waiting to release label Aug 15, 2019
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