From a33aeb3c36c227efa1bceedbcbb58152ec00fc60 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 21 Apr 2022 21:15:00 -0700
Subject: [PATCH] Add a "product name literal in pht()" linter

Summary:
Ref T13658. One challenge in forking Phabricator is product name references in user-visible strings, particularly in "pht()".

Add a linter to identify the use of product name literals in "pht()" text.

The linter could fix these automatically, but it looks like there are fewer than 200 across both Arcanist and Phabricator and some sampling suggests that many are probably clearer when rewritten into generic language anyway.

Test Plan: Ran linter, saw it pop out reasonable warnings.

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21763
---
 src/__phutil_library_map__.php                |  4 ++
 ...nistProductNameLiteralXHPASTLinterRule.php | 67 +++++++++++++++++++
 src/platform/PlatformSymbols.php              | 21 ++++++
 3 files changed, 92 insertions(+)
 create mode 100644 src/lint/linter/xhpast/rules/ArcanistProductNameLiteralXHPASTLinterRule.php
 create mode 100644 src/platform/PlatformSymbols.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index d29a1fe5..20af41e7 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -424,6 +424,7 @@ phutil_register_library_map(array(
     'ArcanistPhutilXHPASTLinterStandard' => 'lint/linter/standards/phutil/ArcanistPhutilXHPASTLinterStandard.php',
     'ArcanistPlusOperatorOnStringsXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPlusOperatorOnStringsXHPASTLinterRule.php',
     'ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase.php',
+    'ArcanistProductNameLiteralXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistProductNameLiteralXHPASTLinterRule.php',
     'ArcanistProjectConfigurationSource' => 'config/source/ArcanistProjectConfigurationSource.php',
     'ArcanistPrompt' => 'toolset/ArcanistPrompt.php',
     'ArcanistPromptResponse' => 'toolset/ArcanistPromptResponse.php',
@@ -914,6 +915,7 @@ phutil_register_library_map(array(
     'PhutilVeryWowEnglishLocale' => 'internationalization/locales/PhutilVeryWowEnglishLocale.php',
     'PhutilWordPressFuture' => 'future/wordpress/PhutilWordPressFuture.php',
     'PhutilXHPASTBinary' => 'parser/xhpast/bin/PhutilXHPASTBinary.php',
+    'PlatformSymbols' => 'platform/PlatformSymbols.php',
     'PytestTestEngine' => 'unit/engine/PytestTestEngine.php',
     'TempFile' => 'filesystem/TempFile.php',
     'TestAbstractDirectedGraph' => 'utils/__tests__/TestAbstractDirectedGraph.php',
@@ -1490,6 +1492,7 @@ phutil_register_library_map(array(
     'ArcanistPhutilXHPASTLinterStandard' => 'ArcanistLinterStandard',
     'ArcanistPlusOperatorOnStringsXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
+    'ArcanistProductNameLiteralXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistProjectConfigurationSource' => 'ArcanistWorkingCopyConfigurationSource',
     'ArcanistPrompt' => 'Phobject',
     'ArcanistPromptResponse' => 'Phobject',
@@ -2007,6 +2010,7 @@ phutil_register_library_map(array(
     'PhutilVeryWowEnglishLocale' => 'PhutilLocale',
     'PhutilWordPressFuture' => 'FutureProxy',
     'PhutilXHPASTBinary' => 'Phobject',
+    'PlatformSymbols' => 'Phobject',
     'PytestTestEngine' => 'ArcanistUnitTestEngine',
     'TempFile' => 'Phobject',
     'TestAbstractDirectedGraph' => 'AbstractDirectedGraph',
diff --git a/src/lint/linter/xhpast/rules/ArcanistProductNameLiteralXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistProductNameLiteralXHPASTLinterRule.php
new file mode 100644
index 00000000..70b57486
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/ArcanistProductNameLiteralXHPASTLinterRule.php
@@ -0,0 +1,67 @@
+<?php
+
+final class ArcanistProductNameLiteralXHPASTLinterRule
+  extends ArcanistXHPASTLinterRule {
+
+  const ID = 134;
+
+  public function getLintName() {
+    return pht('Use of Product Name Literal');
+  }
+
+  public function getLintSeverity() {
+    return ArcanistLintSeverity::SEVERITY_WARNING;
+  }
+
+  public function process(XHPASTNode $root) {
+    $calls = $root->selectDescendantsOfType('n_FUNCTION_CALL');
+
+    $product_names = PlatformSymbols::getProductNames();
+    foreach ($product_names as $k => $product_name) {
+      $product_names[$k] = preg_quote($product_name);
+    }
+
+    $search_pattern = '(\b(?:'.implode('|', $product_names).')\b)i';
+
+    foreach ($calls as $call) {
+      $name = $call->getChildByIndex(0)->getConcreteString();
+
+      if ($name !== 'pht') {
+        continue;
+      }
+
+      $parameters = $call->getChildByIndex(1);
+
+      if (!$parameters->getChildren()) {
+        continue;
+      }
+
+      $identifier = $parameters->getChildByIndex(0);
+      if (!$identifier->isConstantString()) {
+        continue;
+      }
+
+      $literal_value = $identifier->getStringLiteralValue();
+
+      $matches = phutil_preg_match_all($search_pattern, $literal_value);
+      if (!$matches[0]) {
+        continue;
+      }
+
+      $name_list = array();
+      foreach ($matches[0] as $match) {
+        $name_list[phutil_utf8_strtolower($match)] = $match;
+      }
+      $name_list = implode(', ', $name_list);
+
+      $this->raiseLintAtNode(
+        $identifier,
+        pht(
+          'Avoid use of product name literals in "pht()": use generic '.
+          'language or an appropriate method from the "PlatformSymbols" class '.
+          'instead so the software can be forked. String uses names: %s.',
+          $name_list));
+    }
+  }
+
+}
diff --git a/src/platform/PlatformSymbols.php b/src/platform/PlatformSymbols.php
new file mode 100644
index 00000000..7533f06b
--- /dev/null
+++ b/src/platform/PlatformSymbols.php
@@ -0,0 +1,21 @@
+<?php
+
+final class PlatformSymbols
+  extends Phobject {
+
+  public static function getPlatformClientName() {
+    return 'Arcanist';
+  }
+
+  public static function getPlatformServerName() {
+    return 'Phabricator';
+  }
+
+  public static function getProductNames() {
+    return array(
+      self::getPlatformClientName(),
+      self::getPlatformServerName(),
+    );
+  }
+
+}