PHP-CS-Fixer の導入 #996

nanasess · 2024-09-05T08:58:01Z

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

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

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

codecov · 2024-09-05T09:03:25Z

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 ⚠️
data/class/helper/SC_Helper_CSV.php	0.00%	105 Missing ⚠️
data/class/helper/SC_Helper_Transform.php	0.00%	59 Missing ⚠️
data/class/SC_Image.php	2.50%	39 Missing ⚠️
data/class/SC_Fpdf.php	11.76%	30 Missing ⚠️
data/class/api/SC_Api_Operation.php	0.00%	30 Missing ⚠️
data/class/api/SC_Api_Utils.php	0.00%	29 Missing ⚠️
data/class/helper/SC_Helper_PageLayout.php	25.64%	29 Missing ⚠️
data/class/api/operations/ItemLookup.php	0.00%	28 Missing ⚠️
data/class/SC_Initial.php	0.00%	27 Missing ⚠️
... and 44 more

Additional details and impacted files

@@            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	Coverage Δ
tests	`56.23% <45.87%> (-0.07%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

seasoftjapan · 2024-09-05T09:49:30Z

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 · 2024-09-05T12:34:07Z

@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 · 2024-09-05T12:34:24Z

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

nanasess · 2024-09-06T00:59:39Z

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

- string_length_to_empty - ternary_to_elvis_operator

data/class/SC_CartSession.php

seasoftjapan · 2024-09-06T10:13:33Z

data/class/api/operations/CartAdd.php

@@ -41,7 +40,7 @@ class API_CartAdd extends SC_Api_Abstract_Ex
    public function doAction($arrParam)
    {
        $this->arrResponse = array(
-            'Version' => ECCUBE_VERSION);
+            'Version' => ECCUBE_VERSION, );


カンマの後、改行してくれるオプションとか無いでしょうか?
いくつか、気持ちの悪い(中途半端な)印象の整形がありました。

trailing_comma_in_multiline が有効になっていると、複数行の配列で、末尾は改行しないパターンでこのようになってしまうようです。
複数行を強制するルールは実装されていないようなので、 trailing_comma_in_multiline => false にしたいと思います
https://cs.symfony.com/doc/rules/control_structure/trailing_comma_in_multiline.html

気持ち悪いですが、それでも今後の Diff を整えられる全体的なメリットを考えると、ケツカンマの徹底は優先して良いと思います。

とは言え、やはり気持ち悪いし、その箇所は Diff を改善できないので、, ] となる箇所は私が修正しようと思います。どの流れが良いとかありますか?

https://github.com/nanasess/ec-cube2/tree/php-cs-fixer ベースで修正して PR する。

, ] となる箇所を master ベースで修正して、https://github.com/nanasess/ec-cube2/tree/php-cs-fixer へ PR する。

, ] となる箇所を master ベースで修正して、master へ PR する。

別 Issue で後続の対応とする。そのタイミングでルール削除する。

本 PR はルールを戻して、, ] となった箇所を別 Issue で修正する。

@seasoftjapan

本 PR はルールを戻して、, ] となった箇所を別 Issue で修正する。

この対応にしましょうかね。
trailing_comma_in_multiline => false は削除しておきます(@Symfony に含まれているため)

trailing_comma_in_multiline => false は削除して再度 fix しておきました
20138d0

ありがとうございます。
#1017 登録しました。

seasoftjapan · 2024-09-06T10:17:06Z

data/class/api/operations/ItemSearch.php

@@ -125,27 +125,27 @@ protected function getProductsList($searchCondition, $disp_number, $startno, $li
            case 'price':
                $objProduct->setProductsOrder('price02', 'dtb_products_class', 'ASC');
                break;
-            // 販売価格が高い順
+                // 販売価格が高い順


これ、救えないですかね。case のコメントで、case の真上に欲しいです。

'statement_indentation' => ['stick_comment_to_next_continuous_control_statement' => true] とすることで対応できそうです。
https://cs.symfony.com/doc/rules/whitespace/statement_indentation.html
が、 composer の依存関係の問題で、現在 php-cs-fixer 3.9.5 がインストールされています。
このバージョンでは上記オプションは未実装のため、利用できないようです。
PHPUnit のバージョンを上げることで、上記オプションも利用できるようになりそうなので、別途対応しようと思います。

以下 PR 作成しました
#1020

.php-cs-fixer.dist.php

data/class/SC_Batch.php

nanasess · 2024-09-09T00:11:13Z

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

nanasess · 2024-09-29T15:14:03Z

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

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

nanasess · 2024-09-30T17:11:13Z

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

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

各ルールにコメントも入れておきましたのでご確認お願いいたします🙇‍♂️
94d493c

…low-risky=yes

`, ]` となる箇所は別途修正する see EC-CUBE#996 (comment)

nanasess · 2024-10-03T02:45:43Z

GitHub Actions のエラーは以下 PR で解消予定
#1018

seasoftjapan

1点忘れてました。

concat_space => ['spacing' => 'none'] に変更(4系にあわせる)

2系で準拠してきた、Zend Framework PHP 標準コーディング規約と相違するため気になりました。

Laravel 界隈以外では、一般的でない気がするのですが、どうでしょうか?

正直 <sp>.<sp> と打つの面倒くせーと思うこともありますけどね。

これまで主に読みやすさの観点で推奨されていたと思うので、それこそコーディング中はスペースを省略して入力して、PHP-CS-Fixer でスペースを補ってもらうとかも有りかなと思いました。

nanasess · 2024-10-03T13:43:59Z

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

seasoftjapan · 2024-10-03T14:44:37Z

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

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

PHP-CS-Fixer の導入

bfc9f5d

nanasess requested review from seasoftjapan and nobuhiko September 5, 2024 09:00

nanasess marked this pull request as draft September 5, 2024 12:34

nanasess added 2 commits September 6, 2024 10:36

Merge remote-tracking branch 'upstream/master' into php-cs-fixer

4381058

副作用のありそうなルールを除外

9326ac0

- string_length_to_empty - ternary_to_elvis_operator

nanasess force-pushed the php-cs-fixer branch from a796588 to dee413e Compare September 6, 2024 02:23

nanasess marked this pull request as ready for review September 6, 2024 02:43

nanasess enabled auto-merge September 6, 2024 02:43

seasoftjapan mentioned this pull request Sep 6, 2024

strlen($val) をどうするか #999

Open

seasoftjapan reviewed Sep 6, 2024

View reviewed changes

nanasess added 2 commits October 1, 2024 00:43

php-cs-fixer を適用すると削除されるため1行に修正

e96e7fe

nanasess force-pushed the php-cs-fixer branch from dee413e to 872e0eb Compare September 30, 2024 17:01

Merge remote-tracking branch 'upstream/master' into php-cs-fixer

a9a7e00

nanasess force-pushed the php-cs-fixer branch from 872e0eb to 5c6ecce Compare September 30, 2024 17:05

nanasess added 2 commits October 2, 2024 22:30

Apply PHP_CS_FIXER_IGNORE_ENV=1 data/vendor/bin/php-cs-fixer fix --al…

da8ad32

…low-risky=yes

trailing_comma_in_multiline => false は削除

20138d0

`, ]` となる箇所は別途修正する see EC-CUBE#996 (comment)

nanasess force-pushed the php-cs-fixer branch from 5c6ecce to 20138d0 Compare October 2, 2024 13:35

seasoftjapan mentioned this pull request Oct 2, 2024

ソース整形配列末尾のカンマと閉じ括弧を改行する #1017

Closed

seasoftjapan approved these changes Oct 3, 2024

View reviewed changes

nanasess mentioned this pull request Oct 3, 2024

PHPUnit のバージョンアップ #1020

Closed

seasoftjapan requested changes Oct 3, 2024

View reviewed changes

seasoftjapan approved these changes Oct 3, 2024

View reviewed changes

seasoftjapan and others added 3 commits October 4, 2024 22:20

Merge branch 'master' into php-cs-fixer

a14deeb

Fix phpstan

7690b1a

Merge branch 'master' into php-cs-fixer

b3df010

nanasess merged commit 9344c5f into EC-CUBE:master Oct 6, 2024
88 of 90 checks passed

nanasess deleted the php-cs-fixer branch October 6, 2024 16:46

nanasess mentioned this pull request Oct 9, 2024

php-cs-fixer 関連の TODO #1025

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PHP-CS-Fixer の導入 #996

PHP-CS-Fixer の導入 #996

nanasess commented Sep 5, 2024 •

edited

Loading

codecov bot commented Sep 5, 2024 •

edited

Loading

seasoftjapan commented Sep 5, 2024

nanasess commented Sep 5, 2024 •

edited

Loading

nanasess commented Sep 5, 2024

nanasess commented Sep 6, 2024

seasoftjapan Sep 6, 2024

nanasess Sep 30, 2024 •

edited

Loading

seasoftjapan Oct 2, 2024

nanasess Oct 2, 2024 •

edited

Loading

nanasess Oct 2, 2024

seasoftjapan Oct 2, 2024

seasoftjapan Sep 6, 2024

nanasess Sep 30, 2024

nanasess Oct 3, 2024

nanasess commented Sep 9, 2024

nanasess commented Sep 29, 2024

nanasess commented Sep 30, 2024

nanasess commented Oct 3, 2024

seasoftjapan left a comment •

edited

Loading

nanasess commented Oct 3, 2024

seasoftjapan commented Oct 3, 2024

PHP-CS-Fixer の導入 #996

PHP-CS-Fixer の導入 #996

Conversation

nanasess commented Sep 5, 2024 • edited Loading

codecov bot commented Sep 5, 2024 • edited Loading

Codecov Report

seasoftjapan commented Sep 5, 2024

nanasess commented Sep 5, 2024 • edited Loading

nanasess commented Sep 5, 2024

nanasess commented Sep 6, 2024

seasoftjapan Sep 6, 2024

Choose a reason for hiding this comment

nanasess Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

seasoftjapan Oct 2, 2024

Choose a reason for hiding this comment

nanasess Oct 2, 2024 • edited Loading

Choose a reason for hiding this comment

nanasess Oct 2, 2024

Choose a reason for hiding this comment

seasoftjapan Oct 2, 2024

Choose a reason for hiding this comment

seasoftjapan Sep 6, 2024

Choose a reason for hiding this comment

nanasess Sep 30, 2024

Choose a reason for hiding this comment

nanasess Oct 3, 2024

Choose a reason for hiding this comment

nanasess commented Sep 9, 2024

nanasess commented Sep 29, 2024

nanasess commented Sep 30, 2024

nanasess commented Oct 3, 2024

seasoftjapan left a comment • edited Loading

Choose a reason for hiding this comment

nanasess commented Oct 3, 2024

seasoftjapan commented Oct 3, 2024

nanasess commented Sep 5, 2024 •

edited

Loading

codecov bot commented Sep 5, 2024 •

edited

Loading

nanasess commented Sep 5, 2024 •

edited

Loading

nanasess Sep 30, 2024 •

edited

Loading

nanasess Oct 2, 2024 •

edited

Loading

seasoftjapan left a comment •

edited

Loading