Skip to content

Fix handling of HaltCompilerStatement #389

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

Merged
merged 5 commits into from
Oct 5, 2022

Conversation

TysonAndre
Copy link
Contributor

@TysonAndre TysonAndre commented Sep 25, 2022

Make it easier to calculate __COMPILER_HALT_OFFSET__ and make properties other than data non-null.
Combine all tokens after halt compiler statement into an InlineHtml token that goes into HaltCompilerStatement->data if there are bytes.
Related to #381

@TysonAndre TysonAndre force-pushed the halt-compiler-fix branch 2 times, most recently from 63bf6cc to 508ae7b Compare September 25, 2022 19:15
Make it easier to calculate `__COMPILER_HALT_OFFSET__`.
Related to microsoft#381

Address other edge cases
@TysonAndre
Copy link
Contributor Author

TysonAndre commented Sep 25, 2022

This is ready for review. The ParserInvariantTest asserts that the tokens combine into the original file for the test cases of __halt_compiler(). The subsequent commit is just another test case.

All properties should be MissingTokens and non-null
Include whitespace before the data as part of the data for invalid ASTs.
@TysonAndre
Copy link
Contributor Author

One last change: A real InlineHtml node would have no leading whitespace/comments, so make fullStart = start.

  • In places that aren't HaltCompilerStatement (EchoStatement), it's always at the start of a file or after ?>/?>\n, and the newline is part of the close tag token instead of the InlineHtml token

},
{
"error": "SkippedToken",
"kind": "ColonColonToken",
Copy link
Contributor Author

@TysonAndre TysonAndre Sep 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reasoning in updating the tests I'd added in the previous PR is that HaltCompiler should halt the compiler and not parse any more statements, so I'd treat the snippet like __halt_compiler[();]::foo(); with inline html of ::foo();\n
(this token can't be used in other statements/expressions and is a syntax error)

<?php
// invalid
__halt_compiler::foo();

The MissingToken in the original tree were due to token_get_all() returning at most 3 tokens after T_HALT_COMPILER then returning T_INLINE_HTML for the rest

@roblourens roblourens merged commit 2e6da2e into microsoft:main Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants