Skip to content

Microsoft/tolerant-php-parser adoption #323

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

Closed
8 tasks done
mousetraps opened this issue Mar 2, 2017 · 4 comments · Fixed by #357
Closed
8 tasks done

Microsoft/tolerant-php-parser adoption #323

mousetraps opened this issue Mar 2, 2017 · 4 comments · Fixed by #357

Comments

@mousetraps
Copy link
Contributor

mousetraps commented Mar 2, 2017

As discussed in microsoft/tolerant-php-parser#36 (comment), key benefits include:

  • graceful error handling
  • performance + memory usage
  • fully-representative tree

I'm currently working with Felix to put together a functional prototype. This would be a pretty major change, there may be some temporary regressions, and we will certainly need to make some improvements to the parser before it's ready for primetime.

This issue acts as a catch-all to track progress on that work.


Current status:

Working branch:

Features

  • error diagnostics
  • find all symbols
  • find all symbols in workspace
  • [not impacted] format code
  • go to definition
  • find all references
  • hover
  • completion

Known regressions

  • Error diagnostics produced by the parser are not currently "strict" because we parse things more loosely in order to provide error tolerance, so we won't get diagnostics for everything until we add relevant post-parse checks (this will happen on the parser side).
  • The new parser has higher granularity when it comes to Node position info, which results in slightly different (but still correct) position locations in some cases (for instance for a function call hello(/*cursor*/), go-to-definition at the denoted cursor position will not yield a result (but will yield a result if the function call name is selected)
  • The new parser does not have pretty-printing yet, so the definition lines produced are exactly what the user has typed in (including whitespace), rather than a pretty-printed version.
  • Magic constants are treated like normal constants and will therefore appear in "find all references" results. Their fqsen will be resolved to their namespace value if one is defined (even though this is not actually legal).
  • true, false, and null are not included in find all references results (which is probably the expected behavior anyways)
  • variables used within template strings are not currently included in find all references results
  • TODO

Related parser issues

Parser work for this next milestone is tracked here. Any additional issues that should be prioritized?

@razvanphp
Copy link

Sorry to stress on this, but please keep #134 (PHPDoc) & #271 (dependency injection containers) on the feature list, without them, many of the real-world PHP applications are really hard for daily work, my colleagues (with PhpStorm) are still laughing at me when I can't CMD+click while debugging.

@felixfbecker
Copy link
Owner

@razvanphp The adoption will ease the implementation of these features, it is something I talked about with Sara :)

@jens1o
Copy link
Contributor

jens1o commented Mar 28, 2017

any news on this?

@jens1o
Copy link
Contributor

jens1o commented Apr 10, 2017

Nice that this will be handled in the next release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants