Skip to content

Hello psalm - #21787

Merged
MorrisJobke merged 16 commits into
masterfrom
enh/hello-psalm
Aug 18, 2020
Merged

Hello psalm#21787
MorrisJobke merged 16 commits into
masterfrom
enh/hello-psalm

Conversation

@kesselb

@kesselbkesselb commented Jul 10, 2020

Copy link
Copy Markdown
Contributor

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

@kesselb
kesselbforce-pushed the enh/hello-psalm branch 7 times, most recently from 6696326 to 3f2d081CompareJuly 15, 2020 20:22
@kesselb
kesselbforce-pushed the enh/hello-psalm branch 2 times, most recently from 449ef09 to e39e4c9CompareAugust 10, 2020 20:02
@MorrisJobke

MorrisJobke commented Aug 14, 2020

Copy link
Copy Markdown
Member

How nice is this? 😍 👍

@MorrisJobke
MorrisJobke self-requested a review August 14, 2020 09:26
@kesselb

kesselb commented Aug 14, 2020

Copy link
Copy Markdown
ContributorAuthor

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 ;)

@MorrisJobkeMorrisJobke mentioned this pull request Aug 14, 2020
Merged
@MorrisJobke

MorrisJobke commented Aug 14, 2020

Copy link
Copy Markdown
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
Comment threadpsalm.xml
@kesselbkesselb 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>
@MorrisJobkeMorrisJobke 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

MorrisJobke commented Aug 17, 2020

Copy link
Copy Markdown
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

skjnldsv commented Aug 18, 2020

Copy link
Copy Markdown
Member

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

@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>
@MorrisJobkeMorrisJobke added 3. to review Waiting for reviews technical debt 🧱 🤔🚀 labels Aug 18, 2020
@MorrisJobkeMorrisJobke added this to the Nextcloud 20 milestone Aug 18, 2020
@MorrisJobkeMorrisJobke mentioned this pull request Aug 18, 2020
Merged
@MorrisJobke

MorrisJobke commented Aug 18, 2020•
edited
Loading

Copy link
Copy Markdown
Member

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

I will open more PRs there.

@MorrisJobke
MorrisJobkeforce-pushed the enh/hello-psalm branch 3 times, most recently from 940db3d to 977aaceCompareAugust 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>
@nextcloudnextcloud deleted a comment from faily-botBotAug 18, 2020
@MorrisJobke

MorrisJobke commented Aug 18, 2020

Copy link
Copy Markdown
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).

@MorrisJobkeMorrisJobke 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 masterAug 18, 2020
@MorrisJobke
MorrisJobke deleted the enh/hello-psalm branch August 18, 2020 11:38
kesselb
kesselb commented Aug 18, 2020
View reviewed changes
Comment threadbuild/files-checker.php
'ocs',
'package-lock.json',
'package.json',
'psalm.xml',

@kesselbkesselbAug 18, 2020

Copy link
Copy Markdown
ContributorAuthor

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?

@MorrisJobkeMorrisJobkeAug 18, 2020

Copy link
Copy Markdown
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>
@MorrisJobkeMorrisJobke mentioned this pull request Sep 16, 2020
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MorrisJobkeMorrisJobkeMorrisJobke approved these changes
@skjnldsvskjnldsvskjnldsv approved these changes
@ChristophWurstChristophWurstAwaiting requested review from ChristophWurst
@nickvergessennickvergessenAwaiting requested review from nickvergessen
@rullzerrullzerAwaiting requested review from rullzer
@juliusknorrjuliusknorrAwaiting requested review from juliusknorr

Assignees

No one assigned

Labels

4. to releaseReady to be released and/or waiting for tests to finishenhancementtechnical debt🧱 🤔🚀

Projects

None yet

Milestone

Nextcloud 20

Development

Successfully merging this pull request may close these issues.

3 participants

@kesselb@MorrisJobke@skjnldsv