mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-29 02:02:40 +01:00
Add a lint check for deprecated argument order to "implode()"
Summary: Ref T13428. Historically, "implode()" accepts arguments in either order. PHP 7.4+ warns about glue not being first. Add a lint check when the second parameter is a static scalar, implying it is the glue parameter. Test Plan: Ran unit tests. See next change. Maniphest Tasks: T13428 Differential Revision: https://secure.phabricator.com/D20857
This commit is contained in:
parent
3cdfe1fff8
commit
da6d4f85ee
4 changed files with 72 additions and 0 deletions
|
@ -185,6 +185,8 @@ phutil_register_library_map(array(
|
||||||
'ArcanistImplicitFallthroughXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistImplicitFallthroughXHPASTLinterRuleTestCase.php',
|
'ArcanistImplicitFallthroughXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistImplicitFallthroughXHPASTLinterRuleTestCase.php',
|
||||||
'ArcanistImplicitVisibilityXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistImplicitVisibilityXHPASTLinterRule.php',
|
'ArcanistImplicitVisibilityXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistImplicitVisibilityXHPASTLinterRule.php',
|
||||||
'ArcanistImplicitVisibilityXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistImplicitVisibilityXHPASTLinterRuleTestCase.php',
|
'ArcanistImplicitVisibilityXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistImplicitVisibilityXHPASTLinterRuleTestCase.php',
|
||||||
|
'ArcanistImplodeArgumentOrderXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistImplodeArgumentOrderXHPASTLinterRule.php',
|
||||||
|
'ArcanistImplodeArgumentOrderXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistImplodeArgumentOrderXHPASTLinterRuleTestCase.php',
|
||||||
'ArcanistInlineHTMLXHPASTLinterRule' => 'lint/linter/ArcanistInlineHTMLXHPASTLinterRule.php',
|
'ArcanistInlineHTMLXHPASTLinterRule' => 'lint/linter/ArcanistInlineHTMLXHPASTLinterRule.php',
|
||||||
'ArcanistInlineHTMLXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistInlineHTMLXHPASTLinterRuleTestCase.php',
|
'ArcanistInlineHTMLXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistInlineHTMLXHPASTLinterRuleTestCase.php',
|
||||||
'ArcanistInnerFunctionXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInnerFunctionXHPASTLinterRule.php',
|
'ArcanistInnerFunctionXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInnerFunctionXHPASTLinterRule.php',
|
||||||
|
@ -608,6 +610,8 @@ phutil_register_library_map(array(
|
||||||
'ArcanistImplicitFallthroughXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
'ArcanistImplicitFallthroughXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
'ArcanistImplicitVisibilityXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
'ArcanistImplicitVisibilityXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
||||||
'ArcanistImplicitVisibilityXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
'ArcanistImplicitVisibilityXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
|
'ArcanistImplodeArgumentOrderXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
||||||
|
'ArcanistImplodeArgumentOrderXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
'ArcanistInlineHTMLXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
'ArcanistInlineHTMLXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
||||||
'ArcanistInlineHTMLXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
'ArcanistInlineHTMLXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
'ArcanistInnerFunctionXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
'ArcanistInnerFunctionXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
||||||
|
|
|
@ -0,0 +1,39 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
final class ArcanistImplodeArgumentOrderXHPASTLinterRule
|
||||||
|
extends ArcanistXHPASTLinterRule {
|
||||||
|
|
||||||
|
const ID = 129;
|
||||||
|
|
||||||
|
public function getLintName() {
|
||||||
|
return pht('Implode With Glue First');
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getLintSeverity() {
|
||||||
|
return ArcanistLintSeverity::SEVERITY_ERROR;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function process(XHPASTNode $root) {
|
||||||
|
$implosions = $this->getFunctionCalls($root, array('implode'));
|
||||||
|
foreach ($implosions as $implosion) {
|
||||||
|
$parameters = $implosion->getChildOfType(1, 'n_CALL_PARAMETER_LIST');
|
||||||
|
|
||||||
|
if (count($parameters->getChildren()) != 2) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
$parameter = $parameters->getChildByIndex(1);
|
||||||
|
if (!$parameter->isStaticScalar()) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
$this->raiseLintAtNode(
|
||||||
|
$implosion,
|
||||||
|
pht(
|
||||||
|
'When calling "implode()", pass the "glue" argument first. (The '.
|
||||||
|
'other parameter order is deprecated in PHP 7.4 and raises a '.
|
||||||
|
'warning.)'));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
|
@ -0,0 +1,11 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
final class ArcanistImplodeArgumentOrderXHPASTLinterRuleTestCase
|
||||||
|
extends ArcanistXHPASTLinterRuleTestCase {
|
||||||
|
|
||||||
|
public function testLinter() {
|
||||||
|
$this->executeTestsInDirectory(
|
||||||
|
dirname(__FILE__).'/implode-argument-order/');
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
|
@ -0,0 +1,18 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
// This is the correct argument order.
|
||||||
|
implode(' ', $x);
|
||||||
|
|
||||||
|
// This is the legacy argument order which warns in PHP 7.4+.
|
||||||
|
implode($x, ' ');
|
||||||
|
|
||||||
|
// No warning: we can't statically tell which one is the glue.
|
||||||
|
implode($x, $y);
|
||||||
|
|
||||||
|
// No warning: these are likely wrong, but not a glue order problem.
|
||||||
|
implode();
|
||||||
|
implode($x);
|
||||||
|
implode($x, $y, $z);
|
||||||
|
|
||||||
|
~~~~~~~~~~
|
||||||
|
error:7:1:XHP129:Implode With Glue First
|
Loading…
Reference in a new issue