Skip to content
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.

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

fix(API): declared array shape was not correct #1169

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Jun 26, 2024

fixes #1157

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz blizzz added bug Something isn't working 3. to review Waiting for reviews labels Jun 26, 2024
@blizzz blizzz requested review from juliusknorr, elzody and enjeck June 26, 2024 08:29
@juliusknorr juliusknorr merged commit 989a9eb into main Jun 26, 2024
58 checks passed
@juliusknorr juliusknorr deleted the fix/1157/fix-declared-parameter-shape branch June 26, 2024 10:23
@blizzz
Copy link
Member Author

blizzz commented Jun 26, 2024

/backport to stable0.7

@backportbot backportbot bot added the backport-request Pending backport by the backport-bot label Jun 26, 2024
@backportbot backportbot bot removed the backport-request Pending backport by the backport-bot label Jun 26, 2024
@provokateurin
Copy link
Member

This fix is not correct, it now requires a list with and two elements of int and mixed https://psalm.dev/r/74104d149c. You can also see that the generated spec is totally wrong and invalid. It should have been array<int, mixed> to achieve what you want.

@blizzz
Copy link
Member Author

blizzz commented Jul 15, 2024

This fix is not correct, it now requires a list with and two elements of int and mixed https://psalm.dev/r/74104d149c. You can also see that the generated spec is totally wrong and invalid. It should have been array<int, mixed> to achieve what you want.

Thank you for your direct reply. You are right of course.

The odd thing is, that OpenAPI will not build with the correct pairs of brackets:

PHP Fatal error:  Uncaught Error: api1#createRowInTable: Unable to resolve OpenAPI type:
\PHPStan\PhpDocParser\Ast\Type\GenericTypeNode::__set_state(array(
   'type' => 
  \PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode::__set_state(array(
     'name' => 'array',
     'attributes' => 
    array (
    ),
  )),
   'genericTypes' => 
  array (
    0 => 
    \PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode::__set_state(array(
       'name' => 'int',
       'attributes' => 
      array (
      ),
    )),
    1 => 
    \PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode::__set_state(array(
       'name' => 'mixed',
       'attributes' => 
      array (
      ),
    )),
  ),
   'variances' => 
  array (
    0 => 'invariant',
    1 => 'invariant',
  ),
   'attributes' => 
  array (
  ),
))

a plain array<mixed> will do, apart of a general error (✘ Globbing has been deprecated in favor of redocly.yaml’s apis keys.) that also appears without a change. The IDE complains, however, and suggests to replace with array, which will openapi enrage, because it might be empty 🙄

@provokateurin
Copy link
Member

array<mixed> shouldn't be used either. I will look into the error and then hopefully provide a fix here as well.

@blizzz
Copy link
Member Author

blizzz commented Jul 15, 2024

array<mixed> shouldn't be used either. I will look into the error and then hopefully provide a fix here as well.

No, that was a test just for scientific reasons.

@provokateurin
Copy link
Member

It doesn't work because this is impossible in JSON. There is no way to index an object by non-strings, so it has to be a list or the keys need to be changed to strings. Since you cast to int later anyway, changing to string seems fine?
I'll add a better warning to openapi-extractor for these cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenAPI annotation for createRowInTable is likely wrong
3 participants