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

Add a linter rule for determining when single quotes should be used over double quotes.

Summary: Personally, I am a strong fan of this rule. There is currently a similar rule provided by PHP_CodeSniffer.

Test Plan: Wrote and executed unit tests.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9117
This commit is contained in:
Joshua Spence 2014-05-16 19:25:37 -07:00 committed by epriestley
parent 38eda3e86b
commit 35a26718d8
4 changed files with 104 additions and 5 deletions

View file

@ -43,6 +43,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
const LINT_CLOSING_DECL_PAREN = 38; const LINT_CLOSING_DECL_PAREN = 38;
const LINT_REUSED_ITERATOR_REFERENCE = 39; const LINT_REUSED_ITERATOR_REFERENCE = 39;
const LINT_KEYWORD_CASING = 40; const LINT_KEYWORD_CASING = 40;
const LINT_DOUBLE_QUOTE = 41;
private $naminghook; private $naminghook;
private $switchhook; private $switchhook;
@ -99,6 +100,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
self::LINT_CLOSING_DECL_PAREN => 'Declaration Formatting', self::LINT_CLOSING_DECL_PAREN => 'Declaration Formatting',
self::LINT_REUSED_ITERATOR_REFERENCE => 'Reuse of Iterator References', self::LINT_REUSED_ITERATOR_REFERENCE => 'Reuse of Iterator References',
self::LINT_KEYWORD_CASING => 'Keyword Conventions', self::LINT_KEYWORD_CASING => 'Keyword Conventions',
self::LINT_DOUBLE_QUOTE => 'Unnecessary Double Quotes',
); );
} }
@ -132,6 +134,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
self::LINT_CLOSING_DECL_PAREN => $warning, self::LINT_CLOSING_DECL_PAREN => $warning,
self::LINT_REUSED_ITERATOR_REFERENCE => $warning, self::LINT_REUSED_ITERATOR_REFERENCE => $warning,
self::LINT_KEYWORD_CASING => $warning, self::LINT_KEYWORD_CASING => $warning,
self::LINT_DOUBLE_QUOTE => $advice,
// This is disabled by default because it implies a very strict policy // This is disabled by default because it implies a very strict policy
// which isn't necessary in the general case. // which isn't necessary in the general case.
@ -245,6 +248,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
'lintClosingCallParen' => self::LINT_CLOSING_CALL_PAREN, 'lintClosingCallParen' => self::LINT_CLOSING_CALL_PAREN,
'lintClosingDeclarationParen' => self::LINT_CLOSING_DECL_PAREN, 'lintClosingDeclarationParen' => self::LINT_CLOSING_DECL_PAREN,
'lintKeywordCasing' => self::LINT_KEYWORD_CASING, 'lintKeywordCasing' => self::LINT_KEYWORD_CASING,
'lintStrings' => self::LINT_DOUBLE_QUOTE,
); );
foreach ($method_codes as $method => $codes) { foreach ($method_codes as $method => $codes) {
@ -2355,6 +2359,78 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
} }
} }
private function lintStrings(XHPASTNode $root) {
$nodes = $root->selectDescendantsOfTypes(array(
'n_CONCATENATION_LIST',
'n_STRING_SCALAR'));
foreach ($nodes as $node) {
$strings = array();
if ($node->getTypeName() === 'n_CONCATENATION_LIST') {
$strings = $node->selectDescendantsOfType('n_STRING_SCALAR');
} else if ($node->getTypeName() === 'n_STRING_SCALAR') {
$strings = array($node);
if ($node->getParentNode()->getTypeName() === 'n_CONCATENATION_LIST') {
continue;
}
}
$valid = false;
$invalid_nodes = array();
$fixes = array();
foreach ($strings as $string) {
$concrete_string = $string->getConcreteString();
$single_quoted = ($concrete_string[0] === "'");
$contents = substr($concrete_string, 1, -1);
// Double quoted strings are allowed when the string contains the
// following characters.
static $allowed_chars = array(
'\0',
'\n',
'\r',
'\f',
'\t',
'\v',
'\x',
'\b',
'\'',
);
$contains_special_chars = false;
foreach ($allowed_chars as $allowed_char) {
if (strpos($contents, $allowed_char) !== false) {
$contains_special_chars = true;
}
}
if (!$string->isConstantString()) {
$valid = true;
} else if ($contains_special_chars && !$single_quoted) {
$valid = true;
} else if (!$contains_special_chars && !$single_quoted) {
$invalid_nodes[] = $string;
$fixes[$string->getID()] = "'".$contents."'";
}
}
if (!$valid) {
foreach ($invalid_nodes as $invalid_node) {
$this->raiseLintAtNode(
$invalid_node,
self::LINT_DOUBLE_QUOTE,
pht(
'String does not require double quotes. For consistency, '.
'prefer single quotes.'),
$fixes[$invalid_node->getID()]);
}
}
}
}
public function getSuperGlobalNames() { public function getSuperGlobalNames() {
return array( return array(
'$GLOBALS', '$GLOBALS',

View file

@ -0,0 +1,23 @@
<?php
'foobar';
"foobar";
"foobar\n";
"'foobar'";
"foo{$bar}";
'foo"bar"';
pht(
"This string requires \x12\x34 double quotes, but ".
"this string does not. Here, they are used for consistency.");
~~~~~~~~~~
advice:3:1
~~~~~~~~~~
<?php
'foobar';
'foobar';
"foobar\n";
"'foobar'";
"foo{$bar}";
'foo"bar"';
pht(
"This string requires \x12\x34 double quotes, but ".
"this string does not. Here, they are used for consistency.");

View file

@ -15,7 +15,7 @@ switch ($x) {
return; return;
case 4: case 4:
$x++; $x++;
throw new Exception("!"); throw new Exception('!');
case 5: case 5:
break 2; break 2;
case 6: case 6:

View file

@ -1,8 +1,8 @@
<?php <?php
"a"."b"; 'a'.'b';
"a" + "b"; 'a' + 'b';
"a" + $x; 'a' + $x;
$x + $y + $z + "q" + 0; $x + $y + $z + 'q' + 0;
~~~~~~~~~~ ~~~~~~~~~~
error:3:1 error:3:1
error:4:1 error:4:1