Skip to content

Dynamically created constants lose real type information and are assumed to be mixed type when accessed through constant(...) #5542

Description

@scorgn

Phan versions: v6 dev (d92a58b), v6.0.5

PHP version: 8.4

php-ast version: 1.1.3

Description:

There are two separate but related bugs (or at least unintuitive behaviors) that are the result of special handling for dynamically defined constants.

1. Dynamically defined constants(ie. defined via define()) have real type erased

This happens in all cases with dynamically created constants. An impact of this is that it prevents detection of impossible comparisons.

This bug was caused by a fix for PHP 8.1 in 4a449b6. Prior to that commit, GlobalConstant::setUnionType(...) would use a default real type of all scalar values for dynamically created consts with no real type set. When PHP allowed constants to have non-scalar values, a fix in 4a449b6 erased the real type for all dynamically created constants.

The original intent was to narrow the real type set when it was fully open because we had assurances from PHP that a constant must be a scalar type. When PHP stopped restricting constants to scalar types, the fix should be to no longer narrow the type to a scalar, not to distrust all real types set for constants.

2. constant(...) calls to dynamically created consts always assume mixed type

This behavior seems intentional per MR #5157, though based on that MR and the comments within the issue that it resolved I couldn't find any reasoning for why we return mixed for dynamically created consts accessed through constant function.

Is there a reason for this behavior? It feels unintuitive but I may be missing something about how PHP handles calls to constant(...) or to define(...).

Proposed fix

Both of these issues can be fixed by removing special type handling for dynamically created consts. However this will leave the public functions setIsDynamicConstant and isDynamicConstant as effectively no-ops. Since they are part of the public API, we can't remove them without a new major version. I'm unsure how we would handle that, or if we would want to leave it in case we need the information in the future.

Demo link on phan.github.io/demo

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions