Skip to content
This repository has been archived by the owner on Dec 3, 2019. It is now read-only.

Enable php phan #117

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Enable php phan #117

wants to merge 3 commits into from

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Nov 27, 2018

and add the pipeline step in drone ready for phpstan.

Issue #118

A first run of phan reported:

[debug] Running '/usr/bin/php7.1' '-n' '-c' '/tmp/fImIkk' 'vendor-bin/phan/vendor/bin/phan' '--config-file' '.phan/config.php' '--require-config-exists'
appinfo/routes.php:20 PhanUndeclaredVariable Variable $this is undeclared
appinfo/routes.php:23 PhanUndeclaredVariable Variable $this is undeclared
lib/Controller/ChangePasswordController.php:114 PhanUndeclaredClassMethod Call to method __construct from undeclared class \OCA\Encryption\Crypto\Crypt
lib/Controller/ChangePasswordController.php:120 PhanUndeclaredClassMethod Call to method __construct from undeclared class \OCA\Encryption\Util
lib/Controller/ChangePasswordController.php:127 PhanUndeclaredClassMethod Call to method __construct from undeclared class \OCA\Encryption\KeyManager
lib/Controller/ChangePasswordController.php:132 PhanUndeclaredClassMethod Call to method __construct from undeclared class \OCA\Encryption\Session
lib/Controller/ChangePasswordController.php:135 PhanUndeclaredClassMethod Call to method __construct from undeclared class \OCA\Encryption\Recovery
lib/Controller/ChangePasswordController.php:144 PhanUndeclaredClassMethod Call to method isRecoveryKeyEnabled from undeclared class \OCA\Encryption\Recovery
lib/Controller/ChangePasswordController.php:149 PhanUndeclaredClassMethod Call to method checkRecoveryPassword from undeclared class \OCA\Encryption\KeyManager
lib/Controller/ChangePasswordController.php:150 PhanUndeclaredClassMethod Call to method isRecoveryEnabledForUser from undeclared class \OCA\Encryption\Recovery
lib/Controller/UsersController.php:167 PhanTypeMismatchDeclaredParamNullable Doc-block of $userGroups in formatUserForIndex is phpdoc param type array which is not a permitted replacement of the nullable param type ?array declared in the signature ('?T' should be documented as 'T|null' or '?T')
lib/Exception/UserTokenMismatchException.php:26 PhanUndeclaredProperty Reference to undeclared property \OCA\UserManagement\Exception\UserTokenMismatchException->previousException
lib/MetaData.php:127 PhanTypeMismatchProperty Assigning 1|2 to property but \OCA\UserManagement\MetaData->sorting is bool|false
lib/MetaData.php:131 PhanTypeMismatchProperty Assigning 0 to property but \OCA\UserManagement\MetaData->sorting is bool|false
[debug] Restarted process exited 1
Makefile:155: recipe for target 'test-php-phan' failed
make: *** [test-php-phan] Error 1

@@ -164,7 +164,7 @@ public function __construct($appName,

/**
* @param IUser $user
* @param array $userGroups
* @param array|null $userGroups
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes:

lib/Controller/UsersController.php:167 PhanTypeMismatchDeclaredParamNullable Doc-block of $userGroups in formatUserForIndex is phpdoc param type array which is not a permitted 

@@ -23,7 +23,7 @@

class UserTokenMismatchException extends UserTokenException {
public function __construct($message = "", $code = 0, \Exception $previous = null) {
$this->previousException = $this;
$this->previous = $this;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes

lib/Exception/UserTokenMismatchException.php:26 PhanUndeclaredProperty Reference to undeclared property \OCA\UserManagement\Exception\UserTokenMismatchException->previousException

/** @var bool */
protected $sorting = false;
/** @var int */
protected $sorting = self::SORT_NONE;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes:

lib/MetaData.php:127 PhanTypeMismatchProperty Assigning 1|2 to property but \OCA\UserManagement\MetaData->sorting is bool|false
lib/MetaData.php:131 PhanTypeMismatchProperty Assigning 0 to property but \OCA\UserManagement\MetaData->sorting is bool|false

@phil-davis
Copy link
Contributor Author

appinfo/routes.php and lib/Controller/ChangePasswordController.php - I don't really know how they should be fixed, so happy for someone else to suggest...

@phil-davis phil-davis mentioned this pull request Nov 27, 2018
@patrickjahns
Copy link
Contributor

appinfo/routes.php => maybe @DeepDiver1975 @PVince81 have an idea - might need to be refactored. ( If we can't then we can still ignore it )

lib/Controller/ChangePasswordController.php => Is that dependency on encryption app required?
If yes - we need to provide and enable the encryption app - then it will be autoloaded an the classes are available

@codecov
Copy link

codecov bot commented Jan 7, 2019

Codecov Report

Merging #117 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #117   +/-   ##
=========================================
  Coverage     79.11%   79.11%           
  Complexity      214      214           
=========================================
  Files            26       26           
  Lines           905      905           
=========================================
  Hits            716      716           
  Misses          189      189
Impacted Files Coverage Δ Complexity Δ
lib/Controller/UsersController.php 85.97% <ø> (ø) 134 <0> (ø) ⬇️
lib/MetaData.php 90.47% <ø> (ø) 19 <0> (ø) ⬇️
lib/Exception/UserTokenMismatchException.php 100% <100%> (ø) 1 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3cd7c28...9d5f903. Read the comment docs.

@codecov
Copy link

codecov bot commented Jan 7, 2019

Codecov Report

Merging #117 into master will increase coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master     #117     +/-   ##
===========================================
+ Coverage     78.63%   78.73%   +0.1%     
  Complexity      214      214             
===========================================
  Files            26       26             
  Lines           922      917      -5     
===========================================
- Hits            725      722      -3     
+ Misses          197      195      -2
Impacted Files Coverage Δ Complexity Δ
lib/Controller/UsersController.php 85.97% <ø> (-0.09%) 134 <0> (ø)
lib/MetaData.php 90.47% <ø> (ø) 19 <0> (ø) ⬇️
lib/Exception/UserTokenMismatchException.php 100% <100%> (ø) 1 <0> (ø) ⬇️
ajax/togglegroups.php 0% <0%> (ø) 0% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66c9d39...6c8eeb9. Read the comment docs.

@DeepDiver1975
Copy link
Contributor

appinfo/routes.php => maybe @DeepDiver1975 @PVince81 have an idea - might need to be refactored. ( If we can't then we can still ignore it )

these two routes https://github.com/owncloud/user_management/blob/3cd7c282ae55c1752f87d3d36940e41e178d9c50/appinfo/routes.php#L20 need to be refactored and put into a controller .....

@phil-davis
Copy link
Contributor Author

phil-davis commented Jan 7, 2019

@PVince81 @DeepDiver1975 this is a demonstration PR for enabling phan
Please give a priority (might be low) for allocating resources to resolving/refactoring code and then some day put it in a dev sprint... - issue #118

I will leave this here so people can see how to enable phan and get the "error" report.

@patrickjahns patrickjahns removed their request for review December 2, 2019 12:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants