code-lts / doctum

A php API documentation generator, fork of Sami
https://doctum.long-term.support/
MIT License
300 stars 32 forks source link

Intersection type support #59

Closed lukinovec closed 1 year ago

lukinovec commented 1 year ago

Hey! We're trying to run Doctum in the https://github.com/archtechx/tenancy repository (using php doctum.phar update doctum.php --output-format=github --force --ignore-parse-errors --no-ansi --no-progress -v), but the run keeps failing until we delete intersection types from our code. Could this be because Doctum\Parser\NodeVisitor's typeToString() method doesn't handle intersection types? I think those could be handled similarly to union types – using '&' instead of '|'.

Our config file:

use Doctum\Doctum;
use Symfony\Component\Finder\Finder;

$iterator = Finder::create()
    ->files()
    ->name('*.php')
    ->in('src/');

return new Doctum($iterator, [
    'title' => 'Tenancy for Laravel API Documentation',
    'base_url' => 'https://api.tenancyforlaravel.com/',
    'favicon' => 'https://tenancyforlaravel.com/favicon.ico',
]);

The stack trace:

Fatal error: Uncaught Error: Call to undefined method PhpParser\Node\IntersectionType::__toString() in phar:///Users/lukas/Projects/tenancy/doctum.phar/src/Parser/NodeVisitor.php:196
Stack trace:
#0 phar:///Users/lukas/Projects/tenancy/doctum.phar/src/Parser/NodeVisitor.php(329): Doctum\Parser\NodeVisitor->typeToString(Object(PhpParser\Node\IntersectionType))
#1 phar:///Users/lukas/Projects/tenancy/doctum.phar/src/Parser/NodeVisitor.php(73): Doctum\Parser\NodeVisitor->addMethod(Object(PhpParser\Node\Stmt\ClassMethod))
#2 phar:///Users/lukas/Projects/tenancy/doctum.phar/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(200): Doctum\Parser\NodeVisitor->enterNode(Object(PhpParser\Node\Stmt\ClassMethod))
#3 phar:///Users/lukas/Projects/tenancy/doctum.phar/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(114): PhpParser\NodeTraverser->traverseArray(Array)
#4 phar:///Users/lukas/Projects/tenancy/doctum.phar/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(223): PhpParser\NodeTraverser->traverseNode(Object(PhpParser\Node\Stmt\Class_))
#5 phar:///Users/lukas/Projects/tenancy/doctum.phar/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(114): PhpParser\NodeTraverser->traverseArray(Array)
#6 phar:///Users/lukas/Projects/tenancy/doctum.phar/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(223): PhpParser\NodeTraverser->traverseNode(Object(PhpParser\Node\Stmt\Namespace_))
#7 phar:///Users/lukas/Projects/tenancy/doctum.phar/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(91): PhpParser\NodeTraverser->traverseArray(Array)
#8 phar:///Users/lukas/Projects/tenancy/doctum.phar/src/Parser/CodeParser.php(48): PhpParser\NodeTraverser->traverse(Array)
#9 phar:///Users/lukas/Projects/tenancy/doctum.phar/src/Parser/Parser.php(66): Doctum\Parser\CodeParser->parse('<?php\n\ndeclare(...')
#10 phar:///Users/lukas/Projects/tenancy/doctum.phar/src/Project.php(725): Doctum\Parser\Parser->parse(Object(Doctum\Project), Array)
#11 phar:///Users/lukas/Projects/tenancy/doctum.phar/src/Project.php(137): Doctum\Project->parseVersion(Object(Doctum\Version\Version), NULL, Array, false)
#12 phar:///Users/lukas/Projects/tenancy/doctum.phar/src/Console/Command/Command.php(193): Doctum\Project->update(Array, false)
#13 phar:///Users/lukas/Projects/tenancy/doctum.phar/src/Console/Command/UpdateCommand.php(58): Doctum\Console\Command\Command->update(Object(Doctum\Project))
#14 phar:///Users/lukas/Projects/tenancy/doctum.phar/vendor/symfony/console/Command/Command.php(298): Doctum\Console\Command\UpdateCommand->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#15 phar:///Users/lukas/Projects/tenancy/doctum.phar/vendor/symfony/console/Application.php(1040): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#16 phar:///Users/lukas/Projects/tenancy/doctum.phar/vendor/symfony/console/Application.php(301): Symfony\Component\Console\Application->doRunCommand(Object(Doctum\Console\Command\UpdateCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#17 phar:///Users/lukas/Projects/tenancy/doctum.phar/vendor/symfony/console/Application.php(171): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#18 phar:///Users/lukas/Projects/tenancy/doctum.phar/bin/doctum-binary.php(26): Symfony\Component\Console\Application->run()
#19 /Users/lukas/Projects/tenancy/doctum.phar(16): include('phar:///Users/l...')
#20 {main}
  thrown in phar:///Users/lukas/Projects/tenancy/doctum.phar/src/Parser/NodeVisitor.php on line 196
williamdes commented 1 year ago

Hi @lukinovec Thank you for reporting this ! Can you give me one example file I can copy into my tests ?

stancl commented 1 year ago

Hi @williamdes, something like this should be able to reproduce the issue:

class Foo {}
interface Bar {}

class Baz
{
    public function __construct(
        public Foo&Bar $baz,
    ) {}
}

Basically just a class property/method parameter that uses intersection types.

