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

Possible improvements to --show-deprecated #126

Open
stronk7 opened this issue Sep 14, 2022 · 2 comments
Open

Possible improvements to --show-deprecated #126

stronk7 opened this issue Sep 14, 2022 · 2 comments

Comments

@stronk7
Copy link

stronk7 commented Sep 14, 2022

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
Copy link
Collaborator

jrfnl commented Sep 14, 2022

@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
Copy link
Author

stronk7 commented Sep 15, 2022

@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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants