mirror of
https://we.phorge.it/source/arcanist.git
synced 2025-03-27 19:50:19 +01:00
Display other locations of Reused lint errors
Summary: It's not trivial to find them inside 700+ lines long functions. Test Plan: Linted `reused-iterators.lint-test` renamed to `_.php`, saw other locations. Repeated for `reused-local.lint-test`. Repeated for `duplicate-key-in-array.lint-test`. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D4871
This commit is contained in:
parent
4c35af9283
commit
ca37bfb2ab
3 changed files with 41 additions and 16 deletions
|
@ -42,6 +42,22 @@ abstract class ArcanistLinter {
|
|||
return $this->activePath;
|
||||
}
|
||||
|
||||
public function getOtherLocation($offset, $path = null) {
|
||||
if ($path === null) {
|
||||
$path = $this->getActivePath();
|
||||
}
|
||||
|
||||
list($line, $char) = $this->getEngine()->getLineAndCharFromOffset(
|
||||
$path,
|
||||
$offset);
|
||||
|
||||
return array(
|
||||
'path' => $path,
|
||||
'line' => $line + 1,
|
||||
'char' => $char,
|
||||
);
|
||||
}
|
||||
|
||||
public function stopAllLinters() {
|
||||
$this->stopAllLinters = true;
|
||||
return $this;
|
||||
|
|
|
@ -787,7 +787,8 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
$bin_exprs = $for_expr->selectDescendantsOfType('n_BINARY_EXPRESSION');
|
||||
foreach ($bin_exprs as $bin_expr) {
|
||||
if ($bin_expr->getChildByIndex(1)->getConcreteString() == '=') {
|
||||
$var_map[$bin_expr->getChildByIndex(0)->getConcreteString()] = true;
|
||||
$var = $bin_expr->getChildByIndex(0);
|
||||
$var_map[$var->getConcreteString()] = $var;
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -812,7 +813,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
}
|
||||
$name = $var->getConcreteString();
|
||||
$name = trim($name, '&'); // Get rid of ref silliness.
|
||||
$var_map[$name] = true;
|
||||
$var_map[$name] = $var;
|
||||
}
|
||||
|
||||
$used_vars[$foreach_loop->getID()] = $var_map;
|
||||
|
@ -830,12 +831,18 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
$shared = array_intersect_key($outer_vars, $inner_vars);
|
||||
if ($shared) {
|
||||
$shared_desc = implode(', ', array_keys($shared));
|
||||
$this->raiseLintAtNode(
|
||||
$message = $this->raiseLintAtNode(
|
||||
$inner_loop->getChildByIndex(0),
|
||||
self::LINT_REUSED_ITERATORS,
|
||||
"This loop reuses iterator variables ({$shared_desc}) from an ".
|
||||
"outer loop. You might be clobbering the outer iterator. Change ".
|
||||
"the inner loop to use a different iterator name.");
|
||||
|
||||
$locations = array();
|
||||
foreach ($shared as $var) {
|
||||
$locations[] = $this->getOtherLocation($var->getOffset());
|
||||
}
|
||||
$message->setOtherLocations($locations);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -1129,12 +1136,16 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
if ($declarations[$concrete] < $offset) {
|
||||
if (!empty($uses[$concrete]) &&
|
||||
max($uses[$concrete]) > $offset) {
|
||||
$this->raiseLintAtNode(
|
||||
$message = $this->raiseLintAtNode(
|
||||
$var,
|
||||
self::LINT_REUSED_AS_ITERATOR,
|
||||
'This iterator variable is a previously declared local '.
|
||||
'variable. To avoid overwriting locals, do not reuse them '.
|
||||
'as iterator variables.');
|
||||
$message->setOtherLocations(array(
|
||||
$this->getOtherLocation($declarations[$concrete]),
|
||||
$this->getOtherLocation(max($uses[$concrete])),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -1930,13 +1941,18 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
}
|
||||
|
||||
foreach ($keys_warn as $key => $_) {
|
||||
foreach ($nodes_by_key[$key] as $node) {
|
||||
$this->raiseLintAtNode(
|
||||
$node,
|
||||
self::LINT_DUPLICATE_KEYS_IN_ARRAY,
|
||||
"Duplicate key in array initializer. PHP will ignore all ".
|
||||
$node = array_pop($nodes_by_key[$key]);
|
||||
$message = $this->raiseLintAtNode(
|
||||
$node,
|
||||
self::LINT_DUPLICATE_KEYS_IN_ARRAY,
|
||||
"Duplicate key in array initializer. PHP will ignore all ".
|
||||
"but the last entry.");
|
||||
|
||||
$locations = array();
|
||||
foreach ($nodes_by_key[$key] as $node) {
|
||||
$locations[] = $this->getOtherLocation($node->getOffset());
|
||||
}
|
||||
$message->setOtherLocations($locations);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -34,16 +34,9 @@ $f = array(
|
|||
$a => 'var2',
|
||||
);
|
||||
~~~~~~~~~~
|
||||
error:3:3
|
||||
error:5:3
|
||||
error:6:3
|
||||
error:7:3
|
||||
error:8:3
|
||||
error:14:3
|
||||
error:15:3
|
||||
error:19:3
|
||||
error:20:3
|
||||
error:24:3
|
||||
error:25:3
|
||||
error:32:3
|
||||
error:34:3
|
||||
|
|
Loading…
Add table
Reference in a new issue