Vscode-intelephense: Late static binding - Undefined class constant

[tellkonkle]

Describe the bug
Intelephense is trying to find class constant definitions, even when they're being referenced via late static bindings. It's subsequently giving "Undefined class constant" errors. This issue appears to have been introduced in version 1.3.5 or 1.3.6 since it never gave me these errors before.

To Reproduce

<?php

abstract class ModelRecord
{
    public function load($id)
    {
        return $this->db->query('SINGLE * FROM {table} WHERE {primary} = ?')
            ->tag('table', static::TABLE)
            ->tag('where', static::PRIMARY)
            ->bind($id)
            ->run();
    }
}

class User extends ModelRecord
{
    const TABLE   = 'users';
    const PRIMARY = 'id';
}

// Undefined class constant 'TABLE' [8, 36]
// Undefined class constant 'PRIMARY' [9, 36]

Expected behavior
No errors to be present.

Platform and version

• PHP 7.3 via Docker
• Intelephense 1.3.6 (Premium)

[KapitanOczywisty]

ModelRecord has no way of knowing if const is really implemented, without checking every possible implementation. For such cases there are abstract functions:

abstract class ModelRecord
{
    abstract public static function getTable();
    abstract public static function getPrimary();

    public function load($id)
    {
        return $this->db->query('SINGLE * FROM {table} WHERE {primary} = ?')
            ->tag('table', static::getTable())
            ->tag('where', static::getPrimary())
            ->bind($id)
            ->run();
    }
}

class User extends ModelRecord {
    public static function getTable(){ return 'users'; }
    public static function getPrimary(){ return 'id'; }
}

And const should be constant, not dependent on context.

[tellkonkle]

