Skip to content

Rule to check if required file exists #3294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 49 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
c1f3c6f
Create rule for checking paths inside the require keyword
Bellangelo Aug 5, 2024
fb165ba
Test that it cannot read constants from the analyzing file
Bellangelo Aug 5, 2024
749414a
Test that relative paths can produce problems
Bellangelo Aug 5, 2024
deefe60
Test that it can read class constants
Bellangelo Aug 5, 2024
be18ba6
Test that it can read constants
Bellangelo Aug 5, 2024
24a9650
Test that it can read __DIR__
Bellangelo Aug 5, 2024
7c3f635
Test that it cannot read class properties
Bellangelo Aug 5, 2024
4ff25c8
Test that it cannot read variables
Bellangelo Aug 5, 2024
1625225
Test that it cannot class methods
Bellangelo Aug 5, 2024
496c541
Test that it cannot read functions
Bellangelo Aug 5, 2024
03f15fa
Unify filenames
Bellangelo Aug 5, 2024
4889f9e
Const types are not released until PHP 8.3
Bellangelo Aug 5, 2024
0e581eb
Fix code style
Bellangelo Aug 5, 2024
1520ea9
Classes should be abstract or final
Bellangelo Aug 5, 2024
960c71a
getValue() is deprecated
Bellangelo Aug 5, 2024
aff4b04
Put non-important errors under the baseline
Bellangelo Aug 5, 2024
2814cf8
Test files were renamed
Bellangelo Aug 5, 2024
4acc555
Make condition more clear
Bellangelo Aug 7, 2024
4680c6e
Make error message more descriptive
Bellangelo Aug 7, 2024
67ce86b
Remove unused import
Bellangelo Aug 7, 2024
31fa5e0
getConstantStrings() can handle all the manipulation of the path
Bellangelo Aug 7, 2024
50dde41
Make condition more specific
Bellangelo Aug 7, 2024
9f353d8
Class does not accept any arguments in constructor anymore
Bellangelo Aug 7, 2024
bfa3908
Fix conflicts
Bellangelo Aug 22, 2024
2369559
Register class rule
Bellangelo Aug 7, 2024
a2f0e6e
Fix small PHPStan issues
Bellangelo Aug 7, 2024
f20f26f
Fix code style
Bellangelo Aug 7, 2024
9896884
Convert syntax to <=PHP7.4
Bellangelo Aug 7, 2024
54f7779
Handle include() and include_once()
Bellangelo Aug 13, 2024
b723cf2
Rules section is always registered regardless of the conditionalTags
Bellangelo Aug 13, 2024
9035154
Add identifier in rule
Bellangelo Aug 13, 2024
4c85ed7
Tag forces rule to be always registered
Bellangelo Aug 13, 2024
2bed45a
Constants are not readable without the usePathConstantsAsConstantStri…
Bellangelo Aug 13, 2024
76b6f4f
Validate all possible cases
Bellangelo Aug 13, 2024
8ea25b8
remove unused import
Bellangelo Aug 13, 2024
f8db0ec
Handle include paths
Bellangelo Aug 13, 2024
032965a
Simplify tests
Bellangelo Aug 13, 2024
31a86c8
Make test file names consistent
Bellangelo Aug 13, 2024
6abc7ec
Test relative paths
Bellangelo Aug 13, 2024
71ee779
Small code improvements
Bellangelo Aug 13, 2024
3df4d41
Remove unused files
Bellangelo Aug 13, 2024
95e5888
Clean-up files
Bellangelo Aug 13, 2024
4858ceb
Method that works like stream_resolve_include_path but for a specific…
Bellangelo Aug 22, 2024
67ee4d8
Use the working directory that is configured in phpstan
Bellangelo Aug 22, 2024
4e049ea
Allow rule to be ignored
Bellangelo Aug 22, 2024
71d23e4
Use php7.4 syntax
Bellangelo Aug 22, 2024
525a393
Fix return type for < PHP7.4
Bellangelo Aug 22, 2024
e861017
Allow PHPStan to statically infer the possible identifiers
Bellangelo Aug 29, 2024
0743c06
Remove unused import
Bellangelo Aug 29, 2024
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
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,6 @@ parameters:
tooWidePropertyType: true
explicitThrow: true
absentTypeChecks: true
requireFileExists: true
stubFiles:
- ../stubs/bleedingEdge/Rule.stub
6 changes: 6 additions & 0 deletions conf/config.level0.neon
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ conditionalTags:
phpstan.rules.rule: %featureToggles.printfArrayParameters%
PHPStan\Rules\Regexp\RegularExpressionQuotingRule:
phpstan.rules.rule: %featureToggles.validatePregQuote%
PHPStan\Rules\Keywords\RequireFileExistsRule:
phpstan.rules.rule: %featureToggles.requireFileExists%

rules:
- PHPStan\Rules\Api\ApiInstantiationRule
Expand Down Expand Up @@ -309,3 +311,7 @@ services:

-
class: PHPStan\Rules\Regexp\RegularExpressionQuotingRule
-
class: PHPStan\Rules\Keywords\RequireFileExistsRule
arguments:
currentWorkingDirectory: %currentWorkingDirectory%
1 change: 1 addition & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ parameters:
preciseMissingReturn: false
validatePregQuote: false
noImplicitWildcard: false
requireFileExists: false
narrowPregMatches: true
tooWidePropertyType: false
explicitThrow: false
Expand Down
1 change: 1 addition & 0 deletions conf/parametersSchema.neon
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ parametersSchema:
tooWidePropertyType: bool()
explicitThrow: bool()
absentTypeChecks: bool()
requireFileExists: bool()
])
fileExtensions: listOf(string())
checkAdvancedIsset: bool()
Expand Down
138 changes: 138 additions & 0 deletions src/Rules/Keywords/RequireFileExistsRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Keywords;

use PhpParser\Node;
use PhpParser\Node\Expr\Include_;
use PHPStan\Analyser\Scope;
use PHPStan\File\FileHelper;
use PHPStan\Rules\IdentifierRuleError;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\ShouldNotHappenException;
use function array_merge;
use function dirname;
use function explode;
use function get_include_path;
use function is_file;
use function sprintf;
use const PATH_SEPARATOR;

/**
* @implements Rule<Include_>
*/
final class RequireFileExistsRule implements Rule
{

public function __construct(private string $currentWorkingDirectory)
{
}

public function getNodeType(): string
{
return Include_::class;
}

public function processNode(Node $node, Scope $scope): array
{
$errors = [];
$paths = $this->resolveFilePaths($node, $scope);

foreach ($paths as $path) {
if ($this->doesFileExist($path, $scope)) {
continue;
}

$errors[] = $this->getErrorMessage($node, $path);
}

return $errors;
}

/**
* We cannot use `stream_resolve_include_path` as it works based on the calling script.
* This method simulates the behavior of `stream_resolve_include_path` but for the given scope.
* The priority order is the following:
* 1. The current working directory.
* 2. The include path.
* 3. The path of the script that is being executed.
*/
private function doesFileExist(string $path, Scope $scope): bool
{
$directories = array_merge(
[$this->currentWorkingDirectory],
explode(PATH_SEPARATOR, get_include_path()),
[dirname($scope->getFile())],
);

foreach ($directories as $directory) {
if ($this->doesFileExistForDirectory($path, $directory)) {
return true;
}
}

return false;
}

private function doesFileExistForDirectory(string $path, string $workingDirectory): bool
{
$fileHelper = new FileHelper($workingDirectory);
$normalisedPath = $fileHelper->normalizePath($path);
$absolutePath = $fileHelper->absolutizePath($normalisedPath);

return is_file($absolutePath);
}

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

switch ($node->type) {
case Include_::TYPE_REQUIRE:
$type = 'require';
Copy link
Member

Choose a reason for hiding this comment

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

One last simple change: I worry the identifier cannot be statically inferred here because of str_replace. We need that for https://github.com/phpstan/phpstan/actions/workflows/extract-identifiers.yml which updates https://phpstan.org/error-identifiers.

Please assign $identifier = ... in the switch in each case, and use that in RuleErrorBuilder.

Then I'll merge this. Thank you for the effort!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e861017.

Shouldn't this also run as a quality check before merging a PR?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, feel free to contribute this rule 😊

$identifierType = 'require';
break;
case Include_::TYPE_REQUIRE_ONCE:
$type = 'require_once';
$identifierType = 'requireOnce';
break;
case Include_::TYPE_INCLUDE:
$type = 'include';
$identifierType = 'include';
break;
case Include_::TYPE_INCLUDE_ONCE:
$type = 'include_once';
$identifierType = 'includeOnce';
break;
default:
throw new ShouldNotHappenException('Rule should have already validated the node type.');
}

$identifier = sprintf('%s.fileNotFound', $identifierType);

return RuleErrorBuilder::message(
sprintf(
$message,
$type,
$filePath,
),
)->identifier($identifier)->build();
}

/**
* @return array<string>
*/
private function resolveFilePaths(Include_ $node, Scope $scope): array
{
$paths = [];
$type = $scope->getType($node->expr);
$constantStrings = $type->getConstantStrings();

foreach ($constantStrings as $constantString) {
$paths[] = $constantString->getValue();
}

return $paths;
}

}
136 changes: 136 additions & 0 deletions tests/PHPStan/Rules/Keywords/RequireFileExistsRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Keywords;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use function get_include_path;
use function implode;
use function realpath;
use function set_include_path;
use const PATH_SEPARATOR;

