improvement: deprecation ゲート有効化＋未使用の非推奨 public API 削除 (#6933) - #6937
improvement: deprecation ゲート有効化＋未使用の非推奨 public API 削除 (#6933)#6937ttokoro20240902 wants to merge 11 commits into
Conversation
コアに呼び出し元がなく、CSV 出力にも載らず、DB スキーマ変更も伴わない 未使用の @deprecated public API を削除する。仕様（挙動・出力・画面）は不変。 - OrderItem::getTaxRuleId() / setTaxRuleId()（呼び出し元皆無・tax_rule_id カラムは残置） - Product::isEnable() ＋ ProductClass::isEnable()（計算メソッド・相互呼び出しのみ） - Cart::getLock() / setLock() ＋ ORM 非マッピングの $lock プロパティ - transChoice()（trans() の別名・呼び出し元皆無） - TaxProcessor::getTaxDisplayType()（デッドな protected メソッド） - CartService::$cart（未初期化・未使用プロパティ）＋未使用 import Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
呼び出し元がテストのみの @deprecated public API を削除する。 仕様（挙動・出力・画面）は不変。 削除: - Order::getTotalPrice()（getPaymentTotal() の別名。E_USER_DEPRECATED の self 発火源） - CacheUtil::clear()（代替は clearCache()。専用テスト CacheUtilTest ごと削除） - CustomerStatus::NONACTIVE / ACTIVE（現役定数 PROVISIONAL / REGULAR と同値の別名） テストの扱い: - 専用テストは削除: CacheUtilTest、OrderTest::testGetTotalPrice - 道具として使うテストは等価 API へ付け替え: - EditControllerTest: getTotalPrice() → getPaymentTotal() - EntryControllerTest / CustomerRepository(GetQueryBuilderBySearchData)Test / Generator: CustomerStatus::NONACTIVE/ACTIVE → PROVISIONAL/REGULAR Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Symfony のメジャー追従を楽にするため、自コードが Symfony/Doctrine の 非推奨 API を直接呼んだ場合に CI を落とす。weak（発火してもCIを落とさない） から max[direct]=0 へ引き上げる。 - phpunit.xml.dist: 実 PHPUnit CI（unit-test.yml / coverage.yml の phpunit ジョブ）はこの設定を継承するため、ここが強制化の本体。 - coverage.yml: 無効化済み Codeception ジョブの値も整合のため更新。 max[self]=0 は併用しない（StringUtil の E_USER_DEPRECATED は本 Issue の 対象外で、併用すると CI が落ちるため）。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughPHPUnitの非推奨検出を強化し、非推奨APIと顧客ステータス定数を整理しました。PHP 8.4・8.5に対応する入力処理とテスト設定も更新しました。 Changes非推奨検出ゲートと実行手順
エンティティAPIと顧客ステータスの整理
サービス・ユーティリティAPIの削除
PHP互換性と入力処理の更新
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/coverage.yml (1)
259-260: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win無効化された Codeception ジョブではなく、実行されるジョブに設定してください。
この設定は
if: falseの Codeception ジョブ内にあるため、現在のCIでは評価されません。さらに有効化してもcontinue-on-error: trueにより非推奨検出がCIゲートになりません。CI全体のゲートを意図する場合は、PHPUnitジョブへ移動するか、この重複設定を削除してください。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/coverage.yml around lines 259 - 260, Move the SYMFONY_DEPRECATIONS_HELPER setting from the disabled Codeception job to the active PHPUnit job, and remove continue-on-error there so deprecation failures gate CI; if the setting is already present in PHPUnit, delete this duplicate instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/Eccube/Tests/Fixture/Generator.php`:
- Line 912:
createCustomers()のDocBlockに記載されたデフォルトステータスをACTIVEからREGULARへ更新し、実装のCustomerStatus::REGULARと説明を一致させてください。
---
Nitpick comments:
In @.github/workflows/coverage.yml:
- Around line 259-260: Move the SYMFONY_DEPRECATIONS_HELPER setting from the
disabled Codeception job to the active PHPUnit job, and remove continue-on-error
there so deprecation failures gate CI; if the setting is already present in
PHPUnit, delete this duplicate instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 28c17751-aec3-494b-90c9-37192139dbb3
📒 Files selected for processing (19)
.github/workflows/coverage.ymlphpunit.xml.distsrc/Eccube/Entity/Cart.phpsrc/Eccube/Entity/Master/CustomerStatus.phpsrc/Eccube/Entity/Order.phpsrc/Eccube/Entity/OrderItem.phpsrc/Eccube/Entity/Product.phpsrc/Eccube/Entity/ProductClass.phpsrc/Eccube/Resource/functions/trans.phpsrc/Eccube/Service/CartService.phpsrc/Eccube/Service/PurchaseFlow/Processor/TaxProcessor.phpsrc/Eccube/Util/CacheUtil.phptests/Eccube/Tests/Entity/OrderTest.phptests/Eccube/Tests/Fixture/Generator.phptests/Eccube/Tests/Repository/CustomerRepositoryGetQueryBuilderBySearchDataTest.phptests/Eccube/Tests/Repository/CustomerRepositoryTest.phptests/Eccube/Tests/Util/CacheUtilTest.phptests/Eccube/Tests/Web/Admin/Order/EditControllerTest.phptests/Eccube/Tests/Web/EntryControllerTest.php
💤 Files with no reviewable changes (12)
- src/Eccube/Entity/ProductClass.php
- tests/Eccube/Tests/Util/CacheUtilTest.php
- src/Eccube/Entity/OrderItem.php
- src/Eccube/Resource/functions/trans.php
- src/Eccube/Entity/Master/CustomerStatus.php
- src/Eccube/Entity/Order.php
- src/Eccube/Entity/Product.php
- src/Eccube/Entity/Cart.php
- tests/Eccube/Tests/Entity/OrderTest.php
- src/Eccube/Service/CartService.php
- src/Eccube/Util/CacheUtil.php
- src/Eccube/Service/PurchaseFlow/Processor/TaxProcessor.php
| /** @var CustomerStatus $Status */ | ||
| $Status = $options['status'] | ||
| ?? $this->entityManager->find(CustomerStatus::class, CustomerStatus::ACTIVE); | ||
| ?? $this->entityManager->find(CustomerStatus::class, CustomerStatus::REGULAR); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
DocBlockのデフォルト値もREGULARへ更新してください。
実装はCustomerStatus::REGULARをデフォルトにしていますが、createCustomers()のDocBlock（Line 895）は依然としてACTIVEと記載されています。利用者が誤ったステータスを想定しないよう、コメントも同時に更新してください。
修正例
- * `@var` CustomerStatus|null $status 全 Customer に設定する CustomerStatus (デフォルト: ACTIVE)
+ * `@var` CustomerStatus|null $status 全 Customer に設定する CustomerStatus (デフォルト: REGULAR)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/Eccube/Tests/Fixture/Generator.php` at line 912,
createCustomers()のDocBlockに記載されたデフォルトステータスをACTIVEからREGULARへ更新し、実装のCustomerStatus::REGULARと説明を一致させてください。
nanasess
left a comment
nanasess
left a comment
There was a problem hiding this comment.
Part 1（deprecation ゲート）と、削除された public API のうち以下は src/・app/・PHPUnit に呼び出し元が無いことを確認できました。妥当な削除です。
OrderItem::getTaxRuleId/setTaxRuleId/Product::isEnable/ProductClass::isEnable/Cart::getLock/setLock/transChoice()/TaxProcessor::getTaxDisplayType()(protected) /CartService::$cart/Order::getTotalPrice()/CacheUtil::clear()
一方で CustomerStatus::NONACTIVE / ACTIVE の削除は「呼び出し元皆無」ではありません。削除監査が src/ と PHPUnit のみを対象にしており、現行アクティブな Playwright E2E ハーネスを見落としています。修正が必要なため change request とします（詳細はインラインコメント参照）。
🔴 要修正: e2e/setup-fixtures.php が削除された定数を参照
e2e/setup-fixtures.php:50, 56, 140がCustomerStatus::ACTIVE/NONACTIVEを使用。- このファイルは
e2e/global-setup.ts:16がphp e2e/setup-fixtures.phpとして 全 E2E 実行前の globalSetup で必ず実行します（e2e-test.yml/e2e-test-throttling.yml）。
影響:
- 定数削除後、line 50 で
Error: Undefined constant Eccube\Entity\Master\CustomerStatus::ACTIVEが fatal になり、フィクスチャ生成が中断。 global-setup.tsの try/catch が例外を握り潰すため E2E setup は即死しませんが、line 50 以降のテスト会員・仮会員・商品・受注が生成されず、これらに依存する spec（例:front-throttling.spec.tsは globalSetup 生成のテスト用顧客に依存）が失敗します。
修正案（本 PR の Generator.php で既に適用済みの置換と同一）:
- $Status = $entityManager->getRepository(CustomerStatus::class)->find(CustomerStatus::ACTIVE);
+ $Status = $entityManager->getRepository(CustomerStatus::class)->find(CustomerStatus::REGULAR);
...
- $nonActiveStatus = $entityManager->getRepository(CustomerStatus::class)->find(CustomerStatus::NONACTIVE);
+ $nonActiveStatus = $entityManager->getRepository(CustomerStatus::class)->find(CustomerStatus::PROVISIONAL);（ACTIVE=2=REGULAR、NONACTIVE=1=PROVISIONAL で同値です）
補足: codeception/acceptance/_bootstrap.php:158,160 も同定数を参照
CI 無効（coverage.yml if: false）のレガシーですが、削除後は壊れた dead reference として残ります。ついでに同置換するか、少なくとも本 PR の削除判定の対象外である旨を明記すると安全です。
補足: CI（PHPUnit / Playwright）が本 PR で未実行
本 PR は base 4.4 と CONFLICTING 状態のため、main.yml（pull_request トリガ）が一時マージコミットを生成できず起動していません。結果、max[direct]=0 ゲート・E2E ともに CI 上で未検証です。4.4 へ rebase してコンフリクト解消 → main.yml 起動 → 上記 E2E 修正込みで緑を確認する流れを推奨します。
| @@ -28,20 +28,6 @@ | |||
| #[ORM\Cache(usage: 'NONSTRICT_READ_WRITE')] | |||
| class CustomerStatus extends AbstractMasterEntity | |||
There was a problem hiding this comment.
この定数削除（NONACTIVE/ACTIVE）は e2e/setup-fixtures.php:50,56,140 で参照されており、現行の Playwright E2E globalSetup（e2e/global-setup.ts:16 が実行）が Undefined constant で fatal になります。e2e/setup-fixtures.php を ACTIVE→REGULAR / NONACTIVE→PROVISIONAL に修正してください（Generator.php と同一の置換）。レガシーの codeception/acceptance/_bootstrap.php:158,160 も同定数を参照しています。
…on-gate # Conflicts: # .github/workflows/coverage.yml # src/Eccube/Entity/Cart.php # src/Eccube/Entity/Master/CustomerStatus.php # src/Eccube/Entity/Order.php # src/Eccube/Entity/OrderItem.php # src/Eccube/Entity/Product.php # src/Eccube/Entity/ProductClass.php # src/Eccube/Util/CacheUtil.php
レビュー指摘対応。CustomerStatus::NONACTIVE/ACTIVE の削除監査が src/ と PHPUnit のみを対象にしており、以下の現役 E2E ハーネスを見落としていた。 - e2e/setup-fixtures.php … Playwright の globalSetup (e2e/global-setup.ts) が 全 E2E 実行前に必ず実行する。定数削除後は Undefined constant で フィクスチャ生成が中断し、テスト会員・商品・受注が生成されない。 - codeception/acceptance/_bootstrap.php … VAddy スキャン (.github/workflows/vaddy/scan/action.yml) が codecept run -g vaddy を 実行し、@group vaddy の Cest が本ファイルの createCustomer() を使う。 いずれも同値の現行定数へ置換する (ACTIVE=2=REGULAR / NONACTIVE=1=PROVISIONAL) ため挙動は変わらない。あわせて Generator::createCustomers() の DocBlock の デフォルト値表記を実装 (REGULAR) と一致させる。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
deprecation ゲートを有効化する前提として、テスト実行時に発生していた 非推奨警告 11 件を解消する。いずれも挙動は変えない。 src (3 件・null の受け方): - SafeTextmailEscaperExtension: Twig は null の変数もそのまま渡すため エスケーパの引数を ?string で受け、str_replace へ '' を渡す - PluginService::deleteDirs: install()/update() は例外発生時点で未設定の null を渡す仕様のため、null をスキップし phpdoc を string|null に修正 - InvalidItemException: parent::__construct() へ null を渡さない tests (7 件・動的プロパティ生成): - MasterdataTypeTest::$form / LoginHistoryRepository…AdminTest の $Member1・$LoginHistory1-3 / MailControllerTest の $Member・$MailHistories を宣言する。EccubeTestCase::cleanUpProperties() が tearDown で全プロパティに null を代入するため、いずれも nullable で宣言する vendor 由来 (1 件): - HTML パートを持たないメールに対する assertEmailHtmlBodyNotContains() を assertNull($Message->getHtmlBody()) へ変更（MailServiceTest 3 箇所・ WithdrawControllerTest 1 箇所）。元の書き方は symfony/mime 内で str_contains(null, …) の非推奨警告を出すうえ、HTML パートが存在しないため 常に成立する空振りアサーションだった Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…換える (#6933) 当初 SYMFONY_DEPRECATIONS_HELPER を weak から max[direct]=0 に引き上げたが、 この設定は PHPUnit 11 環境では機能しないことが判明したため、PHPUnit 11 が 持つネイティブのゲートへ置き換える。 SYMFONY_DEPRECATIONS_HELPER が効かない理由: - 値を読んで DeprecationErrorHandler を登録するのは symfony/phpunit-bridge の bootstrap.php だが、同ファイルは冒頭で `if (class_exists(PHPUnit\Metadata\Metadata::class)) { return; }` により PHPUnit 10 以上では早期 return する - PHPUnit 10+ 用の SymfonyExtension は ClockMock / DnsMock のみを登録し、 DeprecationErrorHandler は登録しない - CI (unit-test.yml / coverage.yml) は vendor/bin/phpunit を実行するため、 bridge 経由の simple-phpunit も通らない - 実測: 非推奨警告を発生させるテストを vendor/bin/phpunit で実行しても bridge のサマリは出力されず、警告も集計されない 置き換え内容: - SYMFONY_DEPRECATIONS_HELPER を削除（読まれない設定のため） - failOnDeprecation="true" … 非推奨 API の呼び出しで CI を失敗させる - displayDetailsOnTestsThatTriggerDeprecations="true" … 発生箇所を CI ログに出す 前提となる既存の非推奨警告 11 件は先行コミットで解消済みで、フルスイート (2905 テスト) で Deprecations 0 を確認している。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- AGENTS.md / phpunit Skill: 「PHPUnit 11（symfony/phpunit-bridge 経由）」を 「vendor/bin/phpunit を直接実行」に訂正し、bridge の SYMFONY_DEPRECATIONS_HELPER が PHPUnit 10 以上で無効である旨を明記 - AGENTS.md / phpunit Skill / contributing Skill: 実在しない `bin/phpunit` を `vendor/bin/phpunit` に訂正（bin/ には console と template_jp.php のみ） - phpunit Skill「よくある間違い」に 3 項追記 - 未宣言プロパティへの代入は failOnDeprecation で CI red になる - テストのプロパティは nullable 必須（cleanUpProperties が null を代入する） - HTML パートを持たないメールへの assertEmailHtmlBodyNotContains は空振り - contributing Skill「よくある間違い」に 1 項追記 - 非推奨 public API の削除監査は e2e/ と codeception/ も対象に含める Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@nanasess レビューありがとうございます。指摘はすべて実コードで裏取りし対応しました。あわせて Part 1（deprecation ゲート）が実は無効だったことが判明したため、方式を差し替えています。 ① 🔴
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/setup-fixtures.php (1)
20-26: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
boot_env()で読み込んだAPP_ENVを$_SERVER/$_ENVから取得してください。
boot_env()は Symfony Dotenv のままではputenv()を使用せず$_SERVER/$_ENVに値を書き込むため、.envのAPP_ENVをgetenv()だけで参照すると反映されず、Kernel がe2eで起動します。-$appEnv = getenv('APP_ENV') ?: 'e2e'; +$appEnv = $_SERVER['APP_ENV'] + ?? $_ENV['APP_ENV'] + ?? (getenv('APP_ENV') ?: 'e2e');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/setup-fixtures.php` around lines 20 - 26, Update the APP_ENV assignment after boot_env() to read from $_SERVER or $_ENV before falling back to 'e2e', rather than relying only on getenv(). Preserve the existing fallback when neither environment array contains APP_ENV.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@e2e/setup-fixtures.php`:
- Around line 20-26: Update the APP_ENV assignment after boot_env() to read from
$_SERVER or $_ENV before falling back to 'e2e', rather than relying only on
getenv(). Preserve the existing fallback when neither environment array contains
APP_ENV.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a4ff616f-8a1a-493c-a098-576332df8de3
📒 Files selected for processing (16)
.claude/skills/contributing/SKILL.md.claude/skills/phpunit/SKILL.mdAGENTS.mdcodeception/acceptance/_bootstrap.phpe2e/setup-fixtures.phpphpunit.xml.distsrc/Eccube/Entity/Cart.phpsrc/Eccube/Entity/Master/CustomerStatus.phpsrc/Eccube/Entity/Order.phpsrc/Eccube/Entity/OrderItem.phpsrc/Eccube/Entity/Product.phpsrc/Eccube/Entity/ProductClass.phpsrc/Eccube/Service/PluginService.phpsrc/Eccube/Service/PurchaseFlow/InvalidItemException.phpsrc/Eccube/Twig/Extension/SafeTextmailEscaperExtension.phpsrc/Eccube/Util/CacheUtil.php
💤 Files with no reviewable changes (6)
- src/Eccube/Entity/OrderItem.php
- src/Eccube/Entity/Product.php
- src/Eccube/Entity/ProductClass.php
- src/Eccube/Entity/Order.php
- src/Eccube/Util/CacheUtil.php
- src/Eccube/Entity/Master/CustomerStatus.php
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Eccube/Entity/Cart.php
ローカルで除外していたグループ (plugin-service / cache-clear) に 残っていた非推奨警告を解消する。 - PluginServiceTest::testCreateEntityAndTrait: ダミープラグインの Block エンティティに ORM 非マッピングの public $sample を宣言する。 テストが $Block->sample = true と代入していたため PHP 8.2 の 動的プロパティ生成 (deprecated) になっており、failOnDeprecation で CI が失敗していた（12 マトリクス全滅の唯一の原因） - TemplateControllerTest::testChangeTemplateWithEnvOverride: doc-comment の @group cache-clear を #[Group(name: 'cache-clear')] 属性に統一する。 同ファイルの他メソッドは既に属性を使っており取り残しだった。 doc-comment メタデータは PHPUnit 12 で廃止予定 これによりスイート全体の Deprecations / PHPUnit Deprecations がいずれも 0 になった（cache-clear グループは 21 テストのまま変化なし）。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI で PHP 8.4 / 8.5 のジョブのみ失敗していた（8.2 / 8.3 は緑）。原因は failOnDeprecation が PHP 8.4 / 8.5 で新たに非推奨となった呼び出しを検出した ためで、うち 1 件は vendor 内で我々には修正できないものだった。 ゲート範囲を direct のみに限定 (phpunit.xml.dist): PHPUnit 11 の <source> は Symfony の SYMFONY_DEPRECATIONS_HELPER と同じ 分類 (self / direct / indirect) を持つため、当初意図した max[direct]=0 と 同じ範囲になるよう設定する。 - ignoreSelfDeprecations="true" … 自コードの @deprecated 予告 (trigger_error) は対象外 - ignoreIndirectDeprecations="true" … vendor 内部で発生し修正できないものは対象外 (mobiledetect/mobiledetectlib の暗黙 nullable。PHP 8.4 で非推奨) direct な非推奨呼び出しの解消（いずれも挙動不変）: - Order::getMergedProductOrderItems(): 未永続明細では ProductClass の ID が null になり、null を配列キー / オフセットに使っていた（PHP 8.5 で非推奨）。 (string) にキャストする。null は '' として扱われるため挙動は同じ - ProductController::index() / OrderController::index(): $searchData['sortkey'] が null のとき COLUMNS[null] を参照していたため ?? '' で受ける - PluginApiService: curl_close() は PHP 8.0 以降なにもせず 8.5 で非推奨に なったため呼び出しを削除する - CsvExportServiceTest / SjisToUtf8EncodingFilterTest: fgetcsv() の $escape を 現行の既定値 '\\' で明示する（PHP 8.4 で明示指定が必須）。とくに fgetcsvDoesntOccur5cProblem は SJIS 2 バイト目の 0x5c を escape 文字として 誤認しないことの確認なので、既定値を変えずに明示することが重要 なお同ジョブのリトライ 2 回目で失敗する RateLimiterListenerTest 3 件は 1 回目が緑であり、レートリミッタの状態が試行間で残る既知の flaky で本変更とは無関係。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.4 #6937 +/- ##
==========================================
+ Coverage 77.25% 77.27% +0.02%
==========================================
Files 547 547
Lines 27163 27122 -41
==========================================
- Hits 20985 20959 -26
+ Misses 6178 6163 -15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
CI が全 115 チェック成功しました（PHP 8.2 / 8.3 / 8.4 / 8.5 × pgsql / pgsql13 / mysql / sqlite3、Playwright E2E 含む）。前回コメント以降の追加対応を報告します。 CI 1 回目: PHPUnit 全 12 マトリクス失敗 → 原因は 1 件本体スイート（2905 テスト）自体は Failures / Errors 0 で緑でした。ローカルで出ていた 16 件の Failures は 失敗の真因は、ローカルで除外していた
別途ご確認いただきたい点: このテストは あわせて、スイート唯一の残件だった CI 2 回目: PHP 8.4 / 8.5 のみ失敗 → ゲート範囲を direct に限定8.2 / 8.3 は全て緑で、8.4 / 8.5 の 9 ジョブだけが落ちました。 PHPUnit 11 の
そのうえで direct な非推奨呼び出しを解消しました（いずれも挙動不変）。
切り分けた flakyリトライ 2 回目で落ちていた 補足
|
nanasess
left a comment
nanasess
left a comment
There was a problem hiding this comment.
先の change request で指摘した e2e/setup-fixtures.php（および補足の codeception/acceptance/_bootstrap.php）の CustomerStatus::ACTIVE/NONACTIVE 参照が REGULAR/PROVISIONAL へ修正されたことを確認しました。
- ゲートが PHPUnit 11 ネイティブの
failOnDeprecation+ignoreSelfDeprecations/ignoreIndirectDeprecationsに置き換わり、当初意図（direct のみで落とす）を標準機能で表現できています。 - 追加された本体コードの非推奨解消（
OrderController/ProductControllerの??、curl_close()削除、PluginServiceの null ガード、InvalidItemException・SafeTextmailEscaperExtensionの null 対策）はいずれも最小・挙動不変。 - コンフリクト解消により
main.ymlがフル実行され、PHPUnit・dockerbuild(8.2–8.5)・e2e-test 全 suite・e2e-test-throttling が全て pass。ゲートと E2E フィクスチャの両方が実 CI で裏取りされています。
LGTM 👍
| * | ||
| * @deprecated 税率設定は受注作成時に決定するため廃止予定 | ||
| */ | ||
| public function setTaxRuleId(?int $taxRuleId = null): OrderItem |
There was a problem hiding this comment.
この関数は、クーポンプラグインで使用しているようです。
削除する場合は、先にプラグインの修正PRをお願いします。
https://github.com/EC-CUBE/coupon-plugin/blob/112ab3ebc461cf77e2d63af3f59f33273251d392/Service/PurchaseFlow/Processor/CouponProcessor.php#L298
|
追加で見たところ、定期購入プラグインも影響があるかと思います。
|
…on-gate # Conflicts: # tests/Eccube/Tests/Web/Admin/Store/TemplateControllerTest.php
3 participants
概要
Issue #6933 のうち、仕様（実行時の挙動・CSV等の出力・管理/店頭画面・DBスキーマ）を一切変えない範囲で対応する。
@deprecatedpublic API の削除（4.4 の BC 破壊に相乗り）Fixes #6933（の安全に実施できる範囲。据え置き分は後述）
方針
Part 1 — deprecation ゲートを PHPUnit 11 ネイティブの
failOnDeprecationで有効化自コードが非推奨 API を呼んだら CI が失敗するようにし、次期 Symfony（7.4→8）追従を「負債の発掘」から「非推奨を消すだけ」に変えるための早期警告を入れる。
当初は
SYMFONY_DEPRECATIONS_HELPERをweak→max[direct]=0に引き上げる方針だったが、この設定は PHPUnit 11 環境では機能しないことがレビュー中の検証で判明したため、PHPUnit 11 が持つネイティブのゲートへ置き換えた。SYMFONY_DEPRECATIONS_HELPERが効かない理由DeprecationErrorHandlerを登録するのはsymfony/phpunit-bridgeのbootstrap.php（composer のautoload.filesで常時ロードされる）だが、同ファイルは冒頭でif (class_exists(PHPUnit\Metadata\Metadata::class)) { return; }により PHPUnit 10 以上では早期 return するSymfonyExtensionは ClockMock / DnsMock のみを登録し、DeprecationErrorHandlerは登録しない（DeprecationErrorHandlerを参照するのはbootstrap.phpとbin/simple-phpunit.phpだけ）unit-test.yml/coverage.yml）はvendor/bin/phpunitを実行するため、bridge 経由のsimple-phpunitも通らないvendor/bin/phpunitで実行しても bridge のサマリは出力されず、警告も集計されない（SymfonyExtensionを登録しても同じ）置き換え内容（
phpunit.xml.dist）SYMFONY_DEPRECATIONS_HELPERfailOnDeprecation="true"displayDetailsOnTestsThatTriggerDeprecations="true"<source ignoreSelfDeprecations="true" ignoreIndirectDeprecations="true">ゲート範囲を direct に限定した理由
PHPUnit 11 の
<source>は Symfony のSYMFONY_DEPRECATIONS_HELPERと同じ分類を持ちます（IssueTrigger::isDirect()は「first-party/test コードが third-party/PHP の非推奨を呼ぶ」＝Symfony の direct と同義）。当初意図したmax[direct]=0と同じ範囲にするため、次の 2 つを除外しています。trigger_errorする@deprecated予告）max[self]=0を併用しなかった判断と同じmobiledetect/mobiledetectlibの暗黙 nullable 引数。PHP 8.4 で非推奨）これを入れないと PHP 8.4 / 8.5 のジョブが vendor 由来の非推奨で必ず落ちます（8.2 / 8.3 では発生しません）。
coverage.ymlは base の4.4で Codeception カバレッジジョブ自体が削除済み（#6905）のため、本 PR での変更は無くなった。SYMFONY_DEPRECATIONS_HELPERはリポジトリから完全に撤去されている。前提として既存の非推奨警告 19 件を解消（いずれも挙動不変）
PHP 8.2 で検出できた 11 件に加え、CI で PHP 8.4 / 8.5・除外グループ（
plugin-service/cache-clear）で表面化した 8 件も解消している。SafeTextmailEscaperExtension（Twig は null の変数もそのまま渡すためエスケーパ引数を?stringに）/PluginService::deleteDirs（install()/update()が例外発生時点で未設定の null を渡す仕様のため null をスキップ・phpdoc をstring|nullに）/InvalidItemException（parent::__construct()へ null を渡さない）MasterdataTypeTest::$form/LoginHistoryRepository…AdminTestの$Member1・$LoginHistory1-3/MailControllerTestの$Member・$MailHistoriesを宣言。EccubeTestCase::cleanUpProperties()が tearDown で全プロパティに null を代入するため nullable で宣言assertEmailHtmlBodyNotContains()をassertNull($Message->getHtmlBody())に変更（MailServiceTest3 箇所・WithdrawControllerTest1 箇所）。元の書き方はsymfony/mime内でstr_contains(null, …)の警告を出すうえ、HTML パートが存在しないため常に成立する空振りアサーションだったPluginServiceTest::testCreateEntityAndTraitのダミープラグインBlockに ORM 非マッピングのpublic $sampleを宣言。テストが$Block->sample = trueと代入していたため動的プロパティ生成になっていたTemplateControllerTest::testChangeTemplateWithEnvOverride()の@group cache-clearを#[Group]属性へ（同ファイルの他メソッドは既に属性を使用。PHPUnit 12 で廃止予定）Order::getMergedProductOrderItems()（未永続明細でProductClassの ID が null になり null を配列キー / オフセットに使用 →(string)キャスト）/ProductController::index()・OrderController::index()（$searchData['sortkey']が null のときCOLUMNS[null]参照 →?? ''）/PluginApiService（curl_close()は PHP 8.0 以降なにもせず 8.5 で非推奨 → 削除）/fgetcsv()の$escape5 箇所を現行既定値'\\'で明示（PHP 8.4 で明示指定が必須。とくにfgetcsvDoesntOccur5cProblemは SJIS 2 バイト目の 0x5c を escape 文字と誤認しないことの検証なので既定値を変えないことが重要）Part 2 — 削除できるのは「呼び出し元なし／CSV非出力／スキーマ不変」のものだけ
各
@deprecatedpublic API を「①コアに呼び出し元があるか ②CSV 出力に載る保存カラムのアクセサか ③カラム削除マイグレーションを伴うか」で判定し、すべて No のもののみ削除した。削除
OrderItem::getTaxRuleId()/setTaxRuleId()tax_rule_idカラムは残置）Product::isEnable()＋ProductClass::isEnable()Cart::getLock()/setLock()＋$locktransChoice()trans()の別名・呼び出し元皆無TaxProcessor::getTaxDisplayType()CartService::$cartOrder::getTotalPrice()getPaymentTotal()の別名CacheUtil::clear()clearCache()（専用テストごと削除）CustomerStatus::NONACTIVE/ACTIVEPROVISIONAL/REGULARと同値の別名据え置き（削除すると仕様が変わるため本 PR では触らない）
Order::getTax/setTax・ItemHolderInterface::setTax・Order::getDiscount＋tax/discountカラム … CSV 出力「税金」「値引き」列を支える現役。@deprecated予告は残置。BaseInfo::getPhpPath/setPhpPath＋php_pathカラム … 削除にマイグレーション（スキーマ変更）を伴う。併せて訂正したドキュメント
SYMFONY_DEPRECATIONS_HELPERが効かない原因調査の過程で、AGENTS.mdと Skill の記述に誤りがあることが判明したため訂正した。symfony/phpunit-bridge経由）」→「vendor/bin/phpunitを直接実行」（bridge の非推奨検出が PHPUnit 10 以上で無効である旨も明記）bin/phpunit→vendor/bin/phpunit（bin/にはconsoleとtemplate_jp.phpのみ）テスト
CacheUtilTest、OrderTest::testGetTotalPriceEditControllerTest:getTotalPrice()→getPaymentTotal()EntryControllerTest/CustomerRepository(GetQueryBuilderBySearchData)Test/Generator:CustomerStatus::NONACTIVE/ACTIVE→PROVISIONAL/REGULARe2e/setup-fixtures.php（Playwright のglobal-setup.tsが全 E2E 前に実行）codeception/acceptance/_bootstrap.php（VAddy スキャンがcodecept run -g vaddyで実行し、@group vaddyの Cest がcreateCustomer()を使う）RateLimiterListenerTest3 件は 1 回目が緑で、レートリミッタの状態が試行間に残る既知の flaky。failOnPhpunitDeprecationは有効化していない（PHPUnit 自身の非推奨は現時点 0 件だが、マトリクス差で表面化するリスクを避けた）。互換性
failOnDeprecation="true"の有効化により、プラグインのテストが非推奨 API を呼んでいる場合は CI が失敗するようになる。🤖 Generated with Claude Code
Summary by CodeRabbit
破壊的変更
バグ修正
テスト