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

Fix access to undefined stack index on ExpressionParser #67

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

marcioAlmada
Copy link
Owner

@marcioAlmada marcioAlmada commented Sep 30, 2019

Follow up to #64

marcioAlmada and others added 17 commits April 16, 2019 23:49
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
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)
@marcioAlmada
Copy link
Owner Author

1) Yay\ParserOptimizationTest::testExpressionParserWithGoodExpressions with data set
"Expression 446" ('3 << 3 >> 2 . 2', true)

The behavior of unparenthesized expressions containing both '.' and '>>'/'<<'
 will change in PHP 8:  '<<'/'>>' will take a higher precedence

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.

@marcioAlmada marcioAlmada force-pushed the master branch 2 times, most recently from 1e82d5a to a647a0f Compare July 20, 2021 16:36
Comment on lines +1 to +9
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

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

@e1himself
Copy link

1) Yay\ParserOptimizationTest::testExpressionParserWithGoodExpressions with data set
"Expression 446" ('3 << 3 >> 2 . 2', true)

The behavior of unparenthesized expressions containing both '.' and '>>'/'<<'
will change in PHP 8: '<<'/'>>' will take a higher precedence

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.

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.

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.

5 participants