Vypnutí chyby při přístupu k nedefinovanému prvku pole

Školení, která pořádám

Změna chyby při přístupu k nedefinovanému prvku pole z E_NOTICE na E_WARNING v PHP 8 se může směle zařadit do známého PHP: a fractal of bad design. Původní nešťastný návrh, kdy se přístup k nedefinovanému prvku pole považuje za hodný upozornění, zalepuje něčím ještě horším, co rozbíjí programy perfektně funkční v PHP < 8 bez smysluplného důvodu.

Nejprve proč je nešťastné upozorňovat na přístup k nedefinovanému prvku pole: Pole jsou v PHP typicky poměrně dynamické – když napíšu $_GET["select"] == "a" a v $_GET mi žádné "select" nepřijde, tak to není žádný problém. Přístup k nedefinovaným prvkům pole má jasně dané chování – vrátí null. Není to jako v dřevních dobách jazyka C, kde to vrátilo nějakou náhodnou hodnotu, která na tom místě v paměti byla zapsaná dříve. Není žádný důvod, proč kód měnit na isset($_GET["select"]) && $_GET["select"] == "a". Možná si řeknete „obrana proti překlepům“, ale co mi zabrání napsat isset($_GET["selet"]) && $_GET["selet"] == "a" a udělat ten stejný překlep dvakrát?

U jiné ukázky kódu je lpění na ošetřování přístupu k nedefinovaným prvkům pole ještě křiklavější:

<?php
$groups = array();
foreach ($rows as $row) {
    $groups[$row->group]++;
}
?>

Kód mi do pole spočte, kolik řádek z každé skupiny máme. Je krátký, čitelný, přehledný, těžko se v něm udělá chyba. Srovnejte to s tímto monstrem, pokud ošetřuji přístup k nedefinovanému indexu $groups:

<?php
$groups = array();
foreach ($rows as $row) {
    if (!array_key_exists($row->group, $groups)) {
        $groups[$row->group] = 0;
    } else {
        $groups[$row->group]++;
    }
}
?>

Kód je mnohem hůř čitelný, stejné proměnné opakuje třikrát a je náchylnější k chybě. Schválně jsem tam jednu chybu udělal, zkuste ji najít. Phabricator zmírňuje utrpení přidáním funkce idx, s kterou je kód snesitelnější (ale pořád ne tak elegantní jako první verze):

<?php
$groups = array();
foreach ($rows as $row) {
    $groups[$row->group] = idx($groups, $row->group, 0) + 1;
}
?>

Dalším kouskem fraktálu špatného návrhu je i to, že taková funkce není přímo součástí PHP. A aby ten fraktál nebyl malý, tak přidám ještě jednu verzi téhož, která perfektně funguje bez jakýchkoliv upozornění, i když má potenciálně ta stejná rizika jako verze první:

<?php
foreach ($rows as $row) {
    $groups[$row->group][] = 1;
}
$groups = array_map('count', $groups);
?>

Proč to funguje? Přidání prvku pole do neinicilizované hodnoty PHP z nějakého důvodu za sebemenší chybu nepovažuje, i když je zcela obdobné třeba oné inkrementaci. Všimněte si, že pokud je pole $rows neprázdné, tak ani nemusím inicializovat $groups, což osobně považuji za vážnou chybu. PHP přesto ani necekne. Stejně tak necekne ani u tohoto moderního kódu (který má navíc ten problém, že není zpětně kompatibilní):

<?php
$groups = array();
foreach ($rows as $row) {
    $groups[$row->group] = ($grups[$row->group] ?? 0) + 1;
}
?>

Všimněte si překlepu $grups. Všechny chyby zapnuté, moderní kód a PHP na překlep stejně neupozorní, protože operátor ?? stejně jako isset kontroluje, jestli je nastavený celý výraz a u první nenastavené proměnné prostě skončí. Může se to zdát užitečné, ale je to další část fraktálu.

Jak bych to navrhl já

Jak bych chování navrhl já (s výhodou pohledu do budoucnosti)? V první řadě bych oddělil dynamická pole od statických objektů. V PHP jsou pole i objekty uchovávány prakticky stejně, to ale ještě neznamená, že se navenek musí i stejně chovat. Práci s nedeklarovanými vlastnostmi objektu jsem vždycky považoval za prasárnu kromě případů, kdy se k tomu třída přihlásí metodami __get, __set a spol. Nemusely by pak vznikat obezličky typu Nette\SmartObject, které to řeší za PHP. Pokud chci vytvořit objekt bez třídy, můžu použít (object) array('a' => 1), nabízel by se prostor pro syntaxi object('a' => 1), případně zkráceně {'a' => 1} (podobně jako časem vzniklo ['a' => 1]).

