Skip to content

Hello psalm #21787

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
Merged
merged 16 commits into from
Aug 18, 2020
Merged

Hello psalm #21787

merged 16 commits into from
Aug 18, 2020

Conversation

@kesselb
Copy link
Collaborator

@kesselb kesselb commented Jul 10, 2020

Signed-off-by: Daniel Kesselberg mail@danielkesselberg.de

@kesselb kesselb force-pushed the enh/hello-psalm branch 7 times, most recently from 6696326 to 3f2d081 Compare July 15, 2020 20:22
@kesselb kesselb force-pushed the enh/hello-psalm branch 2 times, most recently from 449ef09 to e39e4c9 Compare August 10, 2020 20:02
@MorrisJobke
Copy link
Member

How nice is this? 😍 👍

@MorrisJobke MorrisJobke self-requested a review August 14, 2020 09:26
@kesselb
Copy link
Collaborator Author

kesselb commented Aug 14, 2020

Yep. PHPStorm is going to add support for Psalm soon: https://blog.jetbrains.com/phpstorm/2020/07/phpstan-and-psalm-support-coming-to-phpstorm/.

I'm not sure yet how to continue with this PR. I fixed some errors or added rules to suppress them (like the global template variables $_ and $l). But there are still more than 1000 warnings.

There are some false positives left. But others warning are legit. To continue and merge this PR we could set a baseline (https://psalm.dev/docs/running_psalm/dealing_with_code_issues/#using-a-baseline-file) with the current errors. With the risk that no one will fix the existing issues ;)

@MorrisJobke MorrisJobke mentioned this pull request Aug 14, 2020
Merged
@MorrisJobke
Copy link
Member

There are some false positives left. But others warning are legit. To continue and merge this PR we could set a baseline (https://psalm.dev/docs/running_psalm/dealing_with_code_issues/#using-a-baseline-file) with the current errors. With the risk that no one will fix the existing issues ;)

I guess that is the only way to go for now.

MorrisJobke
MorrisJobke reviewed Aug 14, 2020
View reviewed changes
@kesselb kesselb mentioned this pull request Aug 14, 2020
Merged
MorrisJobke added a commit that referenced this pull request Aug 17, 2020
@MorrisJobke
Fix "misplaced variables" warning of Psalm in PHPDoc statements
88dd3bb 
Ref #21787

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke MorrisJobke mentioned this pull request Aug 17, 2020
Merged
MorrisJobke added a commit that referenced this pull request Aug 17, 2020
@MorrisJobke
Fix "misplaced variables" warning of Psalm in PHPDoc statements
82b7edb 
Ref #21787

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
MorrisJobke added a commit that referenced this pull request Aug 17, 2020
@MorrisJobke
Fix "misplaced variables" warning of Psalm in PHPDoc statements
d7f66c3 
Ref #21787

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Copy link
Member

There are some false positives left. But others warning are legit. To continue and merge this PR we could set a baseline (https://psalm.dev/docs/running_psalm/dealing_with_code_issues/#using-a-baseline-file) with the current errors. With the risk that no one will fix the existing issues ;)

I guess that is the only way to go for now.

Mind to do this now and then we could merge this :)

@skjnldsv
Copy link
Member

I thought it was a dashboard widget to display Psalm quotes 😁

kesselb added 3 commits August 18, 2020 08:58
@kesselb @MorrisJobke
Hello psalm
7257793 
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb @MorrisJobke
Add stubs for phpseclib
8b3b5ae 
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb @MorrisJobke
Add template functions
fd6cdbc 
for some reason they are not loaded by default.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@MorrisJobke MorrisJobke added this to the Nextcloud 20 milestone Aug 18, 2020
@MorrisJobke MorrisJobke mentioned this pull request Aug 18, 2020
Merged
@MorrisJobke
Copy link
Member

MorrisJobke commented Aug 18, 2020 •
edited
Loading

Using stubs found some inconsistencies between the stub and the actual implementation:

I will open more PRs there.

@MorrisJobke MorrisJobke force-pushed the enh/hello-psalm branch 3 times, most recently from 940db3d to 977aace Compare August 18, 2020 10:46
@MorrisJobke
Add psalm baseline
4efca69 
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Add a check for fixes in the psalm baseline
80056e0 
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Add stubs for gd, intl, IntlChar, ldap, redis_cluster and xsl
b70792a 
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Add patches for stubs
c6b251c 
* JetBrains/phpstorm-stubs#893
* JetBrains/phpstorm-stubs#894
* JetBrains/phpstorm-stubs#895
* JetBrains/phpstorm-stubs#896

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Check only the baseline.xml and exclude the psalm.xml from the file c…
42bb6cd 
…heck

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@nextcloud nextcloud deleted a comment from faily-bot bot Aug 18, 2020
@MorrisJobke
Copy link
Member

Here it is - the final rebase that squashed all the fixups into the proper commits. It now also warns about not checked in fixes (so if we fix something it also states that the baseline should be updated).

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 18, 2020
@MorrisJobke MorrisJobke merged commit 92b6740 into master Aug 18, 2020
@MorrisJobke MorrisJobke deleted the enh/hello-psalm branch August 18, 2020 11:38
kesselb
kesselb commented Aug 18, 2020
View reviewed changes
build/files-checker.php
'ocs',
'package-lock.json',
'package.json',
'psalm.xml',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't need psalm baseline here?

Copy link
Member

Choose a reason for hiding this comment

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

No - because it's in the build/ folder which is deleted already. Only new files in the root need to be checked if the release script should either delete them or keep them. I went for the "delete the psalm.xml in the release" way. :)

kesselb pushed a commit that referenced this pull request Aug 31, 2020
@MorrisJobke @kesselb
Fix "misplaced variables" warning of Psalm in PHPDoc statements
620b7bb 
Ref #21787

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke MorrisJobke mentioned this pull request Sep 16, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@rullzer rullzer Awaiting requested review from rullzer

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

Assignees

No one assigned

Labels

4. to release Ready to be released and/or waiting for tests to finish enhancement technical debt

Projects

None yet

Milestone

Nextcloud 20

Development

Successfully merging this pull request may close these issues.

4 participants

@kesselb @MorrisJobke @skjnldsv