Merge branch 'merge_5.x'

nicelocal · Dec 19, 2023 · f1733c3 · f1733c3
2 parents 337c7af + 9b82514
commit f1733c3
Show file tree

Hide file tree

Showing 85 changed files with 1,295 additions and 496 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/UPGRADING.md b/UPGRADING.md
@@ -17,7 +17,7 @@
 
 - [BC] Class `Psalm\Issue\MixedInferredReturnType` was removed
 
-- [BC] Value of constant `Psalm\Type\TaintKindGroup::ALL_INPUT` changed to reflect new `TaintKind::INPUT_SLEEP` and `TaintKind::INPUT_XPATH` have been added. Accordingly, default values for `$taint` parameters of `Psalm\Codebase::addTaintSource()` and `Psalm\Codebase::addTaintSink()` have been changed as well.
+- [BC] Value of constant `Psalm\Type\TaintKindGroup::ALL_INPUT` changed to reflect new `TaintKind::INPUT_EXTRACT`, `TaintKind::INPUT_SLEEP` and `TaintKind::INPUT_XPATH` have been added. Accordingly, default values for `$taint` parameters of `Psalm\Codebase::addTaintSource()` and `Psalm\Codebase::addTaintSink()` have been changed as well.
 
 - [BC] Property `Config::$shepherd_host` was replaced with `Config::$shepherd_endpoint`
 

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/config.xsd b/config.xsd
@@ -433,6 +433,7 @@
             <xs:element name="TaintedCookie" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedCustom" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedEval" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="TaintedExtract" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedFile" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedHeader" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedHtml" type="IssueHandlerType" minOccurs="0" />

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/docs/annotating_code/supported_annotations.md b/docs/annotating_code/supported_annotations.md
@@ -263,9 +263,9 @@ is not within the given namespace.
 <?php
 namespace A\B {
     /**
-    * @internal
-    * @psalm-internal A\B
-    */
+     * @internal
+     * @psalm-internal A\B
+     */
     class Foo { }
 }
 

diff --git a/docs/running_psalm/error_levels.md b/docs/running_psalm/error_levels.md
@@ -286,6 +286,7 @@ Level 5 and above allows a more non-verifiable code, and higher levels are even
  - [TaintedCookie](issues/TaintedCookie.md)
  - [TaintedCustom](issues/TaintedCustom.md)
  - [TaintedEval](issues/TaintedEval.md)
+ - [TaintedExtract](issues/TaintedExtract.md)
  - [TaintedFile](issues/TaintedFile.md)
  - [TaintedHeader](issues/TaintedHeader.md)
  - [TaintedHtml](issues/TaintedHtml.md)

diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md
@@ -234,6 +234,7 @@
  - [TaintedCookie](issues/TaintedCookie.md)
  - [TaintedCustom](issues/TaintedCustom.md)
  - [TaintedEval](issues/TaintedEval.md)
+ - [TaintedExtract](issues/TaintedExtract.md)
  - [TaintedFile](issues/TaintedFile.md)
  - [TaintedHeader](issues/TaintedHeader.md)
  - [TaintedHtml](issues/TaintedHtml.md)

diff --git a/docs/running_psalm/issues/TaintedExtract.md b/docs/running_psalm/issues/TaintedExtract.md
@@ -0,0 +1,10 @@
+# TaintedExtract
+
+Emitted when user-controlled array can be passed into an `extract` call.
+
+```php
+<?php
+
+$array = $_GET;
+extract($array);
+```
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -224,20 +224,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>
@@ -396,6 +403,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;
@@ -125,7 +126,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 {
@@ -248,7 +249,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;
             }
         }