Skip to content

Commit

Permalink
Prevent removing properties on classes with variable assignment
Browse files Browse the repository at this point in the history
  • Loading branch information
@muglug
muglug committed Apr 18, 2019
1 parent 4807ebe commit d6de6ca
Show file tree
Hide file tree
Showing 5 changed files with 223 additions and 100 deletions.
4 changes: 4 additions & 0 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/PropertyAssignmentAnalyzer.php
View file
Expand Up @@ -830,6 +830,10 @@ public static function analyzeStatic(
$prop_name = $stmt->name;

if (!$prop_name instanceof PhpParser\Node\Identifier) {
if ($fq_class_name) {
$codebase->analyzer->addMixedMemberName(strtolower($fq_class_name) . '::$');
}

return;
}

Expand Down
12 changes: 12 additions & 0 deletions src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
View file
Expand Up @@ -565,6 +565,18 @@ public static function analyze(
if (ExpressionAnalyzer::analyze($statements_analyzer, $assign_var->var, $context) === false) {
return false;
}

if (isset($assign_var->var->inferredType)) {
$stmt_var_type = $assign_var->var->inferredType;

if ($stmt_var_type->hasObjectType()) {
foreach ($stmt_var_type->getTypes() as $type) {
if ($type instanceof Type\Atomic\TNamedObject) {
$codebase->analyzer->addMixedMemberName(strtolower($type->value) . '::$');
}
}
}
}
}

if ($var_id) {
Expand Down
215 changes: 116 additions & 99 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/PropertyFetchAnalyzer.php
View file
Expand Up @@ -292,6 +292,14 @@ public static function analyzeInstance(
}

if (!$prop_name) {
if ($stmt_var_type->hasObjectType()) {
foreach ($stmt_var_type->getTypes() as $type) {
if ($type instanceof Type\Atomic\TNamedObject) {
$codebase->analyzer->addMixedMemberName(strtolower($type->value) . '::$');
}
}
}

return null;
}

Expand Down Expand Up @@ -806,125 +814,134 @@ public static function analyzeStatic(
$prop_name = null;
}

if ($fq_class_name &&
$context->check_classes &&
$context->check_variables &&
$prop_name &&
!ExpressionAnalyzer::isMock($fq_class_name)
) {
$var_id = ExpressionAnalyzer::getVarId(
$stmt,
$context->self ?: $statements_analyzer->getFQCLN(),
$statements_analyzer
);
if (!$prop_name) {
if ($fq_class_name) {
$codebase->analyzer->addMixedMemberName(strtolower($fq_class_name) . '::$');
}

$property_id = $fq_class_name . '::$' . $prop_name;
return null;
}

if ($codebase->store_node_types) {
$codebase->analyzer->addNodeReference(
$statements_analyzer->getFilePath(),
$stmt->name,
$property_id
);
}
if (!$fq_class_name
|| !$context->check_classes
|| !$context->check_variables
|| ExpressionAnalyzer::isMock($fq_class_name)
) {
return null;
}

if ($var_id && $context->hasVariable($var_id, $statements_analyzer)) {
// we don't need to check anything
$stmt->inferredType = $context->vars_in_scope[$var_id];
$var_id = ExpressionAnalyzer::getVarId(
$stmt,
$context->self ?: $statements_analyzer->getFQCLN(),
$statements_analyzer
);

if ($context->collect_references) {
// log the appearance
$codebase->properties->propertyExists(
$property_id,
false,
$statements_analyzer,
$context,
new CodeLocation($statements_analyzer->getSource(), $stmt)
);
}
$property_id = $fq_class_name . '::$' . $prop_name;

if ($codebase->store_node_types
&& (!$context->collect_initializations
&& !$context->collect_mutations)
&& isset($stmt->inferredType)
) {
$codebase->analyzer->addNodeType(
$statements_analyzer->getFilePath(),
$stmt->name,
(string) $stmt->inferredType
);
}
if ($codebase->store_node_types) {
$codebase->analyzer->addNodeReference(
$statements_analyzer->getFilePath(),
$stmt->name,
$property_id
);
}

return null;
if ($var_id && $context->hasVariable($var_id, $statements_analyzer)) {
// we don't need to check anything
$stmt->inferredType = $context->vars_in_scope[$var_id];

if ($context->collect_references) {
// log the appearance
$codebase->properties->propertyExists(
$property_id,
false,
$statements_analyzer,
$context,
new CodeLocation($statements_analyzer->getSource(), $stmt)
);
}

if (!$codebase->properties->propertyExists(
$property_id,
false,
$statements_analyzer,
$context,
$context->collect_references ? new CodeLocation($statements_analyzer->getSource(), $stmt) : null
)
if ($codebase->store_node_types
&& (!$context->collect_initializations
&& !$context->collect_mutations)
&& isset($stmt->inferredType)
) {
if (IssueBuffer::accepts(
new UndefinedPropertyFetch(
'Static property ' . $property_id . ' is not defined',
new CodeLocation($statements_analyzer->getSource(), $stmt),
$property_id
),
$statements_analyzer->getSuppressedIssues()
)) {
// fall through
}

return;
$codebase->analyzer->addNodeType(
$statements_analyzer->getFilePath(),
$stmt->name,
(string) $stmt->inferredType
);
}

if (ClassLikeAnalyzer::checkPropertyVisibility(
$property_id,
$context,
$statements_analyzer,
new CodeLocation($statements_analyzer->getSource(), $stmt),
return null;
}

if (!$codebase->properties->propertyExists(
$property_id,
false,
$statements_analyzer,
$context,
$context->collect_references ? new CodeLocation($statements_analyzer->getSource(), $stmt) : null
)
) {
if (IssueBuffer::accepts(
new UndefinedPropertyFetch(
'Static property ' . $property_id . ' is not defined',
new CodeLocation($statements_analyzer->getSource(), $stmt),
$property_id
),
$statements_analyzer->getSuppressedIssues()
) === false) {
return false;
)) {
// fall through
}

$declaring_property_class = $codebase->properties->getDeclaringClassForProperty(
$fq_class_name . '::$' . $prop_name,
true
);
return;
}

$class_storage = $codebase->classlike_storage_provider->get((string)$declaring_property_class);
$property = $class_storage->properties[$prop_name];
if (ClassLikeAnalyzer::checkPropertyVisibility(
$property_id,
$context,
$statements_analyzer,
new CodeLocation($statements_analyzer->getSource(), $stmt),
$statements_analyzer->getSuppressedIssues()
) === false) {
return false;
}

if ($var_id) {
if ($property->type) {
$context->vars_in_scope[$var_id] = ExpressionAnalyzer::fleshOutType(
$codebase,
clone $property->type,
$declaring_property_class,
$declaring_property_class
);
} else {
$context->vars_in_scope[$var_id] = Type::getMixed();
}
$declaring_property_class = $codebase->properties->getDeclaringClassForProperty(
$fq_class_name . '::$' . $prop_name,
true
);

$stmt->inferredType = clone $context->vars_in_scope[$var_id];
$class_storage = $codebase->classlike_storage_provider->get((string)$declaring_property_class);
$property = $class_storage->properties[$prop_name];

if ($codebase->store_node_types
&& (!$context->collect_initializations
&& !$context->collect_mutations)
) {
$codebase->analyzer->addNodeType(
$statements_analyzer->getFilePath(),
$stmt->name,
(string) $stmt->inferredType
);
}
if ($var_id) {
if ($property->type) {
$context->vars_in_scope[$var_id] = ExpressionAnalyzer::fleshOutType(
$codebase,
clone $property->type,
$declaring_property_class,
$declaring_property_class
);
} else {
$stmt->inferredType = Type::getMixed();
$context->vars_in_scope[$var_id] = Type::getMixed();
}

$stmt->inferredType = clone $context->vars_in_scope[$var_id];

if ($codebase->store_node_types
&& (!$context->collect_initializations
&& !$context->collect_mutations)
) {
$codebase->analyzer->addNodeType(
$statements_analyzer->getFilePath(),
$stmt->name,
(string) $stmt->inferredType
);
}
} else {
$stmt->inferredType = Type::getMixed();
}

return null;
Expand Down
6 changes: 6 additions & 0 deletions src/Psalm/Internal/Codebase/ClassLikes.php
View file
Expand Up @@ -961,6 +961,9 @@ private function checkPropertyReferences(ClassLikeStorage $classlike_storage)
if ($property_storage->stmt_location
&& isset($project_analyzer->getIssuesToFix()['PossiblyUnusedProperty'])
&& !$codebase->analyzer->hasMixedMemberName('$' . $property_name)
&& !$codebase->analyzer->hasMixedMemberName(
strtolower($classlike_storage->name) . '::$'
)
&& !IssueBuffer::isSuppressed($issue, $classlike_storage->suppressed_issues)
) {
FileManipulationBuffer::addForCodeLocation(
Expand Down Expand Up @@ -989,6 +992,9 @@ private function checkPropertyReferences(ClassLikeStorage $classlike_storage)
if ($property_storage->stmt_location
&& isset($project_analyzer->getIssuesToFix()['UnusedProperty'])
&& !$codebase->analyzer->hasMixedMemberName('$' . $property_name)
&& !$codebase->analyzer->hasMixedMemberName(
strtolower($classlike_storage->name) . '::$'
)
&& !IssueBuffer::isSuppressed($issue, $classlike_storage->suppressed_issues)
) {
FileManipulationBuffer::addForCodeLocation(
Expand Down
86 changes: 85 additions & 1 deletion tests/FileManipulationTest.php
View file
Expand Up @@ -1772,7 +1772,7 @@ class A {
['PossiblyUnusedProperty'],
true,
],
'removePossiblyUnusedPropertyMixedUse' => [
'dontRemovePossiblyUnusedPropertyWithMixedUse' => [
'<?php
class A {
public $foo = "hello";
Expand All @@ -1793,6 +1793,90 @@ function foo($a) {
['PossiblyUnusedProperty'],
true,
],
'dontRemovePossiblyUnusedPropertyWithVariableFetch' => [
'<?php
class A {
public $foo = "hello";
}
function foo(A $a, string $var) {
echo $a->$var;
}',
'<?php
class A {
public $foo = "hello";
}
function foo(A $a, string $var) {
echo $a->$var;
}',
'7.1',
['PossiblyUnusedProperty'],
true,
],
'dontRemovePossiblyUnusedPropertyWithStaticVariableFetch' => [
'<?php
class A {
public static $foo = "hello";
}
function foo(string $var) {
echo A::$$var;
}',
'<?php
class A {
public static $foo = "hello";
}
function foo(string $var) {
echo A::$$var;
}',
'7.1',
['PossiblyUnusedProperty'],
true,
],
'dontRemovePossiblyUnusedPropertyWithVariableAssignment' => [
'<?php
class A {
public $foo = "hello";
}
function foo(A $a, string $var) {
$a->$var = "hello";
}',
'<?php
class A {
public $foo = "hello";
}
function foo(A $a, string $var) {
$a->$var = "hello";
}',
'7.1',
['PossiblyUnusedProperty'],
true,
],
'dontRemovePossiblyUnusedPropertyWithStaticVariableAssignment' => [
'<?php
class A {
public static $foo = "hello";
}
function foo(string $var) {
A::$$var = "hello";
}',
'<?php
class A {
public static $foo = "hello";
}
function foo(string $var) {
A::$$var = "hello";
}',
'7.1',
['PossiblyUnusedProperty'],
true,
],
'removeUnusedPropertyWithDocblock' => [
'<?php
class A {
Expand Down

0 comments on commit d6de6ca

Please sign in to comment.