Druhá věc je, že pokud nepoužívám dynamické vlastnosti jazyka typu proměnné proměnné ($$a) a dynamické vlastnosti objektů ($this->$a), tak se kontrola může dělat klidně už při kompilaci a nemusí se čekat až do spuštění. Tady PHP zastupuje např. PHPStan.

U polí, kde je struktura pevná a přístup k neexistujícímu prvku opravdu znamená chybu, bych doporučil pracovat raději s objektem, jde např. o funkce vracející řádky z databáze (mysqli_fetch_object místo mysqli_fetch_assoc).

Práce s nedeklarovanými proměnnými je vážná chyba a není žádný důvod ji brát stejně jako práci s neexistujícími prvky pole. Jediný historický důvod je opět stejná reprezentace platných proměnných a prvků pole.

Hlasování

V RFC se jako podklad pro hlasování píše: „Proměnné a vlastnosti objektů jsou typicky staticky známé, což neplatí pro prvky pole, které jsou často dynamické. Některé jazyky (jako JavaScript) přístup k nedefinovanému prvku pole za chybu vůbec nepovažují a projdou bez řečí. Někteří programátoři by tento styl chtěli používat i v PHP a uvítali by, když by tato chyba šla snadno potlačit.“ Hlasování pak dopadlo nejtěsnějším možným výsledkem 42:21 (je potřeba 2/3 hlasů). Žádná možnost snadného potlačení této chyby (třeba ve formě konfigurační direktivy) bohužel nevznikla.

Jak chybu vypnout

Chybu lze vypnout nastavením vlastního ovladače chyb:

<?php
function mute_array_errors($errno, $errstr) {
    return !!preg_match('~^(Trying to access array offset on value of type null|Undefined array key)~', $errstr);
}
set_error_handler('mute_array_errors', E_WARNING);
?>

Chyba nemá jednoznačný kód, takže ji můžeme filtrovat leda tak podle textu chybové hlášky. Další prasárnička…

Za desítky let, co PHP používám, mi tato chyba ničím nepomohla. Nikdy jsem si neřekl: „Sakryš, když bych měl zapnutou tuhle chybu, tak jsem si mohl ušetřit několik hodin ladění.“ Vždycky vedla akorát ke kódu po všech stránkách horšímu. Mnohem lepší službu udělala statická kontrola v podobě PHPStanu nebo dříve aspoň mého php-initialized.

Jakub Vrána, Řešení problému, 1.1.2021, diskuse: 26 (nové: 0)

Diskuse

ikona Jakub Bouček:

Jakube, rozumím, ale souhlasím jen s částí článku, zejména pak absenci unikátního kódu chyby, což by už nikdy nemělo v refaktorované části projektu vzniknout. Ale nesouhlasím s tím že by ta chyba byla zbytečná. Naopak, prakticky v každé aplikaci, která mi přišla do ruky, která pracovala alespoň trochu dynamickou strukturou dat, byl vágní přístup k datům v poli příčinou chyby, kterou bylo těžké odhalit, zejména proto, že příčina chyby vznikla jinde, než se pak projevila, protože nepozorovaně probulala aplikaci. Přístup k nedefinovanému prvku v poli je velmi často příčinou závažných chyb, včetně bezpečnostních. Nepochopil jsem, co ti vadí na operátoru null-coalesce. On řeší právě to, že v kódu vymezuje stavy, kdy je přístup k nedefinovanému prvku pole vědomý a zřetelně se tím vymezuje od kódu, který očekává, že klíč bude existovat.

ikona Jakub Vrána OpenID:

Vadí mi na něm to, že neodhalí chybu, před níž bych měl být nutností jeho použití chráněn.

Jan Škrášek:

"Srovnejte to s tímto monstrem" - to je ale nefer. To je workaround (sice) dostupny i ve starych verzich PHP, coz chapu, ze potrebujes, ale to uz je holt cena, kterou platis za jejich support. V PHP 8 (respektive od 7.4) bys to mel napsat spis takto:

<?php
    $groups
[$row->group] ??= 0;
   
$groups[$row->group]++;
?>

Takovy kod je sice lehce delsi, zato podle me vyrazne snadneji pochopitelnejsi. Je jasne, co je vychozi hodnota. Je jasne, ze nespolehame na to, ze null++ je najednou 1. A hlavne se vyhyba potlacovani chyb (notice) a nenuti me premyslet, co to potencialne muze vyhodit.

