oss-gate / workshop

OSSの開発に未参加または参加したことはあるけどまだ自信がない人を後押しするワークショップ用のリポジトリー
124 stars 547 forks source link

OSS Gate Workshop: PHP Lovers: 2024-02-12: akinoriakatsuka: larastan: Work log #1783

Closed akinoriakatsuka closed 9 months ago

akinoriakatsuka commented 9 months ago

This is a work log of a "OSS Gate workshop". "OSS Gate workshop" is an activity to increase OSS developers. Here's been discussed in Japanese. Thanks.

作業ログ作成時の説明

以下のテンプレートを埋めてタイトルに設定します。埋め方例はスクロールすると見えてきます。

OSS Gate Workshop: ${LOCATION}: ${YEAR}-${MONTH}-${DAY}: ${ACCOUNT_NAME}: ${OSS_NAME}: Work log

タイトル例↓:

OSS Gate Workshop: Tokyo: 2017-01-16: kou: Rabbit: Work log

OSS Gateワークショップ関連情報

akinoriakatsuka commented 9 months ago

サポーターの方と相談して、テーマをlaravel/sailに設定

akinoriakatsuka commented 9 months ago

https://github.com/laravel/sail/blob/1.x/LICENSE.md

laravel/sailはMITライセンスのOSS

akinoriakatsuka commented 9 months ago

https://laravel.com/docs/10.x/installation#docker-installation-using-sail

installation documentation があった

akinoriakatsuka commented 9 months ago

Docker Desktopがインストールされていれば動かせるらしい

4.6.0 (75818)がインストールされていた。

akinoriakatsuka commented 9 months ago

curl -s https://laravel.build/example-app | bash terminal でこのコマンドを打って、最後にパスワード入力をすると、以下の記述が出た

Thank you! We hope you build something incredible. Dive in with: cd example-app && ./vendor/bin/sail up

akinoriakatsuka commented 9 months ago

cd example-app && ./vendor/bin/sail up

コンテナの立ち上げができた

akinoriakatsuka commented 9 months ago

sailは問題なくうごいたので、スキがなさそう。

テーマを larastan/larastan に変更

https://github.com/larastan/larastan/

akinoriakatsuka commented 9 months ago

larastanはMITなのでOSS

https://github.com/larastan/larastan/blob/2.x/LICENSE.md

akinoriakatsuka commented 9 months ago

https://github.com/larastan/larastan/blob/2.x/CONTRIBUTING.md

コントリビューティングがあった。

cloneしてtestしてみる

akinoriakatsuka commented 9 months ago

composer update

46 package suggestions were added by new dependencies, usecomposer suggestto see details. Generating autoload files 87 packages you are using are looking for funding. Use thecomposer fundcommand to find out more! No security vulnerability advisories found

akinoriakatsuka commented 9 months ago

composer test

> phpstan analyse --ansi --memory-limit 256M
Note: Using configuration file /Users/akinori/development/larastan/phpstan.neon.dist.
 137/137 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 [OK] No errors                                                                                                         

> phpunit --colors=always -d memory_limit=1408M
PHPUnit 10.5.10 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.7
Configuration: /Users/akinori/development/larastan/phpunit.xml.dist

.............................................................   61 / 1151 (  5%)
.............................................................  122 / 1151 ( 10%)
.............................................................  183 / 1151 ( 15%)
.............................................................  244 / 1151 ( 21%)
.............................................................  305 / 1151 ( 26%)
.............................................................  366 / 1151 ( 31%)
.............................................................  427 / 1151 ( 37%)
.............................................................  488 / 1151 ( 42%)
.............................................................  549 / 1151 ( 47%)
.............................................................  610 / 1151 ( 52%)
.............................................................  671 / 1151 ( 58%)
.............................................................  732 / 1151 ( 63%)
.............................................................  793 / 1151 ( 68%)
.............................................................  854 / 1151 ( 74%)
.............................................................  915 / 1151 ( 79%)
.............................................................  976 / 1151 ( 84%)
............................................................. 1037 / 1151 ( 90%)
............................................................. 1098 / 1151 ( 95%)
.....................................................         1151 / 1151 (100%)

Time: 00:16.455, Memory: 1.16 GB

OK (1151 tests, 2367 assertions)
akinoriakatsuka commented 9 months ago

testも正常に動いているので、good first issueで絞って上から見ていく作戦

https://github.com/larastan/larastan/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22

akinoriakatsuka commented 9 months ago

まずはこれ

https://github.com/larastan/larastan/issues/1820

akinoriakatsuka commented 9 months ago

再現手順がわかりづらかったので、一旦別のものを見ることにする

akinoriakatsuka commented 9 months ago

これを試してみる

https://github.com/larastan/larastan/pull/1544

withWhereHasが入ったコードでlarastanを実行して、午後からは状況を確認する

akinoriakatsuka commented 9 months ago

