Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Issues found by hphp static analysis #4444

Closed
wants to merge 11 commits into from

3 participants

@rlerdorf

I think I weeded out most of the false positives, but there may be a few still in there. For the TooManyArgument error it doesn't know about func_get_args(), so that is the most common false positive.

--------------------------------
File       : library/Zend/Feed/Reader/Extension/CreativeCommons/Entry.php:56
Reason     : BadConstructorCall
Snippet    : new Zend\Feed\Reader\Extension\CreativeCommons\Feed($this->domDocument, $this->data['type'], $this->xpath)
Line       : $cc = new Feed(
--------------------------------
File       : library/Zend/Code/Scanner/Util.php:29
Reason     : RequiredAfterOptionalParam
Snippet    : $key = null
Line       : public static function resolveImports(&$value, $key = null, stdClass $data)
--------------------------------
File       : library/Zend/Mvc/View/Console/RouteNotFoundStrategy.php:165
Reason     : TooManyArgument
Snippet    : $this->getConsoleUsage($console, $scriptName, $mm, $router)
Line       : $usage = $this->getConsoleUsage($console, $scriptName, $mm, $router);
--------------------------------
File       : library/Zend/Mail/Storage/Folder/Maildir.php:196
Reason     : TooManyArgument
Snippet    : $this->_buildFolderTree($this->rootdir)
Line       : $this->_buildFolderTree($this->rootdir);
--------------------------------
File       : library/Zend/Mvc/View/Http/RouteNotFoundStrategy.php:196
Reason     : TooManyArgument
Snippet    : $this->injectNotFoundReason($model, $e)
Line       : $this->injectNotFoundReason($model, $e);
--------------------------------
File       : library/Zend/ServiceManager/Di/DiAbstractServiceFactory.php:43
Reason     : TooManyArgument
Snippet    : $this->get($requestedName, array(), true)
Line       : return $this->get($requestedName, array(), true);
--------------------------------
File       : library/Zend/ServiceManager/Di/DiAbstractServiceFactory.php:46
Reason     : TooManyArgument
Snippet    : $this->get($serviceName, array(), true)
Line       : return $this->get($serviceName, array(), true);
--------------------------------
File       : library/Zend/ServiceManager/Di/DiServiceFactory.php:84
Reason     : TooManyArgument
Snippet    : $this->get($this->name, $this->parameters, true)
Line       : return $this->get($this->name, $this->parameters, true);
--------------------------------
File       : library/Zend/Code/Generator/DocBlockGenerator.php:174
Reason     : BadArgumentType
Snippet    : parameter 3 of __construct() requires Object - exception, called with String
Line       : throw new Exception\InvalidArgumentException(
--------------------------------
File       : library/Zend/Validator/File/ExcludeMimeType.php:90
Reason     : StatementHasNoEffect
Snippet    : false;
Line       : false;
--------------------------------
File       : library/Zend/Cache/Storage/Adapter/RedisResourceManager.php:332
Reason     : UseVoidReturn
Snippet    : $this->normalizeLibOptionKey($key)
Line       : $constValue = $this->normalizeLibOptionKey($key);
--------------------------------
File       : library/Zend/Serializer/Adapter/PythonPickle.php:424
Reason     : UseVoidReturn
Snippet    : $this->write($k)
Line       : $this->pickle .= $this->write($k) . $this->write($v) . self::OP_SETITEM;
--------------------------------
File       : library/Zend/Serializer/Adapter/PythonPickle.php:424
Reason     : UseVoidReturn
Snippet    : $this->write($v)
Line       : $this->pickle .= $this->write($k) . $this->write($v) . self::OP_SETITEM;
--------------------------------
File       : library/Zend/Serializer/Adapter/PythonPickle.php:444
Reason     : UseVoidReturn
Snippet    : $this->write($v)
Line       : $this->pickle .= $this->write($v) . self::OP_APPEND;
--------------------------------
File       : library/Zend/Cache/Storage/Adapter/MemcachedResourceManager.php:285
Reason     : UseVoidReturn
Snippet    : $this->normalizeLibOptionKey($key)
Line       : $constValue = $this->normalizeLibOptionKey($key);
--------------------------------
File       : library/Zend/Mvc/Controller/AbstractRestfulController.php:316
Reason     : UseVoidReturn
Snippet    : $this->deleteList()
Line       : $return = $this->deleteList();
--------------------------------
File       : library/Zend/Mvc/Controller/AbstractRestfulController.php:354
Reason     : UseVoidReturn
Snippet    : $this->patch($id, $data)
Line       : $return = $this->patch($id, $data);
--------------------------------
File       : library/Zend/Mvc/Controller/AbstractRestfulController.php:363
Reason     : UseVoidReturn
Snippet    : $this->patchList($data)
Line       : $return = $this->patchList($data);
--------------------------------
File       : library/Zend/Mvc/Controller/AbstractRestfulController.php:387
Reason     : UseVoidReturn
Snippet    : $this->replaceList($data)
Line       : $return = $this->replaceList($data);
@weierophinney

@rlerdorf Thanks for doing this! Quick question: is this against current master, or a specific tag?

weierophinney added some commits
@weierophinney weierophinney [#4444] Remove arguments from instantiation
- Object being instantiated has no defined constructor in the inheritance tree.
904ac38
@weierophinney weierophinney [#4444] Make last argument optional
- Cannot have a required argument following an optional argument.
af1313f
@weierophinney weierophinney [#4444] Remove unused argument
- call to getConsoleUsage() was passing an argument the method did not
  accept or use.
eb3915f
@weierophinney weierophinney [#4444] Remove unused argument
- Call to _buildFolderTree() was passing an argument the method did not
  accept or use.
a0c5bbd
@weierophinney weierophinney [#4444] Remove unused argument
- Call to injectNotFoundReason() was passing an argument not accepted or
  used by the method.
95e0c22
@weierophinney weierophinney [#4444] Remove unused arguments
- Calls to get() were passing a third, unused argument; removed.
4d8c5d6
@weierophinney weierophinney [#4444] Fix exception throw
- Missing sprintf()
1e7f312
@weierophinney weierophinney [#4444] Add missing return directive fa00e3a
@weierophinney weierophinney [#4444] Fix usage of lib-option key
- call to normalizeLibOptionKey() uses pass-by-reference, and has no
  return value.
- Fixed code to operate on that assumption.
fb3caa0
@weierophinney weierophinney [#4444] Fix order of operations
- write() appends directly to $pickle property, and does not have a
  return value.
- Broke statements that appended write() operations into multiple statements.
d2234ff
@weierophinney weierophinney [#4444] Fix usage of normalizeLibOptionKey
- normalizeLibOptionKey() uses pass-by-reference, but statement was
  assuming a return value.
- fixed method to use pass-by-reference
ac2ecbb
@weierophinney

@rlerdorf I've patched all but AbstractRestfulController at this time via this PR. The calls to AbstractRestfulController are okay, as the idea is that developers extend the class and override those methods; we used the mechanism of raising an exception as we could not introduce new abstract methods and retain backwards compatibility.

Would having an explicit return null in those methods following the throw make a difference to the analyzer?

@rlerdorf

There are lots of false positives. I wouldn't change any code just to make the analyzer happy. What you probably should do is compile hhvm yourself and write a little filter script that is specific to ZF that filters out the AbstractRestfulController calls and the other false positives and run it occasionally. That is what I do for my stuff.

@rlerdorf

Oh, and to answer your question, this was against master, but you probably figured that out.

@weierophinney

What you probably should do is compile hhvm yourself and write a little filter script that is specific to ZF

@rlerdorf Have you started work on one yet? If so, would you be willing to share it with me? The stuff it found was fantastic. :)

@rlerdorf

A filter specific for ZF you mean? No, I just manually eyeballed it and removed the obvious false positives before sending you the results.

@weierophinney

@rlerdorf Okay, then I'll start looking into it myself. Thanks a ton for sending the feedback!

@mwillbanks mwillbanks was assigned
@mwillbanks mwillbanks closed this pull request from a commit
@mwillbanks mwillbanks Merge branch 'hotfix/4444'
Close #4444
eb37aec
@mwillbanks mwillbanks closed this in eb37aec
@mwillbanks mwillbanks referenced this pull request from a commit
@mwillbanks mwillbanks Merge branch 'hotfix/4444' into develop
Forward Port #4444
1ddb588
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney [#4444] Remove arguments from instantiation
- Object being instantiated has no defined constructor in the inheritance tree.
70d265d
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney [#4444] Make last argument optional
- Cannot have a required argument following an optional argument.
7d7f254
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney [#4444] Remove unused argument
- call to getConsoleUsage() was passing an argument the method did not
  accept or use.
8d78e73
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney [#4444] Remove unused argument
- Call to _buildFolderTree() was passing an argument the method did not
  accept or use.
f687297
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney [#4444] Remove unused argument
- Call to injectNotFoundReason() was passing an argument not accepted or
  used by the method.
19c2897
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney [#4444] Remove unused arguments
- Calls to get() were passing a third, unused argument; removed.
21391fb
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney [#4444] Fix exception throw
- Missing sprintf()
de66071
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney [#4444] Add missing return directive 223975f
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney [#4444] Fix usage of lib-option key
- call to normalizeLibOptionKey() uses pass-by-reference, and has no
  return value.
- Fixed code to operate on that assumption.
149c3bd
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney [#4444] Fix order of operations
- write() appends directly to $pickle property, and does not have a
  return value.
- Broke statements that appended write() operations into multiple statements.
fe46fac
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney [#4444] Fix usage of normalizeLibOptionKey
- normalizeLibOptionKey() uses pass-by-reference, but statement was
  assuming a return value.
- fixed method to use pass-by-reference
166162f
@ghost Unknown referenced this pull request from a commit
@mwillbanks mwillbanks Merge branch 'hotfix/4444'
Close #4444
a3d7064
@ghost Unknown referenced this pull request from a commit
@mwillbanks mwillbanks Merge branch 'hotfix/4444' into develop
Forward Port #4444
a45d2a5
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-mail
@weierophinney weierophinney [zendframework/zf2#4444] Remove unused argument
- Call to _buildFolderTree() was passing an argument the method did not
  accept or use.
2419c7f
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-mail
@mwillbanks mwillbanks Merge branch 'hotfix/4444' 155b787
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-mail
@mwillbanks mwillbanks Merge branch 'hotfix/4444' into develop c1d4cb7
@weierophinney weierophinney referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@weierophinney weierophinney referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@weierophinney weierophinney referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@weierophinney weierophinney referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-servicemanager
@weierophinney weierophinney [zendframework/zf2#4444] Remove unused arguments
- Calls to get() were passing a third, unused argument; removed.
6e2934f
@gianarb gianarb referenced this pull request from a commit in zendframework/zend-servicemanager
@mwillbanks mwillbanks Merge branch 'hotfix/4444' 8c5c414
@gianarb gianarb referenced this pull request from a commit in zendframework/zend-servicemanager
@mwillbanks mwillbanks Merge branch 'hotfix/4444' into develop 7d9869d
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-serializer
@weierophinney weierophinney [zendframework/zf2#4444] Fix order of operations
- write() appends directly to $pickle property, and does not have a
  return value.
- Broke statements that appended write() operations into multiple statements.
a9bac89
@gianarb gianarb referenced this pull request from a commit in zendframework/zend-serializer
@mwillbanks mwillbanks Merge branch 'hotfix/4444' 3196aef
@gianarb gianarb referenced this pull request from a commit in zendframework/zend-serializer
@mwillbanks mwillbanks Merge branch 'hotfix/4444' into develop 92b213f
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-validator
@weierophinney weierophinney [zendframework/zf2#4444] Add missing return directive 82e5866
@gianarb gianarb referenced this pull request from a commit in zendframework/zend-validator
@mwillbanks mwillbanks Merge branch 'hotfix/4444' 02fa5b3
@gianarb gianarb referenced this pull request from a commit in zendframework/zend-validator
@mwillbanks mwillbanks Merge branch 'hotfix/4444' into develop 463e3d7
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-cache
@weierophinney weierophinney [zendframework/zf2#4444] Fix usage of lib-option key
- call to normalizeLibOptionKey() uses pass-by-reference, and has no
  return value.
- Fixed code to operate on that assumption.
b5b0936
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-cache
@weierophinney weierophinney [zendframework/zf2#4444] Fix usage of normalizeLibOptionKey
- normalizeLibOptionKey() uses pass-by-reference, but statement was
  assuming a return value.
- fixed method to use pass-by-reference
2697e3e
@gianarb gianarb referenced this pull request from a commit in zendframework/zend-cache
@mwillbanks mwillbanks Merge branch 'hotfix/4444' de4e2c8
@gianarb gianarb referenced this pull request from a commit in zendframework/zend-cache
@mwillbanks mwillbanks Merge branch 'hotfix/4444' into develop 0b20126
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-feed
@weierophinney weierophinney [zendframework/zf2#4444] Remove arguments from instantiation
- Object being instantiated has no defined constructor in the inheritance tree.
121f01a
@gianarb gianarb referenced this pull request from a commit in zendframework/zend-feed
@mwillbanks mwillbanks Merge branch 'hotfix/4444' fc3b968
@gianarb gianarb referenced this pull request from a commit in zendframework/zend-feed
@mwillbanks mwillbanks Merge branch 'hotfix/4444' into develop 2416664
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 8, 2013
  1. @weierophinney

    [#4444] Remove arguments from instantiation

    weierophinney authored
    - Object being instantiated has no defined constructor in the inheritance tree.
  2. @weierophinney

    [#4444] Make last argument optional

    weierophinney authored
    - Cannot have a required argument following an optional argument.
  3. @weierophinney

    [#4444] Remove unused argument

    weierophinney authored
    - call to getConsoleUsage() was passing an argument the method did not
      accept or use.
  4. @weierophinney

    [#4444] Remove unused argument

    weierophinney authored
    - Call to _buildFolderTree() was passing an argument the method did not
      accept or use.
  5. @weierophinney

    [#4444] Remove unused argument

    weierophinney authored
    - Call to injectNotFoundReason() was passing an argument not accepted or
      used by the method.
  6. @weierophinney

    [#4444] Remove unused arguments

    weierophinney authored
    - Calls to get() were passing a third, unused argument; removed.
  7. @weierophinney

    [#4444] Fix exception throw

    weierophinney authored
    - Missing sprintf()
  8. @weierophinney
  9. @weierophinney

    [#4444] Fix usage of lib-option key

    weierophinney authored
    - call to normalizeLibOptionKey() uses pass-by-reference, and has no
      return value.
    - Fixed code to operate on that assumption.
  10. @weierophinney

    [#4444] Fix order of operations

    weierophinney authored
    - write() appends directly to $pickle property, and does not have a
      return value.
    - Broke statements that appended write() operations into multiple statements.
  11. @weierophinney

    [#4444] Fix usage of normalizeLibOptionKey

    weierophinney authored
    - normalizeLibOptionKey() uses pass-by-reference, but statement was
      assuming a return value.
    - fixed method to use pass-by-reference
This page is out of date. Refresh to see the latest.
View
6 library/Zend/Cache/Storage/Adapter/MemcachedResourceManager.php
@@ -282,14 +282,14 @@ public function getLibOption($id, $key)
throw new Exception\RuntimeException("No resource with id '{$id}'");
}
- $constValue = $this->normalizeLibOptionKey($key);
+ $this->normalizeLibOptionKey($key);
$resource = & $this->resources[$id];
if ($resource instanceof MemcachedResource) {
- return $resource->getOption($constValue);
+ return $resource->getOption($key);
}
- return isset($resource['lib_options'][$constValue]) ? $resource['lib_options'][$constValue] : null;
+ return isset($resource['lib_options'][$key]) ? $resource['lib_options'][$key] : null;
}
/**
View
6 library/Zend/Cache/Storage/Adapter/RedisResourceManager.php
@@ -329,14 +329,14 @@ public function getLibOption($id, $key)
throw new Exception\RuntimeException("No resource with id '{$id}'");
}
- $constValue = $this->normalizeLibOptionKey($key);
+ $this->normalizeLibOptionKey($key);
$resource = & $this->resources[$id];
if ($resource instanceof RedisResource) {
- return $resource->getOption($constValue);
+ return $resource->getOption($key);
}
- return isset($resource['lib_options'][$constValue]) ? $resource['lib_options'][$constValue] : null;
+ return isset($resource['lib_options'][$key]) ? $resource['lib_options'][$key] : null;
}
/**
View
4 library/Zend/Code/Generator/DocBlockGenerator.php
@@ -171,11 +171,11 @@ public function setTag($tag)
if (is_array($tag)) {
$tag = new DockBlockTag($tag);
} elseif (!$tag instanceof DockBlockTag) {
- throw new Exception\InvalidArgumentException(
+ throw new Exception\InvalidArgumentException(sprintf(
'%s expects either an array of method options or an instance of %s\DocBlock\Tag',
__METHOD__,
__NAMESPACE__
- );
+ ));
}
$this->tags[] = $tag;
View
7 library/Zend/Code/Scanner/Util.php
@@ -26,9 +26,12 @@ class Util
* @return void
* @throws Exception\InvalidArgumentException
*/
- public static function resolveImports(&$value, $key = null, stdClass $data)
+ public static function resolveImports(&$value, $key = null, stdClass $data = null)
{
- if (!property_exists($data, 'uses') || !property_exists($data, 'namespace')) {
+ if (!is_object($data)
+ || !property_exists($data, 'uses')
+ || !property_exists($data, 'namespace')
+ ) {
throw new Exception\InvalidArgumentException(sprintf(
'%s expects a data object containing "uses" and "namespace" properties; on or both missing',
__METHOD__
View
4 library/Zend/Feed/Reader/Extension/CreativeCommons/Entry.php
@@ -53,9 +53,7 @@ public function getLicenses()
$licenses = array_unique($licenses);
} else {
- $cc = new Feed(
- $this->domDocument, $this->data['type'], $this->xpath
- );
+ $cc = new Feed();
$licenses = $cc->getLicenses();
}
View
2  library/Zend/Mail/Storage/Folder/Maildir.php
@@ -193,7 +193,7 @@ public function selectFolder($globalName)
throw new Exception\RuntimeException("{$this->currentFolder} is not selectable", 0, $e);
}
// seems like file has vanished; rebuilding folder tree - but it's still an exception
- $this->_buildFolderTree($this->rootdir);
+ $this->_buildFolderTree();
throw new Exception\RuntimeException('seems like the maildir has vanished, I\'ve rebuild the ' .
'folder tree, search for an other folder and try again', 0, $e);
}
View
2  library/Zend/Mvc/View/Console/RouteNotFoundStrategy.php
@@ -162,7 +162,7 @@ public function handleRouteNotFoundError(MvcEvent $e)
$banner = $this->getConsoleBanner($console, $mm);
// Get application usage information
- $usage = $this->getConsoleUsage($console, $scriptName, $mm, $router);
+ $usage = $this->getConsoleUsage($console, $scriptName, $mm);
// Inject the text into view
$result = $banner ? rtrim($banner, "\r\n") : '';
View
2  library/Zend/Mvc/View/Http/RouteNotFoundStrategy.php
@@ -193,7 +193,7 @@ public function prepareNotFoundViewModel(MvcEvent $e)
$model->setTemplate($this->getNotFoundTemplate());
// If displaying reasons, inject the reason
- $this->injectNotFoundReason($model, $e);
+ $this->injectNotFoundReason($model);
// If displaying exceptions, inject
$this->injectException($model, $e);
View
7 library/Zend/Serializer/Adapter/PythonPickle.php
@@ -421,7 +421,9 @@ protected function writeArrayDict($value)
$this->memorize($value);
foreach ($value as $k => $v) {
- $this->pickle .= $this->write($k) . $this->write($v) . self::OP_SETITEM;
+ $this->write($k);
+ $this->write($v);
+ $this->pickle .= self::OP_SETITEM;
}
}
@@ -441,7 +443,8 @@ protected function writeArrayList(array $value)
$this->memorize($value);
foreach ($value as $v) {
- $this->pickle .= $this->write($v) . self::OP_APPEND;
+ $this->write($v);
+ $this->pickle .= self::OP_APPEND;
}
}
View
4 library/Zend/ServiceManager/Di/DiAbstractServiceFactory.php
@@ -40,10 +40,10 @@ public function createServiceWithName(ServiceLocatorInterface $serviceLocator, $
{
$this->serviceLocator = $serviceLocator;
if ($requestedName) {
- return $this->get($requestedName, array(), true);
+ return $this->get($requestedName, array());
}
- return $this->get($serviceName, array(), true);
+ return $this->get($serviceName, array());
}
/**
View
2  library/Zend/ServiceManager/Di/DiServiceFactory.php
@@ -81,7 +81,7 @@ public function __construct(Di $di, $name, array $parameters = array(), $useServ
public function createService(ServiceLocatorInterface $serviceLocator)
{
$this->serviceLocator = $serviceLocator;
- return $this->get($this->name, $this->parameters, true);
+ return $this->get($this->name, $this->parameters);
}
/**
View
2  library/Zend/Validator/File/ExcludeMimeType.php
@@ -87,7 +87,7 @@ public function isValid($value, $file = null)
if (empty($this->type)) {
$this->error(self::NOT_DETECTED);
- false;
+ return false;
}
$mimetype = $this->getMimeType(true);
Something went wrong with that request. Please try again.