Correcting errors reported by Phan PHP static analyzer #9
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- References to undeclared variables were fixed (e.g. in src/Service/CertificateService.php, src/Service/TrustedApplicationService.php) - Instances where no return value was being returned in contexts where a `DomainrobotPromise` was expected were added - Instances where `InvalidArgumentException()`s were intended to be thrown, but invocation resulted in errors due to being called as methods instead of instantiations were fixed - A nonexistent method invocation (`->getId()`) fixed in a case where a `TrustedApplication` object was provided as argument. In this case, the correct method to be called would be `->getUuId()` - Instances of `domainRobotConfig` properties were renamed to `domainrobotConfig` as was intended in one of the previous PRs merged into master - Overall annotations blocks were corrected according to the accepted PHP standard (PHPDoc, Phan annotations, etc.)
|
@jespinal |
|
Great, thanks to you too for taking the time to review this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the company I work for we are using this PHP SDK library in order to integrate our system with InterNetX platform.
While working with the different services provided in this SDK, I've noticed that there are some issues reported by Phan (PHP static analyzer) at the time of using some of them. After examining the issues, I was able to verify most of them, and noticed that the severity goes from low to high/important.
With this PR the following changes are introduced:
domainRobotConfigproperties were renamed todomainrobotConfigas was intended by one of the previous PRs merged into master.src/Service/CertificateService.php,src/Service/TrustedApplicationService.php)DomainrobotPromisewas expected were addedInvalidArgumentException()s were intended to be thrown, but invocation resulted in errors due to being called as methods instead of instantiations were fixed.->getId()) fixed in a case where aTrustedApplicationobject was provided as argument. In this case, the correct method to be called would be->getUuId()HOW TO TEST
If you have automated testing mechanisms for this SDK, they should work just fine. The cases where some conditions would lead to fatal errors (like invocation of nonexistent methods, or throwing exceptions as if they were method callings) should also work.
Running Phan against the
src/Serviceshould not yield any critical error.Feel free to contact me in case of any question or doubt.