Skip to content

Commit

Permalink
Fix #1309 - no PropertyNotSetInConstructor warnings for grandchild of…
Browse files Browse the repository at this point in the history
… class
  • Loading branch information
muglug committed Feb 10, 2019
1 parent 6976528 commit 62fb803
Show file tree
Hide file tree
Showing 9 changed files with 238 additions and 82 deletions.
30 changes: 28 additions & 2 deletions src/Psalm/Internal/Analyzer/ClassAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,27 @@ public function analyze(
)
: $property_type;

/**
* @psalm-suppress ReferenceConstraintViolation
*/
$class_template_params = MethodCallAnalyzer::getClassTemplateParams(
$codebase,
$property_class_storage,
$fq_class_name,
null,
new Type\Atomic\TNamedObject($fq_class_name),
'$this'
);

if ($class_template_params) {
$generic_params = [];
$fleshed_out_type->replaceTemplateTypesWithStandins(
$class_template_params,
$generic_params,
$codebase
);
}

if ($property_type_location && !$fleshed_out_type->isMixed()) {
$fleshed_out_type->check(
$this,
Expand Down Expand Up @@ -834,11 +855,11 @@ function (FunctionLikeParameter $param) {
$fake_constructor_stmts = [
new PhpParser\Node\Stmt\Expression(
new PhpParser\Node\Expr\StaticCall(
new PhpParser\Node\Name(['parent']),
new PhpParser\Node\Name\FullyQualified($constructor_declaring_fqcln),
new PhpParser\Node\Identifier('__construct'),
$fake_constructor_stmt_args,
[
'line' => $class->extends->getLine(),
'startLine' => $class->extends->getLine(),
'startFilePos' => $class->extends->getAttribute('startFilePos'),
'endFilePos' => $class->extends->getAttribute('endFilePos'),
]
Expand All @@ -852,6 +873,11 @@ function (FunctionLikeParameter $param) {
'type' => PhpParser\Node\Stmt\Class_::MODIFIER_PUBLIC,
'params' => $fake_constructor_params,
'stmts' => $fake_constructor_stmts,
],
[
'startLine' => $class->extends->getLine(),
'startFilePos' => $class->extends->getAttribute('startFilePos'),
'endFilePos' => $class->extends->getAttribute('endFilePos'),
]
);

Expand Down
1 change: 1 addition & 0 deletions src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Psalm\Codebase;
use Psalm\Internal\Analyzer\FunctionLike\ReturnTypeAnalyzer;
use Psalm\Internal\Analyzer\FunctionLike\ReturnTypeCollector;
use Psalm\Internal\Analyzer\Statements\Expression\Call\MethodCallAnalyzer;
use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer;
use Psalm\Internal\Codebase\CallMap;
use Psalm\CodeLocation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,6 @@ public static function analyzeInstance(
}
}


$declaring_property_class = (string) $codebase->properties->getDeclaringClassForProperty(
$property_id
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ public static function analyze(
new ReferenceConstraintViolation(
'Variable ' . $var_id . ' is limited to values of type '
. $context->byref_constraints[$var_id]->type
. ' because it is passed by reference',
. ' because it is passed by reference, '
. $assign_value_type->getId() . ' type found',
new CodeLocation($statements_analyzer->getSource(), $assign_var)
),
$statements_analyzer->getSuppressedIssues()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,7 @@ public static function getClassTemplateParams(
Codebase $codebase,
ClassLikeStorage $class_storage,
string $fq_class_name,
string $method_name,
string $method_name = null,
Type\Atomic $lhs_type_part = null,
string $lhs_var_id = null
) {
Expand All @@ -1097,6 +1097,7 @@ public static function getClassTemplateParams(

if (!$template_types) {
if ($calling_class_storage->template_type_extends
&& $method_name
&& !empty($class_storage->overridden_method_ids[$method_name])
&& !isset($class_storage->methods[$method_name]->return_type)
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,82 +84,6 @@ public static function analyze(
$class_storage = $codebase->classlike_storage_provider->get($fq_class_name);

$fq_class_name = $class_storage->name;

if ($stmt->name instanceof PhpParser\Node\Identifier
&& $class_storage->user_defined
&& ($context->collect_mutations || $context->collect_initializations)
) {
$method_id = $fq_class_name . '::' . strtolower($stmt->name->name);

$appearing_method_id = $codebase->getAppearingMethodId($method_id);

if (!$appearing_method_id) {
if (IssueBuffer::accepts(
new UndefinedMethod(
'Method ' . $method_id . ' does not exist',
new CodeLocation($statements_analyzer->getSource(), $stmt),
$method_id
),
$statements_analyzer->getSuppressedIssues()
)) {
return false;
}

return;
}

list($appearing_method_class_name) = explode('::', $appearing_method_id);

$old_context_include_location = $context->include_location;
$old_self = $context->self;
$context->include_location = new CodeLocation($statements_analyzer->getSource(), $stmt);
$context->self = $appearing_method_class_name;

if ($context->collect_mutations) {
$file_analyzer->getMethodMutations($method_id, $context);
} else {
// collecting initializations
$local_vars_in_scope = [];
$local_vars_possibly_in_scope = [];

foreach ($context->vars_in_scope as $var => $_) {
if (strpos($var, '$this->') !== 0 && $var !== '$this') {
$local_vars_in_scope[$var] = $context->vars_in_scope[$var];
}
}

foreach ($context->vars_possibly_in_scope as $var => $_) {
if (strpos($var, '$this->') !== 0 && $var !== '$this') {
$local_vars_possibly_in_scope[$var] = $context->vars_possibly_in_scope[$var];
}
}

if (!isset($context->initialized_methods[$method_id])) {
if ($context->initialized_methods === null) {
$context->initialized_methods = [];
}

$context->initialized_methods[$method_id] = true;

$file_analyzer->getMethodMutations($method_id, $context);

foreach ($local_vars_in_scope as $var => $type) {
$context->vars_in_scope[$var] = $type;
}

foreach ($local_vars_possibly_in_scope as $var => $type) {
$context->vars_possibly_in_scope[$var] = $type;
}
}
}

$context->include_location = $old_context_include_location;
$context->self = $old_self;

if (isset($context->vars_in_scope['$this']) && $old_self) {
$context->vars_in_scope['$this'] = Type::parseString($old_self);
}
}
} elseif ($context->self) {
if ($stmt->class->parts[0] === 'static' && isset($context->vars_in_scope['$this'])) {
$fq_class_name = (string) $context->vars_in_scope['$this'];
Expand Down Expand Up @@ -543,6 +467,82 @@ function (PhpParser\Node\Arg $arg) {

$class_storage = $codebase->classlike_storage_provider->get($fq_class_name);

if ($class_storage->user_defined
&& $context->self
&& ($context->collect_mutations || $context->collect_initializations)
) {
$appearing_method_id = $codebase->getAppearingMethodId($method_id);

if (!$appearing_method_id) {
if (IssueBuffer::accepts(
new UndefinedMethod(
'Method ' . $method_id . ' does not exist',
new CodeLocation($statements_analyzer->getSource(), $stmt),
$method_id
),
$statements_analyzer->getSuppressedIssues()
)) {
return false;
}

return;
}

list($appearing_method_class_name) = explode('::', $appearing_method_id);

if ($codebase->classExtends($context->self, $appearing_method_class_name)) {
$old_context_include_location = $context->include_location;
$old_self = $context->self;
$context->include_location = new CodeLocation($statements_analyzer->getSource(), $stmt);
$context->self = $appearing_method_class_name;

if ($context->collect_mutations) {
$file_analyzer->getMethodMutations($method_id, $context);
} else {
// collecting initializations
$local_vars_in_scope = [];
$local_vars_possibly_in_scope = [];

foreach ($context->vars_in_scope as $var => $_) {
if (strpos($var, '$this->') !== 0 && $var !== '$this') {
$local_vars_in_scope[$var] = $context->vars_in_scope[$var];
}
}

foreach ($context->vars_possibly_in_scope as $var => $_) {
if (strpos($var, '$this->') !== 0 && $var !== '$this') {
$local_vars_possibly_in_scope[$var] = $context->vars_possibly_in_scope[$var];
}
}

if (!isset($context->initialized_methods[$method_id])) {
if ($context->initialized_methods === null) {
$context->initialized_methods = [];
}

$context->initialized_methods[$method_id] = true;

$file_analyzer->getMethodMutations($method_id, $context);

foreach ($local_vars_in_scope as $var => $type) {
$context->vars_in_scope[$var] = $type;
}

foreach ($local_vars_possibly_in_scope as $var => $type) {
$context->vars_possibly_in_scope[$var] = $type;
}
}
}

$context->include_location = $old_context_include_location;
$context->self = $old_self;

if (isset($context->vars_in_scope['$this']) && $old_self) {
$context->vars_in_scope['$this'] = Type::parseString($old_self);
}
}
}

if ($class_storage->deprecated) {
if (IssueBuffer::accepts(
new DeprecatedClass(
Expand Down
4 changes: 4 additions & 0 deletions src/Psalm/IssueBuffer.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ public static function accepts(CodeIssue $e, array $suppressed_issues = [])
}
}

if ($e->getLocation()->getLineNumber() === -1) {
return false;
}

if (self::$recording_level > 0) {
self::$recorded_issues[self::$recording_level][] = $e;

Expand Down
36 changes: 35 additions & 1 deletion tests/PropertyTypeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1278,6 +1278,40 @@ public function foo($b) : void {
}
}'
],
'propertySetInGrandparentExplicitly' => [
'<?php
class A {
/**
* @var string
*/
public $s;
public function __construct(string $s) {
$this->s = $s;
}
}
class B extends A {}
class C extends B {
public function __construct(string $s) {
A::__construct($s);
}
}'
],
'propertySetInGrandparentImplicitly' => [
'<?php
class A {
/**
* @var string
*/
public $s;
public function __construct(string $s) {
$this->s = $s;
}
}
class B extends A {}
class C extends B {}'
],
];
}

Expand Down Expand Up @@ -1655,7 +1689,7 @@ private function __construct() {
}
class B extends A {}',
'error_message' => 'InaccessibleMethod',
'error_message' => 'InaccessibleMethod - src/somefile.php:11',
],
'classInheritsPrivateConstructorWithImplementedConstructor' => [
'<?php
Expand Down
Loading

0 comments on commit 62fb803

Please sign in to comment.