diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php index 94850ab6..126e7320 100644 --- a/src/lint/linter/ArcanistXHPASTLinter.php +++ b/src/lint/linter/ArcanistXHPASTLinter.php @@ -43,6 +43,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { const LINT_CLOSING_DECL_PAREN = 38; const LINT_REUSED_ITERATOR_REFERENCE = 39; const LINT_KEYWORD_CASING = 40; + const LINT_DOUBLE_QUOTE = 41; private $naminghook; private $switchhook; @@ -99,6 +100,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { self::LINT_CLOSING_DECL_PAREN => 'Declaration Formatting', self::LINT_REUSED_ITERATOR_REFERENCE => 'Reuse of Iterator References', 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_REUSED_ITERATOR_REFERENCE => $warning, self::LINT_KEYWORD_CASING => $warning, + self::LINT_DOUBLE_QUOTE => $advice, // This is disabled by default because it implies a very strict policy // which isn't necessary in the general case. @@ -245,6 +248,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { 'lintClosingCallParen' => self::LINT_CLOSING_CALL_PAREN, 'lintClosingDeclarationParen' => self::LINT_CLOSING_DECL_PAREN, 'lintKeywordCasing' => self::LINT_KEYWORD_CASING, + 'lintStrings' => self::LINT_DOUBLE_QUOTE, ); 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() { return array( '$GLOBALS', diff --git a/src/lint/linter/__tests__/xhpast/double-quote.lint-test b/src/lint/linter/__tests__/xhpast/double-quote.lint-test new file mode 100644 index 00000000..b494bd23 --- /dev/null +++ b/src/lint/linter/__tests__/xhpast/double-quote.lint-test @@ -0,0 +1,23 @@ +