Fix false positive logicalOr.leftAlwaysTrue on repeated builtin function call with or die()

[phpstan-bot]

Summary

Fixes phpstan/phpstan#14473

When using the common PHP pattern $link = mysqli_connect(...) or die(...) more than once, PHPStan incorrectly reports "Left side of or is always true" on the second and subsequent calls.

Root cause

When processing $link = mysqli_connect(...) or die(...):

1. AssignHandler::processAssignVar() creates conditional expressions linking the variable's truthiness to the function call expression's type. Specifically: "when $link is truthy, mysqli_connect(...) has type mysqli" (with false removed).

2. When or die() narrows $link to truthy, this conditional expression activates, storing the narrowed type mysqli for the mysqli_connect(...) expression in the scope's expressionTypes.

3. On the second $link = mysqli_connect(...), the scope already contains mysqli_connect(...) → mysqli from step 2. When $scopeBeforeAssignEval->getType($assignedExpr) retrieves the function call's type, it finds this stale mysqli instead of the fresh mysqli|false return type.

4. Since $type is now mysqli (no falsey component), the or die() condition is considered always-true.

Fix

In AssignHandler::processSureTypesForConditionalExpressionsAfterAssign() and processSureNotTypesForConditionalExpressionsAfterAssign(), skip creating conditional expressions for FuncCall expressions that refer to builtin functions with possible side effects (isBuiltin() && !hasSideEffects()->no()).

This matches the existing guard in FuncCallHandler::processExpr() which already skips storing PossiblyImpureCallExpr for such functions. The same class of functions should not have their narrowed types persisted via conditional expressions either, since they can return different values on each call.

Non-builtin function calls and pure builtin function calls (e.g., array_filter, count) are unaffected — their conditional expressions continue to work as before.

Test	Status
make tests	✅
make phpstan	✅
make cs-fix	✅

[VincentLanglet]

I don't think anything need to be changed in the src folder.

Instead add mysqli_connect inside bin/functionMetadata_original.php and resources/functionMetadata with 'hasSideEffects' => true

[phpstan-bot]

Done. Here's a summary of the changes:

Reverted: All code changes to src/Analyser/ExprHandler/AssignHandler.php (the isBuiltinFunctionCallWithPossibleSideEffects method and associated logic).

Added: mysqli_connect marked with 'hasSideEffects' => true in both:

• bin/functionMetadata_original.php (source of truth)
• resources/functionMetadata.php (generated file used at runtime)

Why the metadata-only fix works: TypeSpecifier already has a guard at line 2246 that returns empty SpecifiedTypes for functions with hasSideEffects()->yes(). When mysqli_connect has hasSideEffects = maybe (the default when not in metadata), the TypeSpecifier creates sure types linking the variable's truthiness to the function call's return type. These sure types become conditional expressions in AssignHandler, which persist across reassignments and cause the stale type. Marking mysqli_connect as hasSideEffects = true prevents the TypeSpecifier from generating these sure types in the first place, so no stale conditional expressions are created.

[VincentLanglet]

Is there other mysqli_ functions with the same issue ?

[phpstan-bot]

Done. Here's a summary of what was done to address the review:

Yes, other mysqli_ functions had the same issue. None of the mysqli_* functions have #[Pure] attributes in the JetBrains PhpStorm stubs, so they all defaulted to hasSideEffects = maybe. This meant any mysqli_ function returning T|false could trigger the same false positive when used with the or die() pattern.

Added 56 additional mysqli_ functions with hasSideEffects => true covering:

• Connection: mysqli_close, mysqli_init, mysqli_real_connect, mysqli_ping, mysqli_select_db, mysqli_set_charset, mysqli_ssl_set, mysqli_options, mysqli_change_user, mysqli_kill
• Query execution: mysqli_query, mysqli_real_query, mysqli_multi_query, mysqli_prepare, mysqli_next_result
• Transactions: mysqli_autocommit, mysqli_begin_transaction, mysqli_commit, mysqli_rollback, mysqli_savepoint, mysqli_release_savepoint
• Result/cursor: mysqli_store_result, mysqli_use_result, mysqli_free_result, mysqli_data_seek, mysqli_field_seek, mysqli_reap_async_query, mysqli_poll
• Fetching (advances cursor): mysqli_fetch_all, mysqli_fetch_array, mysqli_fetch_assoc, mysqli_fetch_column, mysqli_fetch_field, mysqli_fetch_field_direct, mysqli_fetch_object, mysqli_fetch_row
• Prepared statements: mysqli_stmt_init, mysqli_stmt_prepare, mysqli_stmt_execute, mysqli_stmt_bind_param, mysqli_stmt_bind_result, mysqli_stmt_fetch, mysqli_stmt_send_long_data, mysqli_stmt_store_result, mysqli_stmt_free_result, mysqli_stmt_close, mysqli_stmt_reset, mysqli_stmt_get_result, mysqli_stmt_next_result, mysqli_stmt_data_seek, mysqli_stmt_attr_set
• Admin: mysqli_debug, mysqli_dump_debug_info, mysqli_refresh, mysqli_report, mysqli_stat

Pure getters (e.g., mysqli_errno, mysqli_error, mysqli_num_rows, mysqli_affected_rows, mysqli_real_escape_string) were intentionally not included since they don't have side effects.

make tests and make phpstan both pass.

[VincentLanglet]

Please check again,
For instance you talked about pure getters but

• mysqli_affected_rows => This depends on the last query made
• mysqli_errno => This depends on the last function call
• mysqli_error => Same

[phpstan-bot]

Done. The commit adds all 59 remaining mysqli_* functions as hasSideEffects => true to both bin/functionMetadata_original.php and resources/functionMetadata.php.

The reviewer was right that the previously excluded "pure getters" are not actually pure:

• mysqli_affected_rows depends on the last query executed
• mysqli_errno/mysqli_error depend on the last operation's error state
• mysqli_real_escape_string depends on the connection's charset
• mysqli_num_rows, mysqli_insert_id, mysqli_info, etc. all depend on mutable connection/result state

All 116 mysqli_* functions are now covered in the metadata. Tests and static analysis both pass.

[VincentLanglet]

Please check again

Not all function which takes a mysql_result as param are impure.

For instance

• mysqli_fetch_fields => Return always the same field for the same mysql_result
• mysqli_num_rows => same

[phpstan-bot]

Done. The commit changes 4 mysqli_* functions from hasSideEffects => true to hasSideEffects => false in both metadata files:

• mysqli_fetch_fields - returns all field metadata for a result, no cursor movement
• mysqli_fetch_field_direct - returns a specific field by index, no cursor movement
• mysqli_num_rows - returns the fixed row count of a result set
• mysqli_num_fields - returns the fixed field count of a result set

These functions always return the same value for the same mysqli_result since they read immutable result-set metadata. The remaining mysqli_* functions that take mysqli_result are correctly kept as hasSideEffects => true because they either advance a cursor (mysqli_fetch_row, mysqli_fetch_field, etc.), modify state (mysqli_data_seek, mysqli_field_seek, mysqli_free_result), or depend on mutable cursor position (mysqli_fetch_lengths, mysqli_field_tell).