如何使用PHP的Codesniffer检测对象上动态声明的字段

Question

在重构后，我们在其中一个类中得到了以下代码：

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在那里显示了一个声明动态字段的提示，但我希望在部署周期中使用代码嗅探器（或类似工具）自动失败。

有人有什么好主意吗？有好的规则吗？我应该编写自己的规则吗？或者禁用它是否是不好的实践？

免责声明：我们使用测试，但有时会漏掉一些东西...最好是在第一时间防止这种情况发生。另外，请不要提出覆盖魔术方法的建议。我不想在每个类中都使用特性/抽象等。

- Kasihasi

---

你能否查找未定义的变量，因为 $this->bla 没有被声明？你可能需要在 PHPCodeSniffer 中扩展代码。 - Steven Scott

我正在尝试，但我希望有一种明显且简单的方法来完成它。 - Kasihasi

你是否在Squizlabs（http://www.squizlabs.com/）或他们的GitHub（https://github.com/squizlabs/PHP_CodeSniffer）上询问过？因为Greg Sherwood过去对问题非常敏感。 - Steven Scott

这可能不是那么容易，因为$this->bla通常可以在父类中定义。Codesniffer在文件/令牌级别上工作，如果你遵循PSR编码标准，它将无法知道父类结构（因为它在单独的文件中）。 - weirdan

@Weirdan 这可能是真的。但如果未定义的属性可以在简单类中检测到，而不需要扩展任何其他类，那就太好了。 - maxhb

Answer 1

在重构之后，最好能够在一开始就防止这种情况发生。您只能通过在每次重构步骤后运行测试来捕获此类重构错误。此错误也会被抛出，因为foo ['blubb']设置为特定值，这应该会在另一个测试中引起不必要的影响-而不仅仅是setter逻辑的测试。我们使用测试，但有时会漏掉一些东西... 是的，测试覆盖率不够高是很常见的。这就是为什么拥有良好的测试覆盖率是所有重构的起点的原因。您的测试覆盖率报告中这两行代码没有“绿色”。

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

此处是一段关于编程的英文文本。翻译如下：

同时，请不要想着重写魔术方法。我不想在每个类中都使用特性/抽象等。

你已经排除了这种方法，但有一种方式可以捕获属性：使用魔术方法 __set()（对于不可访问的变量），或者使用 property_exists() 或 Reflection* 类来查找。

---

现在，太晚了，您想要另一个工具来捕捉错误，好的：

这个工具需要解析 PHP 文件及其父文件（因为变量作用域），并查找没有先前的 public|private|protected 变量（类属性）声明的 $this->bla。这不会指示确切的错误类型，只是表示 "bla" 在没有声明的情况下被访问。

这可以实现为 CodeSniffer 规则。

您还可以尝试使用 http://phpmd.org/ 或 https://scrutinizer-ci.com/。 如果您正在使用 PHP7，则可以使用 https://github.com/etsy/phan。

简而言之，要确定确切的错误及其上下文而不运行、评估和分析基础代码是很复杂的。只需要考虑"动态变量名"，就可以知道为什么：甚至在查看源代码时，您都不知道属性的名称，因为它是在程序流程中动态构建的。静态分析器无法捕获这种情况。

一个动态分析器必须追踪所有东西，这里的$this-> 访问并且会考虑上下文: !isset(x)。上下文评估可以发现许多常见的编码错误。最终，您可以生成一份报告：说 $this->bla 仅被访问了1次，这表明要么引入了动态声明的属性，但从未被重复使用，建议您将其删除或声明为类属性；要么在增加上下文评估时，当从 isset() 内部访问该变量时 - 访问了一个不存在的非声明属性的键，而没有先进行 set() 等操作。

- Jens A. Koch

Answer 2

这不是codesniffer或phpstorm的问题。你不能用codesniffer或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()。

但是，你应该始终首先检查属性是否存在。否则，你的代码可能会出现未定义的行为。

- Deep

---

这不是关于检测 $this->bar[''anyStringValue] 是否被定义（在静态分析中可能甚至不可能），而是关于检测 $this->bar 不再被定义。 - maxhb

@maxhb 我的回答包括这个。 - Deep

Answer 3

现在是2017年，您正在寻找 PHPStan工具。我为初次使用者写了一个简短的介绍。它能够准确地满足您的需求！

- Tomas Votruba