Psalm: Question: How to easily configure own rules of checking?

[klebann]

Hello,
I want to use psalm to find only specific functions in my code and validate arguments.

I was trying to make custom plugin for this but I don't have clue how to write new rule.

I added empty template of plugin to psalm and it doesn't work:

➜ ./vendor/bin/psalm                                   
Scanning files...
Uncaught UnexpectedValueException: psalm-moodle-filter is not a known class in /opt/moodle/mod/checklist/vendor/vimeo/psalm/src/Psalm/Config.php:1235
Stack trace:
#0 /opt/moodle/mod/checklist/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php(584): Psalm\Config->initializePlugins(Object(Psalm\Internal\Analyzer\ProjectAnalyzer))
#1 /opt/moodle/mod/checklist/vendor/vimeo/psalm/src/psalm.php(676): Psalm\Internal\Analyzer\ProjectAnalyzer->check('/opt/moodle/mod...', true)
#2 /opt/moodle/mod/checklist/vendor/vimeo/psalm/psalm(2): require_once('/opt/moodle/mod...')
#3 {main}

Next Psalm\Exception\ConfigException: Failed to load plugin psalm-moodle-filter in /opt/moodle/mod/checklist/vendor/vimeo/psalm/src/Psalm/Config.php:1247
Stack trace:
#0 /opt/moodle/mod/checklist/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php(584): Psalm\Config->initializePlugins(Object(Psalm\Internal\Analyzer\ProjectAnalyzer))
#1 /opt/moodle/mod/checklist/vendor/vimeo/psalm/src/psalm.php(676): Psalm\Internal\Analyzer\ProjectAnalyzer->check('/opt/moodle/mod...', true)
#2 /opt/moodle/mod/checklist/vendor/vimeo/psalm/psalm(2): require_once('/opt/moodle/mod...')
#3 {main}
(Psalm 4.3.1@2feba22a005a18bf31d4c7b9bdb9252c73897476 crashed due to an uncaught Throwable)

Can somebody show me an example of simple plugin for this purpose or tell me how to make this?

To be more specific, i want to search for $DB->...sql...( ) usage and then validate arguments in this method.

[psalm-github-bot[bot]]

Hey @klebann, can you reproduce the issue on https://psalm.dev ?

[orklah]

Hey!

Take a look at this:
https://github.com/orklah/psalm-strict-visibility/blob/master/hooks/StrictVisibility.php

Your plugin should have the same beggining. Once you get to method_storage , you can dump it to take a look at it, it should be a good beggining.

[klebann]

How can i disable all errors from vendor/bin/psalm output and display only this from my plugin?

They are useless in my project:

612 errors found
------------------------------
404 other issues found.
You can display them with --show-info=true
------------------------------

[orklah]

I think the easiest way would be to create a psalm-plugin.xml file that suppresses every native issue in psalm.

