Zet uw statische analyser niet op stil
08 november 2023
3 minutes
Statische analysers zoals psalm en phpstan zijn een zeer krachtig hulpmiddel waarmee je runtime-fouten op productie kunt voorkomen. De kans is groot dat je het gevoel hebt dat ze de hele tijd aan het muggenziften zijn. Of je het leuk vindt of niet, je static analyzer heeft (bijna) altijd gelijk. Dat komt omdat types niet liegen - tenzij ze fout zijn: Als je functie type A verwacht, maar je functie retourneert type A|B|null - dan moet je code ook omgaan met geval B|null! Tenzij je graag urgente bugs op productie wilt oplossen.
We weten allemaal dat er in PHP enkele vreemde eigenaardigheden zitten. Je statische analyzer weet dit ook en zal je vertellen wat er aan de hand is. Dus als een tool als psalm veel klaagt, komt dat omdat je foutgevoelige code hebt geschreven. In plaats van te klagen over de tool, kun je beter nog eens goed naar je code kijken.
Stop de onderdrukking
Iets wat we vaak zien in code reviews is dat je statische analyzer klaagde over een specifiek geval. In plaats van te proberen het probleem te begrijpen en op te lossen, ging je YOLO en voegde @psalm-suppress toe om de error.... te onderdrukken.
Door een suppress statement toe te voegen, demp je jouw statisch analyseprogramma. Op dit punt weet JIJ misschien wat er fout gaat. De andere mensen in je team zullen dit waarschijnlijk niet weten. Maar JIJ hebt de fout voor altijd onderdrukt. Dit betekent dat iemand anders uit het team (of jij in de toekomst), misschien code schrijft die psalm niet meer rapporteert. Dit kan nog steeds resulteren in runtime fouten - wat je zeker niet wilt ?!
Ons advies: Voeg nooit een suppress statement toe, tenzij het echt een intern type probleem is! In dat geval kan je een stub-bestand als alternatief gebruiken of een PR geven aan uw static analyzer. Als je echt een suppress moet toevoegen, documenteer dan het WAAROM - bij voorkeur gekoppeld aan een github issue.
@var is een leugen!
Een andere frequente "dempende" fout bij het gebruik van statische analyzers is het gebruik van interne @var-declaraties. Bijvoorbeeld:
/** @var Foo $foo */
$foo = createFoo();Dit gebeurt meestal als het symbool aan de rechterkant van de = operator een verkeerd type heeft, of niet specifiek genoeg is. Het gebruik van inline @var is om een paar redenen problematisch:
Omdat het gebruikt kan worden om verkeerde type-informatie van het aangeroepen symbool te corrigeren. Uw statische analyser zal het altijd vertrouwen. Maar als er een fout zit in deze annotatie, kan de analyse van de code eronder fout zijn.
Als het terugkeertype verandert, zal uw statische analyzer dit niet weten, wat kan leiden tot runtime-fouten.
De inline
@varmoet worden herhaald boven alle toepassingen van het symbool, wat leidt tot herhaling in de codebase.
In plaats daarvan moet het type bij de bron worden vastgelegd. Als het aangeroepen symbool afkomstig is uit code van derden, kunt u het corrigeren met behulp van een stub-bestand. Als het terugkeertype in elke aanroep verschilt op basis van de doorgegeven argumenten, kunt u generieken gebruiken, of in plaats daarvan een dynamische terugkeertype-uitbreiding schrijven. Als je echt een inline @var moet gebruiken, overweeg dan een alternatief - een assert() aanroep, die een uitzondering kan gooien tijdens runtime, zodat de uitvoering van de code niet doorgaat als niet aan de eisen wordt voldaan.
Deze ongeldige var-declaraties worden vaak toegevoegd wanneer je werkt met ongestructureerde gegevens zoals ruwe geneste JSON-invoerobjecten. In plaats van verschillende type-annotaties toe te voegen, kunt u beter de invoer parsen en proberen het invoertype te dwingen tot het type dat uw toepassing verwacht. In het algemeen: Parse ... Don't validate !
Opmerking: phpstan lanceerde onlangs een leugendetector die @var declaraties inspecteert om te bepalen of ze geldig zijn.
Door uw statische analyser te dempen, verberg je toekomstige productie-fouten. Elke keer dat je een @var declaratie toevoegt aan je codebase, denk bij jezelf: er is waarschijnlijk een betere manier om de fout die ik probeer op te lossen op te lossen! Dezelfde regels gelden als je met typescript werkt: Gebruik geen any of @ts-ignore!
