mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-26 00:32:41 +01:00
Add a linter rule for extending Phobject
Summary: If I understand correctly, all classes should extend from `Phobject`? Test Plan: This needs some further work Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, Korvin Differential Revision: https://secure.phabricator.com/D13275
This commit is contained in:
parent
e97cdc6c9a
commit
ac8367a884
3 changed files with 50 additions and 12 deletions
|
@ -6,6 +6,7 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
const LINT_DEPRECATED_FUNCTION = 3;
|
const LINT_DEPRECATED_FUNCTION = 3;
|
||||||
const LINT_UNSAFE_DYNAMIC_STRING = 4;
|
const LINT_UNSAFE_DYNAMIC_STRING = 4;
|
||||||
const LINT_RAGGED_CLASSTREE_EDGE = 5;
|
const LINT_RAGGED_CLASSTREE_EDGE = 5;
|
||||||
|
const LINT_EXTENDS_PHOBJECT = 6;
|
||||||
|
|
||||||
private $deprecatedFunctions = array();
|
private $deprecatedFunctions = array();
|
||||||
private $dynamicStringFunctions = array();
|
private $dynamicStringFunctions = array();
|
||||||
|
@ -23,14 +24,16 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
|
|
||||||
public function getLintNameMap() {
|
public function getLintNameMap() {
|
||||||
return array(
|
return array(
|
||||||
self::LINT_ARRAY_COMBINE => pht(
|
self::LINT_ARRAY_COMBINE =>
|
||||||
'%s Unreliable', 'array_combine()'),
|
pht('%s Unreliable', 'array_combine()'),
|
||||||
self::LINT_DEPRECATED_FUNCTION => pht(
|
self::LINT_DEPRECATED_FUNCTION =>
|
||||||
'Use of Deprecated Function'),
|
pht('Use of Deprecated Function'),
|
||||||
self::LINT_UNSAFE_DYNAMIC_STRING => pht(
|
self::LINT_UNSAFE_DYNAMIC_STRING =>
|
||||||
'Unsafe Usage of Dynamic String'),
|
pht('Unsafe Usage of Dynamic String'),
|
||||||
self::LINT_RAGGED_CLASSTREE_EDGE => pht(
|
self::LINT_RAGGED_CLASSTREE_EDGE =>
|
||||||
'Class Not %s Or %s', 'abstract', 'final'),
|
pht('Class Not %s Or %s', 'abstract', 'final'),
|
||||||
|
self::LINT_EXTENDS_PHOBJECT =>
|
||||||
|
pht('Class Not Extending %s', 'Phobject'),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -43,6 +46,7 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
}
|
}
|
||||||
|
|
||||||
public function getLintSeverityMap() {
|
public function getLintSeverityMap() {
|
||||||
|
$advice = ArcanistLintSeverity::SEVERITY_ADVICE;
|
||||||
$warning = ArcanistLintSeverity::SEVERITY_WARNING;
|
$warning = ArcanistLintSeverity::SEVERITY_WARNING;
|
||||||
|
|
||||||
return array(
|
return array(
|
||||||
|
@ -50,6 +54,7 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
self::LINT_DEPRECATED_FUNCTION => $warning,
|
self::LINT_DEPRECATED_FUNCTION => $warning,
|
||||||
self::LINT_UNSAFE_DYNAMIC_STRING => $warning,
|
self::LINT_UNSAFE_DYNAMIC_STRING => $warning,
|
||||||
self::LINT_RAGGED_CLASSTREE_EDGE => $warning,
|
self::LINT_RAGGED_CLASSTREE_EDGE => $warning,
|
||||||
|
self::LINT_EXTENDS_PHOBJECT => $advice,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -112,6 +117,7 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
'lintUnsafeDynamicString' => self::LINT_UNSAFE_DYNAMIC_STRING,
|
'lintUnsafeDynamicString' => self::LINT_UNSAFE_DYNAMIC_STRING,
|
||||||
'lintDeprecatedFunctions' => self::LINT_DEPRECATED_FUNCTION,
|
'lintDeprecatedFunctions' => self::LINT_DEPRECATED_FUNCTION,
|
||||||
'lintRaggedClasstreeEdges' => self::LINT_RAGGED_CLASSTREE_EDGE,
|
'lintRaggedClasstreeEdges' => self::LINT_RAGGED_CLASSTREE_EDGE,
|
||||||
|
'lintClassExtendsPhobject' => self::LINT_EXTENDS_PHOBJECT,
|
||||||
);
|
);
|
||||||
|
|
||||||
foreach ($method_codes as $method => $codes) {
|
foreach ($method_codes as $method => $codes) {
|
||||||
|
@ -302,4 +308,29 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function lintClassExtendsPhobject(XHPASTNode $root) {
|
||||||
|
$classes = $root->selectDescendantsOfType('n_CLASS_DECLARATION');
|
||||||
|
|
||||||
|
foreach ($classes as $class) {
|
||||||
|
// TODO: This doesn't quite work for namespaced classes (see T8534).
|
||||||
|
$name = $class->getChildOfType(1, 'n_CLASS_NAME');
|
||||||
|
$extends = $class->getChildByIndex(2);
|
||||||
|
|
||||||
|
if ($name->getConcreteString() == 'Phobject') {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($extends->getTypeName() == 'n_EMPTY') {
|
||||||
|
$this->raiseLintAtNode(
|
||||||
|
$class,
|
||||||
|
self::LINT_EXTENDS_PHOBJECT,
|
||||||
|
pht(
|
||||||
|
'Classes should extend from %s or from some other class. '.
|
||||||
|
'All classes (except for %s itself) should have a base class.',
|
||||||
|
'Phobject',
|
||||||
|
'Phobject'));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -0,0 +1,7 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
abstract class A extends Phobject {}
|
||||||
|
final class B extends A {}
|
||||||
|
final class C {}
|
||||||
|
~~~~~~~~~~
|
||||||
|
advice:5:1
|
|
@ -1,12 +1,12 @@
|
||||||
<?php
|
<?php
|
||||||
|
|
||||||
class A { }
|
class A extends Phobject {}
|
||||||
abstract class B { }
|
abstract class B extends Phobject {}
|
||||||
final class C { }
|
final class C extends Phobject {}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @concrete-extensible
|
* @concrete-extensible
|
||||||
*/
|
*/
|
||||||
class D { }
|
class D extends Phobject {}
|
||||||
~~~~~~~~~~
|
~~~~~~~~~~
|
||||||
warning:3:7
|
warning:3:7
|
||||||
|
|
Loading…
Reference in a new issue