mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-25 08:12:40 +01:00
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
This commit is contained in:
parent
f098e8d863
commit
a33aeb3c36
3 changed files with 92 additions and 0 deletions
|
@ -424,6 +424,7 @@ phutil_register_library_map(array(
|
||||||
'ArcanistPhutilXHPASTLinterStandard' => 'lint/linter/standards/phutil/ArcanistPhutilXHPASTLinterStandard.php',
|
'ArcanistPhutilXHPASTLinterStandard' => 'lint/linter/standards/phutil/ArcanistPhutilXHPASTLinterStandard.php',
|
||||||
'ArcanistPlusOperatorOnStringsXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPlusOperatorOnStringsXHPASTLinterRule.php',
|
'ArcanistPlusOperatorOnStringsXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPlusOperatorOnStringsXHPASTLinterRule.php',
|
||||||
'ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase.php',
|
'ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase.php',
|
||||||
|
'ArcanistProductNameLiteralXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistProductNameLiteralXHPASTLinterRule.php',
|
||||||
'ArcanistProjectConfigurationSource' => 'config/source/ArcanistProjectConfigurationSource.php',
|
'ArcanistProjectConfigurationSource' => 'config/source/ArcanistProjectConfigurationSource.php',
|
||||||
'ArcanistPrompt' => 'toolset/ArcanistPrompt.php',
|
'ArcanistPrompt' => 'toolset/ArcanistPrompt.php',
|
||||||
'ArcanistPromptResponse' => 'toolset/ArcanistPromptResponse.php',
|
'ArcanistPromptResponse' => 'toolset/ArcanistPromptResponse.php',
|
||||||
|
@ -914,6 +915,7 @@ phutil_register_library_map(array(
|
||||||
'PhutilVeryWowEnglishLocale' => 'internationalization/locales/PhutilVeryWowEnglishLocale.php',
|
'PhutilVeryWowEnglishLocale' => 'internationalization/locales/PhutilVeryWowEnglishLocale.php',
|
||||||
'PhutilWordPressFuture' => 'future/wordpress/PhutilWordPressFuture.php',
|
'PhutilWordPressFuture' => 'future/wordpress/PhutilWordPressFuture.php',
|
||||||
'PhutilXHPASTBinary' => 'parser/xhpast/bin/PhutilXHPASTBinary.php',
|
'PhutilXHPASTBinary' => 'parser/xhpast/bin/PhutilXHPASTBinary.php',
|
||||||
|
'PlatformSymbols' => 'platform/PlatformSymbols.php',
|
||||||
'PytestTestEngine' => 'unit/engine/PytestTestEngine.php',
|
'PytestTestEngine' => 'unit/engine/PytestTestEngine.php',
|
||||||
'TempFile' => 'filesystem/TempFile.php',
|
'TempFile' => 'filesystem/TempFile.php',
|
||||||
'TestAbstractDirectedGraph' => 'utils/__tests__/TestAbstractDirectedGraph.php',
|
'TestAbstractDirectedGraph' => 'utils/__tests__/TestAbstractDirectedGraph.php',
|
||||||
|
@ -1490,6 +1492,7 @@ phutil_register_library_map(array(
|
||||||
'ArcanistPhutilXHPASTLinterStandard' => 'ArcanistLinterStandard',
|
'ArcanistPhutilXHPASTLinterStandard' => 'ArcanistLinterStandard',
|
||||||
'ArcanistPlusOperatorOnStringsXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
'ArcanistPlusOperatorOnStringsXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
||||||
'ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
'ArcanistPlusOperatorOnStringsXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
|
'ArcanistProductNameLiteralXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
||||||
'ArcanistProjectConfigurationSource' => 'ArcanistWorkingCopyConfigurationSource',
|
'ArcanistProjectConfigurationSource' => 'ArcanistWorkingCopyConfigurationSource',
|
||||||
'ArcanistPrompt' => 'Phobject',
|
'ArcanistPrompt' => 'Phobject',
|
||||||
'ArcanistPromptResponse' => 'Phobject',
|
'ArcanistPromptResponse' => 'Phobject',
|
||||||
|
@ -2007,6 +2010,7 @@ phutil_register_library_map(array(
|
||||||
'PhutilVeryWowEnglishLocale' => 'PhutilLocale',
|
'PhutilVeryWowEnglishLocale' => 'PhutilLocale',
|
||||||
'PhutilWordPressFuture' => 'FutureProxy',
|
'PhutilWordPressFuture' => 'FutureProxy',
|
||||||
'PhutilXHPASTBinary' => 'Phobject',
|
'PhutilXHPASTBinary' => 'Phobject',
|
||||||
|
'PlatformSymbols' => 'Phobject',
|
||||||
'PytestTestEngine' => 'ArcanistUnitTestEngine',
|
'PytestTestEngine' => 'ArcanistUnitTestEngine',
|
||||||
'TempFile' => 'Phobject',
|
'TempFile' => 'Phobject',
|
||||||
'TestAbstractDirectedGraph' => 'AbstractDirectedGraph',
|
'TestAbstractDirectedGraph' => 'AbstractDirectedGraph',
|
||||||
|
|
|
@ -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
src/platform/PlatformSymbols.php
Normal file
21
src/platform/PlatformSymbols.php
Normal file
|
@ -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(),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
Loading…
Reference in a new issue