Skip to content

Commit

Permalink
Add a "product name literal in pht()" linter
Browse files Browse the repository at this point in the history
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
  • Loading branch information
epriestley committed Apr 25, 2022
1 parent f098e8d commit a33aeb3
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/__phutil_library_map__.php
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@
'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',
Expand Down Expand Up @@ -914,6 +915,7 @@
'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',
Expand Down Expand Up @@ -1490,6 +1492,7 @@
'ArcanistPhutilXHPASTLinterStandard' => 'ArcanistLinterStandard',
'ArcanistPlusOperatorOnStringsXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistProductNameLiteralXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistProjectConfigurationSource' => 'ArcanistWorkingCopyConfigurationSource',
'ArcanistPrompt' => 'Phobject',
'ArcanistPromptResponse' => 'Phobject',
Expand Down Expand Up @@ -2007,6 +2010,7 @@
'PhutilVeryWowEnglishLocale' => 'PhutilLocale',
'PhutilWordPressFuture' => 'FutureProxy',
'PhutilXHPASTBinary' => 'Phobject',
'PlatformSymbols' => 'Phobject',
'PytestTestEngine' => 'ArcanistUnitTestEngine',
'TempFile' => 'Phobject',
'TestAbstractDirectedGraph' => 'AbstractDirectedGraph',
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
}
}

}
21 changes: 21 additions & 0 deletions src/platform/PlatformSymbols.php
Original file line number Diff line number Diff line change
@@ -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(),
);
}

}

0 comments on commit a33aeb3

Please sign in to comment.