williamdes commented 1 year ago

Nice, is there other more complicated examples I should test ?

stancl commented 1 year ago

Might be worth adding:

separately in case the parser handles them differently.

So something like:

class FooAndBar extends Foo implements Bar {}

class Baz
{
    public Foo&Bar $fooAndBar;

    public function baz(Foo&Bar $baz)
    {
        return;
    }

    public function returnFooAndBar(): Foo&Bar
    {
        return new FooAndBar;
    }
}
williamdes commented 1 year ago

I pushed 0e7fb35c055eb15d41cd290dead0d57c55290e40 on QA 5.5

stancl commented 1 year ago

You can probably get rid of tests/phar/data/src/Intersection.php since it doesn't really test anything more than what tests/phar/data/src/RoundaboutIntersection.php tests. And then move the

class Foo {}
interface Bar {}

to tests/phar/data/src/RoundaboutIntersection.php. That will put it all in one file without re-defining Baz or using dependencies from other files.

williamdes commented 1 year ago

Well, one of the two has a namespace But thanks for the hint the goal is to have as much test files as possible so most of the code is hit

williamdes commented 1 year ago

The 5.5.3-dev and 5.5-dev phars are out for testing, please let me know if they solve the issue for you NB: The intersection type is not displayed the right way on html files, I am investigating

See: https://github.com/code-lts/doctum#installation

stancl commented 1 year ago

5.5.3-dev ran without failing now 👍🏻

As for rendering the types, here's one class we have:

class DatabaseConfig
{
    /** The tenant whose database we're dealing with. */
    public Tenant&Model $tenant;

    /** Database username generator (can be set by the developer.) */
    public static Closure|null $usernameGenerator = null;

    /** Database password generator (can be set by the developer.) */
    public static Closure|null $passwordGenerator = null;

    /** Database name generator (can be set by the developer.) */
    public static Closure|null $databaseNameGenerator = null;

   // ...
}

And it renders like this:

CleanShot 2023-04-14 at 4 19 39@2x

The type for $tenant is empty. For the other properties, it shows static. I'm not sure if it should also show the type (Closure|null or ?Closure).

stancl commented 1 year ago

Also I managed to set up the doctum dependency via composer (similar setup as https://github.com/laravel/laravel.com/tree/master/build/doctum), so if you need me to test any changes I can do so without you having to publish phars 👍🏻

williamdes commented 1 year ago

Also I managed to set up the doctum dependency via composer (similar setup as https://github.com/laravel/laravel.com/tree/master/build/doctum), so if you need me to test any changes I can do so without you having to publish phars 👍🏻

very cool, in fact I will always publish phars. it costs nothing for me :) Just a bunch of Yubikey clicks :p

Do you want to make a PR on this project ?

stancl commented 1 year ago

What PR do you have in mind? Is there anything left to do?

williamdes commented 1 year ago

What PR do you have in mind? Is there anything left to do?

I have in mind that the UI of an intersection type is wrong, probably also the one for union types. so that should be fixed and also the UI things you reported But the crashing code part seems to be finished

Any PR for this should use v5.5.x as a base branch

stancl commented 1 year ago

I see. I have some stuff to do on our open-source projects at the moment, but might take a look at this sometime in the future.

GuySartorelli commented 1 year ago

@williamdes the fix for the crash is probably worth tagging as its own patch imo - being able to build the docs at all is a huge improvement and will unblock work I'm currently doing to update our docs site.

williamdes commented 1 year ago

@williamdes the fix for the crash is probably worth tagging as its own patch imo - being able to build the docs at all is a huge improvement and will unblock work I'm currently doing to update our docs site.

Indeed, I will do this shortly Anything else that would need fixing?

GuySartorelli commented 1 year ago

I haven't found anything else that needs fixing.

williamdes commented 1 year ago

I haven't found anything else that needs fixing.

Okay, update on the situation:

williamdes commented 1 year ago

More changes where pushed, I think union types will have some fixes and we will be ready image

williamdes commented 1 year ago

The phar is updated with my latest changes, please try them and let me know

GuySartorelli commented 1 year ago

Hi, Did this get a stable release tag yet? It seems ready to release IMO.

williamdes commented 1 year ago

Hi, Did this get a stable release tag yet? It seems ready to release IMO.

Hi, I will make a release for it Sorry about that, I am a bit slow on my todo list

GuySartorelli commented 1 year ago

No worries, and thanks!

GuySartorelli commented 1 year ago

Sorry but do you have an estimate for when this will be done? It's a blocker for PHP upgrades for one of our projects. We'll use a fork if you think it'll be a while, but we'd prefer not to if you think it will be tagged soon.

williamdes commented 1 year ago

Sorry but do you have an estimate for when this will be done? It's a blocker for PHP upgrades for one of our projects. We'll use a fork if you think it'll be a while, but we'd prefer not to if you think it will be tagged soon.

This week ;) I will be at https://eurorust.eu/ so more time in my hands to manage my OSS projects

GuySartorelli commented 1 year ago

Awesome, thank you so much - and enjoy the conference!

williamdes commented 1 year ago

Awesome, thank you so much - and enjoy the conference!

Thank you !

Here it is: https://github.com/code-lts/doctum/releases/tag/v5.5.3

Please let me know if this turns out well, I decided to release it as it is 🚀 Users had months to test the dev phar ;) I prefer not to wait and have some rougth edges rather than the void 😄