【ご意見ください】PHP8での Warning 解消方針について

nanasess commented 4 hours ago

refs #999

現在、 PHP8 で発生している Warning も解消していきたいのですが、商品一覧ページを対応してみたところ、以下のパッチのようなコードになってしまいます。特に $arrErr の未定義要素のチェックに三項演算子を使用していますが、もう少し簡潔な記述ができないものかと悩んでおります。良いアイディアがありましたらご意見ください🙇‍♂️

diff --git a/data/Smarty/templates/default/products/list.tpl b/data/Smarty/templates/default/products/list.tpl
index 79300d2b6..03cc18850 100644
--- a/data/Smarty/templates/default/products/list.tpl
+++ b/data/Smarty/templates/default/products/list.tpl
@@ -139,7 +139,7 @@
         <!--{/if}-->

         <!--{assign var=id value=$arrProduct.product_id}-->
-        <!--{assign var=arrErr value=$arrProduct.arrErr}-->
+        <!--{assign var=arrErr value=$arrProduct.arrErr|isset ? $arrProduct.arrErr : []}-->
         <!--▼商品-->
         <form name="product_form<!--{$id|h}-->" action="?">
             <input type="hidden" name="<!--{$smarty.const.TRANSACTION_ID_NAME}-->" value="<!--{$transactionid}-->" />
@@ -204,10 +204,10 @@
                                             <!--▼規格1-->
                                             <dt><!--{$tpl_class_name1[$id]|h}-->：</dt>
                                             <dd>
-                                                <select name="classcategory_id1" style="<!--{$arrErr.classcategory_id1|sfGetErrorColor}-->">
-                                                    <!--{html_options options=$arrClassCat1[$id] selected=$arrProduct.classcategory_id1}-->
+                                                <select name="classcategory_id1" style="<!--{sfGetErrorColor($arrErr.classcategory_id1|isset ? $arrErr.classcategory_id1 : '')}-->">
+                                                    <!--{html_options options=$arrClassCat1[$id] selected=$arrProduct.classcategory_id1|isset ? $arrProduct.classcategory_id1 : null}-->
                                                 </select>
-                                                <!--{if $arrErr.classcategory_id1 != ""}-->
+                                                <!--{if $arrErr.classcategory_id1|isset}-->
                                                     <p class="attention">※ <!--{$tpl_class_name1[$id]}-->を入力して下さい。</p>
                                                 <!--{/if}-->
                                             </dd>
@@ -218,9 +218,9 @@
                                             <!--▼規格2-->
                                             <dt><!--{$tpl_class_name2[$id]|h}-->：</dt>
                                             <dd>
-                                                <select name="classcategory_id2" style="<!--{$arrErr.classcategory_id2|sfGetErrorColor}-->">
+                                                <select name="classcategory_id2" style="<!--{sfGetErrorColor($arrErr.classcategory_id2|isset ? $arrErr.classcategory_id2 : '')}-->">
                                                 </select>
-                                                <!--{if $arrErr.classcategory_id2 != ""}-->
+                                                <!--{if $arrErr.classcategory_id2|isset}-->
                                                     <p class="attention">※ <!--{$tpl_class_name2[$id]}-->を入力して下さい。</p>
                                                 <!--{/if}-->
                                             </dd>
@@ -231,8 +231,8 @@
                             <!--{/if}-->
                             <div class="cartin clearfix">
                                 <div class="quantity">
-                                    数量：<input type="text" name="quantity" class="box" value="<!--{$arrProduct.quantity|default:1|h}-->" maxlength="<!--{$smarty.const.INT_LEN}-->" style="<!--{$arrErr.quantity|sfGetErrorColor}-->" />
-                                    <!--{if $arrErr.quantity != ""}-->
+                                    数量：<input type="text" name="quantity" class="box" value="<!--{$arrProduct.quantity|default:1|h}-->" maxlength="<!--{$smarty.const.INT_LEN}-->" style="<!--{$arrErr.quantity|isset ? $arrErr.quantity : null|sfGetErrorColor}-->" />
+                                    <!--{if $arrErr.quantity|isset}-->
                                         <br /><span class="attention"><!--{$arrErr.quantity}--></span>
                                     <!--{/if}-->
                                 </div>
diff --git a/data/class/SC_CartSession.php b/data/class/SC_CartSession.php
index 645d99764..7e938a0d0 100644
--- a/data/class/SC_CartSession.php
+++ b/data/class/SC_CartSession.php
@@ -157,6 +157,22 @@ class SC_CartSession
                     }
                 }
             }
+        } else {
+            $this->cartSession[$product_type_id] = [];
+        }
+
+        // カート内商品の最大要素番号までの要素が存在しない場合、要素を追加しておく
+        for ($i = 0; $i <= $max; $i++) {
+            if (!array_key_exists($i, $this->cartSession[$product_type_id])) {
+                $this->cartSession[$product_type_id][$i] = [
+                    'price' => 0,
+                    'quantity' => 0,
+                    'productsClass' => [
+                        'product_id' => null,
+                        'product_class_id' => null,
+                    ],
+                ];
+            }
         }

         return $max;
