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.

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?

Answer 2

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

Answer 3

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/

Answer 4

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.

Answer 5

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?

Answer 6

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...