From 836768bdccd42ae6a0a95dca7760e25992067ffc Mon Sep 17 00:00:00 2001
From: Asher Baker <asherkin@limetech.io>
Date: Sat, 22 Jul 2017 03:02:01 +0100
Subject: [PATCH] Fix ArcanistPHPCloseTagXHPASTLinterRule always bailing out

Summary:
D13794 changed ArcanistPHPCloseTagXHPASTLinterRule to ignore inline HTML blocks, but selectDescendantsOfType returns an AASTNodeList (which always exists).

Instead, check that the count() of the node list is > 0.

empty.lint-test had to be changed, it wouldn't have been accepted had this rule not been broken before it was commited.

Added tests to cover ArcanistPHPCloseTagXHPASTLinterRule in the future.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D18271
---
 src/__phutil_library_map__.php                         |  2 ++
 src/lint/linter/__tests__/xhpast/empty.lint-test       |  3 ++-
 .../rules/ArcanistPHPCloseTagXHPASTLinterRule.php      |  2 +-
 .../ArcanistPHPCloseTagXHPASTLinterRuleTestCase.php    | 10 ++++++++++
 .../php-close-tag-inline-html-good.lint-test           |  6 ++++++
 .../__tests__/php-close-tag/php-close-tag.lint-test    |  5 +++++
 6 files changed, 26 insertions(+), 2 deletions(-)
 create mode 100644 src/lint/linter/xhpast/rules/__tests__/ArcanistPHPCloseTagXHPASTLinterRuleTestCase.php
 create mode 100644 src/lint/linter/xhpast/rules/__tests__/php-close-tag/php-close-tag-inline-html-good.lint-test
 create mode 100644 src/lint/linter/xhpast/rules/__tests__/php-close-tag/php-close-tag.lint-test

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index d8f10dd4..2a3cfcf8 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -266,6 +266,7 @@ phutil_register_library_map(array(
     'ArcanistPEP8Linter' => 'lint/linter/ArcanistPEP8Linter.php',
     'ArcanistPEP8LinterTestCase' => 'lint/linter/__tests__/ArcanistPEP8LinterTestCase.php',
     'ArcanistPHPCloseTagXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPHPCloseTagXHPASTLinterRule.php',
+    'ArcanistPHPCloseTagXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPHPCloseTagXHPASTLinterRuleTestCase.php',
     'ArcanistPHPCompatibilityXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php',
     'ArcanistPHPCompatibilityXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPHPCompatibilityXHPASTLinterRuleTestCase.php',
     'ArcanistPHPEchoTagXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPHPEchoTagXHPASTLinterRule.php',
@@ -680,6 +681,7 @@ phutil_register_library_map(array(
     'ArcanistPEP8Linter' => 'ArcanistExternalLinter',
     'ArcanistPEP8LinterTestCase' => 'ArcanistExternalLinterTestCase',
     'ArcanistPHPCloseTagXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
+    'ArcanistPHPCloseTagXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistPHPCompatibilityXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistPHPCompatibilityXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistPHPEchoTagXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
diff --git a/src/lint/linter/__tests__/xhpast/empty.lint-test b/src/lint/linter/__tests__/xhpast/empty.lint-test
index 57595708..ddbbaebd 100644
--- a/src/lint/linter/__tests__/xhpast/empty.lint-test
+++ b/src/lint/linter/__tests__/xhpast/empty.lint-test
@@ -1,3 +1,4 @@
-<?php ?>
+<?php
+
 ~~~~~~~~~~
 warning::
diff --git a/src/lint/linter/xhpast/rules/ArcanistPHPCloseTagXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPHPCloseTagXHPASTLinterRule.php
index 95a9918f..8321a27e 100644
--- a/src/lint/linter/xhpast/rules/ArcanistPHPCloseTagXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/rules/ArcanistPHPCloseTagXHPASTLinterRule.php
@@ -12,7 +12,7 @@ final class ArcanistPHPCloseTagXHPASTLinterRule
   public function process(XHPASTNode $root) {
     $inline_html = $root->selectDescendantsOfType('n_INLINE_HTML');
 
-    if ($inline_html) {
+    if (count($inline_html) > 0) {
       return;
     }
 
diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistPHPCloseTagXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistPHPCloseTagXHPASTLinterRuleTestCase.php
new file mode 100644
index 00000000..b7c2067e
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistPHPCloseTagXHPASTLinterRuleTestCase.php
@@ -0,0 +1,10 @@
+<?php
+
+final class ArcanistPHPCloseTagXHPASTLinterRuleTestCase
+  extends ArcanistXHPASTLinterRuleTestCase {
+
+  public function testLinter() {
+    $this->executeTestsInDirectory(dirname(__FILE__).'/php-close-tag/');
+  }
+
+}
diff --git a/src/lint/linter/xhpast/rules/__tests__/php-close-tag/php-close-tag-inline-html-good.lint-test b/src/lint/linter/xhpast/rules/__tests__/php-close-tag/php-close-tag-inline-html-good.lint-test
new file mode 100644
index 00000000..826beb54
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/__tests__/php-close-tag/php-close-tag-inline-html-good.lint-test
@@ -0,0 +1,6 @@
+<?php
+?>
+stuff.
+<?php
+?>
+~~~~~~~~~~
diff --git a/src/lint/linter/xhpast/rules/__tests__/php-close-tag/php-close-tag.lint-test b/src/lint/linter/xhpast/rules/__tests__/php-close-tag/php-close-tag.lint-test
new file mode 100644
index 00000000..54fb1fc5
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/__tests__/php-close-tag/php-close-tag.lint-test
@@ -0,0 +1,5 @@
+<?php
+return;
+?>
+~~~~~~~~~~
+error:3:1