После рефакторинга у нас было что-то вроде этого в одном из наших классов:
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;
}
}
}
Таким образом, в конце $ this-> foo [‘blubb’] был установлен всегда, а не только один раз.
Это происходит из-за волшебных методов PHP. Мы не хотим, чтобы было возможно получить динамический доступ к полям, поэтому я подумал, что просто добавлю правило кодов-различий. Но я не нашел и спросил почему.
PHPStorm показывает поле, объявленное там динамически, но я хочу, чтобы это автоматически завершалось ошибкой с codeniffer (или чем-то подобным) во время нашего цикла развертывания.
Кто-нибудь есть идеи по этому поводу? Есть ли хорошее правило? Должен ли я написать свой собственный и как? Или было бы плохой практикой отключать его?
Отказ от ответственности: мы используем тесты, но иногда вы что-то упускаете … Было бы хорошо, во-первых, предотвратить это. Также, пожалуйста, не предлагайте перезаписывать магические методы. Я не хочу иметь черты / абстрактные черты в каждом классе.
Это не проблема codesiffer или phpstorm. И вы не можете решить эту проблему с codeniffer или IDE. IDE, Codesniffer, phpdocumentor и т. Д. — это «статически» анализ. А для динамического анализа вы можете использовать, например, PHPUnit.
Если вы хотите проверить существование свойства, вы должны использовать функцию 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
Или, может быть, вы можете использовать отражение для собственности http://php.net/manual/en/class.reflectionproperty.php
Если вы хотите проверить «isset», вы должны знать:
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
Когда вы уверены в этом случае проверки, вы можете использовать isset ()
Но вы всегда должны сначала проверить наличие имущества. В противном случае вы можете иметь неопределенное поведение вашего кода.
После рефакторинга
Было бы хорошо, чтобы предотвратить это в первую очередь.
Подобные ошибки рефакторинга можно отследить только путем запуска тестов после каждого шага рефакторинга. Эта ошибка также будет всплывать, потому что foo['blubb']
установлен на определенное значение, и это должно вызвать нежелательный эффект в другом тесте — не только в тесте для логики установщика.
Мы используем тесты, но иногда вы скучаете по вещам …
Да, довольно распространено, что охват не достаточно высок.
Вот почему хорошее тестовое покрытие является отправной точкой для всех рефакторингов.
Эти две строки не были «зелеными» в вашем отчете о покрытии:
if(!isset($this->bla['blubb'])) {
$this->foo['blubb'] = $value;
Также, пожалуйста, не предлагайте перезаписывать магические методы. Я не хочу иметь черты / абстрактные черты в каждом классе.
Вы исключили это, но это один из способов поймать свойства: с помощью магической функции __set()
(для недоступных варов) или property_exists()
или использование Reflection*
классы найти.
Теперь, когда уже слишком поздно, вы хотите, чтобы другой инструмент уловил ошибку, хорошо:
Инструмент должен будет проанализировать файл PHP и его родителей (из-за переменной области) и найти $this->bla
без предварительного public|private|protected
объявление переменной (свойства класса). Это не будет указывать точный тип ошибки, просто что «бла» был доступен без объявления.
Это можно реализовать как правило CodeSniffer.
Вы также можете дать http://phpmd.org/ или же https://scrutinizer-ci.com/ попытка
И, если вы используете PHP7: https://github.com/etsy/phan
ТЛ; тр
Сложно определить точную ошибку и ее контекст без запуска, оценки и анализа базового кода. Просто подумайте о «динамических именах переменных», и вы поймете почему: вы даже не знаете имя свойства, глядя на исходный код, потому что оно динамически создается во время выполнения программы. Статический анализатор этого не поймет.
Динамический анализатор должен отслеживать все вещи, здесь $this->
обращается и учитывает контекст:! isset (x). Оценка контекста может найти много распространенных ошибок кодирования. В конце концов, вы можете создать отчет: говорят, что к $ this-> bla обращались только 1 раз, и это указывает либо на то, что
Сейчас в 2017 году ты ищешь инструмент PHPStan. Я привожу краткое вступление, которое я написал для начинающих пользователей.
Это именно то, что вам нужно!