danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2025-01-21 21:31:13 +01:00
Code Issues Projects Releases Wiki Activity

Fix #2150 - add detection for unnecessary @var annotations

Browse Source 
And also remove them from codebase
This commit is contained in:
Brown 2019-09-19 11:59:43 -04:00
parent e9cd7917a4
commit c5ef2516b5
27 changed files with 138 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
config.xsd
View File

@ -344,6 +344,7 @@
<xs:element name="UnrecognizedStatement" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnresolvableInclude" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnevaluatedCode" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnnecessaryVarAnnotation" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedClosureParam" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedFunctionCall" type="FunctionIssueHandlerType" minOccurs="0" />
<xs:element name="UnusedMethodCall" type="MethodIssueHandlerType" minOccurs="0" />

13
docs/running_psalm/issues.md
View File

@ -2375,6 +2375,19 @@ class A {
}
```
### UnnecessaryVarAnnotation
Emitted when `--find-dead-code` is turned you're using a `@var` annotation on an assignment that Psalm has already identified a type for.
```php
function foo() : string {
return "hello";
}
/** @var string */
$a = foo();
```
### UnrecognizedExpression
Emitted when Psalm encounters an expression that it doesn't know how to handle. This should never happen.

2
src/Psalm/Config/Creator.php
View File

@ -103,13 +103,11 @@ class Creator
$nodes = [];
/** @var string|string[] $path */
foreach ($psr_paths as $paths) {
if (!is_array($paths)) {
$paths = [$paths];
}
/** @var string $path */
foreach ($paths as $path) {
if ($path === '') {
$nodes = array_merge(

1
src/Psalm/DocComment.php
View File

@ -78,7 +78,6 @@ class DocComment
if ($preserve_format) {
foreach ($lines as $m => $line) {
if (preg_match('/^\s?@([\w\-:]+)[\t ]*(.*)$/sm', $line, $matches)) {
/** @var string[] $matches */
list($full_match, $type, $data) = $matches;
$docblock = str_replace($full_match, '', $docblock);

2
src/Psalm/ErrorBaseline.php
View File

@ -97,7 +97,6 @@ class ErrorBaseline
/** @var \DOMElement $filesElement */
$filesElement = $filesElement[0];
/** @var \DOMElement $file */
foreach ($filesElement->getElementsByTagName('file') as $file) {
$fileName = $file->getAttribute('src');
@ -115,7 +114,6 @@ class ErrorBaseline
];
$codeSamples = $issue->getElementsByTagName('code');
/** @var \DOMElement $codeSample */
foreach ($codeSamples as $codeSample) {
$files[$fileName][$issueType]['s'][] = (string) $codeSample->textContent;
}

5
src/Psalm/Internal/Analyzer/CommentAnalyzer.php
View File

@ -334,7 +334,6 @@ class CommentAnalyzer
$info = new FunctionDocblockComment();
if (isset($parsed_docblock['specials']['return']) || isset($parsed_docblock['specials']['psalm-return'])) {
/** @var array<int, string> */
$return_specials = isset($parsed_docblock['specials']['psalm-return'])
? $parsed_docblock['specials']['psalm-return']
: $parsed_docblock['specials']['return'];
@ -355,7 +354,6 @@ class CommentAnalyzer
? $parsed_docblock['specials']['psalm-param']
: []);
/** @var string $param */
foreach ($all_params as $offset => $param) {
$line_parts = self::splitDocLine($param);
@ -400,7 +398,6 @@ class CommentAnalyzer
}
if (isset($parsed_docblock['specials']['param-out'])) {
/** @var string $param */
foreach ($parsed_docblock['specials']['param-out'] as $offset => $param) {
$line_parts = self::splitDocLine($param);
@ -441,7 +438,6 @@ class CommentAnalyzer
}
if (isset($parsed_docblock['specials']['psalm-taint-sink'])) {
/** @var string $param */
foreach ($parsed_docblock['specials']['psalm-taint-sink'] as $param) {
$param = trim($param);
@ -450,7 +446,6 @@ class CommentAnalyzer
}
if (isset($parsed_docblock['specials']['psalm-assert-untainted'])) {
/** @var string $param */
foreach ($parsed_docblock['specials']['psalm-assert-untainted'] as $param) {
$param = trim($param);

4
src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
View File

@ -295,7 +295,6 @@ abstract class FunctionLikeAnalyzer extends SourceAnalyzer implements Statements
$closure_return_type = Type::getMixed();
}
/** @var PhpParser\Node\Expr\Closure $this->function */
$this->function->inferredType = new Type\Union([
new Type\Atomic\TFn(
'Closure',
@ -922,9 +921,6 @@ abstract class FunctionLikeAnalyzer extends SourceAnalyzer implements Statements
];
}
/**
* @var PhpParser\Node\Param
*/
$parser_param = $this->function->getParams()[$offset];
if (!$function_param->type_location || !$function_param->location) {

2
src/Psalm/Internal/Analyzer/ProjectAnalyzer.php
View File

@ -1341,7 +1341,6 @@ class ProjectAnalyzer
$ret = @shell_exec('nproc');
if (is_string($ret)) {
$ret = trim($ret);
/** @var int|false */
$tmp = filter_var($ret, FILTER_VALIDATE_INT);
if (is_int($tmp)) {
return $tmp;
@ -1352,7 +1351,6 @@ class ProjectAnalyzer
$ret = @shell_exec('sysctl -n hw.ncpu');
if (is_string($ret)) {
$ret = trim($ret);
/** @var int|false */
$tmp = filter_var($ret, FILTER_VALIDATE_INT);
if (is_int($tmp)) {
return $tmp;

16
src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php
View File

@ -21,6 +21,7 @@ use Psalm\Issue\PossiblyInvalidIterator;
use Psalm\Issue\PossiblyNullIterator;
use Psalm\Issue\PossibleRawObjectIteration;
use Psalm\Issue\RawObjectIteration;
use Psalm\Issue\UnnecessaryVarAnnotation;
use Psalm\IssueBuffer;
use Psalm\Internal\Scope\LoopScope;
use Psalm\Type;
@ -143,6 +144,21 @@ class ForeachAnalyzer
if (isset($context->vars_in_scope[$var_comment->var_id])
|| $statements_analyzer->isSuperGlobal($var_comment->var_id)
) {
if ($codebase->find_unused_variables
&& $doc_comment
&& isset($context->vars_in_scope[$var_comment->var_id])
&& $context->vars_in_scope[$var_comment->var_id]->getId() === $comment_type->getId()
) {
if (IssueBuffer::accepts(
new UnnecessaryVarAnnotation(
'The @var annotation for ' . $var_comment->var_id . ' is unnecessary',
new CodeLocation($statements_analyzer->getSource(), $stmt, null, true)
)
)) {
// fall through
}
}
$context->vars_in_scope[$var_comment->var_id] = $comment_type;
}
}

1
src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php
View File

@ -55,7 +55,6 @@ class ArrayAnalyzer
$codebase = $statements_analyzer->getCodebase();
/** @var int $int_offset */
foreach ($stmt->items as $int_offset => $item) {
if ($item === null) {
continue;

39
src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
View File

@ -22,6 +22,7 @@ use Psalm\Issue\MixedAssignment;
use Psalm\Issue\NoValue;
use Psalm\Issue\PossiblyUndefinedArrayOffset;
use Psalm\Issue\ReferenceConstraintViolation;
use Psalm\Issue\UnnecessaryVarAnnotation;
use Psalm\IssueBuffer;
use Psalm\Type;
use function is_string;
@ -158,6 +159,20 @@ class AssignmentAnalyzer
continue;
}
if ($codebase->find_unused_variables
&& isset($context->vars_in_scope[$var_comment->var_id])
&& $context->vars_in_scope[$var_comment->var_id]->getId() === $var_comment_type->getId()
) {
if (IssueBuffer::accepts(
new UnnecessaryVarAnnotation(
'The @var annotation for ' . $var_comment->var_id . ' is unnecessary',
new CodeLocation($statements_analyzer->getSource(), $assign_var)
)
)) {
// fall through
}
}
$context->vars_in_scope[$var_comment->var_id] = $var_comment_type;
} catch (\UnexpectedValueException $e) {
if (IssueBuffer::accepts(
@ -204,6 +219,23 @@ class AssignmentAnalyzer
}
if ($comment_type) {
$temp_assign_value_type = $assign_value_type ?: ($assign_value->inferredType ?? null);
if ($codebase->find_unused_variables
&& $temp_assign_value_type
&& $array_var_id
&& $temp_assign_value_type->getId() === $comment_type->getId()
) {
if (IssueBuffer::accepts(
new UnnecessaryVarAnnotation(
'The @var annotation for ' . $array_var_id . ' is unnecessary',
new CodeLocation($statements_analyzer->getSource(), $assign_var)
)
)) {
// fall through
}
}
$assign_value_type = $comment_type;
} elseif (!$assign_value_type) {
if (isset($assign_value->inferredType)) {
@ -362,7 +394,6 @@ class AssignmentAnalyzer
} elseif ($assign_var instanceof PhpParser\Node\Expr\List_
|| $assign_var instanceof PhpParser\Node\Expr\Array_
) {
/** @var int $offset */
foreach ($assign_var->items as $offset => $assign_var_item) {
// $assign_var_item can be null e.g. list($a, ) = ['a', 'b']
if (!$assign_var_item) {
@ -386,8 +417,10 @@ class AssignmentAnalyzer
continue;
}
if (isset($assign_value_type->getTypes()['array'])
&& ($array_atomic_type = $assign_value_type->getTypes()['array'])
$assign_value_types = $assign_value_type->getTypes();
if (isset($assign_value_types['array'])
&& ($array_atomic_type = $assign_value_types['array'])
&& $array_atomic_type instanceof Type\Atomic\ObjectLike
&& !$assign_var_item->key
&& isset($array_atomic_type->properties[$offset]) // if object-like has int offsets

3
src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php
View File

@ -1894,10 +1894,8 @@ class CallAnalyzer
$array_arg_types[] = $array_arg_type;
}
/** @var null|PhpParser\Node\Arg */
$closure_arg = isset($args[$closure_index]) ? $args[$closure_index] : null;
/** @var Type\Union|null */
$closure_arg_type = $closure_arg && isset($closure_arg->value->inferredType)
? $closure_arg->value->inferredType
: null;
@ -2161,7 +2159,6 @@ class CallAnalyzer
continue;
}
/** @var Type\Atomic\TArray */
$array_arg_type = $array_arg_types[$i];
$input_type = $array_arg_type->type_params[1];

16
src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php
View File

@ -43,6 +43,7 @@ use Psalm\Issue\PossiblyInvalidOperand;
use Psalm\Issue\PossiblyUndefinedVariable;
use Psalm\Issue\UndefinedConstant;
use Psalm\Issue\UndefinedVariable;
use Psalm\Issue\UnnecessaryVarAnnotation;
use Psalm\Issue\UnrecognizedExpression;
use Psalm\IssueBuffer;
use Psalm\Type;
@ -1460,6 +1461,20 @@ class ExpressionAnalyzer
continue;
}
if ($codebase->find_unused_variables
&& isset($context->vars_in_scope[$var_comment->var_id])
&& $context->vars_in_scope[$var_comment->var_id]->getId() === $comment_type->getId()
) {
if (IssueBuffer::accepts(
new UnnecessaryVarAnnotation(
'The @var annotation for ' . $var_comment->var_id . ' is unnecessary',
new CodeLocation($statements_analyzer->getSource(), $stmt)
)
)) {
// fall through
}
}
$context->vars_in_scope[$var_comment->var_id] = $comment_type;
}
}
@ -1613,7 +1628,6 @@ class ExpressionAnalyzer
PhpParser\Node\Scalar\Encapsed $stmt,
Context $context
) {
/** @var PhpParser\Node\Expr $part */
foreach ($stmt->parts as $part) {
if (self::analyze($statements_analyzer, $part, $context) === false) {
return false;

3
src/Psalm/Internal/Analyzer/StatementsAnalyzer.php
View File

@ -1119,9 +1119,6 @@ class StatementsAnalyzer extends SourceAnalyzer implements StatementsSource
// Check if we have to remove assignment statemnt as expression (i.e. just "$var = ")
// Consider chain of assignments
/** @var PhpParser\Node\Expr\Assign | PhpParser\Node\Expr\AssignOp |
PhpParser\Node\Expr\AssignRef $assign_exp */
/** @var PhpParser\Node\Expr $rhs_exp */
$rhs_exp = $assign_exp->expr;
if ($rhs_exp instanceof PhpParser\Node\Expr\Assign
|| $rhs_exp instanceof PhpParser\Node\Expr\AssignOp

5
src/Psalm/Internal/Codebase/Reflection.php
View File

@ -100,7 +100,6 @@ class Reflection
? PropertyMap::getPropertyMap()[strtolower($class_name)]
: [];
/** @var \ReflectionProperty $class_property */
foreach ($class_properties as $class_property) {
$property_name = $class_property->getName();
$storage->properties[$property_name] = new PropertyStorage();
@ -173,7 +172,6 @@ class Reflection
$interfaces = $reflected_class->getInterfaces();
/** @var \ReflectionClass $interface */
foreach ($interfaces as $interface) {
$interface_name = $interface->getName();
$this->registerClass($interface);
@ -185,7 +183,6 @@ class Reflection
}
}
/** @var \ReflectionMethod $reflection_method */
foreach ($reflection_methods as $reflection_method) {
$method_reflection_class = $reflection_method->getDeclaringClass();
@ -283,7 +280,6 @@ class Reflection
$storage->params = [];
/** @var \ReflectionParameter $param */
foreach ($params as $param) {
$param_array = $this->getReflectionParamData($param);
$storage->params[] = $param_array;
@ -359,7 +355,6 @@ class Reflection
} else {
$reflection_params = $reflection_function->getParameters();
/** @var \ReflectionParameter $param */
foreach ($reflection_params as $param) {
$param_obj = $this->getReflectionParamData($param);
$storage->params[] = $param_obj;

1
src/Psalm/Internal/Codebase/Scanner.php
View File

@ -544,7 +544,6 @@ class Scanner
// even though we've checked this above, calling the method invalidates it
if (isset($this->classlike_files[$fq_classlike_name_lc])) {
/** @var string */
$file_path = $this->classlike_files[$fq_classlike_name_lc];
$this->files_to_scan[$file_path] = $file_path;
if (isset($this->classes_to_deep_scan[$fq_classlike_name_lc])) {

1
src/Psalm/Internal/PluginManager/Command/DisableCommand.php
View File

@ -53,7 +53,6 @@ class DisableCommand extends Command
$current_dir = (string) getcwd() . DIRECTORY_SEPARATOR;
/** @var string|string[]|bool|null */
$config_file_path = $i->getOption('config');
if ($config_file_path !== null && !is_string($config_file_path)) {
throw new \UnexpectedValueException('Config file path should be a string');

1
src/Psalm/Internal/PluginManager/Command/EnableCommand.php
View File

@ -53,7 +53,6 @@ class EnableCommand extends Command
$current_dir = (string) getcwd() . DIRECTORY_SEPARATOR;
/** @var string|string[]|bool|null */
$config_file_path = $i->getOption('config');
if ($config_file_path !== null && !is_string($config_file_path)) {
throw new \UnexpectedValueException('Config file path should be a string');

1
src/Psalm/Internal/PluginManager/Command/ShowCommand.php
View File

@ -47,7 +47,6 @@ class ShowCommand extends Command
$io = new SymfonyStyle($i, $o);
$current_dir = (string) getcwd() . DIRECTORY_SEPARATOR;
/** @var string|string[]|bool|null */
$config_file_path = $i->getOption('config');
if ($config_file_path !== null && !is_string($config_file_path)) {
throw new \UnexpectedValueException('Config file path should be a string');

3
src/Psalm/Internal/Visitor/PartialParserVisitor.php
View File

@ -69,7 +69,6 @@ class PartialParserVisitor extends PhpParser\NodeVisitorAbstract implements PhpP
$attrs = $node->getAttributes();
if ($cs = $node->getComments()) {
/** @var int */
$stmt_start_pos = $cs[0]->getFilePos();
} else {
/** @var int */
@ -161,7 +160,6 @@ class PartialParserVisitor extends PhpParser\NodeVisitorAbstract implements PhpP
$fake_class = '<?php class _ {' . $method_contents . '}';
/** @var array<PhpParser\Node\Stmt> */
$replacement_stmts = $this->parser->parse(
$fake_class,
$error_handler
@ -189,7 +187,6 @@ class PartialParserVisitor extends PhpParser\NodeVisitorAbstract implements PhpP
$hacky_class_fix = preg_replace('/(->|::)(\n\s*if\s*\()/', '~;$2', $hacky_class_fix);
if ($hacky_class_fix !== $fake_class) {
/** @var array<PhpParser\Node\Stmt> */
$replacement_stmts = $this->parser->parse(
$hacky_class_fix,
$error_handler

1
src/Psalm/Internal/Visitor/ReflectorVisitor.php
View File

@ -1735,7 +1735,6 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse
$existing_params = [];
$storage->params = [];
/** @var PhpParser\Node\Param $param */
foreach ($stmt->getParams() as $param) {
if ($param->var instanceof PhpParser\Node\Expr\Error) {
if (IssueBuffer::accepts(

1
src/Psalm/Internal/Visitor/SimpleNameResolver.php
View File

@ -82,7 +82,6 @@ class SimpleNameResolver extends NodeVisitorAbstract
$attrs = $node->getAttributes();
if ($cs = $node->getComments()) {
/** @var int */
$attrs['startFilePos'] = $cs[0]->getFilePos();
}

6
src/Psalm/Issue/UnnecessaryVarAnnotation.php Normal file
View File

@ -0,0 +1,6 @@
<?php
namespace Psalm\Issue;
class UnnecessaryVarAnnotation extends CodeIssue
{
}

2
src/command_functions.php
View File

@ -153,7 +153,6 @@ function getArguments() : array
$filtered_input_paths = [];
for ($i = 0; $i < count($argv); ++$i) {
/** @var string */
$input_path = $argv[$i];
if (realpath($input_path) !== false) {
@ -232,7 +231,6 @@ function getPathsToCheck($f_paths)
if ($stdin = fgets(STDIN)) {
$filtered_input_paths = preg_split('/\s+/', trim($stdin));
}
/** @var bool */
$blocked = $meta['blocked'];
stream_set_blocking(STDIN, $blocked);
}

1
tests/CodebaseTest.php
View File

@ -161,7 +161,6 @@ class CodebaseTest extends TestCase
$storage->methods['m']->custom_metadata['c'] = 'd';
$storage->properties['prop']->custom_metadata['e'] = 'f';
$storage->methods['m']->params[0]->custom_metadata['g'] = 'h';
/** @var Codebase $codebase */
$codebase->file_storage_provider->get('somefile.php')->custom_metadata['i'] = 'j';
}
}

4
tests/DocumentationTest.php
View File

@ -228,7 +228,9 @@ class DocumentationTest extends TestCase
'<?php' . "\n" . $blocks[0],
$issue_name,
$ignored_issues,
strpos($issue_name, 'Unused') !== false || strpos($issue_name, 'Unevaluated') !== false,
strpos($issue_name, 'Unused') !== false
|| strpos($issue_name, 'Unevaluated') !== false
|| strpos($issue_name, 'Unnecessary') !== false,
];
}

48
tests/UnusedVariableTest.php
View File

@ -1114,6 +1114,17 @@ class UnusedVariableTest extends TestCase
return $foo;
}',
],
'refineForeachVarType' => [
'<?php
function foo() : array {
return ["hello"];
}
/** @var string $s */
foreach (foo() as $s) {
echo $s;
}',
],
];
}
@ -1814,6 +1825,43 @@ class UnusedVariableTest extends TestCase
}',
'error_message' => 'UnusedVariable',
],
'knownVarType' => [
'<?php
function foo() : string {
return "hello";
}
/** @var string */
$a = foo();
echo $a;',
'error_message' => 'UnnecessaryVarAnnotation',
],
'knownVarTypeWithName' => [
'<?php
function foo() : string {
return "hello";
}
/** @var string $a */
$a = foo();
echo $a;',
'error_message' => 'UnnecessaryVarAnnotation',
],
'knownForeachVarType' => [
'<?php
/** @return string[] */
function foo() : array {
return ["hello"];
}
/** @var string $s */
foreach (foo() as $s) {
echo $s;
}',
'error_message' => 'UnnecessaryVarAnnotation',
],
];
}
}