Codeception / module-rest

REST module for Codeception
MIT License
53 stars 27 forks source link

Fixing namespace issue #89

Closed ThomasLandauer closed 1 year ago

ThomasLandauer commented 2 years ago

The line use JsonSerializable; of this file is not being passed on to tests/Support/_generated/AcceptanceTesterActions.php. And with the current phpDoc

@param array|string|JsonSerializable $params

... phpstan is correctly reporting:

Parameter #2 $params of method Tests\Support\AcceptanceTester::sendPatch() expects array|string|Tests\Support_generated\JsonSerializable ...

So the question is: Is there a way to include the needed uses in the generated TesterActions? Or should all occurrences of such "root" classes in the code get prefixed with a \?

Naktibalda commented 1 year ago

The best solution is to move type information to method signature.

public function sendPatch(string $url, array|string|JsonSerializable $params = [], array $files = [])

There was an attempt to generate use statements for docblocks at https://github.com/Codeception/Codeception/pull/6267.

ThomasLandauer commented 1 year ago

So would you say it's better to wait for that other PR? (I certainly can't finish it)

One (small) point against the PR, and in favor of using FQCN in the code: Even if native PHP typehints get added everywhere, the DocBlocks need to stay, cause the docs at https://codeception.com/docs/modules/REST#sendPatch are generated from it, right? And since the use lines aren't shown there, it's more informative to always show the FQCN.

Naktibalda commented 1 year ago

Even if native PHP typehints get added everywhere, the DocBlocks need to stay, cause the docs at https://codeception.com/docs/modules/REST#sendPatch are generated from it, right?

No, I merged PR to generate docs from type declarations a few days ago, because useless docblocks had been removed in many modules: https://github.com/Codeception/codeception.github.com/pull/646

ThomasLandauer commented 1 year ago

OK, so what's the rule for the docs now? If DocBlock is present, take it; if not, take the typehints?

In any case: Showing \Codeception\Util\HttpCode in the docs is certainly better than just HttpCode, isn't it?

So would you say it's better to wait for some sort of automation, or should I start adding FQCN's here and there?

Naktibalda commented 1 year ago

OK, so what's the rule for the docs now? If DocBlock is present, take it; if not, take the typehints?

Yes, that's right.

I removed that @param annotation in https://github.com/Codeception/module-rest/pull/90