-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Preserve heredoc/nowdoc formatting #1007
Comments
I don't think this is related to formatting preservation at all (it is never preserved by the default printer) -- it sounds like you are printing for the default version of PHP 7.4, which assumes flexible doc strings are supported. If you want to generate PHP 7.2 compatible code, you can do so by passing a |
Maybe this was not tested (on my end) in PHP Parser v4 but wasn't it the case before? I would expect to have run into this issue much sooner otherwise as PHP-Scoper has never been compatible with PHP 7.2 code and the PHP version used is the host version |
Ok I'm comparing my PHP-Parser v4 code: $parser = (new ParserFactory())->create(
ParserFactory::PREFER_PHP7,
new Lexer(),
);
$printer = new StandardPrinter(
new Standard(),
); The current code is: $phpVersion = PhpVersion::getHostVersion();
$lexer = $version->isHostVersion() ? new Lexer() : new Emulative($version);
$parser =$version->id >= 80_000
? new Php8($lexer, $version)
: new Php7($lexer, $version);
$printer = new StandardPrinter(
new Standard([
'phpVersion' => $phpVersion,
]),
); I can confirm that for the PHP-Parser v4 the following code: <?php
function foo()
{
$x = <<<'PHP'
$foo
PHP;
} is NOT transformed into: <?php
namespace PhpScoperPrefix;
function foo()
{
$x = <<<'PHP'
$foo
PHP;
} but instead preserved: <?php
namespace PhpScoperPrefix;
function foo()
{
$x = <<<'PHP'
$foo
PHP;
}
I'll add the feature to be able to forcefully set a PHP version, but otherwise currently PHP-Scoper just picks the host version and it worked fine because things like heredocs were not reformatted unless necessary. |
PHP-Parser 4 did not support specifying the PHP version for the pretty printer -- the produced code was always the "most compatible", with an effective target version of PHP 5.3. You can go back to the old behavior by explicitly specifying that as the version. Possibly that's appropriate for PHP-Scoper, as the code is not intended to be read by anyone anyway. |
But wouldn't that result in the opposite, i.e. making indented heredoc into non-indented heredocs? And work strangely for code that is 8.3 and for 8.3? |
It would turn indented into non-indented heredoc, yes -- that's exactly what PHP-Parser 4 did. The non-indented version works on all PHP versions. (Well, all PHP versions that have heredoc.) |
It is not ideal as I would prefer to minimise the diff between the source and scoped code, but I guess if I do want that the I have about 1/3 of the test suites that fail with this mode, so is it just that I'm breaking too much the existing code, am I doing something wrong or is it bugs I should report? |
See the discussion in nikic/PHP-Parser#1007.
See the discussion in nikic/PHP-Parser#1007. This should fix sebastianbergmann/phpunit#5855 although this may result in more ugly code formatting than desired. For this reason the default printer version is 7.2 rather than 5.3 as it would make the code too otherwise.
Yes.
What does "fail" mean here? It generates incorrect code? Output is different because your test expectations used different formatting? |
Hmm I misread some the output. Ok no most of the errors are due to the Which easily explains all the e2e test failures |
Hm, that's weird. |
ok, if thats unexpected ill debug it to see what's going on |
Whilst upgrading PHP-Scoper to PHP-Parser 5, I've run into the issue of sebastianbergmann/phpunit#5855.
This is caused by this change:
Previously the parser was preserving the formatting while possible, this does not seem to be the case anymore. I assume this was a deliberate change in the printer to simplify it and keep the old behaviour with
Printer::printFormatPreserving()
.With PHP-Scoper however, I very easily run into the issue of the printer bailing out (if preserving format), which to be honest is too surprising considering the changes to the AST are not small (you can find plenty of examples here, but the gist of it is, changing names, adding namespaces and
class_alias()
statements orfunction_exists()
statements.).Is
printFormatPreserving()
the way to go in which case more fixing needs to be done to the printer or is there another way?The text was updated successfully, but these errors were encountered: