Merge pull request #10211 from robchett/disable_ignoreInternalFunctio…

…nFalseReturn_by_default Disable ignoreInternalFunction(False|Null)Return by default
vimeo · Oct 16, 2023 · 8f9f8d0 · 8f9f8d0
2 parents 9b00ac0 + 0bd4f9b
commit 8f9f8d0
Show file tree

Hide file tree

Showing 92 changed files with 992 additions and 858 deletions.
diff --git a/UPGRADING.md b/UPGRADING.md
@@ -3,6 +3,8 @@
 
 - The minimum PHP version was raised to PHP 8.1.17.
 
+- [BC] The configuration settings `ignoreInternalFunctionFalseReturn` and `ignoreInternalFunctionNullReturn` are now defaulted to `false`
+
 - [BC] Switched the internal representation of `list<T>` and `non-empty-list<T>` from the TList and TNonEmptyList classes to an unsealed list shape: the TList, TNonEmptyList and TCallableList classes were removed.
   Nothing will change for users: the `list<T>` and `non-empty-list<T>` syntax will remain supported and its semantics unchanged.
   Psalm 5 already deprecates the `TList`, `TNonEmptyList` and `TCallableList` classes: use `\Psalm\Type::getListAtomic`, `\Psalm\Type::getNonEmptyListAtomic` and `\Psalm\Type::getCallableListAtomic` to instantiate list atomics, or directly instantiate TKeyedArray objects with `is_list=true` where appropriate.

diff --git a/bin/update-property-map.php b/bin/update-property-map.php
@@ -93,9 +93,9 @@ function extractClassesFromStatements(array $statements): array
 foreach ($files as $file) {
     $contents = file_get_contents($file);
     // FIXME: find a way to ignore custom entities, for now we strip them.
-    $contents = preg_replace('#&[a-zA-Z\d.\-_]+;#', '', $contents);
-    $contents = preg_replace('#%[a-zA-Z\d.\-_]+;#', '', $contents);
-    $contents = preg_replace('#<!ENTITY[^>]+>#', '', $contents);
+    $contents = (string) preg_replace('#&[a-zA-Z\d.\-_]+;#', '', $contents);
+    $contents = (string) preg_replace('#%[a-zA-Z\d.\-_]+;#', '', $contents);
+    $contents = (string) preg_replace('#<!ENTITY[^>]+>#', '', $contents);
     try {
         $simple = new SimpleXMLElement($contents);
     } catch (Throwable $exception) {

diff --git a/config.xsd b/config.xsd
@@ -49,8 +49,8 @@
         <xs:attribute name="findUnusedBaselineEntry" type="xs:boolean" default="false" />
         <xs:attribute name="hideExternalErrors" type="xs:boolean" default="false" />
         <xs:attribute name="hoistConstants" type="xs:boolean" default="false" />
-        <xs:attribute name="ignoreInternalFunctionFalseReturn" type="xs:boolean" default="true" />
-        <xs:attribute name="ignoreInternalFunctionNullReturn" type="xs:boolean" default="true" />
+        <xs:attribute name="ignoreInternalFunctionFalseReturn" type="xs:boolean" default="false" />
+        <xs:attribute name="ignoreInternalFunctionNullReturn" type="xs:boolean" default="false" />
         <xs:attribute name="includePhpVersionsInErrorBaseline" type="xs:boolean" default="false" />
         <xs:attribute name="inferPropertyTypesFromConstructor" type="xs:boolean" default="true" />
         <xs:attribute name="memoizeMethodCallResults" type="xs:boolean" default="false" />

diff --git a/docs/running_psalm/configuration.md b/docs/running_psalm/configuration.md
@@ -213,7 +213,7 @@ When `true`, Psalm will check that the developer has caught every exception in g
   ignoreInternalFunctionFalseReturn="[bool]"
 >
 ```
-When `true`, Psalm ignores possibly-false issues stemming from return values of internal functions (like `preg_split`) that may return false, but do so rarely. Defaults to `true`.
+When `true`, Psalm ignores possibly-false issues stemming from return values of internal functions (like `preg_split`) that may return false, but do so rarely. Defaults to `false`.
 
 #### ignoreInternalFunctionNullReturn
 
@@ -222,7 +222,7 @@ When `true`, Psalm ignores possibly-false issues stemming from return values of
   ignoreInternalFunctionNullReturn="[bool]"
 >
 ```
-When `true`, Psalm ignores possibly-null issues stemming from return values of internal array functions (like `current`) that may return null, but do so rarely. Defaults to `true`.
+When `true`, Psalm ignores possibly-null issues stemming from return values of internal array functions (like `current`) that may return null, but do so rarely. Defaults to `false`.
 
 #### inferPropertyTypesFromConstructor
 

diff --git a/examples/TemplateChecker.php b/examples/TemplateChecker.php
@@ -160,7 +160,7 @@ protected function checkWithViewClass(Context $context, array $stmts): void
             }
         }
 
-        $pseudo_method_name = preg_replace('/[^a-zA-Z0-9_]+/', '_', $this->file_name);
+        $pseudo_method_name = (string) preg_replace('/[^a-zA-Z0-9_]+/', '_', $this->file_name);
 
         $class_method = new VirtualClassMethod($pseudo_method_name, ['stmts' => []]);
 

diff --git a/examples/plugins/StringChecker.php b/examples/plugins/StringChecker.php
@@ -35,6 +35,7 @@ public static function afterExpressionAnalysis(AfterExpressionAnalysisEvent $eve
                 && strpos($expr->value, 'TestController') === false
                 && preg_match($class_or_class_method, $expr->value)
             ) {
+                /** @psalm-suppress PossiblyInvalidArrayAccess */
                 $absolute_class = preg_split('/[:]/', $expr->value)[0];
                 IssueBuffer::maybeAdd(
                     new InvalidClass(

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="dev-master@f799b68a3cbc91e9056bb972cfe66b7ae0da3f76">
+<files psalm-version="dev-master@77650e7b153ee3a96507ba6902ce2807def23221">
   <file src="examples/TemplateChecker.php">
     <PossiblyUndefinedIntArrayOffset>
       <code><![CDATA[$comment_block->tags['variablesfrom'][0]]]></code>
@@ -12,6 +12,11 @@
       <code>$matches[1]</code>
     </PossiblyUndefinedIntArrayOffset>
   </file>
+  <file src="src/Psalm/Config.php">
+    <PossiblyNullArgument>
+      <code>$deprecated_element_xml</code>
+    </PossiblyNullArgument>
+  </file>
   <file src="src/Psalm/Config/FileFilter.php">
     <PossiblyUndefinedIntArrayOffset>
       <code><![CDATA[explode('::', $method_id)[1]]]></code>
@@ -282,6 +287,11 @@
       <code><![CDATA[$stmt->props[0]]]></code>
     </PossiblyUndefinedIntArrayOffset>
   </file>
+  <file src="src/Psalm/Internal/Fork/Pool.php">
+    <PossiblyFalseArgument>
+      <code>$buffer</code>
+    </PossiblyFalseArgument>
+  </file>
   <file src="src/Psalm/Internal/LanguageServer/LanguageClient.php">
     <InvalidArrayAccess>
       <code>$config</code>

diff --git a/src/Psalm/CodeLocation.php b/src/Psalm/CodeLocation.php
@@ -222,7 +222,7 @@ private function calculateRealLocation(): void
 
             $indentation = (int)strpos($key_line, '@');
 
-            $key_line = trim(preg_replace('@\**/\s*@', '', mb_strcut($key_line, $indentation)));
+            $key_line = trim((string) preg_replace('@\**/\s*@', '', mb_strcut($key_line, $indentation)));
 
             $this->selection_start = $preview_offset + $indentation + $this->preview_start;
             $this->selection_end = $this->selection_start + strlen($key_line);

diff --git a/src/Psalm/Codebase.php b/src/Psalm/Codebase.php
@@ -993,7 +993,7 @@ public function getMarkupContentForSymbolByReference(
         //Direct Assignment
         if (is_numeric($reference->symbol[0])) {
             return new PHPMarkdownContent(
-                preg_replace(
+                (string) preg_replace(
                     '/^[^:]*:/',
                     '',
                     $reference->symbol,
@@ -1032,7 +1032,7 @@ public function getMarkupContentForSymbolByReference(
 
             //Class Property
             if (strpos($reference->symbol, '$') !== false) {
-                $property_id = preg_replace('/^\\\\/', '', $reference->symbol);
+                $property_id = (string) preg_replace('/^\\\\/', '', $reference->symbol);
                 /** @psalm-suppress PossiblyUndefinedIntArrayOffset */
                 [$fq_class_name, $property_name] = explode('::$', $property_id);
                 $class_storage = $this->classlikes->getStorageFor($fq_class_name);
@@ -1204,7 +1204,7 @@ public function getMarkupContentForSymbolByReference(
     public function getSymbolLocationByReference(Reference $reference): ?CodeLocation
     {
         if (is_numeric($reference->symbol[0])) {
-            $symbol = preg_replace('/:.*/', '', $reference->symbol);
+            $symbol = (string) preg_replace('/:.*/', '', $reference->symbol);
             $symbol_parts = explode('-', $symbol);
 
             if (!isset($symbol_parts[0]) || !isset($symbol_parts[1])) {
@@ -1812,7 +1812,7 @@ public function getCompletionItemsForPartialSymbol(
             ) {
                 $file_contents = $this->getFileContents($file_path);
 
-                $class_name = preg_replace('/^.*\\\/', '', $fq_class_name, 1);
+                $class_name = (string) preg_replace('/^.*\\\/', '', $fq_class_name, 1);
 
                 if ($aliases->uses_end) {
                     $position = self::getPositionFromOffset($aliases->uses_end, $file_contents);

diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php
@@ -401,12 +401,12 @@ class Config
     /**
      * @var bool
      */
-    public $ignore_internal_falsable_issues = true;
+    public $ignore_internal_falsable_issues = false;
 
     /**
      * @var bool
      */
-    public $ignore_internal_nullable_issues = true;
+    public $ignore_internal_nullable_issues = false;
 
     /**
      * @var array<string, bool>
@@ -840,6 +840,7 @@ private static function loadDomDocument(string $base_dir, string $file_contents)
         $dom_document->loadXML($file_contents, LIBXML_NONET);
         $dom_document->xinclude(LIBXML_NOWARNING | LIBXML_NONET);
 
+        /** @psalm-suppress PossiblyFalseArgument */
         chdir($oldpwd);
         return $dom_document;
     }
@@ -1103,7 +1104,9 @@ private static function fromXmlAndPaths(
 
         $composer_json = null;
         if (file_exists($composer_json_path)) {
-            $composer_json = json_decode(file_get_contents($composer_json_path), true);
+            $composer_json_contents = file_get_contents($composer_json_path);
+            assert($composer_json_contents !== false);
+            $composer_json = json_decode($composer_json_contents, true);
             if (!is_array($composer_json)) {
                 throw new UnexpectedValueException('Invalid composer.json at ' . $composer_json_path);
             }
@@ -1159,7 +1162,7 @@ private static function fromXmlAndPaths(
                 }
             }
 
-            $config->autoloader = realpath($autoloader_path);
+            $config->autoloader = (string) realpath($autoloader_path);
         }
 
         if (isset($config_xml['cacheDirectory'])) {
@@ -1490,7 +1493,7 @@ public function safeSetCustomErrorLevel(string $issue_key, string $error_level):
     private function loadFileExtensions(SimpleXMLElement $extensions): void
     {
         foreach ($extensions as $extension) {
-            $extension_name = preg_replace('/^\.?/', '', (string)$extension['name'], 1);
+            $extension_name = (string) preg_replace('/^\.?/', '', (string)$extension['name'], 1);
             $this->file_extensions[] = $extension_name;
 
             if (isset($extension['scanner'])) {
@@ -1725,7 +1728,7 @@ private function getPluginClassForPath(Codebase $codebase, string $path, string
     public function shortenFileName(string $to): string
     {
         if (!is_file($to)) {
-            return preg_replace('/^' . preg_quote($this->base_dir, '/') . '/', '', $to, 1);
+            return (string) preg_replace('/^' . preg_quote($this->base_dir, '/') . '/', '', $to, 1);
         }
 
         $from = $this->base_dir;
@@ -1907,7 +1910,7 @@ public static function getParentIssueType(string $issue_type): ?string
         }
 
         if (strpos($issue_type, 'Possibly') === 0) {
-            $stripped_issue_type = preg_replace('/^Possibly(False|Null)?/', '', $issue_type, 1);
+            $stripped_issue_type = (string) preg_replace('/^Possibly(False|Null)?/', '', $issue_type, 1);
 
             if (strpos($stripped_issue_type, 'Invalid') === false && strpos($stripped_issue_type, 'Un') !== 0) {
                 $stripped_issue_type = 'Invalid' . $stripped_issue_type;
@@ -2049,7 +2052,7 @@ public function getReportingLevelForFunction(string $issue_type, string $functio
             if ($level === null && $issue_type === 'UndefinedFunction') {
                 // undefined functions trigger global namespace fallback
                 // so we should also check reporting levels for the symbol in global scope
-                $root_function_id = preg_replace('/.*\\\/', '', $function_id);
+                $root_function_id = (string) preg_replace('/.*\\\/', '', $function_id);
                 if ($root_function_id !== $function_id) {
                     /** @psalm-suppress PossiblyUndefinedStringArrayOffset https://github.com/vimeo/psalm/issues/7656 */
                     $level = $this->issue_handlers[$issue_type]->getReportingLevelForFunction($root_function_id);
@@ -2294,9 +2297,10 @@ public function visitStubFiles(Codebase $codebase, ?Progress $progress = null):
             if (is_file($phpstorm_meta_path)) {
                 $stub_files[] = $phpstorm_meta_path;
             } elseif (is_dir($phpstorm_meta_path)) {
-                $phpstorm_meta_path = realpath($phpstorm_meta_path);
+                $phpstorm_meta_path = (string) realpath($phpstorm_meta_path);
+                $phpstorm_meta_files = glob($phpstorm_meta_path . '/*.meta.php', GLOB_NOSORT);
 
-                foreach (glob($phpstorm_meta_path . '/*.meta.php', GLOB_NOSORT) as $glob) {
+                foreach ($phpstorm_meta_files ?: [] as $glob) {
                     if (is_file($glob) && realpath(dirname($glob)) === $phpstorm_meta_path) {
                         $stub_files[] = $glob;
                     }
@@ -2507,7 +2511,7 @@ public function getPotentialComposerFilePathForClassLike(string $class): ?string
                         && $this->isInProjectDirs($dir . DIRECTORY_SEPARATOR . 'testdummy.php')
                     ) {
                         $maxDepth = $depth;
-                        $candidate_path = realpath($dir) . $pathEnd;
+                        $candidate_path = (string) realpath($dir) . $pathEnd;
                     }
                 }
             }
@@ -2642,7 +2646,9 @@ public function getPHPVersionFromComposerJson(): ?string
 
         if (file_exists($composer_json_path)) {
             try {
-                $composer_json = json_decode(file_get_contents($composer_json_path), true, 512, JSON_THROW_ON_ERROR);
+                $composer_json_contents = file_get_contents($composer_json_path);
+                assert($composer_json_contents !== false);
+                $composer_json = json_decode($composer_json_contents, true, 512, JSON_THROW_ON_ERROR);
             } catch (JsonException $e) {
                 $composer_json = null;
             }

diff --git a/src/Psalm/Config/Creator.php b/src/Psalm/Config/Creator.php
@@ -15,6 +15,7 @@
 use function array_sum;
 use function array_unique;
 use function array_values;
+use function assert;
 use function count;
 use function explode;
 use function file_exists;
@@ -196,8 +197,10 @@ public static function getPaths(string $current_dir, ?string $suggested_dir): ar
                 );
             }
             try {
+                $composer_json_contents = file_get_contents($composer_json_location);
+                assert($composer_json_contents !== false);
                 $composer_json = json_decode(
-                    file_get_contents($composer_json_location),
+                    $composer_json_contents,
                     true,
                     512,
                     JSON_THROW_ON_ERROR,
@@ -261,7 +264,7 @@ private static function getPsr4Or0Paths(string $current_dir, array $composer_jso
                     continue;
                 }
 
-                $path = preg_replace('@[/\\\]$@', '', $path, 1);
+                $path = (string) preg_replace('@[/\\\]$@', '', $path, 1);
 
                 if ($path !== 'tests') {
                     $nodes[] = '<directory name="' . $path . '" />';
@@ -285,9 +288,9 @@ private static function guessPhpFileDirs(string $current_dir): array
 
         /** @var string[] */
         $php_files = array_merge(
-            glob($current_dir . DIRECTORY_SEPARATOR . '*.php', GLOB_NOSORT),
-            glob($current_dir . DIRECTORY_SEPARATOR . '**/*.php', GLOB_NOSORT),
-            glob($current_dir . DIRECTORY_SEPARATOR . '**/**/*.php', GLOB_NOSORT),
+            glob($current_dir . DIRECTORY_SEPARATOR . '*.php', GLOB_NOSORT) ?: [],
+            glob($current_dir . DIRECTORY_SEPARATOR . '**/*.php', GLOB_NOSORT) ?: [],
+            glob($current_dir . DIRECTORY_SEPARATOR . '**/**/*.php', GLOB_NOSORT) ?: [],
         );
 
         foreach ($php_files as $php_file) {

diff --git a/src/Psalm/Config/FileFilter.php b/src/Psalm/Config/FileFilter.php
@@ -12,6 +12,7 @@
 use function array_map;
 use function array_merge;
 use function array_shift;
+use function assert;
 use function count;
 use function explode;
 use function glob;
@@ -136,7 +137,11 @@ public static function loadFromArray(
 
                 if (strpos($prospective_directory_path, '*') !== false) {
                     // Strip meaningless trailing recursive wildcard like "path/**/" or "path/**"
-                    $prospective_directory_path = preg_replace('#(\/\*\*)+\/?$#', '/', $prospective_directory_path);
+                    $prospective_directory_path = (string) preg_replace(
+                        '#(\/\*\*)+\/?$#',
+                        '/',
+                        $prospective_directory_path,
+                    );
                     // Split by /**/, allow duplicated wildcards like "path/**/**/path" and any leading dir separator.
                     /** @var non-empty-list<non-empty-string> $path_parts */
                     $path_parts = preg_split('#(\/|\\\)(\*\*\/)+#', $prospective_directory_path);
@@ -206,7 +211,7 @@ public static function loadFromArray(
 
                     while ($iterator->valid()) {
                         if ($iterator->isLink()) {
-                            $linked_path = readlink($iterator->getPathname());
+                            $linked_path = (string) readlink($iterator->getPathname());
 
                             if (stripos($linked_path, $directory_path) !== 0) {
                                 if ($ignore_type_stats && $filter instanceof ProjectFileFilter) {
@@ -286,7 +291,7 @@ public static function loadFromArray(
                     continue;
                 }
 
-                $file_path = realpath($prospective_file_path);
+                $file_path = (string) realpath($prospective_file_path);
 
                 if (!$file_path && !$allow_missing_files) {
                     throw new ConfigException(
@@ -492,6 +497,7 @@ private static function recursiveGlob(array $parts, bool $only_dir): array
 
         $first_dir = self::slashify($parts[0]);
         $paths = glob($first_dir . '*', GLOB_ONLYDIR | GLOB_NOSORT);
+        assert($paths !== false);
         $result = [];
         foreach ($paths as $path) {
             $parts[0] = $path;

diff --git a/src/Psalm/Config/IssueHandler.php b/src/Psalm/Config/IssueHandler.php
@@ -8,6 +8,7 @@
 
 use function array_filter;
 use function array_map;
+use function assert;
 use function dirname;
 use function in_array;
 use function scandir;
@@ -157,10 +158,12 @@ public function getReportingLevelForVariable(string $var_name): ?string
      */
     public static function getAllIssueTypes(): array
     {
+        $scan = scandir(dirname(__DIR__) . '/Issue', SCANDIR_SORT_NONE);
+        assert($scan !== false);
         return array_filter(
             array_map(
                 static fn(string $file_name): string => substr($file_name, 0, -4),
-                scandir(dirname(__DIR__) . '/Issue', SCANDIR_SORT_NONE),
+                $scan,
             ),
             static fn(string $issue_name): bool => $issue_name !== ''
                 && $issue_name !== 'MethodIssue'

diff --git a/src/Psalm/Context.php b/src/Psalm/Context.php
@@ -844,7 +844,7 @@ public function hasVariable(string $var_name): bool
             return false;
         }
 
-        $stripped_var = preg_replace('/(->|\[).*$/', '', $var_name, 1);
+        $stripped_var = (string) preg_replace('/(->|\[).*$/', '', $var_name, 1);
 
         if ($stripped_var !== '$this' || $var_name !== $stripped_var) {
             $this->cond_referenced_var_ids[$var_name] = true;

diff --git a/src/Psalm/ErrorBaseline.php b/src/Psalm/ErrorBaseline.php
@@ -244,7 +244,7 @@ private static function writeToFile(
             $filesNode->setAttribute('php-version', implode(';' . "\n\t", [...[
                 ('php:' . PHP_VERSION),
             ], ...array_map(
-                static fn(string $extension): string => $extension . ':' . phpversion($extension),
+                static fn(string $extension): string => $extension . ':' . (string) phpversion($extension),
                 $extensions,
             )]));
         }