Type the \OCP\IUserManager::callForSeenUsers closure with Psalm

[ChristophWurst]

Companion of #23743, didn't see there is two methods that take a closure.

[ChristophWurst]

/backport to stable20

[rullzer]

Those double doc blocks with identical data always seem weird to me

[ChristophWurst]

Those double doc blocks with identical data always seem weird to me

you mean the two lines for one parameter? yeah. but that is a limitation of what the original phpdoc can express and what psalm can do. one day I'm sure the phpdoc will be updated. today is not that day.

like we could also only use the @param with psalm syntax but that breaks some tools and IDEs. Hence the secondary psalm-specific line

[MorrisJobke]

you mean the two lines for one parameter? yeah.

We can also just drop it, because it adds nothing over the type hint itself.

[ChristophWurst]

We can also just drop it, because it adds nothing over the type hint itself.

Here it doesn't. But many times we still have a short parameter description and then we shouldn't drop it because that influences how the API docs are rendered.

[rullzer]

I was more meaing the double stuff on the interface and the implementation... this always struck me as error prone to get out of sync.

[ChristophWurst]

Fair enough. It's getting better with php8 and union types but without native support for generics there is still a lot you can't express with php alone.

[ChristophWurst]

Weird that Psalm is failing now.

[kesselb]

Weird that Psalm is failing now.

Why weird? ;) Psalm warns you about the callable definition.

server/lib/private/User/Manager.php

Lines 563 to 566 in ffa8be4

	$return = $callback($user);
	if ($return === false) {
	return;
	}

Because void is defined as return type for the callable. Awesome that Psalm is also able to detect this case.

[ChristophWurst]

Psalm warns you about the callable definition

where do you see that? I only see (unrelated?) changes of the baseline

[kesselb]

Here you go

[kesselb]

It seems (for nextcloud/server) that false is not returned from the callable. Don't know if an app is using this mechanism to interrupt the process.

[faily-bot]

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 34703: failure

mysql8.0-php7.4

Show full log

There were 2 warnings:

1) Test\Files\ViewTest::testRenameFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

2) Test\Files\ViewTest::testCopyFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

--

There was 1 failure:

1) Test\Share20\DefaultShareProviderTest::testUpdateLinkRemovePassword
Failed asserting that 0 matches expected 1.

/drone/src/tests/lib/Share20/DefaultShareProviderTest.php:1901

mysql5.6-php7.3

Show full log

There were 2 warnings:

1) Test\Files\ViewTest::testRenameFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

2) Test\Files\ViewTest::testCopyFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

--

There was 1 failure:

1) Test\Share20\DefaultShareProviderTest::testDeleteUserGroup with data set #3 ('a', 'b', 'c', 'd', false, false)
Failed asserting that actual size 0 matches expected size 1.

/drone/src/tests/lib/Share20/DefaultShareProviderTest.php:2324