PHP-CS-Fixer の導入 - Githubissues

nanasess commented 2 months ago

~~strlen($val) で null チェックしているのを修正するのが主目的~~
- 未定義変数や連想配列での扱いに副作用があるため除外
その他、統一した方が良さそうなルールを適用

PHP8.3で以下のコマンドを実行

PHP_CS_FIXER_IGNORE_ENV=1 data/vendor/bin/php-cs-fixer fix  --allow-risky=yes

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 45.87459% with 820 lines in your changes missing coverage. Please review.

Project coverage is 56.23%. Comparing base (fd8217c) to head (b3df010). Report is 12 commits behind head on master.

Files with missing lines	Patch %	Lines
data/class/helper/SC_Helper_Customer.php	7.19%	129 Missing :warning:
data/class/helper/SC_Helper_CSV.php	0.00%	105 Missing :warning:
data/class/helper/SC_Helper_Transform.php	0.00%	59 Missing :warning:
data/class/SC_Image.php	2.50%	39 Missing :warning:
data/class/SC_Fpdf.php	11.76%	30 Missing :warning:
data/class/api/SC_Api_Operation.php	0.00%	30 Missing :warning:
data/class/api/SC_Api_Utils.php	0.00%	29 Missing :warning:
data/class/helper/SC_Helper_PageLayout.php	25.64%	29 Missing :warning:
data/class/api/operations/ItemLookup.php	0.00%	28 Missing :warning:
data/class/SC_Initial.php	0.00%	27 Missing :warning:
... and 44 more

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #996 +/- ## ========================================== - Coverage 56.29% 56.23% -0.07% ========================================== Files 75 75 Lines 8917 9046 +129 ========================================== + Hits 5020 5087 +67 - Misses 3897 3959 +62 ``` | [Flag](https://app.codecov.io/gh/EC-CUBE/ec-cube2/pull/996/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=EC-CUBE) | Coverage Δ | | |---|---|---| | [tests](https://app.codecov.io/gh/EC-CUBE/ec-cube2/pull/996/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=EC-CUBE) | `56.23% <45.87%> (-0.07%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=EC-CUBE#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

seasoftjapan commented 2 months ago

6f20c7d は、全て PHP-CS-Fixer による変更ですか?

ローカルで bfc9f5d から実行を試したのですが、結果が異なる様子でして。

$ data/vendor/bin/php-cs-fixer fix --allow-risky=yes --dry-run
Loaded config default from "****/.php-cs-fixer.dist.php".
   1) data/class/SC_Customer.php
   2) data/class/helper/SC_Helper_TaxRule.php
   3) data/class/helper/SC_Helper_CSV.php
   4) data/class/SC_Fpdf.php
   5) data/class/pages/guide/LC_Page_Guide_Kiyaku.php
   6) data/class/pages/shopping/LC_Page_Shopping.php
   7) data/class/pages/products/LC_Page_Products_List.php
   8) data/class/pages/admin/contents/LC_Page_Admin_Contents_Recommend.php
   9) data/class/pages/admin/contents/LC_Page_Admin_Contents_RecommendSearch.php
  10) data/class/pages/admin/mail/LC_Page_Admin_Mail_Preview.php
  11) data/class/pages/admin/design/LC_Page_Admin_Design.php
  12) data/class/pages/admin/design/LC_Page_Admin_Design_MainEdit.php
  13) data/class/pages/admin/order/LC_Page_Admin_Order_Mail.php
  14) data/class/pages/admin/order/LC_Page_Admin_Order_Edit.php
  15) data/class/pages/admin/order/LC_Page_Admin_Order_Disp.php
  16) data/class/pages/admin/order/LC_Page_Admin_Order_ProductSelect.php
  17) data/class/pages/admin/products/LC_Page_Admin_Products_Product.php
  18) data/class/pages/mypage/LC_Page_Mypage_Favorite.php
  19) data/module/HTTP/Request.php

Checked all files in 20.817 seconds, 24.000 MB memory used

正しい実行手順を教えていただけますと幸いです。

nanasess commented 2 months ago

@seasoftjapan そのままだと composer.json の config.platform.php: 7.4.0 が有効になっていて、PHP8.0以降のルールが適用されないようです。以下でいかがでしょうか？

PHP_CS_FIXER_IGNORE_ENV=1 data/vendor/bin/php-cs-fixer fix --allow-risky=yes

また、 --diff オプションをつけていただくと、原因がわかりやすいです

nanasess commented 2 months ago

テストが落ちているようなので、一旦 Draft に戻します

nanasess commented 2 months ago

未定義変数、連想配列の扱いに難がありそう

nanasess commented 2 months ago

@seasoftjapan ありがとうございます！おおむね対応可能だと思いますので、順次対応していきますね

nanasess commented 2 months ago

2.13.x のソースに対しても同様のルールを適用できそうなので、これを活用すれば旧バージョンからのマージや PHP8 対応も楽にできそう

nanasess commented 2 months ago

@seasoftjapan 以下のルールを見直してみました

single_quote => true に変更(4系にあわせる)
concat_space => ['spacing' => 'none'] に変更(4系にあわせる)
array_syntax => true に変更(4系にあわせる)
phpdoc_scalar => true に変更(phpdoc の int や bool を統一したい)
data/smarty_extends 及び tests にも適用

各ルールにコメントも入れておきましたのでご確認お願いいたします🙇‍♂️ https://github.com/EC-CUBE/ec-cube2/pull/996/commits/94d493cd94b96a0b337b11a6c09da17869c282ae

nanasess commented 1 month ago

GitHub Actions のエラーは以下 PR で解消予定 https://github.com/EC-CUBE/ec-cube2/pull/1018

nanasess commented 1 month ago

@seasoftjapan 2系と4系で、スペース有無を使い分けるのはナンセンスだなと思い、4系に合わせました。従来のコーディング規約と異なるのは承知の上なんですが、多くのルールを Symfony に準拠しているので、こちらも合わせた方が混乱が少ないかなと思ってます

seasoftjapan commented 1 month ago

ルールに concat_space を追加したと思いましたが、逆に削除したのですね。で、@Symfony に concat_space が設定されていて、デフォルトは ['spacing' => 'one'] であると。

可逆なルールだと思いますし、とりあえずそのスタイルで試してみるのも良さそうですね。

EC-CUBE / ec-cube2

PHP-CS-Fixer の導入 #996

Codecov Report