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 documentation generation when using binded interfaces instead of model classes as typed arguments #494

Merged
merged 3 commits into from
Jul 9, 2022

Conversation

toyi
Copy link
Contributor

@toyi toyi commented Jul 9, 2022

Hi,

The version 3.33.1 introduces a breaking change when you are using a type hinted argument that is an interface binded to a model via the service container.

Since an interface is not instantiable, you get this error when generating the documention:

 Error 

  Cannot instantiate interface App\Interfaces\YourModelInterface.php

  at vendor/knuckleswtf/scribe/camel/Extraction/ExtractedEndpointData.php:262
    258▕             // The argument does not have a type-hint
    259▕             return false;
    260▕         } else {
    261▕             $argumentClassName = $argumentType->getName();
  ➜ 262▕             $argumentInstance = new $argumentClassName;
    263▕             return ($argumentInstance instanceof Model);
    264▕         }
    265▕     }
    266▕ }

      +21 vendor frames 
  22  artisan:37
      Illuminate\Foundation\Console\Kernel::handle()

To resolve this issue, we need to use the service container to resolve the instance instead of just new.

However, there is a caveat, because when you resolve a form request class using the service container, it also execute the authorize method of the ValidatesWhenResolvedTrait trait.

That was the first error I found but alongs the way of fixing it, I found that type hinted arguments with builtin types (e.g. string) also returns a similar error:

 Error 

  Class "string" not found

  at vendor/knuckleswtf/scribe/camel/Extraction/ExtractedEndpointData.php:262
    258▕             // The argument does not have a type-hint
    259▕             return false;
    260▕         } else {
    261▕             $argumentClassName = $argumentType->getName();
  ➜ 262▕             $argumentInstance = new $argumentClassName;
    263▕             return ($argumentInstance instanceof Model);
    264▕         }
    265▕     }
    266▕ }

      +21 vendor frames 
  22  artisan:37
      Illuminate\Foundation\Console\Kernel::handle()

This PR fixes these problems by using the app() helper to resolve any interfaces that can be used as controller method arguments, new for classes (to avoid any side effects of ValidatesWhenResolvedTrait).

Althrough this PR also fixes the error for builtin types, it was not its first goal. For now, theses arguments are treated as "not type hinted".

Of course, any improvements are welcome :)

Have a nice day.

@shalvah
Copy link
Contributor

shalvah commented Jul 9, 2022

Thanks for the catch and the fix!

@shalvah shalvah merged commit c105ef2 into knuckleswtf:master Jul 9, 2022
@toyi
Copy link
Contributor Author

toyi commented Jul 9, 2022

@shalvah My pleasure :) However I just realized I did an awful english mistake, everywhere I wrote "binded", it should be "bound". Can't make a fix for this typo right now but I'll do it tomorrow if you didn't already.

@toyi toyi mentioned this pull request Jul 10, 2022
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.

2 participants