2015-08-13 21 views
8

Nach einem Refactoring dynamischen declarated Feldern auf Objekte erkennen, die wir in einer unserer Klassen so etwas wie dieses hatte:Wie mit codesniffer in PHP

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; 
     } 
    } 
} 

Also am Ende $ this-> foo [ 'blubb' ] wurde immer eingestellt, nicht nur einmal. Dies geschieht aufgrund der magischen Methoden von PHP. Wir wollen nicht, dass es möglich ist, dynamisch auf Felder zuzugreifen, also dachte ich, ich füge einfach eine Codesniffer-Regel hinzu. Aber ich habe keine gefunden und fragte mich warum.

PHPStorm zeigt ein Feld, das dynamisch dort gemeldet wird, aber ich möchte, dass es während des Bereitstellungszyklus automatisch mit Codesniffer (oder ähnlichem) ausfällt.

Hat jemand eine Idee zu diesem Thema? Gibt es eine gute Regel? Soll ich selbst schreiben und wie? Oder wäre es eine schlechte Übung, es zu deaktivieren?

Haftungsausschluss: Wir verwenden Tests, aber manchmal vermisst du Dinge ... Es wäre gut, dies in erster Linie zu verhindern. Auch, bitte überlege dir nicht, die magischen Methoden zu überschreiben. Ich will keine Eigenschaft/Zusammenfassung haben, was auch immer in jeder Klasse.

+1

Können Sie nach undefinierten Variablen suchen, da $ this-> bla nicht deklariert würde? Möglicherweise müssen Sie den Code in PHPCodeSniffer erweitern. –

+0

Ich versuche es, aber ich habe auf einen offensichtlichen und einfachen Weg gehofft. – Kasihasi

+1

Hast du nach Squizlabs (http://www.squizlabs.com/) oder ihrem GitHub (https://github.com/squizlabs/PHP_CodeSniffer), da Greg Sherwood in der Vergangenheit sehr gut auf Fragen reagiert hat. –

Antwort

2

Dies ist kein Codesniffer- oder Phpstorm-Problem. Und Sie können dieses Problem nicht mit Codesniffer oder IDE beheben. IDE, codesniffer, phpdocumentor usw. - das wird "statisch" analysiert. Und für die dynamische Analyse können Sie z.B. phpunit.

Wenn Sie das Vorhandensein einer Eigenschaft überprüfen möchten, müssen Sie die Funktion property_exists() verwenden.

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 

Oder sein können Sie Reflexion für Immobilien http://php.net/manual/en/class.reflectionproperty.php

Wenn Sie überprüfen wollen, für „isset“ Sie wissen müssen, verwenden können:

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 

Wenn Sie sich für diesen Fall von Scheck shure können Sie Verwenden Sie isset()

Aber Sie sollten immer zuerst auf das Vorhandensein der Eigenschaft prüfen. Andernfalls können Sie ein undefiniertes Verhalten Ihres Codes haben.

+0

Es geht nicht darum festzustellen, ob '$ this-> bar ['' anyStringValue 'definiert ist (möglicherweise nicht einmal in der statischen Analyse möglich), es geht darum erkennen, dass $ this-> bar nicht mehr definiert ist. – maxhb

+0

@maxhb Meine Antwort einschließlich darüber. – Deep

2

Nach einem Refactoring

wäre es gut, dies in erster Linie zu verhindern.

Sie können diese Art von Refactoring-Fehlern nur durch Ausführen von Tests nach jedem Refactoring-Schritt erkennen. Dieser Fehler wird auch auftreten, weil foo['blubb'] auf einen bestimmten Wert eingestellt ist und dies sollte einen unerwünschten Effekt in einem anderen Test verursachen - nicht nur im Test für die Setter-Logik.

Wir Tests verwenden, aber manchmal vermisst du Dinge ...

Ja, es ist durchaus üblich, dass die Abdeckung nicht hoch genug ist. Deshalb ist eine gute Testabdeckung der Ausgangspunkt für alle Refactorings.

Diese beiden Linien waren nicht „grün“ in Ihrer Berichterstattung Bericht:

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

Sie bitte auch, kommen nicht die magischen Methoden mit Überschreiben auf. Ich will keine Eigenschaft/Zusammenfassung haben, was auch immer in jeder Klasse.

Sie haben es ausgeschlossen, aber das ist ein Weg, um die Eigenschaften zu fangen: durch die magische Funktion __set() (für unzugängliche VARs) oder property_exists() oder die Verwendung von Reflection* Klassen zu finden.


Nun, das es zu spät ist, wollen Sie ein anderes Werkzeug, um den Fehler zu fangen, ok:

Das Tool müsste die PHP-Datei und deren Eltern (wegen der variablen Umfang) und $this->bla finden analysieren ohne vorherige public|private|protected Variable (Klasseneigenschaft) Deklaration. Dies zeigt nicht den genauen Fehlertyp an, nur dass auf "bla" ohne Deklaration zugegriffen wurde.

Es ist möglich, dies als CodeSniffer-Regel zu implementieren.

Sie können auch http://phpmd.org/ oder https://scrutinizer-ci.com/ einen Versuch geben. Und Sie im Falle verwenden PHP7: https://github.com/etsy/phan

tl; tr

Sein kompliziert ohne Laufen, Auswertung und Analyse der zugrunde liegenden Code den genauen Fehler und seinen Kontext zu bestimmen. Denken Sie nur an "dynamische Variablennamen" und Sie wissen warum: Sie wissen nicht einmal den Namen der Eigenschaft, indem Sie sich den Quellcode ansehen, da dieser dynamisch während des Programmablaufs erstellt wird. Ein statischer Analysator würde das nicht verstehen.

Ein dynamischer Analysator muss alle Dinge verfolgen, hier $this-> greift zu und würde den Kontext berücksichtigen:! Isset (x). Die Kontextauswertung kann viele häufige Programmierfehler finden. Am Ende können Sie einen Bericht erstellen: sagen, dass $ this-> bla nur 1 Mal zugegriffen wurde und zeigt an, dass entweder

  • eine dynamisch deklarierte Eigenschaft eingeführt wurde, aber nie wieder verwendet, mit dem Vorschlag, dass Sie könnte es fallen lassen oder deklarieren als Klasseneigenschaft
  • ODER mit hinzugefügter Kontextevaluierung: das und wenn auf diese Variable von innerhalb eines isset() zugegriffen wurde - dass auf einen nicht existierenden Schlüssel einer nicht deklarierten Eigenschaft zugegriffen wurde, ohne a prior set() usw.
1

Jetzt im Jahr 2017, auf der Suche you'are für tool PHPStan. Ich verlinke kurzes Intro, das ich für erstmalige Nutzer geschrieben habe.

Es macht genau das, was Sie brauchen!