Implement phan stan #75
Conversation
phil-davis
force-pushed
the
implement-phan-stan
branch
from
c6937ec
to
b9e44f6
Compare
| use OCA\Testing\ServerFiles; | ||
| use OCA\Testing\SysInfo; | ||
| use OCP\API; | ||
| use OCA\Testing\Opcache; | ||
| use OCA\Testing\Logfile; | ||
| use OCA\Testing\DavSlowdown; |
There was a problem hiding this comment.
Note: this ended up just being sorting of the use statements. Makes it easier for humans to scan the list. No difference for the computer.
| @@ -44,7 +44,7 @@ public function __construct(IConfig $config, IRequest $request) { | |||
|
|
|||
| /** | |||
| * @param array $parameters | |||
| * @return \OC_OCS_Result | |||
There was a problem hiding this comment.
OC_OCS_Result is deprecated. phan reported this in lots of places. Easily fixed.
| @@ -76,7 +76,7 @@ public function initialize(Server $server) { | |||
| * @param RequestInterface $request request object | |||
| * @param ResponseInterface $response response object | |||
| * @throws \Sabre\DAV\Exception\Forbidden | |||
| * @return boolean | |||
| * @return void | |||
There was a problem hiding this comment.
Error in PHPdoc reported by phan
| @@ -79,7 +79,7 @@ protected function getType($parameters) { | |||
|
|
|||
| /** | |||
| * @param array $parameters | |||
| * @return int | |||
| * @return string | |||
There was a problem hiding this comment.
Error in PHPdoc reported by phan
| } | ||
| $type = $this->getType($parameters); | ||
|
|
||
| $lockingProvider = $this->getLockingProvider(); | ||
|
|
||
| try { | ||
| $lockingProvider->acquireLock($path, $type); | ||
| $this->config->setAppValue('testing', 'locking_' . $path, $type); | ||
| return new \OC_OCS_Result(null, 100); | ||
| $this->config->setAppValue('testing', 'locking_' . $path, (string) $type); |
There was a problem hiding this comment.
$type is int but setAppValue() expects string. phan reported this. We can explicitly cast to string to make it clear that we know we are doing this.
| } | ||
| $type = $this->getType($parameters); | ||
|
|
||
| $lockingProvider = $this->getLockingProvider(); | ||
|
|
||
| try { | ||
| $lockingProvider->changeLock($path, $type); | ||
| $this->config->setAppValue('testing', 'locking_' . $path, $type); | ||
| return new \OC_OCS_Result(null, 100); | ||
| $this->config->setAppValue('testing', 'locking_' . $path, (string) $type); |
There was a problem hiding this comment.
$type is int but setAppValue() expects string. phan reported this. We can explicitly cast to string to make it clear that we know we are doing this.
phil-davis
force-pushed
the
implement-phan-stan
branch
from
b9e44f6
to
6874232
Compare
| when: | ||
| matrix: | ||
| TEST_SUITE: phpunit | ||
| PHAN: true |
There was a problem hiding this comment.
@patrickjahns we can only run phan if PHP_VERSION is 7.*
I put a boolean PHAN in here, and only added it to the matrix for 1 phpunit job of PHP 7.0 7.1 and 7.2. Without that, it will try and run for PHP 5.6 also, which will die.
Alternatives would be:
- put a check in the commands section, and only do
make test-php-phanwhen the PHP version is >=7 - put a check in
Makefileto only really do thephanfor PHP>=7, and emit an information message for PHP<7, so that it will "work" with PHP5.6 but be a NOOP - or?
There was a problem hiding this comment.
@patrickjahns ping ?
There are some other app repos that have good enough code that phan will pass without much (or any) effort.
Before adding drone phan to those, I want to make sure there is a "standard" for doing this in drone.
phil-davis
force-pushed
the
implement-phan-stan
branch
from
088a299
to
57a94a1
Compare
|
@patrickjahns any review comments? merge? |
phil-davis
force-pushed
the
implement-phan-stan
branch
from
57a94a1
to
ac08a3a
Compare
phil-davis
force-pushed
the
implement-phan-stan
branch
from
ac08a3a
to
d321d62
Compare
Description
phanandphpstaninMakefileand drone pipeline stepsphanentries in the drone matrixphaneffectively does the lint/syntax job for PHP 7.*)phan, including moving thelockingfolder code underlibThis currently includes the pending commits from PR #74 which also awaits review and merge.This PR has been rebased after merge of PR #74
Checklist: