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

Bump phan to 1.3.5 and enable on PHP 7.2 7.3 #35818

Merged
merged 6 commits into from
Aug 26, 2019

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Jul 12, 2019

Issue #35697

Backport #35817

The 4th commit is for code that is in stable10 but not master and gets reported by phan

Note: this has been rebased to the "new" master.

@phil-davis
Copy link
Contributor Author

phil-davis commented Jul 12, 2019

ToDo:

  • sort out extra PhanUndeclaredMethod reported in stable10
apps/encryption/lib/AppInfo/Application.php:143 PhanUndeclaredMethod Call to undeclared method \OCP\IServerContainer::getTimeFactory
apps/files_sharing/lib/Scanner.php:78 PhanUndeclaredMethod Call to undeclared method \OC\Files\Storage\Storage::resolvePath
apps/files_sharing/lib/Scanner.php:93 PhanUndeclaredMethod Call to undeclared method \OC\Files\Storage\Storage::resolvePath
apps/provisioning_api/lib/Groups.php:106 PhanUndeclaredMethod Call to undeclared method \OCP\IGroupManager::getSubAdmin
apps/provisioning_api/lib/Groups.php:181 PhanUndeclaredMethod Call to undeclared method \OCP\IGroupManager::getSubAdmin
lib/private/DB/MySqlSchemaColumnDefinitionListener.php:54 PhanUndeclaredMethod Call to undeclared method \OCP\IDBConnection::getDatabaseVersionString
lib/private/Files/ObjectStore/ObjectStoreStorage.php:341 PhanUndeclaredMethod Call to undeclared method \OCP\Files\Storage::getBucket
lib/private/Files/View.php:384 PhanUndeclaredMethod Call to undeclared method \OC\Files\Storage\Local::readdir
lib/private/Group/MetaData.php:192 PhanUndeclaredMethod Call to undeclared method \OCP\IGroupManager::getSubAdmin
lib/private/Share/Share.php:2037 PhanUndeclaredMethod Call to undeclared method \OCP\Share_Backend::getParents
lib/private/Share/Share.php:2061 PhanUndeclaredMethod Call to undeclared method \OCP\Share_Backend::getParents
settings/Controller/UsersController.php:196 PhanUndeclaredMethod Call to undeclared method \OCP\IGroupManager::getSubAdmin
settings/Controller/UsersController.php:314 PhanUndeclaredMethod Call to undeclared method \OCP\IGroupManager::getSubAdmin
settings/Controller/UsersController.php:419 PhanUndeclaredMethod Call to undeclared method \OCP\IGroupManager::getSubAdmin
settings/Controller/UsersController.php:426 PhanUndeclaredMethod Call to undeclared method \OCP\IGroupManager::getSubAdmin
settings/Controller/UsersController.php:757 PhanUndeclaredMethod Call to undeclared method \OCP\IGroupManager::getSubAdmin
settings/Controller/UsersController.php:810 PhanUndeclaredMethod Call to undeclared method \OCP\IGroupManager::getSubAdmin
settings/Controller/UsersController.php:930 PhanUndeclaredMethod Call to undeclared method \OCP\IGroupManager::getSubAdmin
settings/Controller/UsersController.php:972 PhanUndeclaredMethod Call to undeclared method \OCP\IGroupManager::getSubAdmin
settings/Controller/UsersController.php:1056 PhanUndeclaredMethod Call to undeclared method \OCP\IGroupManager::getSubAdmin
settings/Controller/UsersController.php:1057 PhanUndeclaredMethod Call to undeclared method \OCP\IGroupManager::getSubAdmin
settings/Controller/UsersController.php:1138 PhanUndeclaredMethod Call to undeclared method \OCP\IGroupManager::getSubAdmin

@phil-davis
Copy link
Contributor Author

Note: https://drone.owncloud.com/owncloud/core/19398/112

The latest Phan release (Phan 2) requires PHP 7.1+ to run, but PHP 7.0.33-8+ubuntu16.04.1+deb.sury.org+1 and Phan 1.3.5 are installed.

This is why I bumped to only phan 1.3.5

After we drop PHP 7.0 then we can move to phan 2.*.*

@phil-davis phil-davis marked this pull request as ready for review July 12, 2019 12:19
@codecov
Copy link

codecov bot commented Jul 12, 2019

Codecov Report

Merging #35818 into stable10 will decrease coverage by 0.01%.
The diff coverage is 47.45%.

Impacted file tree graph

@@              Coverage Diff               @@
##             stable10   #35818      +/-   ##
==============================================
- Coverage       65.09%   65.08%   -0.02%     
  Complexity      20239    20239              
==============================================
  Files            1300     1300              
  Lines           77238    77269      +31     
  Branches         1301     1301              
==============================================
+ Hits            50279    50289      +10     
- Misses          26574    26595      +21     
  Partials          385      385
