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

Warning message on PHP files consisting of only comments #1157

Closed
fitztrev opened this issue Mar 7, 2019 · 12 comments
Closed

Warning message on PHP files consisting of only comments #1157

fitztrev opened this issue Mar 7, 2019 · 12 comments

Comments

@fitztrev
Copy link

fitztrev commented Mar 7, 2019

When I run the following command, I get a warning message when it found a PHP file where the contents were only comments.

$ ./vendor/bin/rector process app --level laravel58

Contents of app/Test.php:

<?php

// A single-line comment

Output:

PHP Warning:  assert(): assert($itemStartPos >= 0 && $itemEndPos >= 0) failed in
vendor/nikic/php-parser/lib/PhpParser/PrettyPrinterAbstract.php on line 753

Ref nikic/PHP-Parser#589

@TomasVotruba
Copy link
Member

It looks like it's another file. Running it on the app/Tests.php with provided contents works ok.

vendor/bin/rector process app/Test.php --level laravel58

@fitztrev
Copy link
Author

fitztrev commented Mar 8, 2019

It seems the issue is only on PHP 7.2. Just tried on 7.3 and the warning is not there. Thanks!

@fitztrev fitztrev closed this as completed Mar 8, 2019
@TomasVotruba
Copy link
Member

It should work on any PHP. There will be some bug with one of the rules.

If you can provide failing file, we could spare re-creating the issue in the future by someone else on PHP 7.2

@fitztrev
Copy link
Author

fitztrev commented Mar 8, 2019

@TomasVotruba Ok, see below:

$ php -v
// PHP 7.2.4

Steps to reproduce

mkdir rector-issue
cd rector-issue
composer require rector/rector -q

echo "<?php echo 'hello world';"> FileIsOk.php
vendor/bin/rector process FileIsOk.php --level laravel58
// ^^ All good, works as expected

echo "<?php // comment here"> FileIsNotOk.php
vendor/bin/rector process FileIsNotOk.php --level laravel58
// ^^ Issue

@fitztrev fitztrev reopened this Mar 8, 2019
@TomasVotruba
Copy link
Member

TomasVotruba commented Mar 8, 2019

Thank you 👍

What rule from laravel58.yaml (in vendor/rector/config/level/laravel/laravel58.yaml) is causing this? Try commenting out all and use one by one

(I'm affraid this is not related to any rule. This might be caused by printer.)

@fitztrev
Copy link
Author

fitztrev commented Mar 8, 2019

Seems to be all of them -- not related to any specific rule.

@TomasVotruba
Copy link
Member

TomasVotruba commented Mar 8, 2019

Thank you.
Hm, then it's related to the printer. Any clue why PHP 7.2 vs 7.3 is different?
Do you have installed same version of php-parser?

@fitztrev
Copy link
Author

fitztrev commented Mar 8, 2019

Any clue why PHP 7.2 vs 7.3 is different?

$itemEndPos = $origArrItem->getEndTokenPos();

This is int(-1) which is why the assertion fails.

Do you have installed same version of php-parser?

Yes, same version.

@TomasVotruba
Copy link
Member

TomasVotruba commented Mar 8, 2019

On which PHP version this is -1?

$itemEndPos = $origArrItem->getEndTokenPos();

Is is possible your assert.<option> is disabled on PHP 7.3? It just seems weird the same number fails.

@fitztrev
Copy link
Author

fitztrev commented Mar 8, 2019

Ok, that's not it. It's actually -1 in both environments. Result for both PHP 7.2 and 7.3:

var_dump($itemEndPos); // int(-1)
var_dump($itemStartPos >= 0 && $itemEndPos >= 0); // bool(false)

I just looked at the assert options. Here's the difference:

PHP 7.2 - zend.assertions => 1 => 1
PHP 7.3 - zend.assertions => -1 => -1

I guess that explains it?

@TomasVotruba
Copy link
Member

TomasVotruba commented Mar 8, 2019

Yes 👍 I think you have assertions silenced on 7.3.

I'll enable assertions and try it locally, thanks for debugging ❤️

@TomasVotruba
Copy link
Member

Closing as failing test is required and unable to replicate

TomasVotruba added a commit that referenced this issue Nov 5, 2021
rectorphp/rector-src@49aa15d [CodingStyle] Deprecate RemoveUnusedAliasRector, job rather for coding standard tool and opinonated (#1157)
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

No branches or pull requests

2 participants