Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Analyser/ExprHandler/NewHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
$constructorResult = $node;
}, StatementContext::createTopLevel());
if ($constructorResult !== null) {
$throwPoints = array_map(static fn (ThrowPoint $point) => InternalThrowPoint::createFromPublic($point), $constructorResult->getStatementResult()->getThrowPoints());
$throwPoints = array_map(static fn (ThrowPoint $point): InternalThrowPoint => InternalThrowPoint::createFromPublic($point)->replaceScope($scope), $constructorResult->getStatementResult()->getThrowPoints());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to be the only call-site where we use InternalThrowPoint::createFromPublic.

instead of adding replaceScope I think we can just add a Scope $scope parameter to InternalThrowPoint::createFromPublic - we don't need to look for backwards compatibility, because InternalThrowPoint is not annotated @api.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I addressed staabm's review by:

  • Adding an optional ?MutatingScope $scope = null parameter to InternalThrowPoint::createFromPublic()
  • Removing the replaceScope() method
  • Updating the call site in NewHandler.php to pass $scope directly to createFromPublic()

Tests and static analysis both pass.

$impurePoints = $constructorResult->getImpurePoints();
}
} else {
Expand Down
5 changes: 5 additions & 0 deletions src/Analyser/InternalThrowPoint.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ public function canContainAnyThrowable(): bool
return $this->canContainAnyThrowable;
}

public function replaceScope(MutatingScope $scope): self
{
return new self($scope, $this->type, $this->node, $this->explicit, $this->canContainAnyThrowable);
}

public function subtractCatchType(Type $catchType): self
{
return new self($this->scope, TypeCombinator::remove($this->type, $catchType), $this->node, $this->explicit, $this->canContainAnyThrowable);
Expand Down
10 changes: 10 additions & 0 deletions tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1425,6 +1425,16 @@ public function testBug12373(): void
$this->analyse([__DIR__ . '/data/bug-12373.php'], []);
}

public function testBug13920(): void
{
$this->cliArgumentsVariablesRegistered = true;
$this->polluteScopeWithLoopInitialAssignments = true;
$this->checkMaybeUndefinedVariables = true;
$this->polluteScopeWithAlwaysIterableForeach = true;

$this->analyse([__DIR__ . '/data/bug-13920.php'], []);
}

public function testBug14117(): void
{
$this->cliArgumentsVariablesRegistered = true;
Expand Down
26 changes: 26 additions & 0 deletions tests/PHPStan/Rules/Variables/data/bug-13920.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php declare(strict_types = 1);

namespace Bug13920;

class TestOutput {
public function __construct() {}
}

final class MyTest
{
public function testRun(): void
{
$savedArgv = $_SERVER['argv'];

try {
$output = new class() extends TestOutput {
public function __construct()
{
parent::__construct();
}
};
} finally {
$_SERVER['argv'] = $savedArgv;
}
}
}
Loading