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

[5.6] Update Route Facade docblocks to support all possible scenarios #24086

Merged
merged 1 commit into from
May 2, 2018

Conversation

Korri
Copy link
Contributor

@Korri Korri commented May 2, 2018

Made some changes to the docblock of the Route facade and added some to the Illuminate\Routing\RouteRegistrar class in order to allow IDEs to recognize all the following (valid) scenarios:

$callback = function () {};// Make for some shorter code examples

Route::current();
Route::currentRouteName();
Route::currentRouteAction();
Route::bind('user', $callback);
Route::get('user/{name}', $callback)->where('name', '[A-Za-z]+')->name('test');
Route::match(['get', 'post'], '/', $callback)->name('test');
Route::any('foo', $callback)->name('test');
Route::view('/welcome', 'welcome')->name('test');
Route::redirect('/here', '/there', 301)->name('there');
Route::group(['middleware' => 'test'], $callback);
Route::middleware('first')->group($callback);
Route::middleware(['first', 'second'])->group($callback);
Route::domain('{account}.myapp.com')->group($callback);
Route::prefix('admin')->middleware('admin')->as('admin')->name('App\Controllers2')->group($callback);
Route::prefix('admin')->middleware('admin')->as('admin')->name('App\Controllers2')->get('/logout', $callback);

The main motication is to fix the following that is not recognized as valid by phpstorm after mergin pull request #24040 : Route::middleware('first')->group($callback);

@Korri Korri force-pushed the route-facade-methods-doc-cleaning branch from a49c178 to 2d50f5c Compare May 2, 2018 19:01
@taylorotwell taylorotwell merged commit ba7a8d8 into laravel:5.6 May 2, 2018
@taylorotwell
Copy link
Member

The facade docblocks are already becoming tiresome. :/

@browner12
Copy link
Contributor

yah, this feels like it will become a maintenance nightmare

@ntzm
Copy link
Contributor

ntzm commented May 3, 2018

How else do you expect people with IDEs and static analysis tools to deal with Laravel's facades?

@imbrish
Copy link
Contributor

imbrish commented May 3, 2018

Sorry for being ignorant, but doesn't https://github.com/barryvdh/laravel-ide-helper solve this problem already?

@browner12
Copy link
Contributor

I believe it does, but people probably are looking for a solution that doesn't require another dependency.

@Korri
Copy link
Contributor Author

Korri commented May 3, 2018

Out use case is that we use PHPStan, and with good dockblocks, it allows PHPStan to understand facades without any custom plugin.

@Korri Korri deleted the route-facade-methods-doc-cleaning branch June 14, 2018 13:30
@TopFuel TopFuel mentioned this pull request Sep 30, 2018
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.

5 participants