Flag Coverage Δ Complexity Δ
#javascript 53.85% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.26% <47.45%> (-0.02%) 20239 <0> (ø)
Impacted Files Coverage Δ Complexity Δ
lib/public/AppFramework/OCSController.php 95.45% <ø> (ø) 14 <0> (ø) ⬇️
lib/private/legacy/defaults.php 59.25% <ø> (ø) 45 <0> (ø) ⬇️
lib/private/Console/Application.php 0% <ø> (ø) 23 <0> (ø) ⬇️
lib/private/legacy/files.php 0% <ø> (ø) 68 <0> (ø) ⬇️
core/Command/User/ResetPassword.php 70.52% <ø> (ø) 23 <0> (ø) ⬇️
lib/private/User/User.php 88.94% <ø> (ø) 78 <0> (ø) ⬇️
lib/private/Files/Meta/MetaVersionCollection.php 87.5% <ø> (ø) 16 <0> (ø) ⬇️
lib/private/Repair/MoveAvatarIntoSubFolder.php 23.72% <ø> (ø) 18 <0> (ø) ⬇️
apps/dav/lib/Connector/Sabre/TagsPlugin.php 93.54% <ø> (ø) 41 <0> (ø) ⬇️
lib/private/Files/Mount/MountPoint.php 93.5% <ø> (ø) 34 <0> (ø) ⬇️
... and 78 more

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 50df15a...7798c05. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 12, 2019

Codecov Report

Merging #35818 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #35818   +/-   ##
=======================================
  Coverage      54%      54%           
=======================================
  Files          63       63           
  Lines        7403     7403           
  Branches     1307     1307           
=======================================
  Hits         3998     3998           
  Misses       3019     3019           
  Partials      386      386
Flag Coverage Δ
#javascript 54% <ø> (ø) ⬆️

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 8bdd3d9...0397abf. Read the comment docs.

@phil-davis
Copy link
Contributor Author

@patrickjahns @DeepDiver1975 or somebody, please review.

@phil-davis phil-davis force-pushed the stable10-php-phan-20190711 branch 2 times, most recently from 763a7dd to c9950c9 Compare July 30, 2019 04:35
@phil-davis
Copy link
Contributor Author

phil-davis commented Jul 30, 2019

@DeepDiver1975 @patrickjahns if someone could review this, then it could be merged now. That will save having to regenerate it against the "new" master that is coming.

I rebased it this morning and drone passed. It fails codecov by a bit, because it edits "random" files in places that might not have the average 65% code coverage.

@DeepDiver1975 DeepDiver1975 changed the base branch from stable10 to master July 30, 2019 08:54
@phil-davis phil-davis changed the title [stable10] Bump phan to 1.3.5 and enable on PHP 7.2 7.3 Bump phan to 1.3.5 and enable on PHP 7.2 7.3 Aug 1, 2019
@phil-davis
Copy link
Contributor Author

@patrickjahns @DeepDiver1975 or anybody.
This has been rebased to "new" master.
drone CI passes. It fails codecov because the small additions for phan on average are in classes that do not have the project-average code coverage.
IMO it would be nice to get this running again.
Please review.

@phil-davis phil-davis force-pushed the stable10-php-phan-20190711 branch 3 times, most recently from 53cdeaa to 261dd5c Compare August 8, 2019 01:45
@phil-davis
Copy link
Contributor Author

In the drone update we have preserved the "random" drone errors:
https://drone.owncloud.com/owncloud/core/20018/26/9

cURL error 6: Could not resolve host: scality 

@individual-it
Copy link
Member

@phil-davis please resolve conflict

@phil-davis
Copy link
Contributor Author

Conflicts resolved.
@patrickjahns @DeepDiver1975 or somebody - please review.

@phil-davis
Copy link
Contributor Author

I rebased again to make sure that CI is all good with today's current master.
@PVince81 @DeepDiver1975 @patrickjahns or anybody - review please.

@PVince81
Copy link
Contributor

I wonder why our code needs that many string literals for variables and whether there is something we can do to avoid those in the future, like better variable / type declarations ? Or is it the way our classes are constructed ?

Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍 if CI passes

@phil-davis
Copy link
Contributor Author

I wonder why our code needs that many string literals for variables and whether there is something we can do to avoid those in the future, like better variable / type declarations ? Or is it the way our classes are constructed ?

This gets phan running and passing again. Basically I did:

  • if there was already a "comment" typehint (which phan could not understand) or other "obvious" evidence of what type the object is, then I put in the "magic" @phan-var string literal that allows phan to understand the type hint.
  • if the type of the object was not "obviously" guaranteed in some way, then I did @phan-suppress-next-line PhanUndeclaredMethod to suppress the report from phan

"somebody" "someday" should go through these in detail, understand why phan could not deduce the object type, understand if there is a real code problem that could actually go boom in some way, and fix/refactor our code.

@phil-davis
Copy link
Contributor Author

Stupid drone https://drone.owncloud.com/owncloud/core/20351/83/4

behat-chrome-webUIWebdavLockProtection: Error: No such image: docker.io/library/mariadb:10.2

@patrickjahns
Copy link
Contributor

@phil-davis
Instead of needing to use @phan-var please try to use @property for class properties. This should deliver a more clean annotation

https://github.com/phan/phan/wiki/Annotating-Your-Source-Code#property

@phil-davis phil-davis merged commit b9ff4c9 into master Aug 26, 2019
@delete-merged-branch delete-merged-branch bot deleted the stable10-php-phan-20190711 branch August 26, 2019 13:11
@davitol davitol mentioned this pull request Sep 3, 2019
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants