[RFC] embrace PSR, option, pipe, phar

[taoso]

Hello, every body. This is a huge patch. Making this PR is just a request for
comment(RFC). If this PR were acceped, I would make contribution to phpstan;
otherwise, I would maintain my own fork.

Thanks all the developer's awesome work. phpstan is awesome 👍.

embrace psr

• all code including test code has been fix by phpcs according psr1 and psr2
• the coding standard of cs target has been set to be psr1 & psr2
• all testcase related to file lineno has been fixed manually
• use tedivm/stash in favor of psr6

But why?

PSR is for the whole community, and PSR is the future!

embrace php-di/php-di

• use autowiring as much as possible
• use interface instead of tag for extension

But why?

nette/di is awesome. However, it does not support autowiring. So for every
class you need to be auto injected, you have to register them
in the config.neon file. It is boring and ridiculous. By use php-di/php-di,
what is you only need to register is these class they have
constructor parameters which can not be injected like array.

The original implementation use tags to find the rule class and extension class.
As we can see in the code, all the tags are just variants of their responding interface.
So what we need to do is just register rule class and extension class
by their interface and fetch them by interface as well. No tag, less memory burden.

And php-di/php-di implements the container-interop/container-interop, which
has been officially deprecated in favor of PSR-11. PSR again!

embrace option

• let --autoload-file support multiple path, so drop Nette\Loaders\RobotLoader
• add --rule option to set rules to be used
• add --exclude-rule combine with --level option to disable some rule
• add --ignore-path and drop fileExcluder dependence. Finder's customer filter is enough
• add --ignore-error option
• add --extension option
• drop -c option and neo dependence, there is no need any more

But why?

As php has no builtin autoload standard, there are thousands upon thousands
autoload method has been developed. Thanks to the composer project, these chaos
is terminating, and maybe disappeared in the near future. However, we have to
make sure phpstan support the legacy project. So, make --autoload-file support
multiple path will be of convinent. And the RobotLoader is useless.

A new conf/config.php has been introduced, which is used for internal only.
If a rule class does not have any scalar typehint, it will be loaded by php-di
without any config. So if user want to enable their own rule or extension,
what they only need is to define class implements these interface, and DO NOT
add typehint to their __constructor. So, no need to introduce an additional
configure file. So drop the -c option. Further, we could add option for
internal flags like checkThisOnly, checkFunctionArgumentTypes, enableUnionTypes.
All in all, drop the -c option.

embrace cli

• fix process term signal delay
• drop normalPath
• move all output except source error list to stderr
• use list sytle to display error instead of table style

But why?

Call pcntl_signal_dispatch after each file analysis to reduce signal process delay.

Display the file path according to the input opiton. If user use relative path, just
display relative path; if absolute, just absolute. Do less, do clear.

Use list style to display error infomation, so we will get

1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

• Method Note::bar() should return string but returns int in tests/notAutoloaded/extension-demo.php on line 68
• Method Note::bar() invoked with 3 parameters, 0 required in tests/notAutoloaded/extension-demo.php on line 73
• Access to an undefined static property Note::$c in tests/notAutoloaded/extension-demo.php on line 74

[ERROR] Found 3 errors

The progressbar and the [ERROR] infomation will display into stderr and
the error begin with * will be display into stdout. So if you want to use
other util to use phpstan's check result, just read its stdout and filter by
the leading star char.

But why not add some option like --xml or --json? There is no doubt that xml and
json is more friendly for machine reading. But they are hard to read for
human, and parsing them is not so easy. Can you parse xml or json in bash script?
Use the list style is just enough for both machine reading and human reading.
The list is clear and simple. Parsing the result is nothing than filter by
leading star char and split by "in" and "on line", the first part is
message, second file path, and thrid lineno. It works.

The table style is a little more beautiful but more useless. Who will use phpstan?
The phpstom user seems not likely to use phpstan, because phpstom is smart enough.
So phpstan is only for two use case, one for ci like travis, and the other for
developer who does not use IDE but editor like vim, emacs, etc. All these users
are power user. And they are like to let phpstan display the error list to their
favorite editor. So a machine reading friendly style like list style should be used.

embrace phar

• use system tmp dir instead of rootDir/tmp

But why?

You can use phar-composer to package all the phpstan sorce into a phar executable.
But how? Just run composer update --no-dev and go to parent dir and run
phar-composer build phpstan
and you will got you phpstan.phar. Awesome.

For user who do not have their own rule or extension class, use phpstan.phar
is just enough. For user who want to develop their own rule or extension class,
however, they have to implements some phpstan's interface, which are defined
in the phpstan/phpstan package. They have two choice. One is require
phpstan/phpstan for dev only. If use this metho, phpstan.phar is no more needed.
The other choice is to just use phpstan.phar, and implement phpstan's interface
blindly. As phpstan.phar contains these interfaces, no error will occure. But
your IDE will display error for the unexisting phpstan interfaces.

Maybe we have third choice. We can split all the interfaces need to develop
rule and extension into and seprate package like phpstan/phpstan-interface.
Both phpstan/phpstan and the user's own code could implements these interfaces.

[ondrejmirtes]

Hello, thanks for the PR. I think it would be much wiser to decouple the changes into separate RFCs and PRs because this huge chunk of changes that changes the essence and core of the tool so much is unlikely to be accepted.

I see a few useful bits here and there in this PR, but surrounded with a lot of irrelevant changes that break backwards compatibility and change formatting. This will likely result in you wanting to maintain your own fork, but I'd advise against that. You will cut yourself off from being able to benefit from changes in PHPStan; and PHPStan is going to be smarter and faster thanks to those changes.

If I were you, I'd rather go with the way of composition, not modification. If you don't like how PHPStan is configured, create a middle layer tool that will translate CLI options into a neon file that PHPStan accepts. That's the correct way of doing things without breaking backwards compatibility. Some projects already have custom ways to configure PHPStan, see edgedesign/phpqa.

And now, for the specific changes you made:

use system tmp dir

Not everyone wants to cache things in a global system tmp dir. I plan to make this configurable so everyone can set tmp dir of they own choice.

display error into stderr, and just list error to stdout

This is useful. If you create a separate PR with just this change, I can merge it. But make sure it's on the current master, there were some changes related to custom error formatters, see #185.

use psr1 and psr2 coding style

I don't agree with this change as I already pointed out few days ago in #181. As part of this change, you gutted out the current coding standard which does a lot of useful things, see the release notes. These rules are much more useful for code safety and consistency than just some formatting guidelines.

use php-di

Huge BC break. I also don't like how the configuration looks.

only support composer autoload

Huge no-go. There are developers out there that don't use Composer autoloader and need these options in order for the tool to work.

use psr cache

This I can get behind. If you created a separate PR on master, we can discuss about merging it.

I don't agree with moving all the configuration options into CLI (making the analyse command too long, hard to read and limiting to what data structures can I use), so I will skip those.

use interface instead of tag for extension

I don't agree with that. Implementing an interface and tagging a service means two different things. Interface - "this object can be passed to places where this interface is requested". Tag - "I want this service to get injected to this specific place". Tags can also be used to be configured to get some specific services excluded and not be used if the user doesn't want them to.

fix process term signal delay

I worry this might slow down the analysis. Also: it's no longer required on PHP 7.1.

drop normalPath and fileExcluder

Path normalizing is required for a nicer output and for excluding to work. Excluding files is required if the analyzed codebase contains some files with serious errors on purpose. fnmatch does not work very well when the codebase needs to work on Unix and Windows at the same time.

So these changes are fine by me and actually add value:

• display error into stderr, and just list error to stdout
• use psr cache

If you create separate PRs with correct formatting so that the build passes, I'm happy to accept them.

Thanks.

[taoso]

@ondrejmirtes Making this PR is not for merging but discussing. If you are confident in your coding style, system design, why not make this PR open, and let other people join the discussion? Feel free to close this PR, because you are the owner, leader probably.