Come rilevare campi declarated dinamici su oggetti con codesniffer in PHP

Question

Dopo un refactoring, abbiamo avuto qualcosa di simile in una delle nostre classi:

class FooBar 
{ 
    // $foo was $bla before 
    private $foo; 

    public function setBlubbOnArrayOnlyOnce($value) 
    { 
     // $this->bla was forgotten during refactoring. Must be $this->foo 
     if(!isset($this->bla['blubb'])) { 
      $this->foo['blubb'] = $value; 
     } 
    } 
} 

Così, alla fine $ this-> foo [ 'blubb' ] è stato sempre impostato, non solo una volta. Questo succede a causa dei metodi magici di PHP. Non vogliamo che sia possibile accedere ai campi in modo dinamico, quindi ho pensato di aggiungere una regola codesniffer. Ma non ne ho trovato nessuno e mi ha chiesto perché.

PHPStorm mostra un campo dichiarato in modo dinamico lì, ma voglio che questo fallisca automaticamente con codesniffer (o qualcosa di simile) durante il nostro ciclo di distribuzione.

Qualcuno ha un'idea su questo? C'è una buona regola? Dovrei scrivere il mio e come? O sarebbe una cattiva pratica disabilitarlo?

Disclaimer: Usiamo test, ma a volte ti perdi le cose ... Sarebbe bello evitare questo in primo luogo. Inoltre, per favore non venire a sovrascrivere i metodi magici. Non voglio avere un tratto/astratto in ogni classe.

Answer 1

Questo non è un problema di codesniffer o phpstorm. E non puoi voler risolvere questo problema con codesniffer o IDE. IDE, codesniffer, phpdocumentor, ecc. - questo è analizzato "staticamente". E per l'analisi dinamica puoi utilizzare per es. PHPUnit.

Se si desidera verificare l'esistenza della proprietà, è necessario utilizzare la funzione property_exists().

class X 
{ 
    public function __get($name) 
    { 
     $this->{$name} = null; 
     return $this->{$name}; 
    } 
} 

$x = new X(); 
var_dump(property_exists($x, 'foo')); // false 
var_dump($x->foo); // NULL 
var_dump(property_exists($x, 'foo')); // true 

Oppure può essere che si può utilizzare la riflessione per la proprietà http://php.net/manual/en/class.reflectionproperty.php

Se si desidera verificare la presenza di "isset" è necessario conoscere:

var_dump(isset($x), $x); // false + NULL with notice 
$x = null; 
var_dump(isset($x), $x); // false + NULL 
unset($x); 
var_dump(isset($x), $x); // false + NULL without notice 

Quando si Shure per questo caso di controllo è possibile use isset()

Ma si dovrebbe sempre verificare prima l'esistenza della proprietà. Altrimenti puoi avere un comportamento indefinito del tuo codice.

Answer 2

Dopo un refactoring

Sarebbe bene per evitare che questo, in primo luogo.

È possibile rilevare solo questi tipi di errori di refactoring eseguendo test dopo ogni fase di refactoring. Anche questo errore verrà visualizzato, poiché foo['blubb'] è impostato su un valore specifico e ciò dovrebbe causare un effetto indesiderato in un altro test, non solo nel test per la logica setter.

Usiamo test, ma a volte le cose manchi ...

Sì, è abbastanza comune che la copertura non è abbastanza alta. Ecco perché avere una buona copertura di prova è il punto di partenza per tutti i refactoring.

Queste due righe non erano "verde" nel rapporto di copertura:

if(!isset($this->bla['blubb'])) { 
     $this->foo['blubb'] = $value; 

Inoltre, si prega di non venire con sovrascrivendo i metodi magici. Non voglio avere un tratto/astratto in ogni classe.

Hai esclusi, ma questo è un modo per catturare le proprietà: utilizzando la funzione magica __set() (per Vars inaccessibili) o property_exists() o l'uso di Reflection* classi da trovare.

---

Ora, che il suo troppo tardi, si vuole un altro strumento per rilevare l'errore, ok:

Lo strumento avrebbe bisogno di analizzare il file PHP e dei suoi genitori (perché di portata variabile) e trovare $this->bla senza una precedente dichiarazione public|private|protected (proprietà della classe). Questo non indica il tipo esatto di errore, solo che "bla" è stato utilizzato senza dichiarazione.

È possibile implementarlo come regola CodeSniffer.

È anche possibile provare http://phpmd.org/ o https://scrutinizer-ci.com/. E, nel caso in cui si utilizza PHP7: https://github.com/etsy/phan

tl; tr

È complicato per determinare l'errore esatto e il suo contesto senza correre, la valutazione e l'analisi del codice sottostante. Basti pensare ai "nomi dinamici delle variabili" e sai perché: non conosci nemmeno il nome della proprietà guardando il codice sorgente, perché si crea dinamicamente durante il flusso del programma. Un analizzatore statico non lo prenderebbe.

Un analizzatore dinamico deve tracciare tutte le cose, qui gli accessi $this-> e prenderebbe in considerazione il contesto:! Isset (x). La valutazione del contesto può trovare molti errori di codifica comuni. Alla fine si può costruire un rapporto: dicendo che $ this-> bla stato accedere solo 1 volta e che indica o che

• una proprietà dichiarata in modo dinamico è stato introdotto, ma mai riutilizzate, con il suggerimento che si potrebbe rilasciarlo o dichiararlo come proprietà di classe
• OPPURE con una valutazione di contesto aggiunta: che e quando si accede a questa variabile dall'interno di un isset() - che si accede a una chiave non esistente di una proprietà non dichiarata, senza una precedente set(), ecc

Answer 3

Ora nel 2017, you'are alla ricerca di tool PHPStan. Collego una breve introduzione che ho scritto per gli utenti principianti.

Fa esattamente quello che ti serve!