revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-22 23:02:41 +01:00
Code Issues Releases Wiki Activity

Lint undeclared variables in strings

Browse source 
Summary: Depends on D6405.

Test Plan:
New unit test.
Linted libphutil, Arcanist, Phabricator.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6406
This commit is contained in:
Jakub Vrana 2013-07-10 08:26:11 -07:00
parent fbcd686e18
commit f5ceceea87
2 changed files with 29 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
src/lint/linter/ArcanistXHPASTLinter.php
View file

@ -1100,6 +1100,9 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
// because the function calls extract() or uses dynamic variables, // because the function calls extract() or uses dynamic variables,
// preventing us from keeping track of which variables are defined) so we // preventing us from keeping track of which variables are defined) so we
// can stop issuing warnings after that. // can stop issuing warnings after that.
//
// TODO: Support functions defined inside other functions which is commonly
// used with anonymous functions.
$fdefs = $root->selectDescendantsOfType('n_FUNCTION_DECLARATION'); $fdefs = $root->selectDescendantsOfType('n_FUNCTION_DECLARATION');
$mdefs = $root->selectDescendantsOfType('n_METHOD_DECLARATION'); $mdefs = $root->selectDescendantsOfType('n_METHOD_DECLARATION');
@ -1232,7 +1235,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
$declaration_tokens[$var->getID()] = true; $declaration_tokens[$var->getID()] = true;
} }
// Excluded tokens are ones we don't "count" as being uses, described // Excluded tokens are ones we don't "count" as being used, described
// above. Put them into $exclude_tokens. // above. Put them into $exclude_tokens.
$class_statics = $body $class_statics = $body
@ -1260,7 +1263,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
// declared before a loop after the loop, even if you're just assigning // declared before a loop after the loop, even if you're just assigning
// to it. // to it.
$concrete = $var->getConcreteString(); $concrete = $this->getConcreteVariableString($var);
$uses[$concrete][$var->getID()] = $var->getOffset(); $uses[$concrete][$var->getID()] = $var->getOffset();
if (isset($declaration_tokens[$var->getID()])) { if (isset($declaration_tokens[$var->getID()])) {
@ -1272,7 +1275,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
continue; continue;
} }
$all[$var->getID()] = $var; $all[$var->getOffset()] = $concrete;
} }
@ -1361,7 +1364,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
// This is a declaration, exclude it from the "declare variables prior // This is a declaration, exclude it from the "declare variables prior
// to use" check below. // to use" check below.
unset($all[$var->getID()]); unset($all[$var->getOffset()]);
$vars[] = $var; $vars[] = $var;
} }
@ -1376,21 +1379,28 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
$declaration_tokens[$var->getID()] = true; $declaration_tokens[$var->getID()] = true;
} }
foreach (array('n_STRING_SCALAR', 'n_HEREDOC') as $type) {
foreach ($body->selectDescendantsOfType($type) as $string) {
foreach ($string->getStringVariables() as $offset => $var) {
$all[$string->getOffset() + $offset - 1] = '$'.$var;
}
}
}
// Issue a warning for every variable token, unless it appears in a // Issue a warning for every variable token, unless it appears in a
// declaration, we know about a prior declaration, we have explicitly // declaration, we know about a prior declaration, we have explicitly
// exlcuded it, or scope has been made unknowable before it appears. // exlcuded it, or scope has been made unknowable before it appears.
$issued_warnings = array(); $issued_warnings = array();
foreach ($all as $id => $var) { foreach ($all as $offset => $concrete) {
if ($var->getOffset() >= $scope_destroyed_at) { if ($offset >= $scope_destroyed_at) {
// This appears after an extract() or $$var so we have no idea // This appears after an extract() or $$var so we have no idea
// whether it's legitimate or not. We raised a harshly-worded warning // whether it's legitimate or not. We raised a harshly-worded warning
// when scope was made unknowable, so just ignore anything we can't // when scope was made unknowable, so just ignore anything we can't
// figure out. // figure out.
continue; continue;
} }
$concrete = $this->getConcreteVariableString($var); if ($offset >= idx($declarations, $concrete, PHP_INT_MAX)) {
if ($var->getOffset() >= idx($declarations, $concrete, PHP_INT_MAX)) {
// The use appears after the variable is declared, so it's fine. // The use appears after the variable is declared, so it's fine.
continue; continue;
} }
@ -1399,12 +1409,13 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
// to issue another one. // to issue another one.
continue; continue;
} }
$this->raiseLintAtNode( $this->raiseLintAtOffset(
$var, $offset,
self::LINT_UNDECLARED_VARIABLE, self::LINT_UNDECLARED_VARIABLE,
'Declare variables prior to use (even if you are passing them '. 'Declare variables prior to use (even if you are passing them '.
'as reference parameters). You may have misspelled this '. 'as reference parameters). You may have misspelled this '.
'variable name.'); 'variable name.',
$concrete);
$issued_warnings[$concrete] = true; $issued_warnings[$concrete] = true;
} }
} }

7
src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test
View file

@ -160,6 +160,12 @@ function arrow($o, $x) {
echo $o->{$x->{$x->{$x.$x->{$x}}.$x}}; echo $o->{$x->{$x->{$x.$x->{$x}}.$x}};
} }
function strings() {
$a = 1;
echo "$a";
echo "$b";
}
~~~~~~~~~~ ~~~~~~~~~~
disabled:3:1 disabled:3:1
error:30:3 error:30:3
@ -178,3 +184,4 @@ error:121:3 isset() and empty() should not trigger errors.
error:125:3 Should only warn once in this function. error:125:3 Should only warn once in this function.
error:146:8 error:146:8
error:152:9 error:152:9
error:166:9