php-parallel-lint / PHP-Parallel-Lint

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

Possible improvements to --show-deprecated #126

Open stronk7 opened 2 years ago

stronk7 commented 2 years ago

Disclaimer: I've not looked how the --show-deprecated option works internally, and maybe the idea below is not achievable at all.

While using the --show-deprecated option with PHP 8.1 against some large codebase, I was surprised that only a few cases were detected, when that version comes to lots of classes now requiring correct return type-hinting.

So, for example, this file / class:

<?php

class my_ite implements Iterator {
    public function current() {
        return 1;
    }
    public function key() {
        return 1;
    }
    public function next() {
        // do something.
    }
    public function rewind() {
        // do something.
    }
    public function valid() {
        return true;
    }
}

Does, in fact, miss a lot of return types (or #[ReturnTypeWillChange] attributes). See https://3v4l.org/bPN2V

But, parallel-lint does not detect them at all:

$ vendor/bin/parallel-lint --show-deprecated test.php 
PHP 8.1.9 | 10 parallel jobs
.                                                            1/1 (100 %)

Checked 1 files in 0.5 seconds
No syntax error found

(note, neither php -l test.php does)

So, I was guessing if, maybe, when the file (class) being linted does not have unknown dependencies (errors)... it would be possible to, effectively, try to run the file and get all those deprecation messages that aren't shown normally.

Again, surely this is a non-sense and crazy idea, I just discovered that my hypothetical (only, I was happy) 12 deprecation cases detected by --show-deprecated... they are going to be, in reality, some good hundred, heh.

Still I'm not sure to understand why some are detected (for example php_user_filter::filter($in, $out, &$consumed, bool $closing): int was perfectly detected and others (like Iterator::valid() : bool) aren't, unless you effectively "run" the file.

And that's the story and the (crazy) idea, thanks for the tool, it's really useful!

Ciao :-)

jrfnl commented 2 years ago

@stronk7 PHP itself throws some deprecations at compile time, some at runtime. The deprecations thrown at compile time are shown by PHP Parallel Lint, those thrown at runtime are not - and cannot be detected when just linting the code, which is what this tool does.

stronk7 commented 2 years ago

@jrfnl Yes, yes. I understand that. Just was guessing if trying "execute" the files (when safe to do so) may be an acceptable approach to allow the tool to detect more cases. As said, maybe it's a wrong / crazy idea, just it came to my brain when realising that we were missing all those runtime warnings.

Maybe this should be tried with some static tool instead (say PHPCompatibility ;-) for example), and don't try this crazy route of "executing" files whenever possible (that I think I'm going to give a try, just to see how complex / useful it may be in practice).

Ciao :-)

PS: Feel free to close this if you don't see this has any fit within Parallel Lint, NP here!