Travis: add task to lint the PHP files against all supported versions

[jrfnl]

• Includes installing the PHP-Parallel-Lint package to allow for fast linting and comprehensive error reporting.
• Includes installing the PHP console highlighter to enable syntax highlighting of the reporting by PHP-Parallel-Lint.
• Includes adding a Composer script to run the command easily with the correct command-line arguments.
  Note: using @php will make sure that the script is triggered with the same PHP version as Composer is using (if different from the system default PHP version).
  Note: calling the actual parallel-lint script directly instead of via the vendor/bin will allow for the script to run cross-platform as otherwise devs on Windows may run into errors.

Related to #63

[gmazzap]

Thanks. Again.

I will take some more time to merge this because I'm not familiar with the tool, and want to look at it first.

[jrfnl]

@gmazzap Alternatively a pure PHP CLI implementation can be used, but in my experience this tool gives more user-friendly results.

On a practical note: I'm lining up the other CI/QA PRs, but they need to be pulled "in order" as they would all conflict if I pulled them at the same time.
If you want to leave this one for now, would you like me to pull one of the others (and locally re-order) or would you like to take it one at a time in the order I've set them up now ?

[gmazzap]

@jrfnl I just want to have a quick look at the tool before adding it to the requirements, I don't feel super comfortable in adding things I never heard of :)

Regarding the practical note: whichever works best for you. If it helps, I expect to merge this no earlier than tomorrow

[jrfnl]

@gmazzap I'll just pull them in the order they are in, in that case. A few days here or there is not what I'm worried about. PRs which are open for months is ;-)

[gmazzap]

In all honesty, I'm not very keen to introduce it. I'm very careful in adding dependencies, and this PR will introduce 3 new dependencies to do something that I don't feel is so essential.

The codebase is very well covered in tests, all files are loaded in tests, and I will not allow new files that have at least one test. Meaning that there's no way for a file with the wrong syntax to land in the codebase while the test suite passes. Because syntax is checked when files are loaded, and not when executed, even if something is never used (so not covered) in tests PHP will raise an exception / fatal error in case of the wrong syntax.

On top of that, I just assume that all people working on the codebase don't do that on a "plain" text editor, but in an IDE or at least in a text editor that has PHP syntax highlight and linting integrated.

So the proposed tools, in my opinion, would be of some benefit only in very edge cases, and even in those cases, when tests are run, PHP will raise a syntax error, with a message that far to be perfect, is surely easier to google.

I'm not closing this right away, to see if @jrfnl or anyone else has arguments to change my opinion.

[jrfnl]

In all honesty, I'm not very keen to introduce it. I'm very careful in adding dependencies, and this PR will introduce 3 new dependencies to do something that I don't feel is so essential.

As I said above, a pure bash/PHP implementation for this check is also possible. That would mean no dependencies would be introduced.

The codebase is very well covered in tests, all files are loaded in tests, and I will not allow new files that have at least one test. Meaning that there's no way for a file with the wrong syntax to land in the codebase while the test suite passes. Because syntax is checked when files are loaded, and not when executed, even if something is never used (so not covered) in tests PHP will raise an exception / fatal error in case of the wrong syntax.

Until only yesterday, test coverage was not checked, nor published, and this proposal was from before that time...

On top of that, I just assume that all people working on the codebase don't do that on a "plain" text editor, but in an IDE or at least in a text editor that has PHP syntax highlight and linting integrated.

IDEs only lint against one particular PHP version, either the system default or whatever version you have configured the IDE to work with.

So the proposed tools, in my opinion, would be of some benefit only in very edge cases, and even in those cases, when tests are run, PHP will raise a syntax error, with a message that far to be perfect, is surely easier to google.

Parallel lint will show the same error message, but will also show you the line on which the error is encountered. As I said, just a faster and more user-friendly way than the PHP native linting.

I'm not closing this right away, to see if @jrfnl or anyone else has arguments to change my opinion.

Close if you like, but in that case, please have another look at #63 which you gave a 👍 and leave a comment on which things you think is a good idea. I've got plenty other things to do, so please don't waste my time.

[gmazzap]

As I said above, a pure bash/PHP implementation for this check is also possible. That would mean no dependencies would be introduced.

The point is that I don't think that checking for syntax error is necessary at all.

Until only yesterday, test coverage was not checked, nor published, and this proposal was from before that time...

Coverage is competely unrelated.

As said above, it does not matter if a piece of code is excuted or not (in theory coverage could be 0%), PHP checks syntax at parse time, so even if code that contains syntax errors is never executed, the syntax errors are raised. That means that is possible to trigger a fatal syntax error just require-ing a file, and that exactly what happens during tests (because I made sure that all files are required during tests).

It means that when PHP unit tests run on Travis, if any of the files contain a syntax error, Travis build will fail because of that, and that before your (awesome) work from yesterday.

This is why, again, I don't think that a separate check for syntax is required: it is implicitly included in the unit tests.

This of course is true if all the files are required, and it is my responsibility to make sure that no new file is added without it being at least required during tests.

Parallel lint will show the same error message, but will also show you the line on which the error is encountered. As I said, just a faster and more user-friendly way than the PHP native linting.

Yes, that library has nice errors compared to PHP, but I think that IDEs do event a better job, and I don't think that during development people will keep running composer lint. I think that in majority of cases it would only be relevant at build time, and at build time I don't need an additional check for syntax because, as said above, build will fail if any syntax error is there.

Still, there might be cases is which this could be useful to someone in some context, but it is a so edge case IMO that I don't think it is worth to be considered.

Close if you like, but in that case, please have another look at #63 which you gave a 👍 and leave a comment on which things you think is a good idea. I've got plenty other things to do, so please don't waste my time.

Will do. And thanks a ton for your work and the time you're spending on this project.

[jrfnl]

Coverage is competely unrelated.

Well, to me it was as I didn't know that you'd made sure that all files were being tested and I wasn't going to verify this manually ;-)