-
Notifications
You must be signed in to change notification settings - Fork 35
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
Fix access to undefined stack index on ExpressionParser
#67
base: master
Are you sure you want to change the base?
Conversation
Ex.: ``` Error unpacking a non unpackable Ast node on `$(binary_xor?... {` at line 29 with context: [ "^" ] Hint: use a non ellipsis expansion as in `$(binary_xor ? {` ``` cc ircmaxell/php-compiler#29
``` $ast->{'some deep token'} = new Token(T_STRING, 'patch'); ``` Ref: github.com//pull/59
This reverts commit f0d1407.
From now on, the supported signature for expanders is: ``` function(Optional<Ast|TokenStream> $subject, Optional<Engine> $e) : Ast|TokenStream; ``` Support for nested Ast based expanders through Yay DSL also allowed: Ex.: ```php >> { $$(my_cheers_ast_expander( $$(my_hello_ast_expander( $(name_ast_node) )) )); } ``` Given: ```php function my_hello_ast_expander(\Yay\Ast $ast) : \Yay\Ast { return new \Yay\Ast($ast->label(), new Token( T_CONSTANT_ENCAPSED_STRING, "'Hello, {$ast->token()}. From Ast.'", $ast->token()->line() )); } function my_cheers_ast_expander(\Yay\Ast $ast) : \Yay\Ast { $str = str_replace("'", "", (string) $ast->token()); return new \Yay\Ast($ast->label(), new Token( T_CONSTANT_ENCAPSED_STRING, "'{$str} Cheers!'", $ast->token()->line() )); } ``` Thanks @assertchris for reproducing bugs and pushing tests.
- Augment error messages with their respective line and filename using the same formating as the PHP engine itself. This allows IDEs to parse both preprocessor and runtime error message the same way; - Use a `YayPreprocessorError` instead of stdlib exceptions that can come from any random user space code;
Using a stream wrapper requires url include enabled and this is considered too unsafe for most setups. With that in mind I have no reason to maintain this feature. Other methods involving composer autoload trickery or xray (https://github.com/marcioAlmada/xray) extension have been considered preferable according to community usage history.
YAY! Usage: yay yay <file> yay --macros=<macros> yay --macros=<macros> <file> yay -h | --help yay --version Options: -h --help Show this screen. --version Show version. --macros=<macros> PHP glob pattern for macros to be loaded. Ex: my/macros/*.yay [default: ''] Examples: ``` # Process file: yay input.php > output.php # Process file, preload project macros: > yay --macros="./project-macros/*.yay" input.php > output.php # Process stdin: > cat input.php | yay > output.php # Process stdin, preload project macros: > cat input.php | yay --macros="./project-macros/*.yay" > output.php ```
This allows the script to continue if xdebug can't be disabled (most likely bc it isn't there in the first place)
There are still some precedence changes on some operators though, these last failures will require some thought. The ExpressionParser will have to be conditionally different regarding the PHP version. BTW we should be thankfull these warnings were added, this is something nobody would notice without the engine warnings/notices. |
1e82d5a
to
a647a0f
Compare
FROM golang:1.10.1-alpine3.7 as builder | ||
|
||
WORKDIR /go/src/nubank/authorizer | ||
|
||
RUN apk --update add git openssh && \ | ||
rm -rf /var/lib/apt/lists/* && \ | ||
rm /var/cache/apk/* | ||
|
||
RUN go get -u github.com/golang/dep/cmd/dep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Attention: t his is the same Dockerfile that was reported with #71
IMO you should not fix these in your tests. Even though these are rightful warnings from PHP that the code generated might be not what you want. Nevertheless, it's still 100% valid PHP syntax. The tool did exactly what it was told to. So I don't see a problem. |
Follow up to #64