ikona Jakub Vrána OpenID:

Vždyť obdoba tohoto zápisu je v článku taky se všemi jejími neduhami. Příště prosím před komentováním dočíst článek do konce.

Jan Škrášek:

Když to porovnáváš v počtu znaků a možnosti přehlédnutí přeplepu, tak je to jednoznačně jiné a (nej)lepší řešení. Já jsem článek dočetl a dokonce komentoval po druhém čtení. Ani nevím, co ti poradit pro příště.

ikona Jakub Vrána OpenID:

Za tu poznámku o dočtení do konce se omlouvám. Uznávám, že je tato varianta jiná bez nevýhody, kterou má v článku popsané použití ??.

pavel:

Přiznám se, že ten kód, který označujete za "krátký, čitelný, přehledný", mi přijde spíš jako s prominutím "prasárna":

<?php
$groups
= array();
foreach (
$rows as $row) {
   
$groups[$row->group]++;
}
?>

Jak vůbec víte, že po prvním průchodu bude výsledkem toho operátoru "++" hodnota 1? To zjistíte, jen tak, že to vyzkoušíte. Nic přehledného mi na tom nepřijde.

Osobně mi tohle přijde o dost přehlednější:

<?php
$groups
= [];
foreach (
$rows as $row) {
    if (!isset(
$groups[$row->group])) {
   
$groups[$row->group] = 0;
    }
   
$groups[$row->group]++;
}
?>
(isset nebo array_key_exists je v tomto případě jedno)

ikona Jakub Vrána OpenID:

Jako že 0 + 1 = 1 zjistím jenom tak, že to vyzkouším? To snad ne. Nebo že když v dynamicky typovaném jazyce použiji null jako číslo, tak se použije 0, to taky musím vyzkoušet? To snad taky ne. Jde o jasně definované, dokumentované chování: https://www.php.net/language.types.integer#….casting-from-null

Kdo ho nezná, měl by si nejprve doplnit základy.

A rozdíl mezi isset a array_key_exists samozřejmě je, dále v článku je taky popsaný. Když použiji <?php isset($grups[$row->group]) ?> (překlep $grups), tak mám chybný kód, před kterým mě ani snaha ošetřit tuto zbytečnou chybu nezachránila.

pavel:

> když v dynamicky typovaném jazyce použiji null jako číslo, tak se použije 0, to taky musím vyzkoušet?

Pokud se bavíme o php a předpokládám, že ano, tak samozřejmě, že to musíte vyzkoušet. Že se místo null v php vždy použije nula a že "je to jasně dokumentované"? No to snad ne :)

Hádejte, co vypíše tohle:

<?php

$a
= [];
$a['a']++;
$a['b']--;
++
$a['c'];
--
$a['d'];
$a['e'] += 1;
$a['f'] -= 1;

var_dump($a);

?>

Marián Černý:

Mne osobne je jedno, či je to E_NOTICE alebo E_WARNING. Obe sú podľa mňa chyby. Defaultne mám upozorňovanie na chybu zapnuté aj u E_NOTICE. Potom by som ten kód napísal asi takto (pridal by som potlačenie chýb cez @):

<?php
$groups
= [];
foreach (
$rows as $row) {
    @
$groups[$row->group]++;
}
?>

alebo prípadne ešte takto:

<?php
$groups
= [];
foreach (
$rows as $row) {
   
$group = $row->group;
    @
$groups[$group]++;
}
?>

Mimochodom, v prípade, že Vám E_NOTICE nevadí, tak prečo v prvom príklade nevynecháte ešte aj to vytvorenie prázdneho poľa? Viď:

<?php
foreach ($rows as $row) {
    @
$groups[$row->group]++;
}
?>

Marián Černý:

(Ospravedlňujem sa, v tom poslednom príklade to malo byť bez @.)

ikona Jakub Vrána OpenID:

Zavináč je ještě horší než kontrola pomocí isset – potlačí úplně všechny chyby, což třeba v případě netriviálního indexu může být i vevnitř volání nějaké funkce. Opravdu není řešení všude narvat zavináče.

A myšlenka, že není chyba jako chyba, je kostrou celého článku. Neinicializovaná proměnná obvykle značí vážný problém, na neinicializovaný prvek pole je upozorňovat zbytečné.