This is not very easy right now (I think the only solution would be to make an issueHandler with each issue and suppress them one by one. (Maybe this could be a PR to submit to allow suppressing every native issue with a config, or maybe a level 99)

Once you have your own psalm-plugin.xml created this way, you launch vendor/bin/psalm --config=psalm-plugin.xml

[klebann]

@orklah every issue like so?

<MissingPropertyType errorLevel="suppress" />

[weirdan]

The list of all supported issues can be found here: https://github.com/vimeo/psalm/blob/a27c674ceefd8bc1f89ee7fa46c86350f824ee09/config.xsd#L182-L186

[orklah]

@klebann yeah, exactly

@weirdan thanks for the link, I didn't thought of linking the xsd :)

[klebann]

I muted all issues from https://psalm.dev/docs/running_psalm/issues/
At this moment this works for me as expected.

✘1 ➜ vendor/bin/psalm --config=psalm-plugin.xml --no-diff
Scanning files...
Analyzing files...

░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░E░░░░░

ERROR: PrivateStrictVisibility - index.php:128:13 - Calling private method privateMethod2 via proxy (see https://psalm.dev/000)
        $a->privateMethod2(); //This will be flagged


ERROR: PrivateStrictVisibility - index.php:133:13 - Calling private method privateMethod3 via proxy (see https://psalm.dev/000)
        $a->privateMethod3(); //This will be flagged


------------------------------
2 errors found
------------------------------
Psalm can automatically fix 43 of these issues.
Run Psalm again with 
--alter --issues=MissingParamType --dry-run
to see what it can fix.
------------------------------

Checks took 4.61 seconds and used 86.837MB of memory
Psalm was able to infer types for 68.6005% of the codebase

[klebann]

Which API interface should I use to find usuage of $DB->get_record_sql( ... ) and then validate arguments of this specific function?

A usage example would be welcome!

AfterEveryFunctionCallAnalysisInterface would be good for this?

[orklah]

You should already receive every call to get_record_sql in afterMethodCallAnalysis. in my plugin, $method_storage will sometimes contain an object representing the get_record_sql method so you should be able to filter the rest and to fetch it's arguments.

By navigating $expr, you should be able to retrieve arguments passed to the method and their types.

I don't know of any plugin who do exactly that though, so you'll have to do some tweaking on your own.

If you're stuck, just ask, we'll answer when we can 😄

[klebann]

Yes, but $DB definition is in _Moodle data manipulation API_. I don't want to analise Moodle code. I just want to analyse proper usage of this method in my plugin for Moodle.

I dumped all $method_storage but it doesn't show any get_record_sql, probably because there is no definition of $DB in my plugin for Moodle.

I don't know of any plugin who do exactly that though, so you'll have to do some tweaking on your own.

unluckily...

[klebann]

After all, psalm is not able to scan whole Moodle code, it shows error during ./vendor/bin/psalm --init ../moodle:

Calculating best config level based on project files
Scanning files...
Analyzing files...

PHP Fatal error:  Allowed memory size of 8589934592 bytes exhausted (tried to allocate 20480 bytes) in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Type/Union.php on line 282
Fatal error: Allowed memory size of 8589934592 bytes exhausted (tried to allocate 20480 bytes) in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Type/Union.php on line 282
PHP Notice:  Undefined offset: 1 in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php on line 1312
Notice: Undefined offset: 1 in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php on line 1312
PHP Notice:  Trying to get property 'default' of non-object in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php on line 1323
Notice: Trying to get property 'default' of non-object in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php on line 1323
PHP Notice:  Undefined offset: 2 in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php on line 1312
Notice: Undefined offset: 2 in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php on line 1312
PHP Notice:  Trying to get property 'default' of non-object in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php on line 1323
Notice: Trying to get property 'default' of non-object in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php on line 1323
PHP Notice:  Undefined offset: 3 in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php on line 1312
Notice: Undefined offset: 3 in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php on line 1312
PHP Notice:  Trying to get property 'default' of non-object in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php on line 1323
Notice: Trying to get property 'default' of non-object in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php on line 1323
Uncaught Exception: Expecting /opt/psalm/vendor/amphp/amp/lib/internal/placeholder.php:86:2379:-:closure to have storage in /opt/psalm/vendor/amphp/amp/lib/Internal/Placeholder.php
Stack trace in the forked worker:
#0 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/ClosureAnalyzer.php(43): Psalm\Codebase->getClosureStorage('/opt/psalm/vend...', '/opt/psalm/vend...')
#1 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/ClosureAnalyzer.php(72): Psalm\Internal\Analyzer\ClosureAnalyzer->__construct(Object(PhpParser\Node\Expr\Closure), Object(Psalm\Internal\Analyzer\StatementsAnalyzer))
#2 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php(286): Psalm\Internal\Analyzer\ClosureAnalyzer::analyzeExpression(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Object(PhpParser\Node\Expr\Closure), Object(Psalm\Context))
#3 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php(44): Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer::handleExpression(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Object(PhpParser\Node\Expr\Closure), Object(Psalm\Context), false, NULL, false)
#4 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php(197): Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer::analyze(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Object(PhpParser\Node\Expr\Closure), Object(Psalm\Context))
#5 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php(312): Psalm\Internal\Analyzer\Statements\Expression\Call\ArgumentsAnalyzer::analyze(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Array, Array, 'Amp\\Loop::defer', true, Object(Psalm\Context), Object(Psalm\Internal\Type\TemplateResult))
#6 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php(174): Psalm\Internal\Analyzer\Statements\Expression\CallAnalyzer::checkMethodArgs(Object(Psalm\Internal\MethodIdentifier), Array, Object(Psalm\Internal\Type\TemplateResult), Object(Psalm\Context), Object(Psalm\CodeLocation), Object(Psalm\Internal\Analyzer\StatementsAnalyzer))
#7 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php(730): Psalm\Internal\Analyzer\Statements\Expression\Call\StaticMethod\ExistingAtomicStaticCallAnalyzer::analyze(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Object(PhpParser\Node\Expr\StaticCall), Object(PhpParser\Node\Identifier), Array, Object(Psalm\Context), Object(Psalm\Type\Atomic\TNamedObject), Object(Psalm\Internal\MethodIdentifier), 'Amp\\Loop::defer', Object(Psalm\Storage\ClassLikeStorage), false)
#8 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php(179): Psalm\Internal\Analyzer\Statements\Expression\Call\StaticMethod\AtomicStaticCallAnalyzer::handleNamedCall(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Object(PhpParser\Node\Expr\StaticCall), Object(PhpParser\Node\Identifier), Object(Psalm\Context), Object(Psalm\Type\Atomic\TNamedObject), Array, 'Amp\\Loop', false, true)
#9 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php(212): Psalm\Internal\Analyzer\Statements\Expression\Call\StaticMethod\AtomicStaticCallAnalyzer::analyze(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Object(PhpParser\Node\Expr\StaticCall), Object(Psalm\Context), Object(Psalm\Type\Atomic\TNamedObject), false, false, false, true)
#10 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php(155): Psalm\Internal\Analyzer\Statements\Expression\Call\StaticCallAnalyzer::analyze(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Object(PhpParser\Node\Expr\StaticCall), Object(Psalm\Context))
#11 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php(44): Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer::handleExpression(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Object(PhpParser\Node\Expr\StaticCall), Object(Psalm\Context), false, NULL, true)
#12 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php(522): Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer::analyze(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Object(PhpParser\Node\Expr\StaticCall), Object(Psalm\Context), false, NULL, true)
#13 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php(171): Psalm\Internal\Analyzer\StatementsAnalyzer::analyzeStatement(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Object(PhpParser\Node\Stmt\Expression), Object(Psalm\Context), NULL)
#14 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php(355): Psalm\Internal\Analyzer\StatementsAnalyzer->analyze(Array, Object(Psalm\Context))
#15 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php(474): Psalm\Internal\Analyzer\Statements\Block\TryAnalyzer::analyze(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Object(PhpParser\Node\Stmt\TryCatch), Object(Psalm\Context))
#16 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php(171): Psalm\Internal\Analyzer\StatementsAnalyzer::analyzeStatement(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Object(PhpParser\Node\Stmt\TryCatch), Object(Psalm\Context), Object(Psalm\Context))
#17 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php(654): Psalm\Internal\Analyzer\StatementsAnalyzer->analyze(Array, Object(Psalm\Context), Object(Psalm\Context), true)
#18 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/ClassAnalyzer.php(1978): Psalm\Internal\Analyzer\FunctionLikeAnalyzer->analyze(Object(Psalm\Context), Object(Psalm\Internal\Provider\NodeDataProvider), Object(Psalm\Context))
#19 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/ClassAnalyzer.php(1650): Psalm\Internal\Analyzer\ClassAnalyzer->analyzeClassMethod(Object(PhpParser\Node\Stmt\ClassMethod), Object(Psalm\Storage\ClassLikeStorage), Object(Psalm\Internal\Analyzer\TraitAnalyzer), Object(Psalm\Context), Object(Psalm\Context))
#20 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/ClassAnalyzer.php(764): Psalm\Internal\Analyzer\ClassAnalyzer->analyzeTraitUse(Object(Psalm\Aliases), Object(PhpParser\Node\Stmt\TraitUse), Object(Psalm\Internal\Analyzer\ProjectAnalyzer), Object(Psalm\Storage\ClassLikeStorage), Object(Psalm\Context), Object(Psalm\Context), NULL)
#21 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FileAnalyzer.php(211): Psalm\Internal\Analyzer\ClassAnalyzer->analyze(Object(Psalm\Context), Object(Psalm\Context))
#22 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Codebase/Analyzer.php(340): Psalm\Internal\Analyzer\FileAnalyzer->analyze(NULL)
#23 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Fork/Pool.php(193): Psalm\Internal\Codebase\Analyzer->Psalm\Internal\Codebase\{closure}(592, '/opt/moodle/mod...')
#24 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Codebase/Analyzer.php(464): Psalm\Internal\Fork\Pool->__construct(Array, Object(Closure), Object(Closure), Object(Closure), Object(Closure))
#25 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Codebase/Analyzer.php(269): Psalm\Internal\Codebase\Analyzer->doAnalysis(Object(Psalm\Internal\Analyzer\ProjectAnalyzer), 7)
#26 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php(1182): Psalm\Internal\Codebase\Analyzer->analyzeFiles(Object(Psalm\Internal\Analyzer\ProjectAnalyzer), 7, false, false)
#27 /opt/psalm/vendor/vimeo/psalm/src/psalm.php(678): Psalm\Internal\Analyzer\ProjectAnalyzer->checkPaths(Array)
#28 /opt/psalm/vendor/vimeo/psalm/psalm(2): require_once('/opt/psalm/vend...')
#29 {main} in /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Fork/Pool.php:357
Stack trace:
#0 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Fork/Pool.php(389): Psalm\Internal\Fork\Pool->readResultsFromChildren()
#1 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Codebase/Analyzer.php(473): Psalm\Internal\Fork\Pool->wait()
#2 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Codebase/Analyzer.php(269): Psalm\Internal\Codebase\Analyzer->doAnalysis(Object(Psalm\Internal\Analyzer\ProjectAnalyzer), 7)
#3 /opt/psalm/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php(1182): Psalm\Internal\Codebase\Analyzer->analyzeFiles(Object(Psalm\Internal\Analyzer\ProjectAnalyzer), 7, false, false)
#4 /opt/psalm/vendor/vimeo/psalm/src/psalm.php(678): Psalm\Internal\Analyzer\ProjectAnalyzer->checkPaths(Array)
#5 /opt/psalm/vendor/vimeo/psalm/psalm(2): require_once('/opt/psalm/vend...')
#6 {main}
(Psalm 4.3.1@2feba22a005a18bf31d4c7b9bdb9252c73897476 crashed due to an uncaught Throwable)

[orklah]

Can you please explain in length exactly what you want to do?

I don't get why you would want Psalm to analyze the library (instead of your own code calling the library)

[klebann]

I only have this in my code:

global $DB;
$sql = "here is sql query...";
$params = ['id' => $id]
$items = $DB->get_records_sql($sql, $params);

I want to find all usuage of function $DB->get_records_sql in my code
and then i want to check if parrametrs are corect.

[orklah]

So the code you copy/pasted do exactly that.

It will send you an $expr object that represents $DB->get_records_sql($sql, $params);

This $expr object can be navigated (it's a PHP-Parser node) to find everything in the expression (the method name, the params).

Psalm will then help you identify the types for everything (for example, the $sql type and $params type)

EDIT: you shouldn't analyse the moodle library, only your own code. Psalm will retrieve and analyse what it needs in your dependancies

[klebann]

autoupdate.php:

function checklist_course_completion_autoupdate($courseid, $userid) {
    global $DB;

    $sql = "SELECT i.id AS itemid, i.checklist, cl.teacheredit, ck.*, cl.course
              FROM {checklist_item} i
              JOIN {checklist} cl ON cl.id = i.checklist
              LEFT JOIN {checklist_check} ck ON ck.item = i.id AND ck.userid = :userid
             WHERE cl.autoupdate > 0 AND i.linkcourseid = :courseid AND i.itemoptional < :heading";
    $params = ['userid' => $userid, 'courseid' => $courseid, 'heading' => 2];
    $itemchecks = $DB->get_records_sql($sql, $params);
    if (!$itemchecks) {
        return;
    }

    checklist_completion_update_checks($userid, $itemchecks, true);
}

public static function afterMethodCallAnalysis(
        Expr $expr,
        string $method_id,
        string $appearing_method_id,
        string $declaring_method_id,
        Context $context,
        StatementsSource $statements_source,
        Codebase $codebase,
        array &$file_replacements = [],
        Union &$return_type_candidate = null
    ) : void {
        echo $expr->name."\n";

outputs:

➜ vendor/bin/psalm --config=psalm-plugin.xml --no-diff autoupdate.php
Scanning files...
Analyzing files...

░
------------------------------
No errors found!
------------------------------

it doesn't output get_record_sql function...

[klebann]

if I add this to autoupdate.php:

class PrivateTests{
    private function privateMethod1(): void {echo 'private content';}
    private function privateMethod2(): void {echo 'private content';}
    private function privateMethod3(): void {echo 'private content';}

    public function legitCall(): void {
        $this->privateMethod1(); //This won't be flagged
    }

    public function proxyByParam(PrivateTests $a): void {
        $a->privateMethod2(); //This will be flagged
    }

    public function proxyInstantiation(): void {
        $a = new self();
        $a->privateMethod3(); //This will be flagged
    }

}

$a = new PrivateTests();
$b = new PrivateTests();
$a->legitCall();

$a->proxyByParam($b);
$a->proxyInstantiation();

then output looks like this:

✘130 ➜ vendor/bin/psalm --config=psalm-plugin.xml --no-diff autoupdate.php
Scanning files...
Analyzing files...

legitCall
proxyByParam
proxyInstantiation
privateMethod1
privateMethod2
privateMethod3
E

ERROR: PrivateStrictVisibility - autoupdate.php:185:13 - Calling private method privateMethod2 via proxy (see https://psalm.dev/000)
        $a->privateMethod2(); //This will be flagged

[orklah]

That's weird, I'm not sure I see the issue.

Can you open a repo to reproduce the issue? I'll take a look tonight

[klebann]

Maybe it's not method but just function...

[orklah]

function is strpos(), method is datetime->add() for example so method should be more adapted to your need

[klebann]

@orklah https://github.com/klebann/psalb-moodle-plugin
Here is repo which you asked for.
All information how i use that is in README.md

[orklah]

Thanks, I have identified the issue here.

Psalm will only call the hook after a somewhat successful analysis of a method call. On your example, every call to get_records_sql is made on a global variable $DB that is not defined. Psalm is not able to call the hook with so few elements.

It should be fixable with a few tweaks, but I don't know Moodle at all, this may be harder than I think.

• You must know the type of the $DB variable. This seems to be the hard part. I checked the documentation who says The $DB global object is an instance of the moodle_database class but I couldn't find this class in your repo. Not sure why
• You must then define the type of $DB. The cleaner(and more compatible) way to do that would be to add a /** @var moodle_database $DB */ everywhere you import the global variable. This can prove tedious though. However, Psalm will allow you to define the type of a global variable once: https://psalm.dev/docs/running_psalm/configuration/#globals

After that, it should be better. To prove my point, I added

class moodle_database{
    function get_records_sql(){

    }
}

in the autoupdate.php file and

global $DB, $USER;
    /** @var moodle_database $DB */

in the checklist_completion_autoupdate, and the call to the hook is now made.

Can you tell me more about the kind of check you want to make on the arguments? If it's something basic enough, Psalm will surely do it natively without the need of a plugin once you add the global variable in the config

EDIT: if you can't find a way to type $DB, tell me, there could be another way to proceed, but it could prove a lot more tedious

[klebann]

$DB is defined in moodle library, in plugin you only use it as global object.
https://docs.moodle.org/dev/Data_manipulation_API#DB_object

Adding definition of class moodle_database or commenting global variables is working, but it's not what i need. I want to check many plugins of others developers. None of them will add /** @var moodle_database $DB */ for moodle plugin. So we can't change the code of moodle plugin for this purpose.

The plugin am I working with is only an example. It's not even mine. I just want to analyse the code of plugins for security reasons.

Checking arguments is another step. If you are intrested, I wan't to check if this function uses Placeholders
https://docs.moodle.org/dev/Data_manipulation_API#Placeholders

So function get_record_sql is potentialy dangerous. I want to make sure that author of plugin is using Placeholders for arguments in this method. This will prevent SQL-injection.

[orklah]

Ok, I see, but if you have access to psalm-plugin.xml to suppress issues, what's preventing you to add the type in it anyway?

Anyway, if you don't want to, there are two more solutions that I can see:

• You can use another hook, the afterAnalyzeFile to retrieve all the statements for a given file. You'll be able to navigate all those to get to any MethodCall you want. This is kinda tedious however, because you have to recursively navigate through the tree of expressions and statement to get to the MethodCall (for example, the call can be in a try block, itself in an if block, in a foreach, in another if, in a method, in a class which could be the 4th class of the file.
  There are hundreds od statement and expressions types and each must be handled specifically. If this solution tempt you, I'm currently working precisely on this so I could publish my progression and you could build on it.
• The second solution, based on what you tell me is to ditch Psalm altogether. I'm under the impression that you don't need any functionnality Psalm bring to the mix, if I'm not wrong, all you need is https://github.com/nikic/PHP-Parser (which is the underlying structure of statement/expressions I was talking of above. You'd probably need to build a recursive directory crawler and then parse every php file with PHP-Parser and you'd get the same list of expressions/statements as the afterAnalyzeFile hook. It should however be much faster, as all the psalm analysis will be removed

[orklah]

Hmm, I may be wrong on the second option. If the argument is a variable and not directly a literal in situ, you'll be stuck if you use only Php-Parser. Psalm on the other hand will have propagated the content from variable to variable to be able to tell you what's inside.

[klebann]

Ok, I see, but if you have access to psalm-plugin.xml to suppress issues, what's preventing you to add the type in it anyway?

Because I can add psalm-plugin.xml to any moodle plugin. This can be just downloaded and used. Code can be diferend in many cases and I'm not intrested in finding manually every usuage of global $DB and adding comment there.

Hmm, I may be wrong on the second option. If the argument is a variable and not directly a literal in situ, you'll be stuck if you use only Php-Parser. Psalm on the other hand will have propagated the content from variable to variable to be able to tell you what's inside.

That's the reason why I want to use Psalm. Argument can be a variable.
I must check if argument is a static string, if not
I must check if it is a concatenate string, if yes I must check every string that connect to this one... and so on.
I think Psalm could be usefull for making this. I would get Headache making this only using PHP-Parser.

I writed phpgrep function for searching all usages of $DB->...sql...(...) in code:
phpgrep <location_of_code> '${"x:var"}->$f(${"*"})' 'x=$DB' 'f~sql'
but this only search for this function and doesn't allow me to check arguments...

[weirdan]

Can't this be done with appropriate stubs and Psalm's taint analysis? The thing you're trying to do looks quite similar as far as I can see.

[orklah]

Because I can add psalm-plugin.xml to any moodle plugin. This can be just downloaded and used. Code can be diferend in many cases and I'm not intrested in finding manually every usuage of global $DB and adding comment there.

Please re-read this:

Psalm will allow you to define the type of a global variable once: https://psalm.dev/docs/running_psalm/configuration/#globals

[klebann]

Okey, I added:

<globals>
        <var name="DB" type="moodle_database|null" />
    </globals>

Then I added:

class moodle_database{
    function get_records_sql(){

    }
}

Now I'm able to find all get_records_sql() methods. But how to do the same without adding this extra moodle_database class, the definition of this class is in .../lib/setup.php, strictly speaking in moodle files. Is that possible?

[klebann]

[orklah]

You shouldn't need to add the class yourself. I added it because it was absent from your repo. You just have to ensure psalm is able to find the class somehow. This is usually done with autoloading, documentation should tell you more about this if autoloading does not work for you

[klebann]

Is this correct? Seems like it is working

[orklah]

I'd have said it can't work because your require is made after the return, but PHP must require your file (and include it in its known classes) before executing the return, I learned something new 😄

It seems to work well, now you should be able to dump $expr to see what's inside. Just remember $expr comes from PHP-Parser so it doesn't include types from Psalm. You'll have to retrieve the types once you identify what you need in $expr

[orklah]

Beware, if you want to include your psalm-autoloader in any project, the composer class name is random so it won't match. I don't know how composer really works but you should be able to still use it somehow