From 69ab17331bb3a740683c4507c05de3604785c637 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Wed, 21 May 2025 15:20:17 +0200 Subject: [PATCH 1/8] Add some extensions --- composer.json | 16 +- extension.neon | 5 + phpcs.xml.dist | 10 ++ phpstan.neon.dist | 26 ++- src/Context/FeatureContext.php | 2 + src/Context/GivenStepDefinitions.php | 2 +- src/Context/ThenStepDefinitions.php | 1 + ...eUrlFunctionDynamicReturnTypeExtension.php | 169 ++++++++++++++++++ tests/data/parse_url.php | 53 ++++++ .../TestDynamicReturnTypeExtension.php | 28 +++ tests/tests/TestBehatTags.php | 2 + 11 files changed, 309 insertions(+), 5 deletions(-) create mode 100644 extension.neon create mode 100644 src/PHPStan/ParseUrlFunctionDynamicReturnTypeExtension.php create mode 100644 tests/data/parse_url.php create mode 100644 tests/tests/PHPStan/TestDynamicReturnTypeExtension.php diff --git a/composer.json b/composer.json index 8e678002b..3d9d42e58 100644 --- a/composer.json +++ b/composer.json @@ -15,8 +15,12 @@ "php-parallel-lint/php-console-highlighter": "^1.0", "php-parallel-lint/php-parallel-lint": "^1.3.1", "phpcompatibility/php-compatibility": "dev-develop", - "phpstan/extension-installer": "^1.4.3", + "phpstan/extension-installer": "^1.4", "phpstan/phpstan": "^1.12.26", + "phpstan/phpstan-deprecation-rules": "^1.2", + "phpstan/phpstan-phpunit": "^1.4", + "phpstan/phpstan-strict-rules": "^1.6", + "swissspidy/phpstan-no-private": "^0.2.1", "szepeviktor/phpstan-wordpress": "^v1.3.5", "wp-cli/config-command": "^1 || ^2", "wp-cli/core-command": "^1 || ^2", @@ -41,6 +45,11 @@ "branch-alias": { "dev-main": "4.0.x-dev" }, + "phpstan": { + "includes": [ + "extension.neon" + ] + }, "readme": { "sections": [ "Using", @@ -58,6 +67,11 @@ "WP_CLI\\Tests\\": "src" } }, + "autoload-dev": { + "psr-4": { + "WP_CLI\\Tests\\Tests\\": "tests/tests" + } + }, "minimum-stability": "dev", "prefer-stable": true, "bin": [ diff --git a/extension.neon b/extension.neon new file mode 100644 index 000000000..048ae33d3 --- /dev/null +++ b/extension.neon @@ -0,0 +1,5 @@ +services: + - + class: WP_CLI\Tests\PHPStan\ParseUrlFunctionDynamicReturnTypeExtension + tags: + - phpstan.broker.dynamicFunctionReturnTypeExtension diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 24888b4b8..44fb31f0d 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -61,6 +61,16 @@ tests/phpstan/scan-files.php + + tests/data/* + src/PHPStan/* + tests/tests/PHPStan/* + + + + tests/data/* + + diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 01b2d732d..40016adc3 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -1,8 +1,12 @@ +includes: + - extension.neon parameters: level: 6 paths: - src - tests + excludePaths: + - tests/data scanDirectories: - vendor/wp-cli/wp-cli - vendor/phpunit/php-code-coverage @@ -11,9 +15,25 @@ parameters: - tests/phpstan/scan-files.php treatPhpDocTypesAsCertain: false dynamicConstantNames: - - WP_DEBUG - - WP_DEBUG_LOG - - WP_DEBUG_DISPLAY + - WP_DEBUG + - WP_DEBUG_LOG + - WP_DEBUG_DISPLAY ignoreErrors: # Needs fixing in WP-CLI. - message: '#Parameter \#1 \$cmd of function WP_CLI\\Utils\\esc_cmd expects array#' + - message: '#Dynamic call to static method#' + path: 'tests/tests' + strictRules: + disallowedLooseComparison: false + booleansInConditions: false + uselessCast: false + requireParentConstructorCall: false + disallowedConstructs: false + overwriteVariablesWithLoop: false + closureUsesThis: false + matchingInheritedMethodNames: false + numericOperandsInArithmeticOperators: false + strictCalls: true + switchConditionsMatchingType: false + noVariableVariables: false + strictArrayFilter: false diff --git a/src/Context/FeatureContext.php b/src/Context/FeatureContext.php index 150f0d30b..85ced3379 100644 --- a/src/Context/FeatureContext.php +++ b/src/Context/FeatureContext.php @@ -29,6 +29,8 @@ /** * Features context. + * + * @phpstan-ignore class.implementsDeprecatedInterface */ class FeatureContext implements SnippetAcceptingContext { diff --git a/src/Context/GivenStepDefinitions.php b/src/Context/GivenStepDefinitions.php index 3df2d5324..670ae5ba3 100644 --- a/src/Context/GivenStepDefinitions.php +++ b/src/Context/GivenStepDefinitions.php @@ -66,7 +66,7 @@ public function given_a_specific_directory( $empty_or_nonexistent, $dir ): void ); } - $this->remove_dir( $dir ); + self::remove_dir( $dir ); if ( 'empty' === $empty_or_nonexistent ) { mkdir( $dir, 0777, true /*recursive*/ ); } diff --git a/src/Context/ThenStepDefinitions.php b/src/Context/ThenStepDefinitions.php index 9ab0477f9..9dc877b8e 100644 --- a/src/Context/ThenStepDefinitions.php +++ b/src/Context/ThenStepDefinitions.php @@ -566,6 +566,7 @@ public function then_an_email_should_be_sent( $expected ): void { * @param int $return_code Expected HTTP status code. */ public function then_the_http_status_code_should_be( $return_code ): void { + // @phpstan-ignore staticMethod.deprecatedClass $response = Requests::request( 'http://localhost:8080' ); $this->assert_equals( $return_code, $response->status_code ); } diff --git a/src/PHPStan/ParseUrlFunctionDynamicReturnTypeExtension.php b/src/PHPStan/ParseUrlFunctionDynamicReturnTypeExtension.php new file mode 100644 index 000000000..55fef8974 --- /dev/null +++ b/src/PHPStan/ParseUrlFunctionDynamicReturnTypeExtension.php @@ -0,0 +1,169 @@ +|null */ + private $componentTypesPairedConstants = null; + + /** @var array|null */ + private $componentTypesPairedStrings = null; + + /** @var \PHPStan\Type\Type|null */ + private $allComponentsTogetherType = null; + + public function isFunctionSupported( FunctionReflection $functionReflection ): bool { + return $functionReflection->getName() === 'WP_CLI\Utils\parse_url'; + } + + public function getTypeFromFunctionCall( FunctionReflection $functionReflection, FuncCall $functionCall, Scope $scope ): ?Type { + if ( count( $functionCall->getArgs() ) < 1 ) { + return null; + } + + $this->cacheReturnTypes(); + + $componentType = new ConstantIntegerType( -1 ); + + if ( count( $functionCall->getArgs() ) > 1 ) { + $componentType = $scope->getType( $functionCall->getArgs()[1]->value ); + + if ( ! $componentType->isConstantValue()->yes() ) { + return $this->createAllComponentsReturnType(); + } + + $componentType = $componentType->toInteger(); + + if ( ! $componentType instanceof ConstantIntegerType ) { + return $this->createAllComponentsReturnType(); + } + } + + $urlType = $scope->getType( $functionCall->getArgs()[0]->value ); + if ( count( $urlType->getConstantStrings() ) > 0 ) { + $types = []; + foreach ( $urlType->getConstantStrings() as $constantString ) { + try { + // phpcs:ignore Generic.PHP.NoSilencedErrors.Discouraged + $result = @parse_url( $constantString->getValue(), $componentType->getValue() ); + } catch ( \Error $e ) { + $types[] = new ConstantBooleanType( false ); + continue; + } + + $types[] = $scope->getTypeFromValue( $result ); + } + + return TypeCombinator::union( ...$types ); + } + + if ( $componentType->getValue() === -1 ) { + return TypeCombinator::union( $this->createComponentsArray(), new ConstantBooleanType( false ) ); + } + + return $this->componentTypesPairedConstants[ $componentType->getValue() ] ?? new ConstantBooleanType( false ); + } + + private function createAllComponentsReturnType(): Type { + if ( null === $this->allComponentsTogetherType ) { + $returnTypes = [ + new ConstantBooleanType( false ), + new NullType(), + IntegerRangeType::fromInterval( 0, 65535 ), + new StringType(), + $this->createComponentsArray(), + ]; + + $this->allComponentsTogetherType = TypeCombinator::union( ...$returnTypes ); + } + + return $this->allComponentsTogetherType; + } + + private function createComponentsArray(): Type { + $builder = ConstantArrayTypeBuilder::createEmpty(); + + if ( null === $this->componentTypesPairedStrings ) { + throw new \PHPStan\ShouldNotHappenException(); + } + + foreach ( $this->componentTypesPairedStrings as $componentName => $componentValueType ) { + $builder->setOffsetValueType( new ConstantStringType( $componentName ), $componentValueType, true ); + } + + return $builder->getArray(); + } + + private function cacheReturnTypes(): void { + if ( null !== $this->componentTypesPairedConstants ) { + return; + } + + $stringType = new StringType(); + $port = IntegerRangeType::fromInterval( 0, 65535 ); + $falseType = new ConstantBooleanType( false ); + $nullType = new NullType(); + + $stringOrFalseOrNull = TypeCombinator::union( $stringType, $falseType, $nullType ); + $portOrFalseOrNull = TypeCombinator::union( $port, $falseType, $nullType ); + + $this->componentTypesPairedConstants = [ + PHP_URL_SCHEME => $stringOrFalseOrNull, + PHP_URL_HOST => $stringOrFalseOrNull, + PHP_URL_PORT => $portOrFalseOrNull, + PHP_URL_USER => $stringOrFalseOrNull, + PHP_URL_PASS => $stringOrFalseOrNull, + PHP_URL_PATH => $stringOrFalseOrNull, + PHP_URL_QUERY => $stringOrFalseOrNull, + PHP_URL_FRAGMENT => $stringOrFalseOrNull, + ]; + + $this->componentTypesPairedStrings = [ + 'scheme' => $stringType, + 'host' => $stringType, + 'port' => $port, + 'user' => $stringType, + 'pass' => $stringType, + 'path' => $stringType, + 'query' => $stringType, + 'fragment' => $stringType, + ]; + } +} diff --git a/tests/data/parse_url.php b/tests/data/parse_url.php new file mode 100644 index 000000000..dd5ecaded --- /dev/null +++ b/tests/data/parse_url.php @@ -0,0 +1,53 @@ +, user?: string, pass?: string, path?: string, query?: string, fragment?: string}|int<0, 65535>|string|false|null', $value ); + +$value = parse_url( 'http://def.abc', PHP_URL_FRAGMENT ); +assertType( 'null', $value ); + +$value = parse_url( 'http://def.abc#this-is-fragment', PHP_URL_FRAGMENT ); +assertType( "'this-is-fragment'", $value ); + +$value = parse_url( 'http://def.abc#this-is-fragment', 9999 ); +assertType( 'false', $value ); + +$value = parse_url( $string, 9999 ); +assertType( 'false', $value ); + +$value = parse_url( $string, PHP_URL_PORT ); +assertType( 'int<0, 65535>|false|null', $value ); + +$value = parse_url( $string ); +assertType( 'array{scheme?: string, host?: string, port?: int<0, 65535>, user?: string, pass?: string, path?: string, query?: string, fragment?: string}|false', $value ); + +/** @var 'http://def.abc'|'https://example.com' $union */ +$union = $union; +assertType( "array{scheme: 'http', host: 'def.abc'}|array{scheme: 'https', host: 'example.com'}", parse_url( $union ) ); + +/** @var 'http://def.abc#fragment1'|'https://example.com#fragment2' $union */ +$union = $union; +assertType( "'fragment1'|'fragment2'", parse_url( $union, PHP_URL_FRAGMENT ) ); diff --git a/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php b/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php new file mode 100644 index 000000000..97e40e41f --- /dev/null +++ b/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php @@ -0,0 +1,28 @@ + + */ + public function dataFileAsserts(): iterable { + // Path to a file with actual asserts of expected types: + yield from self::gatherAssertTypes( dirname( __DIR__, 2 ) . '/data/parse_url.php' ); + } + + /** + * @dataProvider dataFileAsserts + * @param array ...$args + */ + public function testFileAsserts( string $assertType, string $file, ...$args ): void { + $this->assertFileAsserts( $assertType, $file, ...$args ); + } + + public static function getAdditionalConfigFiles(): array { + return [ dirname( __DIR__, 3 ) . '/extension.neon' ]; + } +} diff --git a/tests/tests/TestBehatTags.php b/tests/tests/TestBehatTags.php index f9c435df6..b35fa511d 100644 --- a/tests/tests/TestBehatTags.php +++ b/tests/tests/TestBehatTags.php @@ -1,5 +1,7 @@ Date: Wed, 21 May 2025 17:53:54 +0200 Subject: [PATCH 2/8] Add extension for `get_flag_value` --- extension.neon | 4 + ...alueFunctionDynamicReturnTypeExtension.php | 112 ++++++++++++++++++ tests/data/get_flag_value.php | 62 ++++++++++ .../TestDynamicReturnTypeExtension.php | 1 + 4 files changed, 179 insertions(+) create mode 100644 src/PHPStan/GetFlagValueFunctionDynamicReturnTypeExtension.php create mode 100644 tests/data/get_flag_value.php diff --git a/extension.neon b/extension.neon index 048ae33d3..8c4d6913c 100644 --- a/extension.neon +++ b/extension.neon @@ -3,3 +3,7 @@ services: class: WP_CLI\Tests\PHPStan\ParseUrlFunctionDynamicReturnTypeExtension tags: - phpstan.broker.dynamicFunctionReturnTypeExtension + - + class: WP_CLI\Tests\PHPStan\GetFlagValueFunctionDynamicReturnTypeExtension + tags: + - phpstan.broker.dynamicFunctionReturnTypeExtension diff --git a/src/PHPStan/GetFlagValueFunctionDynamicReturnTypeExtension.php b/src/PHPStan/GetFlagValueFunctionDynamicReturnTypeExtension.php new file mode 100644 index 000000000..9eb93551f --- /dev/null +++ b/src/PHPStan/GetFlagValueFunctionDynamicReturnTypeExtension.php @@ -0,0 +1,112 @@ +getName() === 'WP_CLI\Utils\get_flag_value'; + } + + public function getTypeFromFunctionCall( + FunctionReflection $functionReflection, + FuncCall $functionCall, + Scope $scope + ): Type { + $args = $functionCall->getArgs(); + + // Ensure we have at least two arguments: $assoc_args and $flag + if ( count( $args ) < 2 ) { + // Not enough arguments, fall back to the function's declared return type or mixed + return $functionReflection->getVariants()[0]->getReturnType(); + } + + $assocArgsType = $scope->getType( $args[0]->value ); + $flagArgType = $scope->getType( $args[1]->value ); + + // Determine the default type + $defaultType = isset( $args[2] ) ? $scope->getType( $args[2]->value ) : new \PHPStan\Type\NullType(); + + // We can only be precise if $flag is a constant string + if ( ! $flagArgType->isConstantValue()->yes() || ( ! $flagArgType->toInteger() instanceof ConstantIntegerType && ! $flagArgType->toString() instanceof ConstantStringType ) ) { + // If $flag is not a constant string, we cannot know which key to check. + // The return type will be a union of the array's possible value types and the default type. + if ( $assocArgsType instanceof ConstantArrayType ) { + $valueTypes = []; + foreach ( $assocArgsType->getValueTypes() as $valueType ) { + $valueTypes[] = $valueType; + } + if ( count( $valueTypes ) > 0 ) { + return TypeCombinator::union( ...$valueTypes ); + } + return $defaultType; // Array is empty or has no predictable value types + } elseif ( $assocArgsType instanceof \PHPStan\Type\ArrayType ) { + return TypeCombinator::union( $assocArgsType->getItemType(), $defaultType ); + } + // Fallback if $assocArgsType isn't a well-defined array type + return new MixedType(); + } + + $flagValue = $flagArgType->getValue(); + + // If $assoc_args is a constant array, we can check if the key exists + if ( $assocArgsType->isConstantValue()->yes() && $assocArgsType->toArray() instanceof ConstantArrayType ) { + $keyTypes = $assocArgsType->getKeyTypes(); + $valueTypes = $assocArgsType->getValueTypes(); + $resolvedValueType = null; + + foreach ( $keyTypes as $index => $keyType ) { + if ( $keyType->isConstantValue()->yes() && $keyType->toString() instanceof ConstantStringType && $keyType->getValue() === $flagValue ) { + $resolvedValueType = $valueTypes[ $index ]; + break; + } + } + + if ( null !== $resolvedValueType ) { + // Key definitely exists, return its type + return $resolvedValueType; + } else { + // Key definitely does not exist, return default type + return $defaultType; + } + } + + // If $assocArgsType is a general ArrayType, we can't be sure if the specific flag exists. + // The function's logic is: isset( $assoc_args[ $flag ] ) ? $assoc_args[ $flag ] : $default; + // So, it's a union of the potential value type from the array and the default type. + if ( $assocArgsType->isArray()->yes() ) { + // We don't know IF the key $flagValue exists. + // PHPStan's ArrayType has an itemType which represents the type of values in the array. + // This is the best we can do for a generic array. + return TypeCombinator::union( $assocArgsType->getItemType(), $defaultType ); + } + + // Fallback for other types of $assocArgsType or if we can't determine. + // This should ideally be the union of what the array could contain for that key and the default. + // For simplicity, if not a ConstantArrayType or ArrayType, return mixed or a broad union. + // In a real-world scenario with more complex types, you might query $assocArgsType->getOffsetValueType(new ConstantStringType($flagValue)) + // and then union with $defaultType. + $offsetValueType = $assocArgsType->getOffsetValueType( new ConstantStringType( $flagValue ) ); + if ( ! $offsetValueType instanceof MixedType || $offsetValueType->isExplicitMixed() ) { + return TypeCombinator::union( $offsetValueType, $defaultType ); + } + + return new MixedType(); // Default fallback + } +} diff --git a/tests/data/get_flag_value.php b/tests/data/get_flag_value.php new file mode 100644 index 000000000..e2f2dcffc --- /dev/null +++ b/tests/data/get_flag_value.php @@ -0,0 +1,62 @@ + 'bar', + 'baz' => 'qux', + ], + 'foo' +); +assertType( "'bar'", $value ); + +$value = get_flag_value( + [ + 'foo' => 'bar', + 'baz' => 'qux', + ], + 'bar' +); +assertType( 'null', $value ); + +$value = get_flag_value( + [ + 'foo' => 'bar', + 'baz' => 'qux', + ], + 'bar', + 123 +); +assertType( '123', $value ); + +$value = get_flag_value( + [ + 'foo' => 'bar', + 'baz' => true, + ], + 'baz', + 123 +); +assertType( 'true', $value ); + +$assoc_args = [ + 'foo' => 'bar', + 'baz' => true, +]; +$key = 'baz'; + +$value = get_flag_value( $assoc_args, $key, 123 ); +assertType( 'true', $value ); + +$value = get_flag_value( $assoc_args, $key2, 123 ); +assertType( "'bar'|true", $value ); diff --git a/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php b/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php index 97e40e41f..0f97dca62 100644 --- a/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php +++ b/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php @@ -12,6 +12,7 @@ class TestDynamicReturnTypeExtension extends \PHPStan\Testing\TypeInferenceTestC public function dataFileAsserts(): iterable { // Path to a file with actual asserts of expected types: yield from self::gatherAssertTypes( dirname( __DIR__, 2 ) . '/data/parse_url.php' ); + yield from self::gatherAssertTypes( dirname( __DIR__, 2 ) . '/data/get_flag_value.php' ); } /** From da3218d19217c2733ef07f60586f83ed7baf2523 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 22 May 2025 11:07:23 +0200 Subject: [PATCH 3/8] Revamp extensions --- extension.neon | 4 + phpcs.xml.dist | 4 + ...alueFunctionDynamicReturnTypeExtension.php | 102 +++++----- ...liRuncommandDynamicReturnTypeExtension.php | 192 ++++++++++++++++++ tests/data/get_flag_value.php | 2 +- tests/data/runcommand.php | 67 ++++++ .../TestDynamicReturnTypeExtension.php | 1 + 7 files changed, 321 insertions(+), 51 deletions(-) create mode 100644 src/PHPStan/WPCliRuncommandDynamicReturnTypeExtension.php create mode 100644 tests/data/runcommand.php diff --git a/extension.neon b/extension.neon index 8c4d6913c..438da3b3f 100644 --- a/extension.neon +++ b/extension.neon @@ -7,3 +7,7 @@ services: class: WP_CLI\Tests\PHPStan\GetFlagValueFunctionDynamicReturnTypeExtension tags: - phpstan.broker.dynamicFunctionReturnTypeExtension + - + class: WP_CLI\Tests\PHPStan\WPCliRuncommandDynamicReturnTypeExtension + tags: + - phpstan.broker.dynamicStaticMethodReturnTypeExtension diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 44fb31f0d..a0ff88646 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -71,6 +71,10 @@ tests/data/* + + src/PHPStan/* + + diff --git a/src/PHPStan/GetFlagValueFunctionDynamicReturnTypeExtension.php b/src/PHPStan/GetFlagValueFunctionDynamicReturnTypeExtension.php index 9eb93551f..57941e980 100644 --- a/src/PHPStan/GetFlagValueFunctionDynamicReturnTypeExtension.php +++ b/src/PHPStan/GetFlagValueFunctionDynamicReturnTypeExtension.php @@ -12,8 +12,9 @@ use PHPStan\Analyser\Scope; use PHPStan\Reflection\FunctionReflection; use PHPStan\Type\Constant\ConstantStringType; -use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\Type; +use PHPStan\Type\MixedType; +use PHPStan\Type\NullType; use PHPStan\Type\TypeCombinator; use function count; @@ -31,82 +32,83 @@ public function getTypeFromFunctionCall( ): Type { $args = $functionCall->getArgs(); - // Ensure we have at least two arguments: $assoc_args and $flag if ( count( $args ) < 2 ) { - // Not enough arguments, fall back to the function's declared return type or mixed + // Not enough arguments, fall back to the function's declared return type. return $functionReflection->getVariants()[0]->getReturnType(); } $assocArgsType = $scope->getType( $args[0]->value ); $flagArgType = $scope->getType( $args[1]->value ); - // Determine the default type - $defaultType = isset( $args[2] ) ? $scope->getType( $args[2]->value ) : new \PHPStan\Type\NullType(); - - // We can only be precise if $flag is a constant string - if ( ! $flagArgType->isConstantValue()->yes() || ( ! $flagArgType->toInteger() instanceof ConstantIntegerType && ! $flagArgType->toString() instanceof ConstantStringType ) ) { - // If $flag is not a constant string, we cannot know which key to check. - // The return type will be a union of the array's possible value types and the default type. - if ( $assocArgsType instanceof ConstantArrayType ) { - $valueTypes = []; - foreach ( $assocArgsType->getValueTypes() as $valueType ) { - $valueTypes[] = $valueType; - } - if ( count( $valueTypes ) > 0 ) { - return TypeCombinator::union( ...$valueTypes ); - } - return $defaultType; // Array is empty or has no predictable value types - } elseif ( $assocArgsType instanceof \PHPStan\Type\ArrayType ) { - return TypeCombinator::union( $assocArgsType->getItemType(), $defaultType ); - } - // Fallback if $assocArgsType isn't a well-defined array type - return new MixedType(); + // 2. Determine the default type + $defaultType = isset( $args[2] ) ? $scope->getType( $args[2]->value ) : new NullType(); + + $flagConstantStrings = $flagArgType->getConstantStrings(); + + if ( count( $flagConstantStrings ) !== 1 ) { + // Flag name is dynamic or not a string. + // Return type is a union of all possible values in $assoc_args + default type. + return $this->getDynamicFlagFallbackType( $assocArgsType, $defaultType ); } - $flagValue = $flagArgType->getValue(); + // 4. Flag is a single constant string. + $flagValue = $flagConstantStrings[0]->getValue(); - // If $assoc_args is a constant array, we can check if the key exists - if ( $assocArgsType->isConstantValue()->yes() && $assocArgsType->toArray() instanceof ConstantArrayType ) { - $keyTypes = $assocArgsType->getKeyTypes(); - $valueTypes = $assocArgsType->getValueTypes(); - $resolvedValueType = null; + // 4.a. If $assoc_args is a single ConstantArray: + $assocConstantArrays = $assocArgsType->getConstantArrays(); + if ( count( $assocConstantArrays ) === 1 ) { + $assocArgsConstantArray = $assocConstantArrays[0]; + $keyTypes = $assocArgsConstantArray->getKeyTypes(); + $valueTypes = $assocArgsConstantArray->getValueTypes(); + $resolvedValueType = null; foreach ( $keyTypes as $index => $keyType ) { - if ( $keyType->isConstantValue()->yes() && $keyType->toString() instanceof ConstantStringType && $keyType->getValue() === $flagValue ) { + $keyConstantStrings = $keyType->getConstantStrings(); + if ( count( $keyConstantStrings ) === 1 && $keyConstantStrings[0]->getValue() === $flagValue ) { $resolvedValueType = $valueTypes[ $index ]; break; } } if ( null !== $resolvedValueType ) { - // Key definitely exists, return its type + // Key definitely exists and has a resolved type. return $resolvedValueType; } else { - // Key definitely does not exist, return default type + // Key definitely does not exist in this constant array. return $defaultType; } } - // If $assocArgsType is a general ArrayType, we can't be sure if the specific flag exists. - // The function's logic is: isset( $assoc_args[ $flag ] ) ? $assoc_args[ $flag ] : $default; - // So, it's a union of the potential value type from the array and the default type. - if ( $assocArgsType->isArray()->yes() ) { - // We don't know IF the key $flagValue exists. - // PHPStan's ArrayType has an itemType which represents the type of values in the array. - // This is the best we can do for a generic array. - return TypeCombinator::union( $assocArgsType->getItemType(), $defaultType ); + // 4.b. $assoc_args is not a single ConstantArray (but $flagValue is known): + // Use getOffsetValueType for other array-like types. + $valueForKeyType = $assocArgsType->getOffsetValueType( new ConstantStringType( $flagValue ) ); + + // The key might exist, or its presence is unknown. + // The function returns $assoc_args[$flag] if set, otherwise $default. + return TypeCombinator::union( $valueForKeyType, $defaultType ); + } + + /** + * Handles the case where the flag name is not a single known constant string. + * The return type is a union of all possible values in $assocArgsType and $defaultType. + */ + private function getDynamicFlagFallbackType( Type $assocArgsType, Type $defaultType ): Type { + $possibleValueTypes = []; + + $assocConstantArrays = $assocArgsType->getConstantArrays(); + if ( count( $assocConstantArrays ) === 1 ) { // It's one specific constant array + $constantArray = $assocConstantArrays[0]; + if ( count( $constantArray->getValueTypes() ) > 0 ) { + $possibleValueTypes = $constantArray->getValueTypes(); + } + } else { + $possibleValueTypes[] = new MixedType(); } - // Fallback for other types of $assocArgsType or if we can't determine. - // This should ideally be the union of what the array could contain for that key and the default. - // For simplicity, if not a ConstantArrayType or ArrayType, return mixed or a broad union. - // In a real-world scenario with more complex types, you might query $assocArgsType->getOffsetValueType(new ConstantStringType($flagValue)) - // and then union with $defaultType. - $offsetValueType = $assocArgsType->getOffsetValueType( new ConstantStringType( $flagValue ) ); - if ( ! $offsetValueType instanceof MixedType || $offsetValueType->isExplicitMixed() ) { - return TypeCombinator::union( $offsetValueType, $defaultType ); + if ( empty( $possibleValueTypes ) ) { + return $defaultType; } - return new MixedType(); // Default fallback + return TypeCombinator::union( $defaultType, ...$possibleValueTypes ); } } diff --git a/src/PHPStan/WPCliRuncommandDynamicReturnTypeExtension.php b/src/PHPStan/WPCliRuncommandDynamicReturnTypeExtension.php new file mode 100644 index 000000000..379e03b5a --- /dev/null +++ b/src/PHPStan/WPCliRuncommandDynamicReturnTypeExtension.php @@ -0,0 +1,192 @@ +getName() === 'runcommand'; + } + + public function getTypeFromStaticMethodCall( + MethodReflection $methodReflection, + StaticCall $methodCall, + Scope $scope + ): Type { + $args = $methodCall->getArgs(); + + /** @var ConstantBooleanType|ConstantStringType $returnOption */ + $returnOption = new ConstantBooleanType( true ); + /** @var ConstantBooleanType|ConstantStringType $parseOption */ + $parseOption = new ConstantBooleanType( false ); + /** @var ConstantBooleanType $exitOnErrorOption */ + $exitOnErrorOption = new ConstantBooleanType( true ); + + $optionsAreStaticallyKnown = true; + + if ( isset( $args[1] ) && $args[1] instanceof Arg ) { + $optionsNode = $args[1]->value; + $optionsType = $scope->getType( $optionsNode ); + + if ( $optionsType->isConstantArray()->yes() ) { + $constantArrayTypes = $optionsType->getConstantArrays(); + if ( count( $constantArrayTypes ) === 1 ) { + $constantArrayType = $constantArrayTypes[0]; + $keyTypes = $constantArrayType->getKeyTypes(); + $valueTypes = $constantArrayType->getValueTypes(); + + foreach ( $keyTypes as $i => $keyType ) { + $keyConstantStrings = $keyType->getConstantStrings(); + if ( count( $keyConstantStrings ) !== 1 ) { + $optionsAreStaticallyKnown = false; + break; + } + $keyName = $keyConstantStrings[0]->getValue(); + $currentOptionValueType = $valueTypes[ $i ]; + + switch ( $keyName ) { + case 'return': + $valueConstantStrings = $currentOptionValueType->getConstantStrings(); + if ( count( $valueConstantStrings ) === 1 && $currentOptionValueType->isScalar()->yes() ) { + $returnOption = $valueConstantStrings[0]; + } elseif ( $currentOptionValueType->isTrue()->yes() ) { + $returnOption = new ConstantBooleanType( true ); + } elseif ( $currentOptionValueType->isFalse()->yes() ) { + $returnOption = new ConstantBooleanType( false ); + } else { + $optionsAreStaticallyKnown = false; + } + break; + case 'parse': + $valueConstantStrings = $currentOptionValueType->getConstantStrings(); + $isExactlyJsonString = ( count( $valueConstantStrings ) === 1 && $valueConstantStrings[0]->getValue() === 'json' && $currentOptionValueType->isScalar()->yes() ); + + if ( $isExactlyJsonString ) { + $parseOption = $valueConstantStrings[0]; + } elseif ( $$currentOptionValueType->isFalse()->yes() ) { + $parseOption = new ConstantBooleanType( false ); + } else { + // Not a single, clear constant we handle for a "known" path + $parseOption = new ConstantBooleanType( false ); // Default effect + $optionsAreStaticallyKnown = false; + } + break; + case 'exit_error': + if ( $currentOptionValueType->isTrue()->yes() ) { + $exitOnErrorOption = new ConstantBooleanType( true ); + } elseif ( $currentOptionValueType->isFalse()->yes() ) { + $exitOnErrorOption = new ConstantBooleanType( false ); + } else { + $optionsAreStaticallyKnown = false; + } + break; + } + if ( ! $optionsAreStaticallyKnown ) { + break; + } + } + } else { + $optionsAreStaticallyKnown = false; + } + } else { + $optionsAreStaticallyKnown = false; + } + } + + if ( ! $optionsAreStaticallyKnown ) { + return TypeCombinator::union( $this->getFallbackUnionTypeWithoutNever(), new NeverType() ); + } + + $normalReturnType = $this->determineNormalReturnType( $returnOption, $parseOption ); + + if ( $exitOnErrorOption->getValue() === true ) { + if ( $normalReturnType instanceof NeverType ) { + return $normalReturnType; + } + return TypeCombinator::union( $normalReturnType, new NeverType() ); + } + + return $normalReturnType; + } + + /** + * @param ConstantBooleanType|ConstantStringType $returnOptionValue + * @param ConstantBooleanType|ConstantStringType $parseOptionValue + */ + private function determineNormalReturnType( Type $returnOptionValue, Type $parseOptionValue ): Type { + $returnConstantStrings = $returnOptionValue->getConstantStrings(); + $return_val = count( $returnConstantStrings ) === 1 ? $returnConstantStrings[0]->getValue() : null; + + $parseConstantStrings = $parseOptionValue->getConstantStrings(); + $parseIsJson = count( $parseConstantStrings ) === 1 && $parseConstantStrings[0]->getValue() === 'json'; + + if ( 'all' === $return_val ) { + return $this->createAllObjectType(); + } + if ( 'return_code' === $return_val ) { + return new IntegerType(); + } + if ( 'stderr' === $return_val ) { + return new StringType(); + } + if ( $returnOptionValue->isTrue()->yes() || 'stdout' === $return_val ) { + if ( $parseIsJson ) { + return TypeCombinator::union( + new ArrayType( new MixedType(), new MixedType() ), + new NullType() + ); + } + return new StringType(); + } + if ( $returnOptionValue->isFalse()->yes() ) { + return new NullType(); + } + + return new MixedType( true ); + } + + private function createAllObjectType(): Type { + $propertyTypes = [ + 'stdout' => new StringType(), + 'stderr' => new StringType(), + 'return_code' => new IntegerType(), + ]; + $optionalProperties = []; + return new ObjectShapeType( $propertyTypes, $optionalProperties ); + } + + private function getFallbackUnionTypeWithoutNever(): Type { + return TypeCombinator::union( + new StringType(), + new IntegerType(), + $this->createAllObjectType(), + new ArrayType( new MixedType(), new MixedType() ), + new ObjectWithoutClassType(), + new NullType() + ); + } +} diff --git a/tests/data/get_flag_value.php b/tests/data/get_flag_value.php index e2f2dcffc..9a6b39c26 100644 --- a/tests/data/get_flag_value.php +++ b/tests/data/get_flag_value.php @@ -59,4 +59,4 @@ assertType( 'true', $value ); $value = get_flag_value( $assoc_args, $key2, 123 ); -assertType( "'bar'|true", $value ); +assertType( "123|'bar'|true", $value ); diff --git a/tests/data/runcommand.php b/tests/data/runcommand.php new file mode 100644 index 000000000..3f1828bbe --- /dev/null +++ b/tests/data/runcommand.php @@ -0,0 +1,67 @@ + true ] ); +assertType( 'string', $value ); + +$value = WP_CLI::runcommand( 'plugin list --format=json', [ 'return' => false ] ); +assertType( 'null', $value ); + +$value = WP_CLI::runcommand( 'plugin list --format=json', [ 'return' => 'all' ] ); +assertType( 'object{stdout: string, stderr: string, return_code: int}', $value ); + +$value = WP_CLI::runcommand( 'plugin list --format=json', [ 'return' => 'stdout' ] ); +assertType( 'string', $value ); + +$value = WP_CLI::runcommand( 'plugin list --format=json', [ 'return' => 'stderr' ] ); +assertType( 'string', $value ); + +$value = WP_CLI::runcommand( 'plugin list --format=json', [ 'return' => 'return_code' ] ); +assertType( 'int', $value ); + +$value = WP_CLI::runcommand( + 'plugin list --format=json', + [ + 'return' => true, + 'parse' => 'json', + ] +); +assertType( 'array|null', $value ); + +$value = WP_CLI::runcommand( + 'plugin list --format=json', + [ + 'return' => 'stdout', + 'parse' => 'json', + ] +); +assertType( 'array|null', $value ); + +$value = WP_CLI::runcommand( + 'plugin list --format=json', + [ + 'return' => 'stdout', + 'exit_error' => true, + ] +); +assertType( 'string', $value ); + +$value = WP_CLI::runcommand( + 'plugin list --format=json', + [ + 'return' => 'stdout', + 'exit_error' => false, + ] +); +assertType( 'string', $value ); diff --git a/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php b/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php index 0f97dca62..0d4d09baf 100644 --- a/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php +++ b/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php @@ -13,6 +13,7 @@ public function dataFileAsserts(): iterable { // Path to a file with actual asserts of expected types: yield from self::gatherAssertTypes( dirname( __DIR__, 2 ) . '/data/parse_url.php' ); yield from self::gatherAssertTypes( dirname( __DIR__, 2 ) . '/data/get_flag_value.php' ); + yield from self::gatherAssertTypes( dirname( __DIR__, 2 ) . '/data/runcommand.php' ); } /** From 10e9e753764e17f07794de076f7ed66cb7a9fdf8 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 22 May 2025 15:23:27 +0200 Subject: [PATCH 4/8] Move defaults to extension --- extension.neon | 15 +++++++++++++++ phpstan.neon.dist | 12 ------------ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/extension.neon b/extension.neon index 438da3b3f..6cf489519 100644 --- a/extension.neon +++ b/extension.neon @@ -11,3 +11,18 @@ services: class: WP_CLI\Tests\PHPStan\WPCliRuncommandDynamicReturnTypeExtension tags: - phpstan.broker.dynamicStaticMethodReturnTypeExtension +parameters: + strictRules: + disallowedLooseComparison: false + booleansInConditions: false + uselessCast: false + requireParentConstructorCall: false + disallowedConstructs: false + overwriteVariablesWithLoop: false + closureUsesThis: false + matchingInheritedMethodNames: false + numericOperandsInArithmeticOperators: false + strictCalls: false + switchConditionsMatchingType: false + noVariableVariables: false + strictArrayFilter: false diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 40016adc3..3c2a0976c 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -24,16 +24,4 @@ parameters: - message: '#Dynamic call to static method#' path: 'tests/tests' strictRules: - disallowedLooseComparison: false - booleansInConditions: false - uselessCast: false - requireParentConstructorCall: false - disallowedConstructs: false - overwriteVariablesWithLoop: false - closureUsesThis: false - matchingInheritedMethodNames: false - numericOperandsInArithmeticOperators: false strictCalls: true - switchConditionsMatchingType: false - noVariableVariables: false - strictArrayFilter: false From 6bc08ba945a5f694b183a6401a63704fd6789663 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 22 May 2025 17:53:28 +0200 Subject: [PATCH 5/8] Update extension config --- extension.neon | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/extension.neon b/extension.neon index 6cf489519..c231d5465 100644 --- a/extension.neon +++ b/extension.neon @@ -12,7 +12,10 @@ services: tags: - phpstan.broker.dynamicStaticMethodReturnTypeExtension parameters: + dynamicConstantNames: + - FOO strictRules: + allRules: false disallowedLooseComparison: false booleansInConditions: false uselessCast: false @@ -26,3 +29,23 @@ parameters: switchConditionsMatchingType: false noVariableVariables: false strictArrayFilter: false + +# Add the schema from phpstan-strict-rules so it's available without loading the extension +# and the above configuration works. +parametersSchema: + strictRules: structure([ + allRules: anyOf(bool(), arrayOf(bool())), + disallowedLooseComparison: anyOf(bool(), arrayOf(bool())), + booleansInConditions: anyOf(bool(), arrayOf(bool())) + uselessCast: anyOf(bool(), arrayOf(bool())) + requireParentConstructorCall: anyOf(bool(), arrayOf(bool())) + disallowedConstructs: anyOf(bool(), arrayOf(bool())) + overwriteVariablesWithLoop: anyOf(bool(), arrayOf(bool())) + closureUsesThis: anyOf(bool(), arrayOf(bool())) + matchingInheritedMethodNames: anyOf(bool(), arrayOf(bool())) + numericOperandsInArithmeticOperators: anyOf(bool(), arrayOf(bool())) + strictCalls: anyOf(bool(), arrayOf(bool())) + switchConditionsMatchingType: anyOf(bool(), arrayOf(bool())) + noVariableVariables: anyOf(bool(), arrayOf(bool())) + strictArrayFilter: anyOf(bool(), arrayOf(bool())) + ]) From 17e4af910db417242bbdb97ce28904ea800c8d35 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 22 May 2025 18:01:47 +0200 Subject: [PATCH 6/8] Make data provider static --- tests/tests/PHPStan/TestDynamicReturnTypeExtension.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php b/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php index 0d4d09baf..a54e37e7d 100644 --- a/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php +++ b/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php @@ -9,7 +9,7 @@ class TestDynamicReturnTypeExtension extends \PHPStan\Testing\TypeInferenceTestC /** * @return iterable */ - public function dataFileAsserts(): iterable { + public static function dataFileAsserts(): iterable { // Path to a file with actual asserts of expected types: yield from self::gatherAssertTypes( dirname( __DIR__, 2 ) . '/data/parse_url.php' ); yield from self::gatherAssertTypes( dirname( __DIR__, 2 ) . '/data/get_flag_value.php' ); From 3d086e50e2d149e44d58e9632e9b2b3e82ac4bfa Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Wed, 11 Jun 2025 15:11:06 +0200 Subject: [PATCH 7/8] Fix data provider --- tests/tests/PHPStan/TestDynamicReturnTypeExtension.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php b/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php index a54e37e7d..19fb98679 100644 --- a/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php +++ b/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php @@ -20,6 +20,7 @@ public static function dataFileAsserts(): iterable { * @dataProvider dataFileAsserts * @param array ...$args */ + #[DataProvider( 'dataFileAsserts' )] // phpcs:ignore PHPCompatibility.Attributes.NewAttributes.PHPUnitAttributeFound public function testFileAsserts( string $assertType, string $file, ...$args ): void { $this->assertFileAsserts( $assertType, $file, ...$args ); } From f7896919f3da9075e9ae155333f0b292ceea39e7 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Wed, 11 Jun 2025 15:13:14 +0200 Subject: [PATCH 8/8] Add missing import --- tests/tests/PHPStan/TestDynamicReturnTypeExtension.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php b/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php index 19fb98679..801d87527 100644 --- a/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php +++ b/tests/tests/PHPStan/TestDynamicReturnTypeExtension.php @@ -4,6 +4,8 @@ namespace WP_CLI\Tests\Tests\PHPStan; +use PHPUnit\Framework\Attributes\DataProvider; + class TestDynamicReturnTypeExtension extends \PHPStan\Testing\TypeInferenceTestCase { /**