diff --git a/data/class/helper/SC_Helper_Maker.php b/data/class/helper/SC_Helper_Maker.php
index 3370ca6df..2f989797b 100644
--- a/data/class/helper/SC_Helper_Maker.php
+++ b/data/class/helper/SC_Helper_Maker.php
@@ -45,9 +45,9 @@ class SC_Helper_Maker
         if (!$has_deleted) {
             $where .= ' AND del_flg = 0';
         }
-        $arrRet = $objQuery->select('*', 'dtb_maker', $where, [$maker_id]);
+        $arrRet = $objQuery->getRow('*', 'dtb_maker', $where, [$maker_id]);

-        return $arrRet[0];
+        return $arrRet;
     }

     /**
diff --git a/data/class/pages/products/LC_Page_Products_List.php b/data/class/pages/products/LC_Page_Products_List.php
index c97724b12..6d740a387 100644
--- a/data/class/pages/products/LC_Page_Products_List.php
+++ b/data/class/pages/products/LC_Page_Products_List.php
@@ -338,7 +338,7 @@ class LC_Page_Products_List extends LC_Page_Ex
         if (strlen($arrSearchData['maker_id']) > 0) {
             $objMaker = new SC_Helper_Maker_Ex();
             $maker = $objMaker->getMaker($arrSearchData['maker_id']);
-            $arrSearch['maker'] = $maker['name'];
+            $arrSearch['maker'] = isset($maker['name']) ? $maker['name'] : null;
         }

         // 商品名検索条件

nanasess commented 4 hours ago

個別の案件では、  でエラーの有無をチェックせずに、<span class="attention"></span> が直接埋め込まれているコードが大量にあったため、以下のような modifier を作成しました

/**
 * $arrErrに対してのみ利用する想定のmodifier
 * $arrErrの中から指定したKeyのエラーを探して返す
 *
 * php8でのWarning対応のために作成。
 * エラーが発生したときのみ$arrErrにKeyが設定される設計なので、テンプレート側でKeyが含まれない可能性を考慮する必要がある。
 * 用途を$arrErrに限定することを明示するためにfind_errというmodifier名にしている。
 *
 * @param ?array $arrErr $arrErr配列。 nullのケースがありうる。
 * @param string $key 探したいエラーのKey。
 *
 * @return string Keyに該当するValueを返す。Keyが存在しない場合はnullを返す。
 *
 * @example <span class="attention"><!--{$arrErr|find_err:"shop_kana"}--></span>
 */
function smarty_modifier_find_err(?array $arrErr, string $key)
{
    if (!is_array($arrErr)) {
        return null;
    }

    return $arrErr[$key] ?? null;
}

これなら簡潔に書けて正規表現で一括置換もできるので便利なのですが、 modifier の名前がいまいちなのと、直感的に何をしているか解りづらいので却下かなぁと。

nanasess commented 3 hours ago

|default を使えばずいぶん簡潔にできそう。 $arrErr が null の場合があるので、そこだけ PHP で空の配列を作成しておく必要がありそう。

thx: https://x.com/tadsan/status/1850198026817823213

diff --git a/data/Smarty/templates/default/products/list.tpl b/data/Smarty/templates/default/products/list.tpl
index 79300d2b6..24deaeae6 100644
--- a/data/Smarty/templates/default/products/list.tpl
+++ b/data/Smarty/templates/default/products/list.tpl
@@ -139,7 +139,7 @@
         <!--{/if}-->

         <!--{assign var=id value=$arrProduct.product_id}-->
-        <!--{assign var=arrErr value=$arrProduct.arrErr}-->
+        <!--{assign var=arrErr value=$arrProduct.arrErr|default:[]}-->
         <!--▼商品-->
         <form name="product_form<!--{$id|h}-->" action="?">
             <input type="hidden" name="<!--{$smarty.const.TRANSACTION_ID_NAME}-->" value="<!--{$transactionid}-->" />
@@ -204,10 +204,10 @@
                                             <!--▼規格1-->
                                             <dt><!--{$tpl_class_name1[$id]|h}-->：</dt>
                                             <dd>
-                                                <select name="classcategory_id1" style="<!--{$arrErr.classcategory_id1|sfGetErrorColor}-->">
-                                                    <!--{html_options options=$arrClassCat1[$id] selected=$arrProduct.classcategory_id1}-->
+                                                <select name="classcategory_id1" style="<!--{sfGetErrorColor($arrErr.classcategory_id1|default:'')}-->">
+                                                    <!--{html_options options=$arrClassCat1[$id] selected=$arrProduct.classcategory_id1|default:''}-->
                                                 </select>
