Lint dependent files too

[integer]

arc-phpstan have had serious problem with not matching all problems in codebase. It is because arcanist call linters only with changed files. This is absolutely OK when you linting something easy like formatting. But static analysis is much more complex.

For example when you add new mandatory parameter to method and you do not change all places where you call this method, you want to PHPStan find this bug. But to find this PHPStan needs analyse all files where method is used not only changed file. Without analysing dependent files PHPStan cannot find this bug. You can feel safe because no errors in static analyse and problem will be found by customers. This is really bad.

How to solve it? You can run PHPStan over whole project but it is slow. There is another way too. PHPStan can give you list of dependent files for every class. You can get it with phpstan dump-deps. With this list of dependent files we can add dependent files to analyse. This is faster than running PHPStan over all files in project and you can be sure that analyse not missed any error.

[ondrejmirtes]

The main workflow should be:

1. Run dump-deps on a full project. Save the JSON somewhere.
2. Get changed files that were changed since the last dump-deps run. You can do it with Git for example.
3. Run PHPStan on changed + dependent files retrieved from the dependency tree.
4. Update the tree from 1). You don't have to run full dump-deps. You can merge the full tree with a dumped tree from the changed files only. This needs an advanced algorithm.

BTW a personal plug: I run a proprietary service that helps with this. You run PHPStan through a custom API client that hides away this logic. It saves and updates the dump-deps tree on a server, and updates it when you run it in CI on the main branch (usually master). Because this dumped tree file is shared, you actually never have to run full PHPStan locally, you can always take advantage of the latest dependency tree.

There is a how-to on using it: https://gist.github.com/ondrejmirtes/5ed53cbcf1eb76060a844b54ec6a9dcd (the most relevant use-case for you is number 3). Let me know if you're interested and I'll provide you with a token to test it.

[cmmata]

This sounds interesting, as you say at the moment it doesn't check dependent files and that could be a problem. But I think it can be overkill anyway with big repositories. Could you add an extra option in .arclint like level to activate or not this new functionality? That should be more compatible with actual users too.

In my case for example, the company I'm working for uses Phabricator and we check with phpstan only the files we're working with. Then we open a new Diff, and the CI server runs a full phpstan check (while we are working on other stuff and not waiting). In this case, we could analyze dependent files if it's fast enough, or rely on the CI's full check if it's making us wait. The result will be the same.

Oh, and please fetch the repo changes because I merged another pull request this morning. Thank you.

[jaydiablo]

I haven't tested this yet, but it is definitely something that should be added to this library. The current behavior of only running against changed files does present problems (and IMO should be considered a bug). We forked this lib and modified it so that it does a full run, just once, when arc lint is ran (https://github.com/diablomedia/arc-phpstan/tree/run-once) but would be much nicer if it could just run on the changed files and anything dependent on them.

I'll try to patch this into my environment in the near future and give it a go.

[ondrejmirtes]

I have a lot of performance improvements planned this year in PHPStan itself, so make sure not to spend too much time on this meanwhile 😊

[cmmata]

@ondrejmirtes What do you mean with that? That phpstan will be fast enough to run a full scan every time? Or that it will scan dependant files without asking for it (or something similar)?

[ondrejmirtes]

Yeah, it will do something like that without any change to the outside behaviour, it will “just work” 😊

[cmmata]

That's great! 😄 Do you know more or less when will it be available? If it's coming soon, maybe there's no need to spend time on this. But if it's something like at the end of this year maybe it's worth a temporal workaround

[ondrejmirtes]

You know how it is with software estimates, but I definitely think it will be in the first 6 months of 2020 :)

[ondrejmirtes]

FYI first of several planned performance improvements just landed in PHPStan 0.12.9 as experimental flag. Put this in your phpstan.neon to run PHPStan in parallel processes, and let me know if you find any bugs. Also what the performance gains are like for you :)

parameters:
    featureToggles:
        parallel: true

My plan for upcoming weeks is to fix bugs in this feature and turn it on for everyone, and then continue even further with the optimizations :)

[cmmata]

Great! I'll take a look

[ondrejmirtes]

Goods news! Check out this article (https://medium.com/@ondrejmirtes/from-minutes-to-seconds-massive-performance-gains-in-phpstan-163be88d1519) and the latest PHPStan 0.12.12 release :)

You will not need any special work anymore and PHPStan will be fast by default!