Skip to content

Commit 4b8b7dd

Browse files
staabmstaabm
committed
Fix "Differing behaviors when requiring or including relatively vs using __DIR__"
1 parent 171a4f7 commit 4b8b7dd

5 files changed

Lines changed: 146 additions & 13 deletions

File tree

‎src/Rules/Keywords/RequireFileExistsRule.php‎

Lines changed: 60 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,21 @@
33
namespace PHPStan\Rules\Keywords;
44

55
use PhpParser\Node;
6+
use PhpParser\Node\Arg;
7+
use PhpParser\Node\Expr;
8+
use PhpParser\Node\Expr\FuncCall;
69
use PhpParser\Node\Expr\Include_;
10+
use PhpParser\Node\Name\FullyQualified;
711
use PHPStan\Analyser\Scope;
812
use PHPStan\DependencyInjection\AutowiredParameter;
913
use PHPStan\DependencyInjection\RegisteredRule;
1014
use PHPStan\File\FileHelper;
15+
use PHPStan\Node\Printer\ExprPrinter;
1116
use PHPStan\Rules\IdentifierRuleError;
1217
use PHPStan\Rules\Rule;
1318
use PHPStan\Rules\RuleErrorBuilder;
1419
use PHPStan\ShouldNotHappenException;
20+
use PHPStan\Type\Constant\ConstantStringType;
1521
use function array_merge;
1622
use function dirname;
1723
use function explode;
@@ -30,6 +36,7 @@ final class RequireFileExistsRule implements Rule
3036
public function __construct(
3137
#[AutowiredParameter]
3238
private string $currentWorkingDirectory,
39+
private ExprPrinter $exprPrinter,
3340
)
3441
{
3542
}
@@ -41,15 +48,28 @@ public function getNodeType(): string
4148

