PHP Syntax checking pre-source control

Question

Referring to Is there a static code analyzer [like Lint] for PHP files? -- I am looking at how to assess the content of PHP files before they are committed by developers. Whichever solution(s) are appropriate will be triggered via SVN hooks similar to the answer: Is it possible to check PHP file syntax from PHP?

I came across this Automatic Syntax checking of PHP files when checking into SVN which is the angle I'm going for, however ... php -l isn't quite sufficient.

For example, given the code:

if ($foo == 'bar') { 
     echo $foo;
}

This results in:

2012/01/15 02:51:14 [error] 694#0: *164 FastCGI sent in stderr: "PHP Notice: Undefined variable: foo

Compared to:

if (isset($foo)) { echo $foo; }

Some of this comes down to educating coders on best practices. Unfortunately, some don't learn as quickly as others, and the only way to ensure that compliance to coding standards is met, is to reduce what is going into SVN that has been untested or isn't compliant.

From the first link in this question, I have tried:

• php -l
  • doesn't notify about the problem with $foo
• phplint-pure-c-1.0_20110223 - PHPLint
  • doesn't notify about the problem with $foo

    if ($foo == 'bar') {
                     \_ HERE

==== /mnt/hgfs/workspace/scratch-pad/phpinfo.php:44: Warning: comparing (unknown) == (string): cannot check the comparison between unknown types

• phpcs - PHP Code Sniffer
  • doesn't notify about the problem with $foo despite PHP Sanity Check indicating it was the right answer
• SimpleTest
  • Is very nice, but requires the developers who are writing the bad code to write good unit tests...

All are interesting in their own way, but none are catching these problems that really are only being found at runtime.

Appreciate input / thoughts on this topic.

EDIT

There was one poster who suggested that PHPLint was the right way to go. I thought, OK! Let's try it again given that there is a new version: phplint-pure-c-1.1_20120202:

 <?php
 if ($foo == 'bar') {
     echo $foo;
 }
 ?>

Simple test .................... and, it works and reports 1 error, 1 warning. However, if the following is added BEFORE the if statement:

 <?php
 if (isset($foo) && $foo == 'bar') { echo 'man'; }
 if ($foo == 'bar') { 
     echo $foo;
 }
 ?>

it does not work, and reports 0 errors, 2 warnings.

Disabled

Honestly, let them write unit tests. Syntax check will not find most of the faults. Unit testing will. – Atiana

Be very careful doing this kind of thing in an svn pre-commit hook. Infact I'd say flat out don't do it. You can't see the output of an svn pre-commit hook unless the commit is rejected - that can be incredibly disruptive when you block commits based on none-critical errors. It's better to implement such checks on developer's machines unless you value conformance above productivity. Tying into your build system would be more appropriate given you use svn. – Tarryn

there will always be some cases where something doesn't work the way you'd want to. That doesn't change the fact that phplint did what you originally asked it for. Mind you, I didn't actually say you should use it - I only implied that it fit to the requirements you posed. – Sapowith

@Sapowith i know. appreciate the effort and response ... it's the best of a bad bunch of solutions :) – Disabled

I agree with @Tarryn here. Tying it to a build system will be a much more pleasant experience for your developers, and it will be easier to configure. – Subprincipal

Answer 1

I think this might be a bit hard for an analyser to give warnings about. The code you've given might work with the help register_globals, for example. Also, it might be defined in some other file that is including this file. For those reasons, PHP files should be analyzed with full context of other files for this to be really reliable, and PHP/server configuration should also be either available or defined to the analyzing mechanism.

That said, are you sure phplint doesn't do what you want to?

There is an online validator that you can use to test it. Given the input:

<?php

echo $foo;

the result was:

        echo $foo;
                  \_ HERE
==== 3: ERROR: variable `$foo' has not been assigned
END parsing of test-qBlPWw
==== ?: notice: unused package `dummy.php'
==== ?: notice: unused module `standard'
Overall test results: 1 errors, 0 warnings.

whereas with isset() it didn't find any issues.

EDIT: so for this other test case:

<?php

if ($foo == 'bar') echo $foo;

On Linux Mint 8 the response is:

$ src/phplint test.php 
/home/vadmin/phplint/phplint-pure-c-1.0_20110223/test.php:3: ERROR: variable `$foo' has not been assigned
/home/vadmin/phplint/phplint-pure-c-1.0_20110223/test.php:3: Warning: comparing (unknown) == (string): cannot check the comparison between unknown types
Overall test results: 1 errors, 1 warnings.

and with this:

<?php

$foo = '1';
if ($foo == 1) echo $foo;

it is:

$ src/phplint test.php 
/home/vadmin/phplint/phplint-pure-c-1.0_20110223/test.php:6: ERROR: comparing (string) == (int)
Overall test results: 1 errors, 0 warnings.

so isn't it working like it should, and reporting the problem properly?

Sapowith

hi, i didn't use the online validator as we wont be sending our code across the wire outside of the org...i downloaded and compiled the PHPLint code and ran locally which didn't return what you've shown – Disabled

Yes, I didn't mean you should be using the online validator for your application... I meant that online validator should be the same as local one. On locally installed phplint-windows-1.020110223 the result is the same: BEGIN parsing of example.php 1: <?php 2: 3: echo $foo; echo $foo; _ HERE ==== example.php:3: ERROR: variable `$foo' has not been assigned END parsing of example.php Overall test results: 1 errors, 0 warnings. – Sapowith

nice. compiled phplint-pure-c-1.0_20110223 on linux gives different results. awesome. – Disabled

Interesting. I tested also with Mint 8 and phplint-pure-c-1.0_20110223 and got the same results also there: echo $foo; _ HERE ==== /home/vadmin/phplint/phplint-pure-c-1.0_20110223/test.php:3: ERROR: variable `$foo' has not been assigned Overall test results: 1 errors, 0 warnings. child process exited abnormally with code 1 – Sapowith

Oh, I see you changed the test case now. So was it that with this new test case it does not work, but for the one that was in the question initially, it works? – Sapowith

Yes, sorry @eis. the original test case wasn't entered properly – Disabled

@sdolgy: Added the new test case to the answer. – Sapowith

Answer 2

You might want to combine phpcs (to adhere to coding standards) and a new project by Sebastian Bergmann: https://github.com/sebastianbergmann/hphpa This utilizes the static compiler by facebook to check for errors such as your looking for... Might be too much as a pre commit hook, but a hook into your build system might suffice?

Pyrope

Answer 3

All of these smart super power tools that eavesdrop every door and look into every keyhole will never be able to compete withe the stupid and blunt action of RUNNING the code.

What is the value of having compilable, syntactically valid php files in the repo? You can make zounds of such files, commit them on a regular basis to the repo and, rest assured, all of them contribute to the project and add a certain reliable feature, because, well, they went through the pre-commit hook to check their validity?

There is a cr@pload of problems with the code written by humans, syntax and missing vars being only the tip of the iceberg. Unit testing (as noted by @NikiC) helps quite a bit. It's the responsibility of the developer to make reliable, working, documented code and test it before committing. Silly mistakes of using undeclared vars is something that the IDE can point out (Zend Studio does, for instance). Your goal is to create good working software and unit tests are key here. This should be the main concern in my opinion. Valid php files is a very loose requirement...

Jemima

Answer 4

Could you use a third party compiler that has more compile time options, like phc ( http://www.phpcompiler.org/doc/latest/runningphc.html#compiling-web-applications) ? (or possibly hiphop?)

Then I thought: you need Perl::Critic for php.

Critiquing PHP-code / PerlCritic for PHP?

(also google : perl critic for php )

I wish I could be more concretely helpful, but sometimes it's just an idea that gets you to the solution. That is what I have to offer :)

David

Zucchetto

Answer 5

Oh yeah, what you need is PHPUnderControl! It will check your syntax, automatically chack your unit tests, do a C.R.A.P. index, and more good stuff. It's basically the bomb!

Check it out, here is the URL: http://phpundercontrol.org/

Intendance

Answer 6

while not a command line checker, PHPStorm has to be one of the best IDE's out there.

It has various inspections which can detect the sort of problems that you've mentioned. Also, it automatically re-runs these inspections on the files you're committing to version control, checking for undefined variables, poor quality code and "todos".

However the problem with these inspections is that they can't know everything, so they sometimes err on the side of being 'suggestions' or 'warnings' rather than error.

However, it is quite good at what it does, and it can these sorts of problems while you're editing and, usually it results in fixing the errors well before any commit action anyway.

Periodate