diff --git a/build/collision-detector.json b/build/collision-detector.json index c5171fcc96a..390d572c254 100644 --- a/build/collision-detector.json +++ b/build/collision-detector.json @@ -9,6 +9,7 @@ "../tests/PHPStan/Parser/data/cleaning-property-hooks-after.php", "../tests/PHPStan/Rules/Functions/data/duplicate-function.php", "../tests/PHPStan/Rules/Classes/data/duplicate-class.php", + "../tests/PHPStan/Rules/data/file-reference.php", "../tests/PHPStan/Rules/Names/data/multiple-namespaces.php", "../tests/PHPStan/Rules/Names/data/no-namespace.php", "../tests/notAutoloaded", diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 140a90ae453..fe739660996 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -24,3 +24,4 @@ parameters: newOnNonObject: true unnecessaryNullCoalesce: true finiteTypesInHaystack: true + checkFileReferences: true diff --git a/conf/config.neon b/conf/config.neon index 3012ca68af2..e640f7c3eb3 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -50,6 +50,7 @@ parameters: newOnNonObject: false unnecessaryNullCoalesce: false finiteTypesInHaystack: false + checkFileReferences: false fileExtensions: - php checkAdvancedIsset: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index cf5d3b4ef6b..812756609f9 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -53,6 +53,7 @@ parametersSchema: newOnNonObject: bool() unnecessaryNullCoalesce: bool() finiteTypesInHaystack: bool() + checkFileReferences: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/src/File/FileExistenceChecker.php b/src/File/FileExistenceChecker.php new file mode 100644 index 00000000000..dbc31d58f9b --- /dev/null +++ b/src/File/FileExistenceChecker.php @@ -0,0 +1,100 @@ +exists($path, $scriptDirectory, [], false); + } + + /** + * Like fileExists(), but a directory at the resolved path also counts as existing. + * + * @param list $baseDirectories + */ + public function pathExists(string $path, string $scriptDirectory, array $baseDirectories): bool + { + return $this->exists($path, $scriptDirectory, $baseDirectories, true); + } + + /** + * @param list $baseDirectories + */ + private function exists(string $path, string $scriptDirectory, array $baseDirectories, bool $allowDirectory): bool + { + $scriptHelper = new FileHelper($scriptDirectory); + $resolvedBaseDirectories = []; + foreach ($baseDirectories as $baseDirectory) { + // a relative base directory is resolved against the directory of the analysed script + $resolvedBaseDirectories[] = $scriptHelper->absolutizePath($baseDirectory); + } + + $directories = array_merge( + $resolvedBaseDirectories, + [$this->currentWorkingDirectory], + explode(PATH_SEPARATOR, get_include_path()), + [$scriptDirectory], + ); + + foreach ($directories as $directory) { + $absolutePath = (new FileHelper($directory))->absolutizePath($path); + + if (is_file($absolutePath)) { + return true; + } + + if ($allowDirectory && is_dir($absolutePath)) { + return true; + } + } + + return false; + } + +} diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index 0ba5e1a7a57..c994839d9f6 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -10,6 +10,7 @@ use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; +use PHPStan\File\FileExistenceChecker; use PHPStan\Reflection\ConstantReflection; use PHPStan\Reflection\ExtendedParameterReflection; use PHPStan\Reflection\ParameterReflection; @@ -37,6 +38,7 @@ use function array_last; use function array_merge; use function count; +use function dirname; use function implode; use function in_array; use function is_int; @@ -44,11 +46,14 @@ use function lcfirst; use function max; use function sprintf; +use function strtolower; #[AutowiredService] final class FunctionCallParametersCheck { + private const FILE_REFERENCE_ATTRIBUTE = 'JetBrains\PhpStorm\FileReference'; + public function __construct( private RuleLevelHelper $ruleLevelHelper, private NullsafeCheck $nullsafeCheck, @@ -63,6 +68,9 @@ public function __construct( private bool $checkExtraArguments, #[AutowiredParameter] private bool $checkMissingTypehints, + private ?FileExistenceChecker $fileExistenceChecker = null, + #[AutowiredParameter(ref: '%featureToggles.checkFileReferences%')] + private bool $checkFileReferences = false, ) { } @@ -324,6 +332,10 @@ public function check( $errors[] = $error; } + foreach ($this->checkFileReferences($argumentsWithParameters, $scope, $funcCall) as $error) { + $errors[] = $error; + } + if (!$this->checkArgumentTypes && !$this->checkArgumentsPassedByReference) { return $errors; } @@ -782,6 +794,110 @@ private function processArguments( return [$errors, $newArguments]; } + /** + * Validates constant-string arguments passed to parameters annotated with + * #[JetBrains\PhpStorm\FileReference], reusing the include/require file resolution. + * + * @param array $argumentsWithParameters + * @return list + */ + private function checkFileReferences(array $argumentsWithParameters, Scope $scope, Node\Expr\FuncCall|Node\Expr\MethodCall|Node\Expr\StaticCall|Node\Expr\New_ $funcCall): array + { + if (!$this->checkFileReferences || $this->fileExistenceChecker === null) { + return []; + } + + if ($this->isFileExistenceTestingCall($funcCall, $scope)) { + // e.g. is_file()/file_exists() legitimately receive paths that may not exist + return []; + } + + $errors = []; + $scriptDirectory = dirname($scope->getFile()); + foreach ($argumentsWithParameters as $i => [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine, $parameter]) { + if ($unpack) { + continue; + } + if (!$parameter instanceof ExtendedParameterReflection) { + continue; + } + + $baseDirectories = $this->getFileReferenceBaseDirectories($parameter); + if ($baseDirectories === null) { + continue; + } + + $argumentValueType ??= $scope->getType($argumentValue); + foreach ($argumentValueType->getConstantStrings() as $constantString) { + $path = $constantString->getValue(); + if ($path === '') { + continue; + } + if ($this->fileExistenceChecker->pathExists($path, $scriptDirectory, $baseDirectories)) { + continue; + } + + $errors[] = RuleErrorBuilder::message(sprintf( + 'Path "%s" passed to %s does not exist.', + $path, + lcfirst($this->describeParameter($parameter, $argumentName ?? $i + 1)), + )) + ->identifier('argument.fileReference') + ->line($argumentLine) + ->build(); + } + } + + return $errors; + } + + private function isFileExistenceTestingCall(Node\Expr\FuncCall|Node\Expr\MethodCall|Node\Expr\StaticCall|Node\Expr\New_ $funcCall, Scope $scope): bool + { + if (!$funcCall instanceof Node\Expr\FuncCall || !$funcCall->name instanceof Node\Name) { + return false; + } + + if (!$this->reflectionProvider->hasFunction($funcCall->name, $scope)) { + return false; + } + + $functionName = strtolower($this->reflectionProvider->getFunction($funcCall->name, $scope)->getName()); + + return in_array($functionName, FileExistenceChecker::FILE_EXISTENCE_FUNCTIONS, true); + } + + /** + * @return list|null Base directories from the attribute's $basePath, or null when the parameter is not a file reference + */ + private function getFileReferenceBaseDirectories(ExtendedParameterReflection $parameter): ?array + { + $isFileReference = false; + $baseDirectories = []; + foreach ($parameter->getAttributes() as $attribute) { + if ($attribute->getName() !== self::FILE_REFERENCE_ATTRIBUTE) { + continue; + } + $isFileReference = true; + + $basePathType = $attribute->getArgumentTypes()['basePath'] ?? null; + if ($basePathType === null) { + continue; + } + foreach ($basePathType->getConstantStrings() as $basePath) { + if ($basePath->getValue() === '') { + continue; + } + $baseDirectories[] = $basePath->getValue(); + } + } + + if (!$isFileReference) { + return null; + } + + return $baseDirectories; + } + private function describeParameter(ParameterReflection $parameter, int|string|null $positionOrNamed): string { $parts = []; diff --git a/src/Rules/Keywords/RequireFileExistsRule.php b/src/Rules/Keywords/RequireFileExistsRule.php index 4ea63f57159..b9c4b4c519c 100644 --- a/src/Rules/Keywords/RequireFileExistsRule.php +++ b/src/Rules/Keywords/RequireFileExistsRule.php @@ -11,6 +11,7 @@ use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\RegisteredRule; +use PHPStan\File\FileExistenceChecker; use PHPStan\File\FileHelper; use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Rules\IdentifierRuleError; @@ -18,13 +19,8 @@ use PHPStan\Rules\RuleErrorBuilder; use PHPStan\ShouldNotHappenException; use PHPStan\Type\Constant\ConstantStringType; -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 @@ -33,26 +29,12 @@ final class RequireFileExistsRule implements Rule { - /** - * Functions that, when they return true, guarantee the path exists on the - * filesystem, so guarding a require/include with them suppresses the error. - */ - private const FILE_EXISTENCE_FUNCTIONS = [ - 'file_exists', - 'is_file', - 'is_readable', - 'is_writable', - 'is_writeable', - 'is_executable', - ]; - public function __construct( - #[AutowiredParameter] - private string $currentWorkingDirectory, private ExprPrinter $exprPrinter, #[AutowiredParameter(ref: '%featureToggles.magicDirInInclude%')] private bool $checkMagicDirInInclude, private FileHelper $fileHelper, + private FileExistenceChecker $fileExistenceChecker, ) { } @@ -91,37 +73,9 @@ public function processNode(Node $node, Scope $scope): array 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); - $absolutePath = $fileHelper->absolutizePath($path); - - return is_file($absolutePath); + return $this->fileExistenceChecker->fileExists($path, dirname($scope->getFile())); } private function getErrorMessage(Include_ $node, string $filePath): IdentifierRuleError @@ -203,7 +157,7 @@ private function resolveFilePaths(Expr $expr, Scope $scope, bool &$magicDirFallb private function isInFileExists(Include_ $node, Scope $scope): bool { - foreach (self::FILE_EXISTENCE_FUNCTIONS as $funcName) { + foreach (FileExistenceChecker::FILE_EXISTENCE_FUNCTIONS as $funcName) { $expr = new FuncCall(new FullyQualified($funcName), [ new Arg($node->expr), ]); diff --git a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php index 105257b4993..f720c39c8c4 100644 --- a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php +++ b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\Classes; +use PHPStan\File\FileExistenceChecker; use PHPStan\Rules\ClassCaseSensitivityCheck; use PHPStan\Rules\ClassForbiddenNameCheck; use PHPStan\Rules\ClassNameCheck; @@ -49,6 +50,8 @@ protected function getRule(): Rule checkArgumentsPassedByReference: true, checkExtraArguments: true, checkMissingTypehints: true, + fileExistenceChecker: self::getContainer()->getByType(FileExistenceChecker::class), + checkFileReferences: true, ), new ClassNameCheck( new ClassCaseSensitivityCheck($reflectionProvider, checkInternalClassCaseSensitivity: true), @@ -728,4 +731,18 @@ public function testInstantiationWithNonObjectType(): void ]); } + public function testFileReferenceAttribute(): void + { + $this->analyse([__DIR__ . '/../data/file-reference.php'], [ + [ + 'Path "file-reference-missing.php" passed to parameter #1 $path does not exist.', + 71, + ], + [ + 'Path "file-reference-missing.php" passed to parameter #1 $path does not exist.', + 84, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 44bc0a56d08..0801c9521ab 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\Functions; +use PHPStan\File\FileExistenceChecker; use PHPStan\Rules\FunctionCallParametersCheck; use PHPStan\Rules\NullsafeCheck; use PHPStan\Rules\PhpDoc\UnresolvableTypeHelper; @@ -49,6 +50,8 @@ protected function getRule(): Rule checkArgumentsPassedByReference: true, checkExtraArguments: true, checkMissingTypehints: true, + fileExistenceChecker: self::getContainer()->getByType(FileExistenceChecker::class), + checkFileReferences: true, ), ); } @@ -3029,4 +3032,22 @@ public function testBug11494(): void ]); } + public function testFileReferenceAttribute(): void + { + $this->analyse([__DIR__ . '/../data/file-reference.php'], [ + [ + 'Path "file-reference-missing.php" passed to parameter #1 $path does not exist.', + 57, + ], + [ + 'Path "file-reference-nested.php" passed to parameter #1 $path does not exist.', + 59, + ], + [ + 'Path "file-reference-missing.php" passed to parameter #1 $path does not exist.', + 67, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Keywords/RequireFileExistsRuleNoConstantPathTest.php b/tests/PHPStan/Rules/Keywords/RequireFileExistsRuleNoConstantPathTest.php index f953876bae3..5e12029571c 100644 --- a/tests/PHPStan/Rules/Keywords/RequireFileExistsRuleNoConstantPathTest.php +++ b/tests/PHPStan/Rules/Keywords/RequireFileExistsRuleNoConstantPathTest.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\Keywords; +use PHPStan\File\FileExistenceChecker; use PHPStan\File\FileHelper; use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Rules\Rule; @@ -18,10 +19,10 @@ class RequireFileExistsRuleNoConstantPathTest extends RuleTestCase protected function getRule(): Rule { return new RequireFileExistsRule( - $this->currentWorkingDirectory, self::getContainer()->getByType(ExprPrinter::class), true, self::getContainer()->getByType(FileHelper::class), + new FileExistenceChecker($this->currentWorkingDirectory), ); } diff --git a/tests/PHPStan/Rules/Keywords/RequireFileExistsRuleTest.php b/tests/PHPStan/Rules/Keywords/RequireFileExistsRuleTest.php index 7cad2911c20..a185b943da8 100644 --- a/tests/PHPStan/Rules/Keywords/RequireFileExistsRuleTest.php +++ b/tests/PHPStan/Rules/Keywords/RequireFileExistsRuleTest.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\Keywords; +use PHPStan\File\FileExistenceChecker; use PHPStan\File\FileHelper; use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Rules\Rule; @@ -23,10 +24,10 @@ class RequireFileExistsRuleTest extends RuleTestCase protected function getRule(): Rule { return new RequireFileExistsRule( - $this->currentWorkingDirectory, self::getContainer()->getByType(ExprPrinter::class), true, self::getContainer()->getByType(FileHelper::class), + new FileExistenceChecker($this->currentWorkingDirectory), ); } diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php index eda813a24b2..190916100d7 100644 --- a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\Methods; +use PHPStan\File\FileExistenceChecker; use PHPStan\Rules\FunctionCallParametersCheck; use PHPStan\Rules\NullsafeCheck; use PHPStan\Rules\PhpDoc\UnresolvableTypeHelper; @@ -59,6 +60,8 @@ protected function getRule(): Rule checkArgumentsPassedByReference: true, checkExtraArguments: true, checkMissingTypehints: true, + fileExistenceChecker: self::getContainer()->getByType(FileExistenceChecker::class), + checkFileReferences: true, ), ); } @@ -4258,4 +4261,17 @@ public function testBug14893(): void ]); } + public function testFileReferenceAttribute(): void + { + $this->checkThisOnly = false; + $this->checkNullables = true; + $this->checkUnionTypes = true; + $this->analyse([__DIR__ . '/../data/file-reference.php'], [ + [ + 'Path "file-reference-missing.php" passed to parameter #1 $path does not exist.', + 76, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php index cc31101ece2..85119d52cd5 100644 --- a/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\Methods; +use PHPStan\File\FileExistenceChecker; use PHPStan\Rules\ClassCaseSensitivityCheck; use PHPStan\Rules\ClassForbiddenNameCheck; use PHPStan\Rules\ClassNameCheck; @@ -70,6 +71,8 @@ protected function getRule(): Rule checkArgumentsPassedByReference: true, checkExtraArguments: true, checkMissingTypehints: true, + fileExistenceChecker: self::getContainer()->getByType(FileExistenceChecker::class), + checkFileReferences: true, ), ); } @@ -1057,4 +1060,15 @@ public function testClassExistOnCall(): void $this->analyse([__DIR__ . '/data/class-exists-on-static-call.php'], []); } + public function testFileReferenceAttribute(): void + { + $this->checkThisOnly = false; + $this->analyse([__DIR__ . '/../data/file-reference.php'], [ + [ + 'Path "file-reference-missing.php" passed to parameter #1 $path does not exist.', + 80, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/data/file-reference-base/file-reference-nested.php b/tests/PHPStan/Rules/data/file-reference-base/file-reference-nested.php new file mode 100644 index 00000000000..b576167ec6b --- /dev/null +++ b/tests/PHPStan/Rules/data/file-reference-base/file-reference-nested.php @@ -0,0 +1,3 @@ += 8.0 + +namespace JetBrains\PhpStorm { + // Declared locally: PHPStan does not autoload phpstorm-stubs' meta/attributes/FileReference.php (it is absent from PhpStormStubsMap). Drop this once the attribute is registered there. + #[\Attribute(\Attribute::TARGET_PARAMETER)] + class FileReference + { + + public function __construct(string $basePath = '') + { + } + + } + +} + +namespace FileReferenceTest { + + use JetBrains\PhpStorm\FileReference; + + function loadFile(#[FileReference] string $path): void + { + } + + function loadFileWithBasePath(#[FileReference('file-reference-base')] string $path): void + { + } + + class Loader + { + + public function __construct(#[FileReference] string $path) + { + } + + public function load(#[FileReference] string $path): void + { + } + + public static function loadStatic(#[FileReference] string $path): void + { + } + + } + + class PromotedLoader + { + + public function __construct(#[FileReference] public string $path) + { + } + + } + + // function calls + loadFile('file-reference-existing.php'); + loadFile('file-reference-missing.php'); + loadFile('file-reference-base'); + loadFile('file-reference-nested.php'); + + // non-constant path is not checked + $dynamicPath = (string) rand(); + loadFile($dynamicPath); + + // base path resolves against the analysed file directory + loadFileWithBasePath('file-reference-nested.php'); + loadFileWithBasePath('file-reference-missing.php'); + + // constructor calls + new Loader('file-reference-existing.php'); + new Loader('file-reference-missing.php'); + + // method calls + $loader = new Loader('file-reference-existing.php'); + $loader->load('file-reference-existing.php'); + $loader->load('file-reference-missing.php'); + + // static calls + Loader::loadStatic('file-reference-existing.php'); + Loader::loadStatic('file-reference-missing.php'); + + // promoted constructor property + new PromotedLoader('file-reference-existing.php'); + new PromotedLoader('file-reference-missing.php'); + +}