Page MenuHomePhabricator

D9117.id21698.diff
No OneTemporary

D9117.id21698.diff

diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php
--- a/src/lint/linter/ArcanistXHPASTLinter.php
+++ b/src/lint/linter/ArcanistXHPASTLinter.php
@@ -43,6 +43,7 @@
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 @@
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 @@
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 @@
'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,76 @@
}
}
+ 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,
+ 'String does not require double quotes; use single quotes instead.',
+ $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
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/double-quote.lint-test
@@ -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.");
diff --git a/src/lint/linter/__tests__/xhpast/switches.lint-test b/src/lint/linter/__tests__/xhpast/switches.lint-test
--- a/src/lint/linter/__tests__/xhpast/switches.lint-test
+++ b/src/lint/linter/__tests__/xhpast/switches.lint-test
@@ -15,7 +15,7 @@
return;
case 4:
$x++;
- throw new Exception("!");
+ throw new Exception('!');
case 5:
break 2;
case 6:
diff --git a/src/lint/linter/__tests__/xhpast/wrong-concat-operator.lint-test b/src/lint/linter/__tests__/xhpast/wrong-concat-operator.lint-test
--- a/src/lint/linter/__tests__/xhpast/wrong-concat-operator.lint-test
+++ b/src/lint/linter/__tests__/xhpast/wrong-concat-operator.lint-test
@@ -1,8 +1,8 @@
<?php
-"a"."b";
-"a" + "b";
-"a" + $x;
-$x + $y + $z + "q" + 0;
+'a'.'b';
+'a' + 'b';
+'a' + $x;
+$x + $y + $z + 'q' + 0;
~~~~~~~~~~
error:3:1
error:4:1

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 16, 11:09 PM (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7707811
Default Alt Text
D9117.id21698.diff (5 KB)

Event Timeline