似たもの探し

情報収集

例えば、with〇〇があれば修正の参考になるかもしれない。

akinoriakatsuka commented 9 months ago

laravelのプロジェクトでwithWhereHasを含むコードを書いてみて、larastanで状況が再現するか試してみる

akinoriakatsuka commented 9 months ago
  1. laravelのプロジェクトにlarastanを入れる

composer require larastan/larastan:^2.0 --dev

./composer.json has been updated
Running composer update larastan/larastan
Loading composer repositories with package information
Updating dependencies
Lock file operations: 3 installs, 0 updates, 0 removals
  - Locking larastan/larastan (v2.8.1)
  - Locking phpmyadmin/sql-parser (5.9.0)
  - Locking phpstan/phpstan (1.10.57)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 3 installs, 0 updates, 0 removals
  - Downloading larastan/larastan (v2.8.1)
  - Installing phpstan/phpstan (1.10.57): Extracting archive
  - Installing phpmyadmin/sql-parser (5.9.0): Extracting archive
  - Installing larastan/larastan (v2.8.1): Extracting archive
2 package suggestions were added by new dependencies, use `composer suggest` to see details.
Generating optimized autoload files
> Illuminate\Foundation\ComposerScripts::postAutoloadDump
> @php artisan package:discover --ansi

   INFO  Discovering packages.  

  laravel/sail ...................................................................... DONE
  laravel/sanctum ................................................................... DONE
  laravel/tinker .................................................................... DONE
  nesbot/carbon ..................................................................... DONE
  nunomaduro/collision .............................................................. DONE
  nunomaduro/termwind ............................................................... DONE
  spatie/laravel-ignition ........................................................... DONE

86 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
> @php artisan vendor:publish --tag=laravel-assets --ansi --force

   INFO  No publishable resources for tag [laravel-assets].  

No security vulnerability advisories found
akinoriakatsuka commented 9 months ago

larastanを入れてphpstanを実行

larastanの手順通りにphpstan.neonを作成

includes:
    - vendor/larastan/larastan/extension.neon

parameters:

    paths:
        - app/

    # Level 9 is the highest level
    level: 5

#    ignoreErrors:
#        - '#PHPDoc tag @var#'
#
#    excludePaths:
#        - ./*/*/FileToBeExcluded.php
#
#    checkMissingIterableValueType: false
akinoriakatsuka commented 9 months ago

初期状態ではエラーは特になし

$ ./vendor/bin/phpstan analyse                    
Note: Using configuration file /Users/akinori/development/example-app/phpstan.neon.
 19/19 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 [OK] No errors        
akinoriakatsuka commented 9 months ago

issueにあるようにlevelを8に設定

akinoriakatsuka commented 9 months ago

静的解析で、実行するわけではないので、Userクラスに雑に貼り付けて、phpstanを実行してみる

withWhereHasはコメントアウトしておいて、with->whreHasを有効化してとりあえず、phpstanの実行を試みる

<?php

namespace App\Models;

// use Illuminate\Contracts\Auth\MustVerifyEmail;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Foundation\Auth\User as Authenticatable;
use Illuminate\Notifications\Notifiable;
use Laravel\Sanctum\HasApiTokens;

class User extends Authenticatable
{
    use HasApiTokens, HasFactory, Notifiable;

    /**
     * The attributes that are mass assignable.
     *
     * @var array<int, string>
     */
    protected $fillable = [
        'name',
        'email',
        'password',
    ];

    /**
     * The attributes that should be hidden for serialization.
     *
     * @var array<int, string>
     */
    protected $hidden = [
        'password',
        'remember_token',
    ];

    /**
     * The attributes that should be cast.
     *
     * @var array<string, string>
     */
    protected $casts = [
        'email_verified_at' => 'datetime',
        'password' => 'hashed',
    ];

    public function handle(): void
    {
        $videos = Video::query()
            // ->withWhereHas('liveStreamVideo', function ($query): void {
            //     $query->where('status', LivestreamVideoStatus::Scheduled)
            //         ->whereNotNull('video_sdk_room_id')
            //         ->where('start_at', '<=', now());
            // })
            ->with('liveStreamVideo:id,video_id,status')
            ->whereHas('liveStreamVideo', function ($query): void {
                $query->where('status', LivestreamVideoStatus::Scheduled)
                    ->whereNotNull('video_sdk_room_id')
                    ->where('start_at', '<=', now());
            })
            ->get();

        $videos->each(function ($video): void {
            $video->liveStreamVideo?->update([
                'status' => LivestreamVideoStatus::Active,
            ]);
        });
    }

}
akinoriakatsuka commented 9 months ago

雑に貼り付けたので、クラスがないというエラーがでる

