Merge remote-tracking branch 'vimeo/5.x' into HEAD

danog · Dec 19, 2023 · 9b82514 · 9b82514
2 parents a75d26a + b38530e
commit 9b82514
Show file tree

Hide file tree

Showing 72 changed files with 1,064 additions and 381 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -135,6 +135,7 @@ jobs:
       CHUNK_COUNT: "${{ matrix.count }}"
       CHUNK_NUMBER: "${{ matrix.chunk }}"
       PARALLEL_PROCESSES: 5
+      BRANCH_NAME: ${{ github.head_ref || github.ref_name }}
 
     steps:
       - name: Set up PHP

diff --git a/bin/tests-github-actions.sh b/bin/tests-github-actions.sh
@@ -3,13 +3,15 @@
 set -eu
 
 function get_seeded_random() {
-  openssl enc -aes-256-ctr -pass pass:"vimeo/psalm" -nosalt </dev/zero 2>/dev/null
+  local -r branch_name="$1"
+  openssl enc -aes-256-ctr -pass pass:"$branch_name" -nosalt </dev/zero 2>/dev/null
 }
 
 function run {
   local -r chunk_count="$1"
   local -r chunk_number="$2"
   local -r parallel_processes="$3"
+  local -r branch_name="$4"
 
   local -r phpunit_cmd='
 echo "::group::{}";
@@ -23,7 +25,7 @@ exit "$exit_code"'
 
   mkdir -p build/parallel/ build/phpunit/logs/
 
-  find tests -name '*Test.php' | shuf --random-source=<(get_seeded_random) > build/tests_all
+  find tests -name '*Test.php' | shuf --random-source=<(get_seeded_random "$branch_name") > build/tests_all
   # split incorrectly splits the lines by byte size, which means that the number of tests per file are as evenly distributed as possible
   #split --number="l/$chunk_number/$chunk_count" build/tests_all > build/tests_split
   local -r lines=$(wc -l <build/tests_all)
@@ -47,5 +49,6 @@ exit "$exit_code"'
 if [ -z "${CHUNK_COUNT:-}" ]; then echo "Did not find env var CHUNK_COUNT."; exit 1; fi
 if [ -z "${CHUNK_NUMBER:-}" ]; then echo "Did not find env var CHUNK_NUMBER."; exit 1; fi
 if [ -z "${PARALLEL_PROCESSES:-}" ]; then echo "Did not find env var PARALLEL_PROCESSES."; exit 1; fi
+if [ -z "${BRANCH_NAME:-}" ]; then echo "Did not find env var BRANCH_NAME."; exit 1; fi
 
-run "$CHUNK_COUNT" "$CHUNK_NUMBER" "$PARALLEL_PROCESSES"
+run "$CHUNK_COUNT" "$CHUNK_NUMBER" "$PARALLEL_PROCESSES" "$BRANCH_NAME"
diff --git a/composer.json b/composer.json
@@ -31,7 +31,7 @@
         "dnoegel/php-xdg-base-dir": "^0.1.1",
         "felixfbecker/advanced-json-rpc": "^3.1",
         "felixfbecker/language-server-protocol": "^1.5.2",
-        "fidry/cpu-core-counter": "^0.4.1 || ^0.5.1",
+        "fidry/cpu-core-counter": "^0.4.1 || ^0.5.1 || ^1.0.0",
         "netresearch/jsonmapper": "^1.0 || ^2.0 || ^3.0 || ^4.0",
         "nikic/php-parser": "^4.16",
         "sebastian/diff": "^4.0 || ^5.0",
@@ -112,16 +112,16 @@
         "psalter"
     ],
     "scripts": {
-        "cs": "phpcs -ps",
-        "cs-fix": "phpcbf -ps",
-        "lint": "parallel-lint ./src ./tests",
+        "cs": "@php phpcs -ps",
+        "cs-fix": "@php phpcbf -ps",
+        "lint": "@php parallel-lint ./src ./tests",
         "phpunit": [
             "Composer\\Config::disableProcessTimeout",
             "paratest -f --runner=WrapperRunner"
         ],
         "phpunit-std": [
             "Composer\\Config::disableProcessTimeout",
-            "phpunit"
+            "@php phpunit"
         ],
         "verify-callmap": "@php phpunit tests/Internal/Codebase/InternalCallMapHandlerTest.php",
         "psalm": "@php ./psalm",

diff --git a/dictionaries/CallMap.php b/dictionaries/CallMap.php
@@ -1552,7 +1552,7 @@
 'decbin' => ['string', 'num'=>'int'],
 'dechex' => ['string', 'num'=>'int'],
 'decoct' => ['string', 'num'=>'int'],
-'define' => ['bool', 'constant_name'=>'string', 'value'=>'mixed', 'case_insensitive='=>'bool'],
+'define' => ['bool', 'constant_name'=>'string', 'value'=>'array|scalar|null', 'case_insensitive='=>'false'],
 'define_syslog_variables' => ['void'],
 'defined' => ['bool', 'constant_name'=>'string'],
 'deflate_add' => ['string|false', 'context'=>'DeflateContext', 'data'=>'string', 'flush_mode='=>'int'],
@@ -12923,8 +12923,8 @@
 'strrpos' => ['int|false', 'haystack'=>'string', 'needle'=>'string', 'offset='=>'int'],
 'strspn' => ['int', 'string'=>'string', 'characters'=>'string', 'offset='=>'int', 'length='=>'?int'],
 'strstr' => ['string|false', 'haystack'=>'string', 'needle'=>'string', 'before_needle='=>'bool'],
-'strtok' => ['string|false', 'string'=>'string', 'token'=>'string'],
-'strtok\'1' => ['string|false', 'string'=>'string'],
+'strtok' => ['non-empty-string|false', 'string'=>'string', 'token'=>'string'],
+'strtok\'1' => ['non-empty-string|false', 'string'=>'string'],
 'strtolower' => ['lowercase-string', 'string'=>'string'],
 'strtotime' => ['int|false', 'datetime'=>'string', 'baseTimestamp='=>'?int'],
 'strtoupper' => ['string', 'string'=>'string'],

diff --git a/dictionaries/CallMap_73_delta.php b/dictionaries/CallMap_73_delta.php
@@ -64,6 +64,10 @@
       'old' => ['int', 'scale'=>'int'],
       'new' => ['int', 'scale='=>'int'],
     ],
+    'define' => [
+        'old' => ['bool', 'constant_name'=>'string', 'value'=>'array|scalar|null', 'case_insensitive='=>'bool'],
+        'new' => ['bool', 'constant_name'=>'string', 'value'=>'array|scalar|null', 'case_insensitive='=>'false'],
+    ],
     'ldap_compare' => [
       'old' => ['bool|int', 'ldap'=>'resource', 'dn'=>'string', 'attribute'=>'string', 'value'=>'string'],
       'new' => ['bool|int', 'ldap'=>'resource', 'dn'=>'string', 'attribute'=>'string', 'value'=>'string', 'controls='=>'array'],

diff --git a/dictionaries/CallMap_historical.php b/dictionaries/CallMap_historical.php
@@ -9937,7 +9937,7 @@
     'decbin' => ['string', 'num'=>'int'],
     'dechex' => ['string', 'num'=>'int'],
     'decoct' => ['string', 'num'=>'int'],
-    'define' => ['bool', 'constant_name'=>'string', 'value'=>'mixed', 'case_insensitive='=>'bool'],
+    'define' => ['bool', 'constant_name'=>'string', 'value'=>'array|scalar|null', 'case_insensitive='=>'bool'],
     'define_syslog_variables' => ['void'],
     'defined' => ['bool', 'constant_name'=>'string'],
     'deflate_add' => ['string|false', 'context'=>'resource', 'data'=>'string', 'flush_mode='=>'int'],
@@ -14333,8 +14333,8 @@
     'strrpos' => ['int|false', 'haystack'=>'string', 'needle'=>'string|int', 'offset='=>'int'],
     'strspn' => ['int', 'string'=>'string', 'characters'=>'string', 'offset='=>'int', 'length='=>'int'],
     'strstr' => ['string|false', 'haystack'=>'string', 'needle'=>'string|int', 'before_needle='=>'bool'],
-    'strtok' => ['string|false', 'string'=>'string', 'token'=>'string'],
-    'strtok\'1' => ['string|false', 'string'=>'string'],
+    'strtok' => ['non-empty-string|false', 'string'=>'string', 'token'=>'string'],
+    'strtok\'1' => ['non-empty-string|false', 'string'=>'string'],
     'strtolower' => ['lowercase-string', 'string'=>'string'],
     'strtotime' => ['int|false', 'datetime'=>'string', 'baseTimestamp='=>'int'],
     'strtoupper' => ['string', 'string'=>'string'],

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -219,20 +219,27 @@
     </PossiblyUndefinedIntArrayOffset>
   </file>
   <file src="src/Psalm/Internal/Analyzer/Statements/UnusedAssignmentRemover.php">
+    <PossiblyUndefinedArrayOffset>
+      <code>$token_list[$iter]</code>
+    </PossiblyUndefinedArrayOffset>
     <PossiblyUndefinedIntArrayOffset>
       <code>$token_list[$iter]</code>
       <code>$token_list[$iter]</code>
       <code>$token_list[$iter]</code>
       <code>$token_list[$iter]</code>
       <code>$token_list[0]</code>
-      <code>$token_list[1]</code>
     </PossiblyUndefinedIntArrayOffset>
   </file>
   <file src="src/Psalm/Internal/Analyzer/StatementsAnalyzer.php">
     <PossiblyUndefinedArrayOffset>
       <code><![CDATA[$stmt->expr->getArgs()[0]]]></code>
     </PossiblyUndefinedArrayOffset>
   </file>
+  <file src="src/Psalm/Internal/Cli/Psalm.php">
+    <PossiblyInvalidArgument>
+      <code><![CDATA[$options['f'] ?? null]]></code>
+    </PossiblyInvalidArgument>
+  </file>
   <file src="src/Psalm/Internal/Cli/Refactor.php">
     <PossiblyUndefinedIntArrayOffset>
       <code>$identifier_name</code>
@@ -378,6 +385,32 @@
       <code>$cs[0]</code>
     </PossiblyUndefinedIntArrayOffset>
   </file>
+  <file src="src/Psalm/Internal/PluginManager/Command/DisableCommand.php">
+    <RedundantCondition>
+      <code>$config_file_path !== null</code>
+    </RedundantCondition>
+    <ReservedWord>
+      <code><![CDATA[$input->getArgument('pluginName')]]></code>
+      <code><![CDATA[$input->getOption('config')]]></code>
+    </ReservedWord>
+  </file>
+  <file src="src/Psalm/Internal/PluginManager/Command/EnableCommand.php">
+    <RedundantCondition>
+      <code>$config_file_path !== null</code>
+    </RedundantCondition>
+    <ReservedWord>
+      <code><![CDATA[$input->getArgument('pluginName')]]></code>
+      <code><![CDATA[$input->getOption('config')]]></code>
+    </ReservedWord>
+  </file>
+  <file src="src/Psalm/Internal/PluginManager/Command/ShowCommand.php">
+    <RedundantCondition>
+      <code>$config_file_path !== null</code>
+    </RedundantCondition>
+    <ReservedWord>
+      <code><![CDATA[$input->getOption('config')]]></code>
+    </ReservedWord>
+  </file>
   <file src="src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayMapReturnTypeProvider.php">
     <PossiblyUndefinedIntArrayOffset>
       <code>$callable_method_name</code>

diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php
@@ -1160,7 +1160,7 @@ private static function fromXmlAndPaths(
                 }
 
                 // we need an absolute path for checks
-                if ($path[0] !== '/' && DIRECTORY_SEPARATOR === '/') {
+                if (Path::isRelative($path)) {
                     $prospective_path = $base_dir . DIRECTORY_SEPARATOR . $path;
                 } else {
                     $prospective_path = $path;

diff --git a/src/Psalm/Config/FileFilter.php b/src/Psalm/Config/FileFilter.php
@@ -9,6 +9,7 @@
 use RecursiveDirectoryIterator;
 use RecursiveIteratorIterator;
 use SimpleXMLElement;
+use Symfony\Component\Filesystem\Path;
 
 use function array_filter;
 use function array_map;
@@ -127,7 +128,7 @@ public static function loadFromArray(
                 $resolve_symlinks = (bool) ($directory['resolveSymlinks'] ?? false);
                 $declare_strict_types = (bool) ($directory['useStrictTypes'] ?? false);
 
-                if ($directory_path[0] === '/' && DIRECTORY_SEPARATOR === '/') {
+                if (Path::isAbsolute($directory_path)) {
                     /** @var non-empty-string */
                     $prospective_directory_path = $directory_path;
                 } else {
@@ -250,7 +251,7 @@ public static function loadFromArray(
             foreach ($config['file'] as $file) {
                 $file_path = (string) ($file['name'] ?? '');
 
-                if ($file_path[0] === '/' && DIRECTORY_SEPARATOR === '/') {
+                if (Path::isAbsolute($file_path)) {
                     /** @var non-empty-string */
                     $prospective_file_path = $file_path;
                 } else {

diff --git a/src/Psalm/Internal/Analyzer/ClosureAnalyzer.php b/src/Psalm/Internal/Analyzer/ClosureAnalyzer.php
@@ -15,7 +15,6 @@
 use Psalm\Issue\UndefinedVariable;
 use Psalm\IssueBuffer;
 use Psalm\Type;
-use Psalm\Type\Atomic\TMixed;
 use Psalm\Type\Atomic\TNamedObject;
 use Psalm\Type\Union;
 
@@ -135,12 +134,6 @@ public static function analyzeExpression(
 
                 $use_var_id = '$' . $use->var->name;
 
-                // insert the ref into the current context if passed by ref, as whatever we're passing
-                // the closure to could execute it straight away.
-                if ($use->byRef && !$context->hasVariable($use_var_id)) {
-                    $context->vars_in_scope[$use_var_id] = new Union([new TMixed()], ['by_ref' => true]);
-                }
-
                 if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph
                     && $context->hasVariable($use_var_id)
                 ) {
@@ -156,7 +149,7 @@ public static function analyzeExpression(
                 }
 
                 $use_context->vars_in_scope[$use_var_id] =
-                    $context->hasVariable($use_var_id) && !$use->byRef
+                    $context->hasVariable($use_var_id)
                     ? $context->vars_in_scope[$use_var_id]
                     : Type::getMixed();
 
@@ -207,7 +200,12 @@ public static function analyzeExpression(
         $use_context->calling_method_id = $context->calling_method_id;
         $use_context->phantom_classes = $context->phantom_classes;
 
-        $closure_analyzer->analyze($use_context, $statements_analyzer->node_data, $context, false);
+        $byref_vars = [];
+        $closure_analyzer->analyze($use_context, $statements_analyzer->node_data, $context, false, $byref_vars);
+
+        foreach ($byref_vars as $key => $value) {
+            $context->vars_in_scope[$key] = $value;
+        }
 
         if ($closure_analyzer->inferred_impure
             && $statements_analyzer->getSource() instanceof FunctionLikeAnalyzer
@@ -231,7 +229,7 @@ public static function analyzeExpression(
     /**
      * @return  false|null
      */
-    public static function analyzeClosureUses(
+    private static function analyzeClosureUses(
         StatementsAnalyzer $statements_analyzer,
         PhpParser\Node\Expr\Closure $stmt,
         Context $context,
@@ -270,21 +268,6 @@ public static function analyzeClosureUses(
                     continue;
                 }
 
-                if ($use->byRef) {
-                    $context->vars_in_scope[$use_var_id] = Type::getMixed();
-                    $context->vars_possibly_in_scope[$use_var_id] = true;
-
-                    if (!$statements_analyzer->hasVariable($use_var_id)) {
-                        $statements_analyzer->registerVariable(
-                            $use_var_id,
-                            new CodeLocation($statements_analyzer, $use->var),
-                            null,
-                        );
-                    }
-
-                    return null;
-                }
-
                 if (!isset($context->vars_possibly_in_scope[$use_var_id])) {
                     if ($context->check_variables) {
                         if (IssueBuffer::accepts(
@@ -331,14 +314,6 @@ public static function analyzeClosureUses(
 
                     continue;
                 }
-            } elseif ($use->byRef) {
-                $new_type = new Union([new TMixed()], [
-                    'parent_nodes' => $context->vars_in_scope[$use_var_id]->parent_nodes,
-                ]);
-
-                $context->remove($use_var_id);
-
-                $context->vars_in_scope[$use_var_id] = $new_type;
             }
         }
 

diff --git a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
@@ -47,6 +47,8 @@
 use Psalm\Issue\UnusedDocblockParam;
 use Psalm\Issue\UnusedParam;
 use Psalm\IssueBuffer;
+use Psalm\Node\Expr\VirtualVariable;
+use Psalm\Node\Stmt\VirtualWhile;
 use Psalm\Plugin\EventHandler\Event\AfterFunctionLikeAnalysisEvent;
 use Psalm\Storage\ClassLikeStorage;
 use Psalm\Storage\FunctionLikeParameter;
@@ -151,14 +153,18 @@ public function __construct($function, SourceAnalyzer $source, FunctionLikeStora
 
     /**
      * @param bool          $add_mutations  whether or not to add mutations to this method
+     * @param array<string, Union> $byref_vars
+     * @param-out array<string, Union> $byref_vars
      * @return false|null
      * @psalm-suppress PossiblyUnusedReturnValue unused but seems important
+     * @psalm-suppress ComplexMethod Unavoidably complex
      */
     public function analyze(
         Context $context,
         NodeDataProvider $type_provider,
         ?Context $global_context = null,
         bool $add_mutations = false,
+        array &$byref_vars = []
     ): ?bool {
         $storage = $this->storage;
 
@@ -188,7 +194,13 @@ public function analyze(
                 || !in_array("UnusedPsalmSuppress", $storage->suppressed_issues)
             ) {
                 foreach ($storage->suppressed_issues as $offset => $issue_name) {
-                    IssueBuffer::addUnusedSuppression($this->getFilePath(), $offset, $issue_name);
+                    IssueBuffer::addUnusedSuppression(
+                        $storage->location !== null
+                            ? $storage->location->file_path
+                            : $this->getFilePath(),
+                        $offset,
+                        $issue_name,
+                    );
                 }
             }
         }
@@ -231,9 +243,8 @@ public function analyze(
 
         $statements_analyzer = new StatementsAnalyzer($this, $type_provider);
 
+        $byref_uses = [];
         if ($this instanceof ClosureAnalyzer && $this->function instanceof Closure) {
-            $byref_uses = [];
-
             foreach ($this->function->uses as $use) {
                 if (!is_string($use->var->name)) {
                     continue;
@@ -348,6 +359,31 @@ public function analyze(
             (bool) $template_types,
         );
 
+        if ($byref_uses) {
+            $ref_context = clone $context;
+            $var = '$__tmp_byref_closure_if__' . (int) $this->function->getAttribute('startFilePos');
+
+            $ref_context->vars_in_scope[$var] = Type::getBool();
+
+            $var = new VirtualVariable(
+                substr($var, 1),
+            );
+            $virtual_while = new VirtualWhile(
+                $var,
+                $function_stmts,
+            );
+
+            $statements_analyzer->analyze(
+                [$virtual_while],
+                $ref_context,
+            );
+
+            foreach ($byref_uses as $var_id => $_) {
+                $byref_vars[$var_id] = $ref_context->vars_in_scope[$var_id];
+                $context->vars_in_scope[$var_id] = $ref_context->vars_in_scope[$var_id];
+            }
+        }
+
         if ($storage->pure) {
             $context->pure = true;
         }