Add a linter rule for public properties

Summary: Add a linter rule which advises against public class properties.

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14641

src/__phutil_library_map__.php

@@ -290,6 +290,8 @@ phutil_register_library_map(array(
     'ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase.php',
     'ArcanistPregQuoteMisuseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPregQuoteMisuseXHPASTLinterRule.php',
     'ArcanistPregQuoteMisuseXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPregQuoteMisuseXHPASTLinterRuleTestCase.php',
+    'ArcanistPublicPropertyXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPublicPropertyXHPASTLinterRule.php',
+    'ArcanistPublicPropertyXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPublicPropertyXHPASTLinterRuleTestCase.php',
     'ArcanistPuppetLintLinter' => 'lint/linter/ArcanistPuppetLintLinter.php',
     'ArcanistPuppetLintLinterTestCase' => 'lint/linter/__tests__/ArcanistPuppetLintLinterTestCase.php',
     'ArcanistPyFlakesLinter' => 'lint/linter/ArcanistPyFlakesLinter.php',
@@ -694,6 +696,8 @@ phutil_register_library_map(array(
     'ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistPregQuoteMisuseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistPregQuoteMisuseXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
+    'ArcanistPublicPropertyXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
+    'ArcanistPublicPropertyXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistPuppetLintLinter' => 'ArcanistExternalLinter',
     'ArcanistPuppetLintLinterTestCase' => 'ArcanistExternalLinterTestCase',
     'ArcanistPyFlakesLinter' => 'ArcanistExternalLinter',

src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php

@@ -237,6 +237,7 @@ abstract class ArcanistXHPASTLinterRule extends Phobject {
 
     switch ($modifier_list->getTypeName()) {
       case 'n_CLASS_ATTRIBUTES':
+      case 'n_CLASS_MEMBER_MODIFIER_LIST':
       case 'n_METHOD_MODIFIER_LIST':
         break;
 

src/lint/linter/xhpast/rules/ArcanistPublicPropertyXHPASTLinterRule.php

@@ -0,0 +1,35 @@
+<?php
+
+final class ArcanistPublicPropertyXHPASTLinterRule
+  extends ArcanistXHPASTLinterRule {
+
+  const ID = 130;
+
+  public function getLintName() {
+    return pht('Use of `%s` Properties', 'public');
+  }
+
+  public function getLintSeverity() {
+    return ArcanistLintSeverity::SEVERITY_ADVICE;
+  }
+
+  public function process(XHPASTNode $root) {
+    $members = $root->selectDescendantsOfType(
+      'n_CLASS_MEMBER_DECLARATION_LIST');
+
+    foreach ($members as $member) {
+      $modifiers = $this->getModifiers($member);
+
+      if (isset($modifiers['public'])) {
+        $this->raiseLintAtNode(
+          $member,
+          pht(
+            '`%s` properties should be avoided. Instead of exposing '.
+            'the property value directly, consider using getter '.
+            'and setter methods.',
+            'public'));
+      }
+    }
+  }
+
+}

src/lint/linter/xhpast/rules/__tests__/ArcanistPublicPropertyXHPASTLinterRuleTestCase.php

@@ -0,0 +1,10 @@
+<?php
+
+final class ArcanistPublicPropertyXHPASTLinterRuleTestCase
+  extends ArcanistXHPASTLinterRuleTestCase {
+
+  public function testLinter() {
+    $this->executeTestsInDirectory(dirname(__FILE__).'/public-property/');
+  }
+
+}

src/lint/linter/xhpast/rules/__tests__/public-property/class.lint-test

@@ -0,0 +1,9 @@
+<?php
+
+class SomeClass {
+  private $x;
+  protected $y;
+  public $z;
+}
+~~~~~~~~~~
+advice:6:3