/**
* @extends RuleTestCase<RequireFileExistsRule>
*/
class RequireFileExistsRuleTest extends RuleTestCase
{

private RequireFileExistsRule $rule;

public function setUp(): void
{
parent::setUp();

$this->rule = $this->getDefaultRule();
}

protected function getRule(): Rule
{
return $this->rule;
}

public static function getAdditionalConfigFiles(): array
{
return [
__DIR__ . '/../../Analyser/usePathConstantsAsConstantString.neon',
];
}

private function getDefaultRule(): RequireFileExistsRule
{
return new RequireFileExistsRule(__DIR__ . '/../');
}

public function testBasicCase(): void
{
$this->analyse([__DIR__ . '/data/require-file-simple-case.php'], [
[
'Path in include() "a-file-that-does-not-exist.php" is not a file or it does not exist.',
11,
],
[
'Path in include_once() "a-file-that-does-not-exist.php" is not a file or it does not exist.',
12,
],
[
'Path in require() "a-file-that-does-not-exist.php" is not a file or it does not exist.',
13,
],
[
'Path in require_once() "a-file-that-does-not-exist.php" is not a file or it does not exist.',
14,
],
]);
}

public function testFileDoesNotExistConditionally(): void
{
$this->analyse([__DIR__ . '/data/require-file-conditionally.php'], [
[
'Path in include() "a-file-that-does-not-exist.php" is not a file or it does not exist.',
9,
],
[
'Path in include_once() "a-file-that-does-not-exist.php" is not a file or it does not exist.',
10,
],
[
'Path in require() "a-file-that-does-not-exist.php" is not a file or it does not exist.',
11,
],
[
'Path in require_once() "a-file-that-does-not-exist.php" is not a file or it does not exist.',
12,
],
]);
}

public function testRelativePath(): void
{
$this->analyse([__DIR__ . '/data/require-file-relative-path.php'], [
[
'Path in include() "data/include-me-to-prove-you-work.txt" is not a file or it does not exist.',
8,
],
[
'Path in include_once() "data/include-me-to-prove-you-work.txt" is not a file or it does not exist.',
9,
],
[
'Path in require() "data/include-me-to-prove-you-work.txt" is not a file or it does not exist.',
10,
],
[
'Path in require_once() "data/include-me-to-prove-you-work.txt" is not a file or it does not exist.',
11,
],
]);
}

public function testRelativePathWithIncludePath(): void
{
$includePaths = [realpath(__DIR__)];
$includePaths[] = get_include_path();

set_include_path(implode(PATH_SEPARATOR, $includePaths));

try {
$this->analyse([__DIR__ . '/data/require-file-relative-path.php'], []);
} finally {
set_include_path($includePaths[1]);
}
}

public function testRelativePathWithSameWorkingDirectory(): void
{
$this->rule = new RequireFileExistsRule(__DIR__);

try {
$this->analyse([__DIR__ . '/data/require-file-relative-path.php'], []);
} finally {
$this->rule = $this->getDefaultRule();
}
}

}
Empty file.
12 changes: 12 additions & 0 deletions tests/PHPStan/Rules/Keywords/data/require-file-conditionally.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php declare(strict_types=1);

$path = __DIR__ . '/include-me-to-prove-you-work.txt';

if (rand(0,1)) {
$path = 'a-file-that-does-not-exist.php';
}

include $path;
include_once $path;
require $path;
require_once $path;
11 changes: 11 additions & 0 deletions tests/PHPStan/Rules/Keywords/data/require-file-relative-path.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php declare(strict_types=1);

include 'include-me-to-prove-you-work.txt';
include_once 'include-me-to-prove-you-work.txt';
require 'include-me-to-prove-you-work.txt';
require_once 'include-me-to-prove-you-work.txt';

include 'data/include-me-to-prove-you-work.txt';
include_once 'data/include-me-to-prove-you-work.txt';
require 'data/include-me-to-prove-you-work.txt';
require_once 'data/include-me-to-prove-you-work.txt';
14 changes: 14 additions & 0 deletions tests/PHPStan/Rules/Keywords/data/require-file-simple-case.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php declare(strict_types=1);

$fileThatExists = __DIR__ . '/include-me-to-prove-you-work.txt';
$fileThatDoesNotExist = 'a-file-that-does-not-exist.php';

include $fileThatExists;
include_once $fileThatExists;
require $fileThatExists;
require_once $fileThatExists;

include $fileThatDoesNotExist;
include_once $fileThatDoesNotExist;
require $fileThatDoesNotExist;
require_once $fileThatDoesNotExist;
Loading