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.

Sign up for GitHub

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

Jump to bottom

Update to psalm 5 #37390

Merged
merged 1 commit into from
May 2, 2023
Merged

Update to psalm 5 #37390

merged 1 commit into from
May 2, 2023

Conversation

provokateurin
Copy link
Member

@provokateurin provokateurin commented Mar 24, 2023 •
edited
Loading

Summary

Checklist

@provokateurin provokateurin force-pushed the update/psalm-5 branch 2 times, most recently from 0da5bdd to f752b68 Compare March 27, 2023 10:46
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 27, 2023
View reviewed changes
apps/files_sharing/lib/SharedStorage.php Outdated
*/
public function setAvailability($available) {
public function setAvailability($isAvailable) {

Check notice

Code scanning / Psalm

MissingReturnType

Method OCA\Files_Sharing\SharedStorage::setAvailability does not have a return type, expecting void
apps/user_ldap/lib/Configuration.php Fixed Show fixed Hide fixed
lib/private/AllConfig.php Fixed Show fixed Hide fixed
apps/files_trashbin/lib/Trashbin.php Fixed Show fixed Hide fixed
apps/user_ldap/lib/Access.php Outdated
@@ -354,7 +354,7 @@
return @$this->invokeLDAPMethod('exopPasswd', $userDN, '', $password) ||
@$this->invokeLDAPMethod('modReplace', $userDN, $password);
} catch (ConstraintViolationException $e) {
throw new HintException('Password change rejected.', \OC::$server->getL10N('user_ldap')->t('Password change rejected. Hint: ') . $e->getMessage(), (int)$e->getCode());
throw new HintException('Password change rejected.', \OC::$server->getL10N('user_ldap')->t('Password change rejected. Hint: ') . $e->getMessage(), $e->getCode());

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OC\Server::getL10N has been marked as deprecated
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 28, 2023
View reviewed changes
lib/private/Files/Cache/HomeCache.php Fixed Show fixed Hide fixed
apps/files_trashbin/lib/Trashbin.php Fixed Show fixed Hide fixed
apps/files_trashbin/lib/Trashbin.php Fixed Show fixed Hide fixed
apps/files_trashbin/lib/Trashbin.php Fixed Show fixed Hide fixed
apps/files_trashbin/lib/Trashbin.php Fixed Show fixed Hide fixed
apps/federatedfilesharing/lib/FederatedShareProvider.php Fixed Show fixed Hide fixed
apps/federatedfilesharing/lib/FederatedShareProvider.php Fixed Show fixed Hide fixed
apps/files_external/lib/MountConfig.php Fixed Show fixed Hide fixed
apps/user_ldap/lib/Configuration.php Fixed Show fixed Hide fixed
@provokateurin provokateurin force-pushed the update/psalm-5 branch 4 times, most recently from 9bc45e9 to c7ec317 Compare March 30, 2023 05:55
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 30, 2023
View reviewed changes
apps/updatenotification/lib/Controller/AdminController.php Outdated
@@ -99,7 +99,7 @@

// Create a new job and store the creation date
$this->jobList->add(ResetTokenBackgroundJob::class);
$this->config->setAppValue('core', 'updater.secret.created', $this->timeFactory->getTime());
$this->config->setAppValue('core', 'updater.secret.created', (string)$this->timeFactory->getTime());

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OCP\AppFramework\Utility\ITimeFactory::getTime has been marked as deprecated
@provokateurin provokateurin mentioned this pull request Mar 30, 2023
Closed
8 tasks
@provokateurin provokateurin force-pushed the update/psalm-5 branch 3 times, most recently from d634595 to 0be733b Compare March 30, 2023 15:23
@provokateurin provokateurin marked this pull request as ready for review March 30, 2023 15:28
This was referenced Mar 31, 2023
Merged
Closed
Merged
Merged
Closed
Merged
@provokateurin provokateurin marked this pull request as draft April 3, 2023 06:09
@provokateurin provokateurin mentioned this pull request Apr 3, 2023
Merged
4 tasks
This was referenced Apr 14, 2023
Closed
Closed
Merged
Closed
@provokateurin provokateurin marked this pull request as ready for review April 25, 2023 13:49
@provokateurin
Copy link
Member Author

I already tried fixing a lot of the new found issues, but it's getting out of hand. What I did now is just ignore all the new errors so that we can finally have psalm 5.

@provokateurin
Copy link
Member Author

No idea what the failing security analysis means, the output by psalm seems to be incorrect.

@provokateurin provokateurin mentioned this pull request Apr 25, 2023
Open
@kesselb
Copy link
Contributor

kesselb commented Apr 25, 2023

image

Haha ;) An error on line 0 is not allowed 🤣

kesselb
kesselb approved these changes Apr 25, 2023
View reviewed changes
Copy link
Contributor

@kesselb kesselb left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@provokateurin
Copy link
Member Author

Yeah I filed an issue at vimeo/psalm#9712, I hope it gets fixed soon.

@provokateurin
Use psalm 5
9fddbea 
Signed-off-by: jld3103 <[email protected]>
@provokateurin provokateurin enabled auto-merge May 2, 2023 11:43
@provokateurin provokateurin merged commit 436a088 into master May 2, 2023
@provokateurin provokateurin deleted the update/psalm-5 branch May 2, 2023 12:55
@ChristophWurst ChristophWurst mentioned this pull request Nov 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@come-nc come-nc come-nc approved these changes

@kesselb kesselb kesselb approved these changes

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@miaulalala miaulalala Awaiting requested review from miaulalala

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@provokateurin @ChristophWurst @kesselb @come-nc