Navíc článek si stěžuje hlavně na povýšení této zbytečné chyby na varování v PHP 8. Když se jí chci zbavit vypnutím varování, spláchnu s tím i všechna ostatní potenciálně užitečná varování.

pavel:

One-liner kompatibilní ve všech verzích:

<?php
$groups
= array_count_values(array_column($rows, 'group'));
?>

viz: https://3v4l.org/XiXec

ikona Jakub Vrána OpenID:

Pěkné, ale to samozřejmě s pointou článku nijak nesouvisí. Navíc v PHP 5 (které třeba Adminer pořád podporuje) to nefunguje.

Magdalena Bobromyla:

Jakube,
* Dopustil ses logického klamu zvaného "moving the goldpost"
a psychologické projekce.
* Článek se vztahuje k PHP 8; kdyby's náhodou nevěděl, píšeš o tom hned v první větě. (Plus, v této diskuzi kritizuješ ostatní, aby si článek přečetli; v tomto případě to platí na tebe.)
* To, co s tvým článkem skutečně nesouvisí, je Adminer, který tu bez sebemenšího důvodu vytahuješ. Všimni si té ironie ve tvé licoměrnosti.
Přeju ti hezký den

lubos:

Tato zmena je sucastou dlhodobeho boja medzi dvoma trendami v PHP najma v poslednych 2-3 rokoch:
1/ zachovanie historicky povodnej typovej volnosti (ktora vytvara priestor pre programatorske chyby ale tiez ma na svedomi rozsirenie PHP v minulosti)
2/ a sprisnenie typovosti (strict mode) - po ktorej volaju tvorcovia modernych frameworkov a profi programatori.
Je velmi tazke nastavit hranicu, aby obidva tabory boli happy. Osobne nemam s touto zmenou problem, vzdy kodujem skor defenzivne a osetrujem cez isset(), empty() alebo array_key_exists(). A debugger mi vzdy zastane a upozorni na chybu (nusphere, nie xdebug). Povodna notice vsak podla mna postacovala.

koubel:

Přesně tak. Prostě Jakub jednoznačně pláče nad špatným hrobem a to doslova. Majorita dnes vyvíjí se zapnutými E_NOTICE, což ostatně doporučuje i manuál a asi to zapínají všichni i na produkci. Prostě chtějí mít kód bez jakýchkoli chyb.

Je to krásně vidět podle toho hlasování, stará garda tam chtěla nechat ten notice.

Ty argumenty Jakuba jsou v pohodě, ale je holt součástí té menšiny.

Ostatně Jakube, mohl jsi hlasovat, příště lépe sleduj co se v PHP děje. Pak si s Rasmusem zanadáváte někdy v létě u piva a za rok a půl už budeš mít na nový rok pohodu.

Sice už PHP léta nesleduji, ale jak tak koukám, tak je dost možné že v PHP 10 bude jen jeden typ chyb, nebo všechno dokonce vyhodí výjimku ala Java/C#.
Zdarec hoši.

ikona Jakub Vrána OpenID:

Hlasovat jsem nemohl, nejsem committer do php-src.

David Grudl:

Tohle je hodně nefér trefa do PHP 8 :)

Přitom se vlastně skoro nic nezměnilo, jen detail, který žádný běžný program nerozbije. Jen byla změněna kategorie u jedné chyby. Souhlasím s tím, že ta změna byla zbytečná, a to proto, že nevidím smysl rozlišovat mezi E_WARNING a E_NOTICE.

Že jazyk upozorňuje na přístup k neexistujícímu prvku pole je jeho legitimní vlastnost a hádám, že naprostou většinou vývojářů považovaná za správnou. Dělal to tak vždycky a dělá to tak dál.

Chyba se v PHP před verzí 8 filtrovala úplně stejným způsobem, jako nyní, jen se změní druhý parametr v set_error_handler. Proč je to najednou takový problém?

BTW  operátor ?? nekontroluje úplně celý výraz, třeba $a[$b] ?? 1 vyhodí varování, pokud proměnná $b neexistuje.

ikona Jakub Vrána OpenID:

Problém to je proto, že vypnout všechny E_NOTICE žádnou velkou škodu neudělalo, ale vypnout všechny E_WARNING ano.

Jan Barášek:

A není lepší psát kód tak, aby se nemusela vypínat žádná chybová hlášení a kód běžel v maximálně striktním režimu, kde má vývojář kontrolu nad každou věcí? :)

ikona Jakub Vrána OpenID:

Na několika příkladech v článku ukazuji, že to vede ke krkolomnějšímu a hůře čitelnému kódu, který nás od skutečných chyb stejně neuchrání.

v6ak:

Myslím, že smysl mají někdy oba přístupy – jak hodit chybu, tak vrátit nějakou prázdnou hodnotu. Ale ono v rámci jedné kolekce lze mít obojí. A v některých jazycích to jde:

Python dict:

* Přístup přes slovník[klíč] háže v případě neexistujícího klíče výjimku.
* Přístup přes slovník.get(klíč) vrací v případě neexistujícího klíče None.
* Případně lze u metody get druhým parametrem specifikovat svoji „prázdnou“ návratovou hodnotu.
* Trošku škoda, že Python něco podobného nemá i pro list, nebo o tom aspoň nevím.

Scala Map:

* Situace je dost podobná jako u Pythonu.
* Ovšem Scala se snaží vyhnout hodnotě null, takže metoda get vrací Option. S tím se mi jednak lépe pracuje (má metody jako map, flatMap, filter apod.) a jednak to jen tak neprobublá někam, kde se s Option nepočítá. (Což by mimochodem byl benefit i u dynamicky typovaného jazyka – tam by to nejspíš způsobilo výjimku i v případě existence prvku.)
* Scala navíc má podobný mechanismus i pro sekvence.

PHP volí – řekl bych – nejhorší možnost. Zaprvé nabízí (pokud se to nezměnilo) jen jednu možnost. Zadruhé je tato možnost takový kočkopes – vrátí to null na program *nějak* pokračuje, ale zároveň to hodí warning, který je defaultně vypisován uživateli a který nelze snadno ošetřit v programu.

ikona Jakub Vrána OpenID:

PHP má dvě dost podobné struktury – pole a objekty. Pole by mohly se chovat jedním způsobem, objekty druhým.

lenochware:

Plně souhlasím s článkem. PHP pole jsem vždycky vnímal jako obecné dynamické úložiště a nepovažuji to za stejný případ jako nedeklarované proměnné nebo atributy objektu.

Dotaz na neexistující klíč vidím spíš jako třeba sql select na neexistující id, případně jquery/dom selector. Obojí vrátí prázdnou množinu a toto chování mi připadá velmi užitečné.

Stejně to podle mě spousta lidí vyřeší tak, že zavede nějakou funkci array_get($pole, $klíč) a výsledek bude úplně stejný, jako by to warning/notice neházelo, akorát se kód bude hemžit array_gety()

Ovšem ty notice to hází snad už od 5ky, čili mi připadá zbytečné se nad tím po 15 letech rozhořčovat. To že to switchli na warning je sice utahování šroubu, ale nic co by mě moc překvapovalo.

hever:

Dlouhá léta jsem měl přesně tyto notice přes set_error_handler() zahazoval. Přesně souhlasím s tím, že nedefinovaný prvek pole je prostě null (tj. prázdný string nebo nula nebo false ...) a tanečky s isset() jsem považoval za nesmyslné.

Až nedávno jsem se zamýšlel nad tím, jestli bych se neměl začít chovat konformněji a s příchodem ?? jsem se pokusil vše přepsat, aby se vlk nažral a koza tolik neutrpěla - dost taky kvůli tomu, že režie množství vyhazování a zahazování těchto notice zpomalovala aplikaci. Nicméně, po přepsání všech míst v kódu jsem se po pár dnech zpětně podíval na to co jsem to vlastně udělal, pocítil marnost a zahazování této chyby do error_handleru zase vrátil. Ikdyž tedy, většinu těch úprav jsem nechal, ale ne že bych cítil, že mají smysl, ale jen jako podvolení se situaci a zamezení notice (ještě jsem na php7.4).

V nějakých minoritních případech toto chování k odhalení chyby pomůže, uznávám. V globálu ale, myslím si, ty tanečky kolem toho za to nestojí. Navržené Jakubovo řešení, kdy se objekty a pole chovají jinak, mě dává smysl.

Diskuse je zrušena z důvodu spamu.

avatar © 2005-2024 Jakub Vrána. Publikované texty můžete přetiskovat pouze se svolením autora. Ukázky kódu smíte používat s uvedením autora a URL tohoto webu bez dalších omezení Creative Commons. Můžeme si tykat. Skripty předpokládají nastavení: magic_quotes_gpc=Off, magic_quotes_runtime=Off, error_reporting=E_ALL & ~E_NOTICE a očekávají předchozí zavolání mysql_set_charset. Skripty by měly být funkční v PHP >= 4.3 a PHP >= 5.0.