PHP's native late static bindings allow for this use case even if it's not "best practice." This has never caused errors before until v1.3.6 (ish). Is it possible to disable this check? Intelephense should NOT be sniffing for definitions that it knows it can't properly ascertain (late static bindings are, by nature, things Intelephense can't look for as you described - so you shouldn't be looking for them), as is evidenced by the fact that this version also broke closures that are bound to a different $this context (method not defined), which completely screws up my DOM library that uses $this inside closures to reference the selected DOM node to mimic the jQuery API. I'll submit separate bug report on that later today (assuming it's not already submitted).

[KapitanOczywisty]

I'm only implying that this is a gray area which can no longer work in the future. In that case, I'm not sure how extension should behave. Personally I'm against silently skipping checks, but I'm heavy strict user. In general late static bindings issues in IDE often can be solved with abstract. About clousures please open issue, we're take a look.

For now you can disable check: "intelephense.diagnostics.undefinedClassConstants": false. I can try to suggest another idea:

abstract class ModelRecord
{
    // define "abstract" const
    const TABLE = '';
    const PRIMARY = '';
    public function load($id)
    {
        // optional check if implemented
        if(self::TABLE === static::TABLE OR self::PRIMARY === static::PRIMARY)
            throw new UnexpectedValueException("TABLE or PRIMARY const not implemented");

        var_dump(static::TABLE, static::PRIMARY);
    }
}

class User extends ModelRecord
{
    const TABLE   = 'users';
    const PRIMARY = 'id';
}
class Contact extends ModelRecord
{
}

(new User)->load(1);
(new Contact)->load(2);

https://3v4l.org/AlUal

[tellkonkle]

PHP is a loosely typed language that simply has strict type support now, so that "loose" aspect of the language should always be something to consider for this type of tool. In my use case, there is a function (not shown in my example for brevity's sake) that checks for definitions and throws exceptions as applicable (it's ran in the constructor). I can probably rework these areas a bit to accommodate for this change (I like your suggestion on the overrides to simulate abstract constants), but it sucks to have a framework built that now needs reworked to accommodate changes to a popular code sniffing tool. But I'll try to make it work. Thank you for your patience. Cheers! Edit: I'll let you determine if/when to close this issue.

[aoikarasu]

Really there's still no way to suppress a warning message?
Nothing even similar to what ESLint has, e.g. /* eslint-disable strict */ ?

[marveloo]

I think it would be great if we could define constants using PHP doc blocks:

abstract class A {
    /**
     * @var string MY_CONST
     */

    public static function print()
    {
        echo static::MY_CONST;
    }
}

[KapitanOczywisty]

I think it would be great if we could define constants using PHP doc blocks:

@marveloo Is there any use case when you cannot use const and you need phpdoc? Constants can be overridden in descendent classes. Only problem is with overriding consts from interfaces, but this is for a reason.

[marveloo]

Well, that's true, we can use const to declare constants, but there is no way to declare abstract constants. The good thing about using undeclared constants is that they behave as if they were abstract ones, i.e. we are required to declare them later in subclasses. And as for phpdocs, well, that would be a way to declare abstract constants.

[KapitanOczywisty]

This would be counter intuitive behavior, but even if phpdoc consts would be supported intelephense doesn't enforce any implementations yet neither from interfaces nor abstract classes. In general using constants is a bad way to do that, they are designed for something completely different.

[marveloo]

We all have different beliefs and habits and that is ok. Anyway let's leave our biases towards what is good or bad design aside for now. The thing is that this is a feature of PHP (whether we like it or not) and I think the tools we use should not become the limiting factor in utilizing these features.

So now, as I'm thinking of it, perhaps the idea for intelephense would be to skip checks for undeclared static:: constants in abstract classes, because classes that make use of undeclared constants are not capable to be used on their own, so there is much sense for developers to declare those classes as abstract ones.

[tellkonkle]

I've adjusted my code to simply declare "abstract" constants as NULL and override them with the real values in child classes. I totally forgot you could override class constants in PHP until KapitanOczywisty pointed it out. So for my particular use case I'm now happy. 😊

[marveloo]

When we give some constant a value (whether it's null or anything) we barely can consider it as an abstract one because in that case we have to do a manual check for its value (as @KapitanOczywisty did here) so as to ensure that nobody has forgotten to redeclare it in a subclass. Anyway, that's still a way of dealing with it.

[KapitanOczywisty]

@marveloo This is not even "good or bad" design this is just not what const are designed for. My code is to deal with situation when there are consts already in place, it's not recommended when you creating new classes, It's "less bad". I'm strongly against removing check for single case. PHP is loosely typed, but there is some standards which are necessary to use extension.

If there is need for abstract consts maybe you should make suggestion in https://wiki.php.net/rfc

[marveloo]

Not sure I understand what you mean by "this is just not what const are designed for". By definition constant is a named value which cannot be changed during the runtime. This is exactly how constants work in PHP.

[KapitanOczywisty]

Unfortunately there is no rfc for late static bindings and it's hard to say "what author had in mind", but judging by manual and articles from that era, this was for methods and static parameters. I think that constants are working only because they are stored the same way as methods in zend_class_entry thus it was easy to get this working or was working already. (I wasn't there so don't quote me on that) Also I couldn't find any mention about calling constants via static:: in official manual, which is weird - they had to be aware that this is working with constants as well.

In general you should be able to write literal value in place of const and code should run. If the same reference to a constant (static::TABLE) can result in different value depending on context is not that "constant" anymore. Not everything what is possible in language is intended feature.

[marveloo]

Well, I take your point, but that would be a bit classical view of constants. Things evolve. static::CONSTANTS are not uncommon among vendors these days. Symfony and Laraval use them, to name a few of them. And we may even find some occurrences of undeclared static constants there too. It's no way an issue of high priority, but I wouldn't mind to see them supported by intelephense sometime.

[FastieSystems]

This happens not only with class constants but with normal global constants. Recently I made a change that moved the definition of about 25 constants to runtime. This is perfectly legal PHP and, of course, the code runs perfectly.

I suppose there is a fine line between me knowing what I'm doing and a tool not understanding at the same level. I certainly do not advocate suppressing errors for undefined constants in general and I have no idea what to suggest that might make Intelephense understand a particular context.

I am about to go back and undo that change to avoid this problem. Fortunately, the code is not widely deployed and I can adjust quickly. Still, today is my first day with Intelephense (I switched from Felix Becker's extension) and it is a bit frustrating to have working code flagged with errors. Warnings, okay; I know I can improve.

This is more of an observation than a complaint. I don't regret installing Intelephense at all.

[FastieSystems]

In my second day of using Intelephense, I see the problem is bigger than I thought. 15 years ago I devised a system for loading user-defined settings from a database and stuffing them into global constants. The typical site has about 100 of these in addition the 25 I mentioned above (I've already dealt with that problem). My solution is extremely efficient, with minimal overhead, and I am loath to change it.

But I have a suggested solution. Intelephense obviously knows about PHP's own set of global constants, such as DIRECTORY_SEPARATOR and PHP_VERSION and CASE_UPPER and about 2500 more. All that is needed is a way to define my small collection of constants so Intelephense knows about them, too. Perhaps the capability already exists and I just don't know about it.

Whether this can be extended to the late binding issue described here I cannot say.

[FastieSystems]

It's no way an issue of high priority,

I see your point but in my case the problem turns out to be more severe. I'd say it has at least above average priority. The undeclared constants flagged by Intelephense are declared at runtime and the code being flagged works perfectly. I'm finding it difficult to reconcile working code with the red flags I'm getting.

[bmewburn]

@FastieSystems you can add some stubs to declare these constants. Just create a file, declare all your constants, and drop it somewhere in your folder where it won't get executed but will be read by intelephense.

[FastieSystems]

@bmewburn Yes, that's what I'm doing. Seems a bit inelegant to me but I realize it will work. Thank you.