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

Support nested generics and closures type hint #17

Conversation

KentarouTakeda
Copy link

Fixed an issue where an incorrect parsing result was returned when the type declared in @param was a nested generic type or closure.

A simple solution was already in place for generic types, but it did not support cases where multiple whitespaces were included due to nesting. Closures also use whitespace when defined accurately, but this was not supported either.

I fixed this issue by implementing a simple parser.

Issues this pull request resolves

Testing overview

I not only added many test cases, but also tested Laravel IDE Helper with this patch applied. The results changed as follows:

Resolving barrydh/laravel-ide-helper#1505

1) Barryvdh\LaravelIdeHelper\Tests\Console\ModelsCommand\ArrayCastsWithComment\Test::test
Failed asserting that two strings are identical.

Snapshots can be updated by passing `-d --update-snapshots` through PHPUnit's CLI arguments.
--- Expected
+++ Actual
@@ @@
  * @property string $cast_to_bool
  * @property string $cast_to_boolean
  * @property string $cast_to_object
- * @property array $cast_to_array
- * @property array $cast_to_json
- * @property \Illuminate\Support\Collection $cast_to_collection
  * @property string $cast_to_date
  * @property string $cast_to_datetime
  * @property string $cast_to_date_serialization

/tmp/barryvdh/laravel-ide-helper/tests/SnapshotPhpDriver.php:24
/tmp/barryvdh/laravel-ide-helper/vendor/spatie/phpunit-snapshot-assertions/src/Snapshot.php:55
/tmp/barryvdh/laravel-ide-helper/vendor/spatie/phpunit-snapshot-assertions/src/MatchesSnapshots.php:199
/tmp/barryvdh/laravel-ide-helper/vendor/spatie/phpunit-snapshot-assertions/src/MatchesSnapshots.php:57
/tmp/barryvdh/laravel-ide-helper/tests/Console/ModelsCommand/AbstractModelsCommand.php:58
/tmp/barryvdh/laravel-ide-helper/tests/Console/ModelsCommand/ArrayCastsWithComment/Test.php:26

Failed the xfail-equivalent test. The issue appears to have been fixed.

Fixed multiple issues with Laravel IDE Helper

2) Barryvdh\LaravelIdeHelper\Tests\MethodTest::testEloquentBuilderOutput
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 '/**
  * Set the relationships that should be eager loaded.
  *
- * @param string|array $relations
- * @param string|\Closure|null $callback
+ * @param array<array-key, array|(\Closure(\Illuminate\Database\Eloquent\Relations\Relation<*,*,*>): mixed)|string>|string $relations
+ * @param (\Closure(\Illuminate\Database\Eloquent\Relations\Relation<*,*,*>): mixed)|string|null $callback
  * @return \Illuminate\Database\Eloquent\Builder|static
  * @static
  */'

/tmp/barryvdh/laravel-ide-helper/barryvdh-laravel-ide-helper/tests/MethodTest.php:76

This is a test that recently failed due to a change in the Laravel Framework. The patch changed the reason for the failure.

For reference, without this patch, the failure reason will be as follows:

1) Barryvdh\LaravelIdeHelper\Tests\MethodTest::testEloquentBuilderOutput
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 '/**
  * Set the relationships that should be eager loaded.
  *
- * @param string|array $relations
- * @param string|\Closure|null $callback
+ * @param \Illuminate\Database\Eloquent\array<array-key, array|(\Closure(\Illuminate\Database\Eloquent\Relations\Relation<*,*,*>):  mixed)|string>|string  $relations
+ * @param \Illuminate\Database\Eloquent\(\Closure(\Illuminate\Database\Eloquent\Relations\Relation<*,*,*>):  mixed)|string|null  $callback
  * @return \Illuminate\Database\Eloquent\Builder|static 
  * @static 
  */'

/home/runner/work/laravel-ide-helper/laravel-ide-helper/tests/MethodTest.php:76

Before the patch, the output was obviously broken. As already shown, this will now be accurate.

@barryvdh barryvdh merged commit bba116b into barryvdh:master Oct 16, 2024
10 checks passed
@KentarouTakeda KentarouTakeda deleted the nested-generics-and-closures-type-hint branch October 16, 2024 11:15
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