Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Issues found by hphp static analysis #4444

Closed
wants to merge 11 commits into from

Conversation

weierophinney
Copy link
Member

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
Copy link
Member

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

- Object being instantiated has no defined constructor in the inheritance tree.
- Cannot have a required argument following an optional argument.
- call to getConsoleUsage() was passing an argument the method did not
  accept or use.
- Call to _buildFolderTree() was passing an argument the method did not
  accept or use.
- Call to injectNotFoundReason() was passing an argument not accepted or
  used by the method.
- Calls to get() were passing a third, unused argument; removed.
- call to normalizeLibOptionKey() uses pass-by-reference, and has no
  return value.
- Fixed code to operate on that assumption.
- write() appends directly to $pickle property, and does not have a
  return value.
- Broke statements that appended write() operations into multiple statements.
- normalizeLibOptionKey() uses pass-by-reference, but statement was
  assuming a return value.
- fixed method to use pass-by-reference
@weierophinney
Copy link
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
Copy link
Author

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
Copy link
Author

rlerdorf commented May 8, 2013

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

@weierophinney
Copy link
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
Copy link
Author

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
Copy link
Member

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

@ghost ghost assigned mwillbanks May 8, 2013
@mwillbanks mwillbanks closed this in eb37aec May 8, 2013
mwillbanks pushed a commit that referenced this pull request May 8, 2013
weierophinney added a commit to zendframework/zend-mail that referenced this pull request May 14, 2015
- Call to _buildFolderTree() was passing an argument the method did not
  accept or use.
weierophinney pushed a commit to zendframework/zend-mail that referenced this pull request May 14, 2015
weierophinney pushed a commit to zendframework/zend-mail that referenced this pull request May 14, 2015
weierophinney added a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
- Calls to get() were passing a third, unused argument; removed.
gianarb pushed a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
weierophinney added a commit to zendframework/zend-serializer that referenced this pull request May 15, 2015
- write() appends directly to $pickle property, and does not have a
  return value.
- Broke statements that appended write() operations into multiple statements.
gianarb pushed a commit to zendframework/zend-serializer that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-serializer that referenced this pull request May 15, 2015
weierophinney added a commit to zendframework/zend-validator that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-validator that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-validator that referenced this pull request May 15, 2015
weierophinney added a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
- call to normalizeLibOptionKey() uses pass-by-reference, and has no
  return value.
- Fixed code to operate on that assumption.
weierophinney added a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
- normalizeLibOptionKey() uses pass-by-reference, but statement was
  assuming a return value.
- fixed method to use pass-by-reference
gianarb pushed a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
weierophinney added a commit to zendframework/zend-feed that referenced this pull request May 15, 2015
- Object being instantiated has no defined constructor in the inheritance tree.
gianarb pushed a commit to zendframework/zend-feed that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-feed that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants