Would PHPLint's advice be beneficial to our community?

Question

I recently found this static PHP code analyzer called PHPLint. One of the things it can do, is look at code like this:

/**
 * @return string
 */
function getText() {
  return 5;
}

And complain that a 5, is not a string. It seems to me like this could potentially reduce bugs, and perhaps even security issues in our code. One nice thing is that I see it could give is that for return values from function, all you ever need to check is (=== null). If it's not null, then you have the data you expected.

At the surface, this seems like a possibly great addition to our codebases, core and contrib alike. But perhaps there are problems with this that I don't see?

I started applying appropriate doc-blocks to own of my own modules, and one of the issue I hit myself is that because core frequently uses arrays with mixed data in them, there are limits to what kinds of checks PHPLint can do, which causes a lot of warnings.

Edit: One concrete example of an issue that this could prevent is this notice from php 5.4. This code has always failed, it's just that no one noticed until PHP 5.4 made the implicit casting a notice. In this case the bug is relatively harmless, but more subtle errors and security problems could creep in this way.

Answer 1

In theory, there sure are benefits. There are however some issues with doing it, as you partially found out yourself already.

• There will be like a million false warnings, might just be as hard to find the real issues as when simply reading the code.
• Drupal has a quite a few custom coding standards. For example, we're currently not using type declarations in docblocks except for @return $class.

I don't know how configurable PHPLint is, maybe there are a few checks that would make sense to run on our code.

For coding style issues, it might make more sense to use http://drupal.org/project/drupalcs