Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small PHPStorm reported errors #376

Closed
aaron-junot opened this issue Oct 18, 2018 · 1 comment
Closed

Small PHPStorm reported errors #376

aaron-junot opened this issue Oct 18, 2018 · 1 comment
Labels
V2 V2 Client

Comments

@aaron-junot
Copy link

While working on #375, I noticed that there were other errors that did not have anything to do with tags such as @property, @return and @throws. I did not fix these errors because they did not have anything to do with #277 and #358. However, I think it's useful to document my findings, and maybe one day, we will do something about them. At this time, I don't believe any of them are pressing enough to warrant spending any development time on them, but here they are for documentation purposes, broken down by class.

Issues according to PHPStorm:

Recurly_Base:
1. Uses DOMDocument without including ext-dom in composer.json (this lib is included in php installs by default)
2. Uses LIBXML_NOBLANKS without including ext-libxml in composer.json (this lib is included in php installs by default)
3. Uses $this->getNodeName() which is implemented by subclasses, not this abstract class
4. Uses $this->updateErrorAttributes() which is implemented by subclasses, not this abstract class
5. Uses XML_TEXT_NODE without including ext-dom in composer.json (this lib is included in php installs by default)
6. Uses XML_ELEMENT_NODE without including ext-dom in composer.json (this lib is included in php installs by default)
7. Accesses protected property _objects of Recurly_Pager
8. Complains about how $node->getAttribute() is not found, even though it definitely exists according to the PHP docs
9. Complains about how $node->hasAttribute() is not found, even though it definitely exists according to the PHP docs
10. Complains about how $new_obj->setHref() is not found, even though it's always a Recurly_Resource which has this method because it extends Recurly_Base

Recurly_Client
1. Uses mb_internal_encoding without including ext-mbstring in composer.json (even though the docs say this is a builtin)

Recurly_CurrencyList
1. Complains about how $node->appendChild() is not found, even though it definitely exists according to the PHP docs
2. Complains about how $doc->createElement() is not found, even though it definitely exists according to the PHP docs

Recurly_CustomField
1. Complains about how $node->appendChild() is not found, even though it definitely exists according to the PHP docs
2. Complains about how $doc->createElement() is not found, even though it definitely exists according to the PHP docs

Recurly_CustomFieldList
1. Complains about how $node->appendChild() is not found, even though it definitely exists according to the PHP docs
2. Complains about how $doc->createElement() is not found, even though it definitely exists according to the PHP docs
3. Complains about how $customFieldsNode->hasChildNodes() is not found, even though it definitely exists according to the PHP docs

Recurly_PushNotification
1. Uses simplexml_load_string without including ext-mbstring in composer.json (even though the docs say this is a builtin)
2. Same with SimpleXMLElement

Recurly_js
1. Do we even need this class?
2. Because of some weird syntax I don't recognize, PHPStorm can't find $rjs->get_signature()

Recurly_Resource
1. Uses DOMDocument without including ext-dom in composer.json (this lib is included in php installs by default)
2. Uses LIBXML_NOEMPTYTAG without including ext-libxml in composer.json (this lib is included in php installs by default)

Recurly_Response
1. Can't find $object->getErrors()

Recurly_Subscription
1. Similar problems with the cant find node stuff

Recurly_SubscriptionAddOn
1. Similar problems with the cant find node stuff

@aaron-junot
Copy link
Author

In addition to these issues, there is also a lot of commented out code and TODOs that should be addressed.

My thought is that the commented out code has not been in use in a very long time, so clearly it is unneeded.

//} else if ($this->_attributes->include($key)) {
// return null;

// FIXME: This is the pattern keys follow for paged results, which creates
// duplicate values. I consider that a bug.
// $this->assertEquals((1 - $i % $page_size) + 1, $pager->key());
// The nested results follow a much sainer:
// $this->assertEquals($i, $pager->key());

//print "Node: {$node->nodeType} -- {$node->nodeName}\n";

These are the TODOs:

// TODO: If there are more dots, then apply these to sub elements

# TODO: Should test the rest of the parsing.

@bhelx bhelx added the V2 V2 Client label Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
V2 V2 Client
Projects
None yet
Development

No branches or pull requests

2 participants