improvement: deprecation ゲート有効化＋未使用の非推奨 public API 削除 (#6933)#6937
Open
ttokoro20240902 wants to merge 3 commits into
Open
improvement: deprecation ゲート有効化＋未使用の非推奨 public API 削除 (#6933)#6937ttokoro20240902 wants to merge 3 commits into
ttokoro20240902 wants to merge 3 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>
📝 WalkthroughWalkthroughSymfonyの非推奨検出設定を強化し、複数の非推奨public APIを削除しました。CustomerStatusの定数名と関連テスト、受注テストのAPI参照も更新されています。 Changes非推奨検出ゲート
エンティティAPI削除
サービス・ユーティリティ整理
会員ステータスとテスト更新
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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と説明を一致させてください。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
Issue #6933 のうち、仕様（実行時の挙動・CSV等の出力・管理/店頭画面・DBスキーマ）を一切変えない範囲で対応する。
@deprecatedpublic API の削除（4.4 の BC 破壊に相乗り）Fixes #6933（の安全に実施できる範囲。据え置き分は後述）
方針
Part 1 —
SYMFONY_DEPRECATIONS_HELPERをmax[direct]=0へweak（発火してもCIを落とさない）からmax[direct]=0（自コードが Symfony/Doctrine の非推奨 API を直接呼んだら CI 失敗）へ引き上げる。次期 Symfony（7.4→8）追従を「負債の発掘」から「非推奨を消すだけ」に変えるための早期警告。phpunit.xml.dist… 実 PHPUnit CI（unit-test.yml/coverage.ymlの phpunit ジョブ）はこの設定を継承するため、ここが強制化の本体。coverage.yml… 無効化済み Codeception ジョブの値も整合のため更新。max[self]=0は併用しない。StringUtilのE_USER_DEPRECATEDは本 Issue の対象外で、併用すると CI が落ちるためdirect単独とした。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()の別名（self 発火源）CacheUtil::clear()clearCache()（専用テストごと削除）CustomerStatus::NONACTIVE/ACTIVEPROVISIONAL/REGULARと同値の別名据え置き（削除すると仕様が変わるため本 PR では触らない）
Order::getTax/setTax・ItemHolderInterface::setTax・Order::getDiscount＋tax/discountカラム … CSV 出力「税金」「値引き」列を支える現役。@deprecated予告は残置。BaseInfo::getPhpPath/setPhpPath＋php_pathカラム … 削除にマイグレーション（スキーマ変更）を伴う。テスト
CacheUtilTest、OrderTest::testGetTotalPriceEditControllerTest:getTotalPrice()→getPaymentTotal()EntryControllerTest/CustomerRepository(GetQueryBuilderBySearchData)Test/Generator:CustomerStatus::NONACTIVE/ACTIVE→PROVISIONAL/REGULARmax[direct]=0実適用で direct 発火 0 を確認。互換性
🤖 Generated with Claude Code
Summary by CodeRabbit
変更
品質改善