Skip to content

Commit

Permalink
Add a linter rule to prevent hardcoded class names
Browse files Browse the repository at this point in the history
Summary: Add a linter rule to advise against the use of hardcoded class names. Hardcoded class names make the code harder to refactor and it is generally preferable to use the `__CLASS__` magic constant instead.

Test Plan: This works, but needs some polish.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12605
  • Loading branch information
joshuaspence committed May 12, 2015
1 parent 73aca1f commit 9090efc
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 1 deletion.
41 changes: 40 additions & 1 deletion src/lint/linter/ArcanistXHPASTLinter.php
Expand Up @@ -61,6 +61,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
const LINT_INNER_FUNCTION = 59;
const LINT_DEFAULT_PARAMETERS = 60;
const LINT_LOWERCASE_FUNCTIONS = 61;
const LINT_CLASS_NAME_LITERAL = 62;

private $blacklistedFunctions = array();
private $naminghook;
Expand Down Expand Up @@ -191,6 +192,8 @@ public function getLintNameMap() {
=> pht('Default Parameters'),
self::LINT_LOWERCASE_FUNCTIONS
=> pht('Lowercase Functions'),
self::LINT_CLASS_NAME_LITERAL
=> pht('Class Name Literal'),
);
}

Expand Down Expand Up @@ -240,6 +243,7 @@ public function getLintSeverityMap() {
self::LINT_INNER_FUNCTION => $warning,
self::LINT_DEFAULT_PARAMETERS => $warning,
self::LINT_LOWERCASE_FUNCTIONS => $advice,
self::LINT_CLASS_NAME_LITERAL => $advice,
);
}

Expand Down Expand Up @@ -307,7 +311,7 @@ public function setLinterConfigurationValue($key, $value) {

public function getVersion() {
// The version number should be incremented whenever a new rule is added.
return '24';
return '25';
}

protected function resolveFuture($path, Future $future) {
Expand Down Expand Up @@ -395,6 +399,7 @@ protected function resolveFuture($path, Future $future) {
'lintInnerFunctions' => self::LINT_INNER_FUNCTION,
'lintDefaultParameters' => self::LINT_DEFAULT_PARAMETERS,
'lintLowercaseFunctions' => self::LINT_LOWERCASE_FUNCTIONS,
'lintClassNameLiteral' => self::LINT_CLASS_NAME_LITERAL,
);

foreach ($method_codes as $method => $codes) {
Expand Down Expand Up @@ -3727,6 +3732,40 @@ private function lintLowercaseFunctions(XHPASTNode $root) {
}
}

private function lintClassNameLiteral(XHPASTNode $root) {
$class_declarations = $root->selectDescendantsOfType('n_CLASS_DECLARATION');

foreach ($class_declarations as $class_declaration) {
$class_name = $class_declaration
->getChildOfType(1, 'n_CLASS_NAME')
->getConcreteString();

$strings = $class_declaration->selectDescendantsOfType('n_STRING_SCALAR');

foreach ($strings as $string) {
$contents = substr($string->getSemanticString(), 1, -1);
$replacement = null;

if ($contents == $class_name) {
$replacement = '__CLASS__';
}

$regex = '/\b'.preg_quote($class_name, '/').'\b/';
if (!preg_match($regex, $contents)) {
continue;
}

$this->raiseLintAtNode(
$string,
self::LINT_CLASS_NAME_LITERAL,
pht(
"Don't hard-code class names, use %s instead.",
'__CLASS__'),
$replacement);
}
}
}

/**
* Retrieve all calls to some specified function(s).
*
Expand Down
29 changes: 29 additions & 0 deletions src/lint/linter/__tests__/xhpast/class-name-literal.lint-test
@@ -0,0 +1,29 @@
<?php
class MyClass {
public function someMethod() {
return 'MyClass';
}

public function someOtherMethod() {
$x = 'MyClass is awesome';
$y = 'MyClassIsAwesome';
return __CLASS__;
}
}
~~~~~~~~~~
error:2:7
advice:4:12
advice:8:10
~~~~~~~~~~
<?php
class MyClass {
public function someMethod() {
return __CLASS__;
}

public function someOtherMethod() {
$x = 'MyClass is awesome';
$y = 'MyClassIsAwesome';
return __CLASS__;
}
}

0 comments on commit 9090efc

Please sign in to comment.