ModelCriteria::delete() -> $this

[spidfire]

I got an error not 100% sure it's a PHP compat issue but it does look like it

Details
Type: Error
Code: 0
Message: Using $this when not in object context
File:  ..../vendor/propel/propel/src/Propel/Runtime/ActiveQuery/ModelCriteria.php
Line: 1497
Trace
#0 ..../src/Models/Environment/Base/EnvironmentToServerQuery.php(700): Propel\Runtime\ActiveQuery\ModelCriteria::delete()

After some dive into the code I came across this issue:

It generates a static reference ModelCriteria::delete and inside this ref there is a test on if (0 === count($this->getMap())) { which refers to $this.

Generated code from a Base Query file:

    return $con->transaction(static function () use ($con, $criteria) {
            $affectedRows = 0; // initialize var to track total num of affected rows

            EnvironmentToServerTableMap::removeInstanceFromPool($criteria);

            $affectedRows += ModelCriteria::delete($con);
            EnvironmentToServerTableMap::clearRelatedInstancePool();

            return $affectedRows;
        });

https://github.com/propelorm/Propel2/blob/776f746747c62962f4b70b3a3ac1de2b249ffd35/src/Propel/Generator/Builder/Om/QueryBuilder.php#L1795

https://github.com/propelorm/Propel2/blob/776f746747c62962f4b70b3a3ac1de2b249ffd35/src/Propel/Runtime/ActiveQuery/ModelCriteria.php#L1499

[dereuromark]

What PHP version are you using? I am a bit surprised that the current 7.1-7.4 test suite doesn't emit this kind of error yet, as I am sure this would have to also been used internally somewhere.

[spidfire]

7.4.8 but now I switched to 7.3.20 and I still got the same error... I do run on CentOS maybe It's configured more strict by default?

[spidfire]

<?php

class ModelCriteria {
    
    function delete(){
        echo "Works";        
    }    
}
ModelCriteria::delete();

A short test on https://sandbox.onlinephpfunctions.com/

5.3.29 -> Just works 5.4 -> Strict -> Non-static method Test::test() cannot be called statically 7 -> Deprecated Non-static method TestClass::test() should not be called statically in

The only way I could make it work is by using:

error_reporting(E_ALL ^ E_DEPRECATED);

[spidfire]

<?php
error_reporting(E_ALL ^ E_DEPRECATED);

class ModelCriteria {
    
    private function getMap(){
        return ['yolo' => 'test'];
    }
    public function delete(ConnectionInterface $con = null) {
        if(0 === count($this->getMap())){
            echo "err";
        }
        echo "Works";
        
    }
    
}

ModelCriteria::delete();

Creates the same error

<br />
<b>Fatal error</b>:  Uncaught Error: Using $this when not in object context in [...][...]:10
Stack trace:
#0 [...][...](20): ModelCriteria::delete()
#1 {main}
  thrown in <b>[...][...]</b> on line <b>10</b><br />

[dereuromark]

Interesting That is a pretty dirty method in general. delete() should never be called statically due to the many inside dynamic calls (>= 4).

[spidfire]

I'm still puzzled by why It's only bugging out on this environment and not the others. I've got like 8 other systems running with propel

[spidfire]

Hmmm after I rebuild the code it seemed fine again. I think my PhpCsFixerV2 has broken something

This is the change that broke it I think:

-        return $con->transaction(static function () use ($con, $criteria) {
+        return $con->transaction(function () use ($con, $criteria) {

[spidfire]

But still a good piece of code to review

[dereuromark]

Do you want to make a PR here for whats left to do?