$ ./vendor/bin/phpstan analyse app/Models/User.php --memory-limit=2G
Note: Using configuration file /Users/akinori/development/example-app/phpstan.neon.
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ --------------------------------------------------------------------------------- 
  Line   User.php                                                                         
 ------ --------------------------------------------------------------------------------- 
  :48    Call to static method query() on an unknown class App\Models\Video.              
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols              
  :56    Access to constant Scheduled on an unknown class                                 
         App\Models\LivestreamVideoStatus.                                                
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols              
  :64    Access to constant Active on an unknown class App\Models\LivestreamVideoStatus.  
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols              
 ------ --------------------------------------------------------------------------------- 

 [ERROR] Found 3 errors                                                                     
akinoriakatsuka commented 9 months ago

with->whreHasをwithWhereHasを使うように変更して、phpstanの実行を試みる

$videos = Video::query()
            ->withWhereHas('liveStreamVideo', function ($query): void {
                $query->where('status', LivestreamVideoStatus::Scheduled)
                    ->whereNotNull('video_sdk_room_id')
                    ->where('start_at', '<=', now());
            })
            // ->with('liveStreamVideo:id,video_id,status')
            // ->whereHas('liveStreamVideo', function ($query): void {
            //     $query->where('status', LivestreamVideoStatus::Scheduled)
            //         ->whereNotNull('video_sdk_room_id')
            //         ->where('start_at', '<=', now());
            // })
            ->get();
akinoriakatsuka commented 9 months ago

withWhereHasを使ってもエラーの数は3で変わらない もう修正されている?

$ ./vendor/bin/phpstan analyse app/Models/User.php --memory-limit=2G
Note: Using configuration file /Users/akinori/development/example-app/phpstan.neon.
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ --------------------------------------------------------------------------------- 
  Line   User.php                                                                         
 ------ --------------------------------------------------------------------------------- 
  :48    Call to static method query() on an unknown class App\Models\Video.              
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols              
  :50    Access to constant Scheduled on an unknown class                                 
         App\Models\LivestreamVideoStatus.                                                
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols              
  :64    Access to constant Active on an unknown class App\Models\LivestreamVideoStatus.  
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols              
 ------ --------------------------------------------------------------------------------- 

 [ERROR] Found 3 errors                                                                     
akinoriakatsuka commented 9 months ago

$ ./vendor/bin/phpstan analyse app/Models/User.php --memory-limit=2G Note: Using configuration file /Users/akinori/development/example-app/phpstan.neon. 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


Line User.php


:56 Relation 'livestreamVideo' is not found in App\Models\Video model.
:56 Relation 'livestreamVideo' is not found in App\Models\Video model.
:66 Access to an undefined property
Illuminate\Database\Eloquent\Model::$livestreamVideo.
💡 Learn more:
https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
:67 Access to undefined constant
App\Models\LivestreamVideoStatus::Active.


[ERROR] Found 4 errors

github-actions[bot] commented 9 months ago

おつかれさまでした!

ワークショップの終了にともないissueを閉じますが、このまま作業メモとして使っても構いません :ok_hand:

ワークショップの感想を集めています!

ブログなどに書かれた際は、このページへリンクの追加をお願いします :pray:

またの参加をお待ちしています!

akinoriakatsuka commented 9 months ago

(作業メモ継続して使います)

とりあえず、真面目にモデルとか作ってエラーを消していくことにする。

akinoriakatsuka commented 9 months ago

app/Jobs/ChangeStatusOfLiveStreamVideo.phpにファイルを作ってより近い状況で再現を試みる


$ ./vendor/bin/phpstan analyse app/Jobs/ChangeStatusOfLiveStreamVideo.php --memory-limit=2G
Note: Using configuration file /Users/akinori/development/example-app/phpstan.neon.
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ --------------------------------------------------------------------------------------- 
  Line   ChangeStatusOfLiveStreamVideo.php                                                      
 ------ --------------------------------------------------------------------------------------- 
  :42    Access to an undefined property Illuminate\Database\Eloquent\Model::$liveStreamVideo.  
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property   
 ------ --------------------------------------------------------------------------------------- 

 [ERROR] Found 1 error                                                                                          
akinoriakatsuka commented 9 months ago

with と WhereHasが分かれているメソッドでは$videoPHPStan: Illuminate\Database\Eloquent\Collection<int, App\Models\Video> $videosこの型になっている。

withWhereHadでは$videoPHPStan: Illuminate\Database\Eloquent\Collection<int, Illuminate\Database\Eloquent\Model> $videos2この型になっている

App\Models\Videoになるべきところが、Illuminate\Database\Eloquent\Modelになっているので、リレーションがないと言われている。

akinoriakatsuka commented 9 months ago

withWhereHasを含んだメソッドチェーンでもApp\Models\Videoのコレクションが返るとPHPStanに知らせればよさそう

akinoriakatsuka commented 9 months ago

https://phpstan.org/developing-extensions/rules

落ち着いてこちらを調べる