JakubOnderka / PHP-Parallel-Lint

This tool check syntax of PHP files faster than serial check with fancier output.
Other
647 stars 61 forks source link

Parser error attributed to "line -1" #136

Closed Krinkle closed 4 years ago

Krinkle commented 5 years ago

The following example code:

<?php

namespace ContentTranslation;

class ResourceLoaderCxUiDataModule extends \ResourceLoaderFileModule {
    /**
     * @param ResourceLoaderContext $context
     * @return string JavaScript code
     */
    public function getScript( ResourceLoaderContext $context ) {
        return $this->getDataScript( $context ) . parent::getScript( $context );
    }

    private function getDataScript( ResourceLoaderContext $context )) {
        return Xml::encodeJsCall( 'mw.config.set', [ [
            'wgCxUiData' => [
                'mainPage' => Title::newMainPage( $context )->getPrefixedText()
            ],
        ] ] );
    }

    /**
     * @param ResourceLoaderContext $context
     * @return array
     */
    public function getDefinitionSummary( ResourceLoaderContext $context ) {
        // Used for the module version hash
        $summary = parent::getDefinitionSummary( $context );
        $summary[] = [
            'dataScript' => $this->getDataScript( $context ),
        ];
        return $summary;
    }
}

This contains a syntax error in the getDataScript signature.

Using jakub-onderka/php-parallel-lint (v1.0.0), the error is:

Parse error: ./includes/ResourceLoaderCxUiDataModule.phpUnexpected ')', expecting ';' or '{' in ./includes/ResourceLoaderCxUiDataModule.php on line -1

I'm glad it found the mistake and it wasn't hard to find, but it would've been even nicer if it also mentioned the line number.

jrfnl commented 5 years ago

I've tried to reproduce this issue using v 1.0.0, but failed.

Might help to know what PHP version you are using and possibly what OS.

PL-136

Krinkle commented 5 years ago

I can't find the original build failure but based on ops logs, I believe it would have been PHP 7.2.8.

jrfnl commented 5 years ago

Tried again, now with PHP 7.2.8 and still can't reproduce this (on Windows). Tried with the last release as well as with the latest master.

Krinkle commented 5 years ago

My original report was from a @Wikimedia repository. We have since found other cases which are still reproducible, reported at https://phabricator.wikimedia.org/T202581.

Looks like it wasn't from PHP 7.2, but from HHVM 3.18 (we run both side-by-side currently).

jrfnl commented 5 years ago

In that case, I'll have to leave it for someone else to figure out. I haven't got HHVM installed, so won't be able to test it.

Daimona commented 5 years ago

At a quick glance, it sounds like a regex problem due to different formats between PHP and HHVM. Compare https://github.com/JakubOnderka/PHP-Parallel-Lint/blob/89553b3b5f772230dd350806ab4103cee380d2eb/src/Error.php#L114 with https://3v4l.org/4EYDe

3v4l seems to be reporting line -1 as well, but that should be another problem.

Of course, there may be other regexps that work with Zend only, but this one seems to be the only responsible for the wrong syntax error line.

Krinkle commented 4 years ago

The project continues under a new GitHub organisation at: https://github.com/php-parallel-lint/PHP-Parallel-Lint.

(This move is recognised in the project's readme, see commit).