Psalm: Question regarding custom loader

Created on 9 Dec 2018  路  19Comments  路  Source: vimeo/psalm

I have a 9-ish year old, almost 100% custom PHP codebase that uses lazy loading, but does not use autoloaders. It has a system of loader functions like 'instance_model', which are analogous to NodeJS's module system. These resolve names using a statically built array.

Source files are loaded from two different directories, one being the project itself, and a global library with common code shared by all sites using the codebase. If the same file exists in both, the websites version implicitly replaces the global one. The global one isn't included at all in that case.

I assume that psalm can handle the multiple source directories if I define multiple 'directory' tags, but how would I make it aware of the file override system and custom loaders? I've read a page on running psalm on wordpress that implies it is possible, but not found anything in the documentation that says how.

I'm mainly interested in detecting undefined functions/variables, for when I rename things.

All 19 comments

But you're using an autoloader, right?

Nope, loading is all done using include_once through the following function:

function instance_component(string $prefix, array $valid_components, string $name) {
    include_once $valid_components[$name];
    $clsname = $prefix . '_' . $name;
    return new $clsname();}

Then instance_model for example, is implemented as follows. There are a number of these helper functions.

function instance_model(string $model_name) {
    return instance_component('mdl', $GLOBALS['fw_valid_models'], $model_name);}

The global comes from a statically generated array which may be cached by opcache. This lists all of the files of each type of component (controller, admin_controller, model, view, helper, gateway, action). They need to be segregated as such as these are also used for meta-programming as well as loading.

I assume I would need to tell psalm that 'instance_model("test")' for instance, means "new mdl_test()". Or is it able to follow through the string concatenations and work that out? To include the files, it would be easy to generate a static include list from the existing array for testing purposes.

I need to tell psalm that 'instance_model("test")' for instance, means "new mdl_test()". To include the files, it would be easy to generate a static include list from the existing array.

OK, that's non-trivial, and will require you to write a plugin that implements Psalm\Plugin\Hook\AfterFunctionCallAnalysisInterface and sets $return_type_candidate appropriately.

Something like this:

<?php
namespace YourCoolNamespace;

use PhpParser;
use Psalm\Codebase;
use Psalm\CodeLocation;
use Psalm\Context;
use Psalm\FileManipulation;
use Psalm\StatementsSource;
use Psalm\Type;
use Psalm\Plugin\Hook;

class InstanceModelReturnTypeUpdater implements Hook\AfterFunctionCallAnalysisInterface
{
    /**
     * Called after an expression has been checked
     *
     * @param  PhpParser\Node\Expr  $expr
     * @param  Context              $context
     * @param  string[]             $suppressed_issues
     * @param  FileManipulation[]   $file_replacements
     *
     * @return null
     */
    public static function afterMethodCallAnalysis(
        PhpParser\Node\Expr\FuncCall $expr,
        string $function_id,
        Context $context,
        StatementsSource $statements_source,
        Codebase $codebase,
        array &$file_replacements = [],
        Union &$return_type_candidate = null
    ) {
        if ($function_id === 'instance_model' && isset($expr->args[0])) {
            $first_arg = $expr->args[0]->value;

            if ($first_arg instanceof PhpParser\Node\Scalar\String_) {
                $fq_class_name = 'mdl_' . $first_arg->value;
                $return_type_candidate = new Union([new TNamedObject($fq_class_name)]);
            }
        }
    }
}

Psalm still won't load those classes automatically, so you'll never be able to scan individual files - just the whole set at once.

That looks straightforward enough, Can I do this on 'instance_component' once like the following, as it is the base of the system, or will I need to do it separately for each of the helpers?

     if ($function_id === 'instance_component' && isset($expr->args[0])) {
            $first_arg = $expr->args[0]->value;
            $third_arg = $expr->args[2]->value;

            if ($first_arg instanceof PhpParser\Node\Scalar\String_ &&
                $first_arg instanceof PhpParser\Node\Scalar\String_)
           {
                $fq_class_name = $first_arg->value . '_' . $third_arg->value;
                $return_type_candidate = new Union([new TNamedObject($fq_class_name)]);
            }
        }

yup, you can do that too (without a separate file)

Though if you run that through Psalm it'll shout at you for $first_arg instanceof PhpParser\Node\Scalar\String_ && $first_arg instanceof PhpParser\Node\Scalar\String_

Meant $third_arg...

@muglug Thanks for your help. Does psalm/phpdoc have the ability to specify the contents of deeply nested, or recursively defined arrays? My code has taken a lot of inspiration from Clojure in this regards and it works fine for my needs, but having a way of documenting them would be good. Clojure has 'spec' and JSON schema does something similar, but I'm not aware of anything similar for native PHP arrays.

While originally a dynamic language, it's common use is becoming increasingly java/C# like.

Psalm supports a syntax object-like arrays: https://getpsalm.org/docs/supported_annotations/#object-like-arrays

But that syntax does not support recursive types, unfortunately

That's somewhat surprising actually. This usage isn't unknown in PHP and Wordpress for instance has similar nested arrays in the plugin API. Would be nice to have something like this:

  • @psalm-param array{0: string, 1: array{0: string, 1: string}}[] $arr

That is, an array of 'object arrays', one of the keys of which is another array. How does psalm's type inference currently handle such structures?

Oh that works fine. It's nested, but not recursive.

So while Psalm allows type aliasing in the form

/** @psalm-type Foo = A|array<int, A> */

it does not allow

/** @psalm-type Foo = A|array<int, Foo> */

TypeScript does allow recursive types, FWIW

That makes sense, thanks for correcting my misunderstanding.

Do you have any other questions, or can I close?

That's it, thanks for your help.

You're welcome!

I've now had time to try using Psalm on this code. I've specified the path to the root of the framework code in a 'directory' tag and I assume that Psalm loads all files recursively down from this directory?

It isn't finding a number of helper functions defined in code/common.php under this root. I'm unsure why as they are defined in the global scope, and don't depend on the custom loader. It's included as follows:

    $c_path = dirname(__FILE__);
    include $c_path."/common.php";

Assuming the files are being loaded recursively, I assume how they are being included wouldn't matter?

Would you mind opening a new issue? It'd be great if you could separate things like

file1.php

// some code

file2.php

// some code

so I understand what exactly you're trying to do (maybe it's clear given everything above, but it's late and my brain's slowed to a crawl)

No problem, will do so tomorrow.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Pierstoval picture Pierstoval  路  3Comments

LeSuisse picture LeSuisse  路  3Comments

muglug picture muglug  路  3Comments

Ocramius picture Ocramius  路  3Comments

staabm picture staabm  路  3Comments