Issues found by hphp static analysis #4444

Closed
wants to merge 11 commits into
from

Projects

None yet

3 participants

@rlerdorf
rlerdorf commented May 8, 2013

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
Member

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

weierophinney added some commits May 8, 2013
@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
Member

@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
rlerdorf commented May 8, 2013

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
rlerdorf commented May 8, 2013

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

@weierophinney
Member

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
rlerdorf commented May 8, 2013

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
Member

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

@mwillbanks mwillbanks was assigned May 8, 2013
@mwillbanks mwillbanks added a commit that closed this pull request May 8, 2013
@mwillbanks mwillbanks Merge branch 'hotfix/4444'
Close #4444
eb37aec
@mwillbanks mwillbanks closed this in eb37aec May 8, 2013
@mwillbanks mwillbanks added a commit that referenced this pull request May 8, 2013
@mwillbanks mwillbanks Merge branch 'hotfix/4444' into develop
Forward Port #4444
1ddb588
@weierophinney weierophinney added a commit to zendframework/zend-mail that referenced this pull request May 14, 2015
@weierophinney weierophinney [zendframework/zendframework#4444] Remove unused argument
- Call to _buildFolderTree() was passing an argument the method did not
  accept or use.
2419c7f
@weierophinney weierophinney pushed a commit to zendframework/zend-mail that referenced this pull request May 14, 2015
@mwillbanks mwillbanks Merge branch 'hotfix/4444' 155b787
@weierophinney weierophinney pushed a commit to zendframework/zend-mail that referenced this pull request May 14, 2015
@mwillbanks mwillbanks Merge branch 'hotfix/4444' into develop c1d4cb7
@weierophinney weierophinney added a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#4444] Remove unused arguments
- Calls to get() were passing a third, unused argument; removed.
6e2934f
@gianarb gianarb pushed a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
@mwillbanks mwillbanks Merge branch 'hotfix/4444' 8c5c414
@gianarb gianarb pushed a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
@mwillbanks mwillbanks Merge branch 'hotfix/4444' into develop 7d9869d
@weierophinney weierophinney added a commit to zendframework/zend-serializer that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#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 pushed a commit to zendframework/zend-serializer that referenced this pull request May 15, 2015
@mwillbanks mwillbanks Merge branch 'hotfix/4444' 3196aef
@gianarb gianarb pushed a commit to zendframework/zend-serializer that referenced this pull request May 15, 2015
@mwillbanks mwillbanks Merge branch 'hotfix/4444' into develop 92b213f
@weierophinney weierophinney added a commit to zendframework/zend-validator that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#4444] Add missing return directive 82e5866
@gianarb gianarb pushed a commit to zendframework/zend-validator that referenced this pull request May 15, 2015
@mwillbanks mwillbanks Merge branch 'hotfix/4444' 02fa5b3
@gianarb gianarb pushed a commit to zendframework/zend-validator that referenced this pull request May 15, 2015
@mwillbanks mwillbanks Merge branch 'hotfix/4444' into develop 463e3d7
@weierophinney weierophinney added a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#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 added a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#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 pushed a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
@mwillbanks mwillbanks Merge branch 'hotfix/4444' de4e2c8
@gianarb gianarb pushed a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
@mwillbanks mwillbanks Merge branch 'hotfix/4444' into develop 0b20126
@weierophinney weierophinney added a commit to zendframework/zend-feed that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#4444] Remove arguments from instantiation
- Object being instantiated has no defined constructor in the inheritance tree.
121f01a
@gianarb gianarb pushed a commit to zendframework/zend-feed that referenced this pull request May 15, 2015
@mwillbanks mwillbanks Merge branch 'hotfix/4444' fc3b968
@gianarb gianarb pushed a commit to zendframework/zend-feed that referenced this pull request May 15, 2015
@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