Skip to content

Fixing namespace issue - #89

Closed
ThomasLandauer wants to merge 1 commit into
Codeception:masterfrom
ThomasLandauer:patch-2
Closed

Fixing namespace issue#89
ThomasLandauer wants to merge 1 commit into
Codeception:masterfrom
ThomasLandauer:patch-2

Conversation

@ThomasLandauer

Copy link
Copy Markdown
Member

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 \?

The line `use JsonSerializable;` of this file is not being passed on to `tests/Support/_generated/AcceptanceTesterActions.php`. And with the current phpDoc
```php
@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 `use`s in the generated `TesterAction`s` Or should all occurrences of such "root" classes in the code get prefixed with a `\`?
@Arhell
Arhell requested a review from Naktibalda September 9, 2022 07:49
@Naktibalda

Copy link
Copy Markdown
Member

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 Codeception/Codeception#6267.

@ThomasLandauer

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
Member

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: Codeception/codeception.github.com#646

@ThomasLandauer

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
Member

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 #90

@Naktibalda Naktibalda closed this Sep 11, 2022
@ThomasLandauer
ThomasLandauer deleted the patch-2 branch September 11, 2022 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants