diff --git a/.github/workflows/claude-regression-tests.yml b/.github/workflows/claude-regression-tests.yml deleted file mode 100644 index 440c6b4180..0000000000 --- a/.github/workflows/claude-regression-tests.yml +++ /dev/null @@ -1,137 +0,0 @@ -# https://help.github.com/en/categories/automating-your-workflow-with-github-actions - -name: "Claude Regression Tests" - -on: - workflow_run: - workflows: ["Issue Bot"] - types: - - completed - -concurrency: - group: claude-regression-tests-${{ github.event.workflow_run.head_branch || github.run_id }} - cancel-in-progress: true - -jobs: - regression-tests: - name: "Generate regression tests" - if: >- - github.event.workflow_run.event == 'pull_request' && - github.event.workflow_run.conclusion == 'success' - runs-on: blacksmith-4vcpu-ubuntu-2404 - timeout-minutes: 60 - - steps: - - name: "Get PR details and affected issues" - id: context - env: - GH_TOKEN: ${{ secrets.PHPSTAN_BOT_TOKEN }} - PR_JSON: ${{ toJSON(github.event.workflow_run.pull_requests) }} - RUN_ID: ${{ github.event.workflow_run.id }} - HEAD_SHA: ${{ github.event.workflow_run.head_sha }} - REPO: ${{ github.repository }} - run: | - # Skip if last commit was our own (prevent feedback loop) - COMMIT_MSG=$(gh api "repos/$REPO/commits/$HEAD_SHA" --jq '.commit.message' 2>/dev/null || true) - if [[ "$COMMIT_MSG" == "Add regression test for #"* ]]; then - echo "Last commit was regression test addition, skipping" - echo "skip=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - - PR_NUMBER=$(echo "$PR_JSON" | jq -r '.[0].number // empty') - if [ -z "$PR_NUMBER" ]; then - echo "No PR associated with this workflow run" - echo "skip=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - - PR_AUTHOR=$(gh pr view "$PR_NUMBER" --repo "$REPO" --json author --jq '.author.login') - if [ "$PR_AUTHOR" != "phpstan-bot" ]; then - echo "PR author is $PR_AUTHOR, not phpstan-bot - skipping" - echo "skip=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - - # Read the Issue Bot summary from workflow run logs - ISSUE_NUMBERS=$(gh run view "$RUN_ID" --repo "$REPO" --log 2>/dev/null \ - | grep -oP '\[#\K\d+(?=\]\(https://github\.com/phpstan/phpstan/issues/)' \ - | sort -un \ - | tr '\n' ' ' \ - | xargs) - - if [ -z "$ISSUE_NUMBERS" ]; then - echo "No affected issues found in Issue Bot summary" - echo "skip=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - - PR_BRANCH=$(gh pr view "$PR_NUMBER" --repo "$REPO" --json headRefName --jq '.headRefName') - - echo "number=$PR_NUMBER" >> "$GITHUB_OUTPUT" - echo "branch=$PR_BRANCH" >> "$GITHUB_OUTPUT" - echo "issues=$ISSUE_NUMBERS" >> "$GITHUB_OUTPUT" - echo "skip=false" >> "$GITHUB_OUTPUT" - - echo "### Context" >> "$GITHUB_STEP_SUMMARY" - echo "PR: #$PR_NUMBER (branch: $PR_BRANCH)" >> "$GITHUB_STEP_SUMMARY" - echo "Affected issues: $ISSUE_NUMBERS" >> "$GITHUB_STEP_SUMMARY" - - - name: "Checkout" - if: steps.context.outputs.skip != 'true' - uses: actions/checkout@v4 - with: - ref: ${{ steps.context.outputs.branch }} - fetch-depth: 0 - token: ${{ secrets.PHPSTAN_BOT_TOKEN }} - - - name: "Install PHP" - if: steps.context.outputs.skip != 'true' - uses: "shivammathur/setup-php@v2" - with: - coverage: "none" - php-version: "8.4" - ini-file: development - extensions: mbstring - - - name: "Install dependencies" - if: steps.context.outputs.skip != 'true' - uses: "ramsey/composer-install@v3" - - - name: "Install Claude Code" - if: steps.context.outputs.skip != 'true' - run: npm install -g @anthropic-ai/claude-code - - - name: "Generate regression tests" - if: steps.context.outputs.skip != 'true' - env: - CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} - GH_TOKEN: ${{ secrets.PHPSTAN_BOT_TOKEN }} - ISSUE_NUMBERS: ${{ steps.context.outputs.issues }} - run: | - git config user.name "phpstan-bot" - git config user.email "ondrej+phpstanbot@mirtes.cz" - - for ISSUE_NUMBER in $ISSUE_NUMBERS; do - echo "Generating regression test for issue #$ISSUE_NUMBER" - echo "### Issue #$ISSUE_NUMBER" >> "$GITHUB_STEP_SUMMARY" - - claude -p \ - --model claude-opus-4-6 \ - --output-format stream-json \ - --verbose \ - --dangerously-skip-permissions \ - "/regression-test $ISSUE_NUMBER - - Do not create a branch or push - this will be handled automatically." - done - - - name: "Push" - if: steps.context.outputs.skip != 'true' - run: | - if [ "$(git rev-parse HEAD)" = "$(git rev-parse @{u})" ]; then - echo "No new commits to push" - exit 0 - fi - - git push diff --git a/.github/workflows/issue-bot.yml b/.github/workflows/issue-bot.yml index e2d09fb52c..969d6da6b2 100644 --- a/.github/workflows/issue-bot.yml +++ b/.github/workflows/issue-bot.yml @@ -192,9 +192,11 @@ jobs: GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} run: | set +e - ./console.php evaluate >> $GITHUB_STEP_SUMMARY + ./console.php evaluate > tmp/step-summary.md exit_code="$?" + cat tmp/step-summary.md >> $GITHUB_STEP_SUMMARY + if [[ "$exit_code" == "2" ]]; then echo "::notice file=.github/workflows/issue-bot.yml,line=3 ::Issue bot detected open issues which are affected by this pull request - see https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" exit 0 @@ -202,6 +204,13 @@ jobs: exit $exit_code + - name: "Upload step summary" + if: github.event_name == 'pull_request' + uses: actions/upload-artifact@v4 + with: + name: step-summary + path: issue-bot/tmp/step-summary.md + - name: "Evaluate results - push" working-directory: "issue-bot" if: "github.repository_owner == 'phpstan' && github.ref == 'refs/heads/2.1.x'" @@ -220,3 +229,84 @@ jobs: fi exit $exit_code + + regression-tests: + name: "Generate regression tests" + needs: evaluate + if: github.event_name == 'pull_request' && github.event.pull_request.user.login == 'phpstan-bot' + runs-on: blacksmith-4vcpu-ubuntu-2404 + timeout-minutes: 60 + + steps: + - name: "Check for feedback loop" + id: check + env: + GH_TOKEN: ${{ secrets.PHPSTAN_BOT_TOKEN }} + run: | + COMMIT_MSG=$(gh api "repos/${{ github.repository }}/commits/${{ github.event.pull_request.head.sha }}" --jq '.commit.message' 2>/dev/null || true) + if [[ "$COMMIT_MSG" == "Add regression test for #"* ]]; then + echo "Last commit was regression test addition, skipping" + echo "skip=true" >> "$GITHUB_OUTPUT" + else + echo "skip=false" >> "$GITHUB_OUTPUT" + fi + + - name: "Checkout" + if: steps.check.outputs.skip != 'true' + uses: actions/checkout@v4 + with: + ref: ${{ github.head_ref }} + fetch-depth: 0 + token: ${{ secrets.PHPSTAN_BOT_TOKEN }} + + - name: "Download step summary" + if: steps.check.outputs.skip != 'true' + uses: actions/download-artifact@v4 + with: + name: step-summary + path: ./tmp + + - name: "Install PHP" + if: steps.check.outputs.skip != 'true' + uses: "shivammathur/setup-php@v2" + with: + coverage: "none" + php-version: "8.4" + ini-file: development + extensions: mbstring + + - name: "Install dependencies" + if: steps.check.outputs.skip != 'true' + uses: "ramsey/composer-install@v3" + + - name: "Install Claude Code" + if: steps.check.outputs.skip != 'true' + run: npm install -g @anthropic-ai/claude-code + + - name: "Generate regression tests" + if: steps.check.outputs.skip != 'true' + env: + CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + GH_TOKEN: ${{ secrets.PHPSTAN_BOT_TOKEN }} + run: | + git config user.name "phpstan-bot" + git config user.email "ondrej+phpstanbot@mirtes.cz" + + claude -p \ + --model claude-opus-4-6 \ + --output-format stream-json \ + --verbose \ + --dangerously-skip-permissions \ + "Read the file ./tmp/step-summary.md which contains the Issue Bot step summary from CI in Markdown format. Interpret this Markdown to figure out which GitHub issues need regression tests added. For each affected issue (where behavior changed), run /regression-test with the issue number. + + Do not create a branch or push - this will be handled automatically." + + - name: "Push" + if: steps.check.outputs.skip != 'true' + run: | + if [ "$(git rev-parse HEAD)" = "$(git rev-parse @{u})" ]; then + echo "No new commits to push" + exit 0 + fi + + git push diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 594c91f841..67e6009761 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3818,7 +3818,7 @@ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self $scope->getNamespace(), $scope->expressionTypes, $scope->nativeExpressionTypes, - array_merge($specifiedTypes->getNewConditionalExpressionHolders(), $scope->conditionalExpressions), + $this->mergeConditionalExpressions($specifiedTypes->getNewConditionalExpressionHolders(), $scope->conditionalExpressions), $scope->inClosureBindScopeClasses, $scope->anonymousFunctionReflection, $scope->inFirstLevelStatement, @@ -4007,6 +4007,25 @@ private function intersectConditionalExpressions(array $otherConditionalExpressi return $newConditionalExpressions; } + /** + * @param array $newConditionalExpressions + * @param array $existingConditionalExpressions + * @return array + */ + private function mergeConditionalExpressions(array $newConditionalExpressions, array $existingConditionalExpressions): array + { + $result = $existingConditionalExpressions; + foreach ($newConditionalExpressions as $exprString => $holders) { + if (!array_key_exists($exprString, $result)) { + $result[$exprString] = $holders; + } else { + $result[$exprString] = array_merge($result[$exprString], $holders); + } + } + + return $result; + } + /** * @param array $conditionalExpressions * @param array $ourExpressionTypes diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 4a19e9cc49..79ea0e8344 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4364,6 +4364,13 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto $matchScope = $matchScope->filterByFalseyValue($filteringExpr); } + if (!$hasDefaultCond && !$hasAlwaysTrueCond && $condType->isBoolean()->yes() && $condType->isConstantScalarValue()->yes()) { + if ($this->isScopeConditionallyImpossible($matchScope)) { + $hasAlwaysTrueCond = true; + $matchScope = $matchScope->addTypeToExpression($expr->cond, new NeverType()); + } + } + $isExhaustive = $hasDefaultCond || $hasAlwaysTrueCond; if (!$isExhaustive) { $remainingType = $matchScope->getType($expr->cond); @@ -7598,6 +7605,60 @@ private function getFilteringExprForMatchArm(Expr\Match_ $expr, array $condition ); } + /** + * Checks if a scope's conditional expressions form a contradiction, + * meaning no combination of variable values is possible. + * Used for match(true) exhaustiveness detection. + */ + private function isScopeConditionallyImpossible(MutatingScope $scope): bool + { + $boolVars = []; + foreach ($scope->getDefinedVariables() as $varName) { + $varType = $scope->getVariableType($varName); + if ($varType->isBoolean()->yes() && !$varType->isConstantScalarValue()->yes()) { + $boolVars[] = $varName; + } + } + + if ($boolVars === []) { + return false; + } + + // Check if any boolean variable's both truth values lead to contradictions + foreach ($boolVars as $varName) { + $varExpr = new Variable($varName); + + $truthyScope = $scope->filterByTruthyValue($varExpr); + $truthyContradiction = $this->scopeHasNeverVariable($truthyScope, $boolVars); + if (!$truthyContradiction) { + continue; + } + + $falseyScope = $scope->filterByFalseyValue($varExpr); + $falseyContradiction = $this->scopeHasNeverVariable($falseyScope, $boolVars); + if ($falseyContradiction) { + return true; + } + } + + return false; + } + + /** + * @param string[] $varNames + */ + private function scopeHasNeverVariable(MutatingScope $scope, array $varNames): bool + { + foreach ($varNames as $varName) { + $type = $scope->getVariableType($varName); + if ($type instanceof NeverType) { + return true; + } + } + + return false; + } + private function inferForLoopExpressions(For_ $stmt, Expr $lastCondExpr, MutatingScope $bodyScope): MutatingScope { // infer $items[$i] type from for ($i = 0; $i < count($items); $i++) {...} diff --git a/src/Analyser/SpecifiedTypes.php b/src/Analyser/SpecifiedTypes.php index fd9ddda81d..7fb3381ae5 100644 --- a/src/Analyser/SpecifiedTypes.php +++ b/src/Analyser/SpecifiedTypes.php @@ -188,6 +188,16 @@ public function unionWith(SpecifiedTypes $other): self $result = $result->setAlwaysOverwriteTypes(); } + $conditionalExpressionHolders = $this->newConditionalExpressionHolders; + foreach ($other->newConditionalExpressionHolders as $exprString => $holders) { + if (!array_key_exists($exprString, $conditionalExpressionHolders)) { + $conditionalExpressionHolders[$exprString] = $holders; + } else { + $conditionalExpressionHolders[$exprString] = array_merge($conditionalExpressionHolders[$exprString], $holders); + } + } + $result->newConditionalExpressionHolders = $conditionalExpressionHolders; + return $result->setRootExpr($rootExpr); } diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 11023bb1cd..64f9841cf0 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -645,6 +645,16 @@ public function specifyTypesInCondition( $rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context)->setRootExpr($expr); $types = $context->true() ? $leftTypes->unionWith($rightTypes) : $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope)); if ($context->false()) { + $leftTypesForHolders = $leftTypes; + $rightTypesForHolders = $rightTypes; + if ($context->truthy()) { + if ($leftTypesForHolders->getSureTypes() === [] && $leftTypesForHolders->getSureNotTypes() === []) { + $leftTypesForHolders = $this->specifyTypesInCondition($scope, $expr->left, TypeSpecifierContext::createFalsey())->setRootExpr($expr); + } + if ($rightTypesForHolders->getSureTypes() === [] && $rightTypesForHolders->getSureNotTypes() === []) { + $rightTypesForHolders = $this->specifyTypesInCondition($rightScope, $expr->right, TypeSpecifierContext::createFalsey())->setRootExpr($expr); + } + } $result = new SpecifiedTypes( $types->getSureTypes(), $types->getSureNotTypes(), @@ -653,10 +663,10 @@ public function specifyTypesInCondition( $result = $result->setAlwaysOverwriteTypes(); } return $result->setNewConditionalExpressionHolders(array_merge( - $this->processBooleanNotSureConditionalTypes($scope, $leftTypes, $rightTypes), - $this->processBooleanNotSureConditionalTypes($scope, $rightTypes, $leftTypes), - $this->processBooleanSureConditionalTypes($scope, $leftTypes, $rightTypes), - $this->processBooleanSureConditionalTypes($scope, $rightTypes, $leftTypes), + $this->processBooleanNotSureConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders), + $this->processBooleanNotSureConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders), + $this->processBooleanSureConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders), + $this->processBooleanSureConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders), ))->setRootExpr($expr); } diff --git a/tests/PHPStan/Analyser/nsrt/bug-7008.php b/tests/PHPStan/Analyser/nsrt/bug-7008.php new file mode 100644 index 0000000000..646478ce92 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-7008.php @@ -0,0 +1,23 @@ +treatPhpDocTypesAsCertain: false in your %configurationFile%.'; $this->analyse([__DIR__ . '/data/bug-3632.php'], [ + [ + 'Instanceof between null and Bug3632\NiceClass will always evaluate to false.', + 32, + $tipText, + ], [ 'Instanceof between Bug3632\NiceClass and Bug3632\NiceClass will always evaluate to true.', 36, diff --git a/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php index 2b1833c369..8f2e2ea4d1 100644 --- a/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php @@ -157,6 +157,24 @@ public function testBug7937(): void $this->analyse([__DIR__ . '/data/bug-7937.php'], []); } + public function testBug11903(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->reportAlwaysTrueInLastCondition = true; + $this->analyse([__DIR__ . '/data/bug-11903.php'], [ + [ + 'Negated boolean expression is always true.', + 13, + 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', + ], + [ + 'Negated boolean expression is always true.', + 21, + 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', + ], + ]); + } + public static function dataReportAlwaysTrueInLastCondition(): iterable { yield [false, [ diff --git a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php index 5f081aee3d..917f016c4a 100644 --- a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php @@ -405,4 +405,15 @@ public function testBug13048(): void $this->analyse([__DIR__ . '/data/bug-13048.php'], []); } + #[RequiresPhp('>= 8.0')] + public function testBug13303(): void + { + $this->analyse([__DIR__ . '/data/bug-13303.php'], [ + [ + 'Match expression does not handle remaining value: true', + 34, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-11903.php b/tests/PHPStan/Rules/Comparison/data/bug-11903.php new file mode 100644 index 0000000000..2baca425d4 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-11903.php @@ -0,0 +1,27 @@ += 8.0 + +namespace Bug13303; + +function a(bool $b, bool $c): int { + return match(true) { + $b && $c => 1, + !$b && !$c => 2, + !$b && $c => 3, + $b && !$c => 4, + }; +} + +function b(bool $b, bool $c): int { + return match(true) { + $b && $c, + !$b && !$c => 1, + !$b && $c, + $b && !$c => 2, + }; +} + +function c(bool $b, bool $c): int { + return match(true) { + $b === true && $c === true => 1, + $b === false && $c === false => 2, + $b === false && $c === true => 3, + $b === true && $c === false => 4, + }; +} + +function d(bool $b, bool $c): int { + // Not exhaustive - should still report error + return match(true) { + $b && $c => 1, + !$b && !$c => 2, + !$b && $c => 3, + }; +}