4249
public function processNode(Node $node, Scope $scope): array
4350
{
51+
if ($this->isInFileExists($node, $scope)) {
52+
return [];
53+
}
54+
4455
$errors = [];
45-
$paths = $this->resolveFilePaths($node, $scope);
56+
$usedMagicDirFallback = false;
57+
$paths = $this->resolveFilePaths($node->expr, $scope, $usedMagicDirFallback);
4658

4759
foreach ($paths as $path) {
60+
$path = $path->getValue();
61+
4862
if ($this->doesFileExist($path, $scope)) {
4963
continue;
5064
}
5165

52-
$errors[] = $this->getErrorMessage($node, $path);
66+
if ($usedMagicDirFallback) {
67+
$pathExpr = $this->exprPrinter->printExpr($node->expr);
68+
} else {
69+
$pathExpr = '"' . $path . '"';
70+
}
71+
72+
$errors[] = $this->getErrorMessage($node, $pathExpr);
5373
}
5474

5575
return $errors;
@@ -90,7 +110,7 @@ private function doesFileExistForDirectory(string $path, string $workingDirector
90110

91111
private function getErrorMessage(Include_ $node, string $filePath): IdentifierRuleError
92112
{
93-
$message = 'Path in %s() "%s" is not a file or it does not exist.';
113+
$message = 'Path in %s() %s is not a file or it does not exist.';
94114

95115
switch ($node->type) {
96116
case Include_::TYPE_REQUIRE:
@@ -125,19 +145,49 @@ private function getErrorMessage(Include_ $node, string $filePath): IdentifierRu
125145
}
126146

127147
/**
128-
* @return array<string>
148+
* @return list<ConstantStringType>
129149
*/
130-
private function resolveFilePaths(Include_ $node, Scope $scope): array
150+
private function resolveFilePaths(Expr $expr, Scope $scope, bool &$magicDirFallback): array
131151
{
132-
$paths = [];
133-
$type = $scope->getType($node->expr);
134-
$constantStrings = $type->getConstantStrings();
152+
$magicDirFallback = false;
135153

136-
foreach ($constantStrings as $constantString) {
137-
$paths[] = $constantString->getValue();
154+
if (!$expr instanceof Expr\BinaryOp\Concat) {
155+
return $scope->getType($expr)->getConstantStrings();
138156
}
139157

158+
if ($expr->left instanceof Node\Scalar\MagicConst\Dir) {
159+
$magicDirFallback = true;
160+
161+
$paths = [];
162+
foreach ($scope->getType($expr->right)->getConstantStrings() as $constantString) {
163+
$paths[] = new ConstantStringType(dirname($scope->getFile()) . $constantString->getValue());
164+
}
165+
return $paths;
166+
}
167+
168+
$paths = [];
169+
$rightPaths = $this->resolveFilePaths($expr->right, $scope, $magicDirFallback);
170+
foreach ($this->resolveFilePaths($expr->left, $scope, $magicDirFallback) as $left) {
171+
foreach ($rightPaths as $rightPath) {
172+
$paths[] = new ConstantStringType($left->getValue() . $rightPath->getValue());
173+
}
174+
}
140175
return $paths;
141176
}
142177

178+
private function isInFileExists(Include_ $node, Scope $scope): bool
179+
{
180+
foreach (['file_exists', 'is_file'] as $funcName) {
181+
$expr = new FuncCall(new FullyQualified($funcName), [
182+
new Arg($node->expr),
183+
]);
184+
185+
if ($scope->getType($expr)->isTrue()->yes()) {
186+
return true;
187+
}
188+
}
189+
190+
return false;
191+
}
192+
143193
}

‎tests/PHPStan/Rules/Keywords/RequireFileExistsRuleNoConstantPathTest.php‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Keywords;
4+
5+
use PHPStan\Node\Printer\ExprPrinter;
6+
use PHPStan\Rules\Rule;
7+
use PHPStan\Testing\RuleTestCase;
8+
9+
/**
10+
* @extends RuleTestCase<RequireFileExistsRule>
11+
*/
12+
class RequireFileExistsRuleNoConstantPathTest extends RuleTestCase
13+
{
14+
15+
private string $currentWorkingDirectory = __DIR__ . '/../';
16+
17+
protected function getRule(): Rule
18+
{
19+
return new RequireFileExistsRule(
20+
$this->currentWorkingDirectory,
21+
self::getContainer()->getByType(ExprPrinter::class),
22+
);
23+
}
24+
25+
public function testBug12203NoConstantPath(): void
26+
{
27+
$this->analyse([__DIR__ . '/data/bug-12203.php'], [
28+
[
29+
'Path in require_once() "../bug-12203-sure-does-not-exist.php" is not a file or it does not exist.',
30+
5,
31+
],
32+
[
33+
"Path in require_once() __DIR__ . '/../bug-12203-sure-does-not-exist.php' is not a file or it does not exist.",
34+
6,
35+
],
36+
[
37+
"Path in require_once() __DIR__ . '/' . \$path . '/' . \$file is not a file or it does not exist.",
38+
10,
39+
],
40+
[
41+
'Path in require_once() __DIR__ . "{$path}/{$file}" is not a file or it does not exist.',
42+
12,
43+
],
44+
]);
45+
}
46+
47+
public function testInFileExists(): void
48+
{
49+
$this->analyse([__DIR__ . '/data/include-in-file-exists.php'], []);
50+
}
51+
52+
}

‎tests/PHPStan/Rules/Keywords/RequireFileExistsRuleTest.php‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22

33
namespace PHPStan\Rules\Keywords;
44

5+
use PHPStan\Node\Printer\ExprPrinter;
56
use PHPStan\Rules\Rule;
67
use PHPStan\Testing\RuleTestCase;
78
use function get_include_path;
89
use function implode;
910
use function realpath;
1011
use function set_include_path;
11-
use const DIRECTORY_SEPARATOR;
1212
use const PATH_SEPARATOR;
1313

1414
/**
@@ -21,7 +21,10 @@ class RequireFileExistsRuleTest extends RuleTestCase
2121

2222
protected function getRule(): Rule
2323
{
24-
return new RequireFileExistsRule($this->currentWorkingDirectory);
24+
return new RequireFileExistsRule(
25+
$this->currentWorkingDirectory,
26+
self::getContainer()->getByType(ExprPrinter::class),
27+
);
2528
}
2629

2730
public static function getAdditionalConfigFiles(): array
@@ -130,10 +133,23 @@ public function testBug12203(): void
130133
5,
131134
],
132135
[
133-
'Path in require_once() "' . __DIR__ . DIRECTORY_SEPARATOR . 'data/../bug-12203-sure-does-not-exist.php" is not a file or it does not exist.',
136+
"Path in require_once() __DIR__ . '/../bug-12203-sure-does-not-exist.php' is not a file or it does not exist.",
134137
6,
135138
],
139+
[
140+
"Path in require_once() __DIR__ . '/' . \$path . '/' . \$file is not a file or it does not exist.",
141+
10,
142+
],
143+
[
144+
'Path in require_once() __DIR__ . "{$path}/{$file}" is not a file or it does not exist.',
145+
12,
146+
],
136147
]);
137148
}
138149

150+
public function testInFileExists(): void
151+
{
152+
$this->analyse([__DIR__ . '/data/include-in-file-exists.php'], []);
153+
}
154+
139155
}

‎tests/PHPStan/Rules/Keywords/data/bug-12203.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,9 @@
44

55
require_once '../bug-12203-sure-does-not-exist.php';
66
require_once __DIR__ . '/../bug-12203-sure-does-not-exist.php';
7+
8+
$path = '..';
9+
$file = 'bug-12203-sure-does-not-exist.php';
10+
require_once __DIR__ . '/'. $path .'/'. $file;
11+
12+
require_once __DIR__ . "$path/$file";

‎tests/PHPStan/Rules/Keywords/data/include-in-file-exists.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?php
2+
3+
if (file_exists(__DIR__ . '/../vendor/autoload.php')) {
4+
require __DIR__ . '/../vendor/autoload.php';
5+
}
6+
7+
if (is_file(__DIR__ . '/../vendor/autoload.php')) {
8+
require __DIR__ . '/../vendor/autoload.php';
9+
}

0 commit comments

Comments
 (0)