1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-12-01 19:22:41 +01:00

Write a linter rule for unexpected return values

Summary:
Although it is technically possible to return a value from a PHP constructor, it is rather odd and should be avoided. See some discussion on [[http://stackoverflow.com/q/11904255 | StackOverflow]]. Consider the following example:

```lang=php
class SomeClass {
  public function __construct() {
    return 'quack';
  }
}
```

This doesn't work:

```lang=php, counterexample
echo new SomeClas();
```

This, strangely, does work:

```lang=php
echo id(new SomeClass())->__construct();
```

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14516
This commit is contained in:
Joshua Spence 2015-11-19 19:34:32 +11:00
parent 149e7895fb
commit 3ac313ad1e
6 changed files with 86 additions and 0 deletions

View file

@ -333,6 +333,8 @@ phutil_register_library_map(array(
'ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRuleTestCase.php', 'ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRuleTestCase.php',
'ArcanistUndeclaredVariableXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php', 'ArcanistUndeclaredVariableXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUndeclaredVariableXHPASTLinterRule.php',
'ArcanistUndeclaredVariableXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistUndeclaredVariableXHPASTLinterRuleTestCase.php', 'ArcanistUndeclaredVariableXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistUndeclaredVariableXHPASTLinterRuleTestCase.php',
'ArcanistUnexpectedReturnValueXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnexpectedReturnValueXHPASTLinterRule.php',
'ArcanistUnexpectedReturnValueXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistUnexpectedReturnValueXHPASTLinterRuleTestCase.php',
'ArcanistUnitConsoleRenderer' => 'unit/renderer/ArcanistUnitConsoleRenderer.php', 'ArcanistUnitConsoleRenderer' => 'unit/renderer/ArcanistUnitConsoleRenderer.php',
'ArcanistUnitRenderer' => 'unit/renderer/ArcanistUnitRenderer.php', 'ArcanistUnitRenderer' => 'unit/renderer/ArcanistUnitRenderer.php',
'ArcanistUnitTestEngine' => 'unit/engine/ArcanistUnitTestEngine.php', 'ArcanistUnitTestEngine' => 'unit/engine/ArcanistUnitTestEngine.php',
@ -707,6 +709,8 @@ phutil_register_library_map(array(
'ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistUnaryPrefixExpressionSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistUndeclaredVariableXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistUndeclaredVariableXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistUndeclaredVariableXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistUndeclaredVariableXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistUnexpectedReturnValueXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistUnexpectedReturnValueXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistUnitConsoleRenderer' => 'ArcanistUnitRenderer', 'ArcanistUnitConsoleRenderer' => 'ArcanistUnitRenderer',
'ArcanistUnitRenderer' => 'Phobject', 'ArcanistUnitRenderer' => 'Phobject',
'ArcanistUnitTestEngine' => 'Phobject', 'ArcanistUnitTestEngine' => 'Phobject',

View file

@ -0,0 +1,48 @@
<?php
final class ArcanistUnexpectedReturnValueXHPASTLinterRule
extends ArcanistXHPASTLinterRule {
const ID = 92;
public function getLintName() {
return pht('Unexpected `%s` Value', 'return');
}
public function getLintSeverity() {
return ArcanistLintSeverity::SEVERITY_WARNING;
}
public function process(XHPASTNode $root) {
$methods = $root->selectDescendantsOfType('n_METHOD_DECLARATION');
foreach ($methods as $method) {
$method_name = $method
->getChildOfType(2, 'n_STRING')
->getConcreteString();
switch (strtolower($method_name)) {
case '__construct':
case '__destruct':
$returns = $method->selectDescendantsOfType('n_RETURN');
foreach ($returns as $return) {
$return_value = $return->getChildByIndex(0);
if ($return_value->getTypeName() == 'n_EMPTY') {
continue;
}
$this->raiseLintAtNode(
$return,
pht(
'Unexpected `%s` value in `%s` method.',
'return',
$method_name));
}
break;
}
}
}
}

View file

@ -0,0 +1,11 @@
<?php
final class ArcanistUnexpectedReturnValueXHPASTLinterRuleTestCase
extends ArcanistXHPASTLinterRuleTestCase {
public function testLinter() {
$this->executeTestsInDirectory(
dirname(__FILE__).'/unexpected-return-value/');
}
}

View file

@ -0,0 +1,8 @@
<?php
class SomeClass {
public function __construct() {
return null;
}
}
~~~~~~~~~~
warning:4:5

View file

@ -0,0 +1,8 @@
<?php
class SomeClass {
public function __destruct() {
return null;
}
}
~~~~~~~~~~
warning:4:5

View file

@ -0,0 +1,7 @@
<?php
class SomeClass {
public function __construct() {
return;
}
}
~~~~~~~~~~