-                                                <!--{if $arrErr.classcategory_id1 != ""}-->
+                                                <!--{if $arrErr.classcategory_id1|isset}-->
                                                     <p class="attention">※ <!--{$tpl_class_name1[$id]}-->を入力して下さい。</p>
                                                 <!--{/if}-->
                                             </dd>
@@ -218,9 +218,9 @@
                                             <!--▼規格2-->
                                             <dt><!--{$tpl_class_name2[$id]|h}-->：</dt>
                                             <dd>
-                                                <select name="classcategory_id2" style="<!--{$arrErr.classcategory_id2|sfGetErrorColor}-->">
+                                                <select name="classcategory_id2" style="<!--{sfGetErrorColor($arrErr.classcategory_id2|default:'')}-->">
                                                 </select>
-                                                <!--{if $arrErr.classcategory_id2 != ""}-->
+                                                <!--{if $arrErr.classcategory_id2|isset}-->
                                                     <p class="attention">※ <!--{$tpl_class_name2[$id]}-->を入力して下さい。</p>
                                                 <!--{/if}-->
                                             </dd>
@@ -231,8 +231,8 @@
                             <!--{/if}-->
                             <div class="cartin clearfix">
                                 <div class="quantity">
-                                    数量：<input type="text" name="quantity" class="box" value="<!--{$arrProduct.quantity|default:1|h}-->" maxlength="<!--{$smarty.const.INT_LEN}-->" style="<!--{$arrErr.quantity|sfGetErrorColor}-->" />
-                                    <!--{if $arrErr.quantity != ""}-->
+                                    数量：<input type="text" name="quantity" class="box" value="<!--{$arrProduct.quantity|default:1|h}-->" maxlength="<!--{$smarty.const.INT_LEN}-->" style="<!--{sfGetErrorColor($arrErr.quantity|default:'')}-->" />
+                                    <!--{if $arrErr.quantity|isset}-->
                                         <br /><span class="attention"><!--{$arrErr.quantity}--></span>
                                     <!--{/if}-->
                                 </div>
diff --git a/data/class/SC_CartSession.php b/data/class/SC_CartSession.php
index 645d99764..7e938a0d0 100644
--- a/data/class/SC_CartSession.php
+++ b/data/class/SC_CartSession.php
@@ -157,6 +157,22 @@ class SC_CartSession
                     }
                 }
             }
+        } else {
+            $this->cartSession[$product_type_id] = [];
+        }
+
+        // カート内商品の最大要素番号までの要素が存在しない場合、要素を追加しておく
+        for ($i = 0; $i <= $max; $i++) {
+            if (!array_key_exists($i, $this->cartSession[$product_type_id])) {
+                $this->cartSession[$product_type_id][$i] = [
+                    'price' => 0,
+                    'quantity' => 0,
+                    'productsClass' => [
+                        'product_id' => null,
+                        'product_class_id' => null,
+                    ],
+                ];
+            }
         }

         return $max;
diff --git a/data/class/helper/SC_Helper_Maker.php b/data/class/helper/SC_Helper_Maker.php
index 3370ca6df..2f989797b 100644
--- a/data/class/helper/SC_Helper_Maker.php
+++ b/data/class/helper/SC_Helper_Maker.php
@@ -45,9 +45,9 @@ class SC_Helper_Maker
         if (!$has_deleted) {
             $where .= ' AND del_flg = 0';
         }
-        $arrRet = $objQuery->select('*', 'dtb_maker', $where, [$maker_id]);
+        $arrRet = $objQuery->getRow('*', 'dtb_maker', $where, [$maker_id]);

-        return $arrRet[0];
+        return $arrRet;
     }

     /**
diff --git a/data/class/pages/products/LC_Page_Products_List.php b/data/class/pages/products/LC_Page_Products_List.php
index c97724b12..6d740a387 100644
--- a/data/class/pages/products/LC_Page_Products_List.php
+++ b/data/class/pages/products/LC_Page_Products_List.php
@@ -338,7 +338,7 @@ class LC_Page_Products_List extends LC_Page_Ex
         if (strlen($arrSearchData['maker_id']) > 0) {
             $objMaker = new SC_Helper_Maker_Ex();
             $maker = $objMaker->getMaker($arrSearchData['maker_id']);
-            $arrSearch['maker'] = $maker['name'];
+            $arrSearch['maker'] = isset($maker['name']) ? $maker['name'] : null;
         }

         // 商品名検索条件

seasoftjapan commented 3 hours ago

$arrSearch['maker'] = isset($maker['name']) ? $maker['name'] : null; は、Null 合体演算子で書けません?

seasoftjapan commented 3 hours ago

$arrErr.quantity とかは、Smarty でなんとかして欲しい気もしますが、そんな動きは無いですかね・・・

EC-CUBE / ec-cube2

【ご意見ください】PHP8での Warning 解消方針について #1045