fix: processing expressions in ActiveRecordAttributeArrayAnalyzer (#54)

Author: mspirkov

README.md

@@ -228,22 +228,24 @@ Customer::find()->with('orders.oops')->all();       // ✗ typo — no such rela
 
 #### Active Record condition validation
 
-`findOne()`, `findAll()`, and `deleteAll()` take a plain array condition (`['attribute' => value]`, with an array value matched as an `IN (...)` condition) — and so does the second, condition argument of `updateAll()` / `updateAllCounters()`. Like `attributeLabels()` and `scenarios()`, this is never checked against the model until the query actually runs. This rule checks that every attribute name in a condition array exists on the queried ActiveRecord model (the same `@property`-aware resolution as `activeRecordRelationValidation`) and that its value's type is compatible with the attribute's declared type. Only array literals with a resolvable string key are checked; primary-key-only lookups (`findOne(1)`, `findOne([1, 2])`) and dynamically-built condition arrays are left alone.
+`findOne()`, `findAll()`, and `deleteAll()` take a plain array condition (`['attribute' => value]`, with an array value matched as an `IN (...)` condition) — and so does the second, condition argument of `updateAll()` / `updateAllCounters()`. Like `attributeLabels()` and `scenarios()`, this is never checked against the model until the query actually runs. This rule checks that every attribute name in a condition array exists on the queried ActiveRecord model (the same `@property`-aware resolution as `activeRecordRelationValidation`) and that its value's type is compatible with the attribute's declared type. Only array literals with a resolvable string key are checked; primary-key-only lookups (`findOne(1)`, `findOne([1, 2])`) and dynamically-built condition arrays are left alone. A value implementing `yii\db\ExpressionInterface` (e.g. `new Expression('NOW()')`) is accepted for any attribute regardless of its declared type — `yii\db\conditions\HashConditionBuilder` builds it as raw SQL instead of type-casting it, and does so per-value inside an `IN (...)` array too.
 
 ```php
 /**
  * @property int $id
  * @property int $status
+ * @property string $updated_at
  */
 final class Customer extends ActiveRecord { /* ... */ }
 
-Customer::findOne(1);                                // ✓ primary key lookup, not a condition hash
-Customer::findOne(['status' => 1]);                  // ✓
-Customer::findOne(['status' => [1, 2]]);             // ✓ IN (1, 2)
-Customer::findOne(['statuss' => 1]);                 // ✗ typo — unknown attribute
-Customer::findOne(['status' => '1']);                // ✗ wrong type — int expected
-Customer::deleteAll(['statuss' => 1]);               // ✗ typo — unknown attribute
-Customer::updateAll(['status' => 1], ['idd' => 5]);  // ✗ typo — unknown attribute in the condition
+Customer::findOne(1);                                         // ✓ primary key lookup, not a condition hash
+Customer::findOne(['status' => 1]);                           // ✓
+Customer::findOne(['status' => [1, 2]]);                      // ✓ IN (1, 2)
+Customer::findOne(['updated_at' => new Expression('NOW()')]); // ✓ raw SQL, not type-checked
+Customer::findOne(['statuss' => 1]);                          // ✗ typo — unknown attribute
+Customer::findOne(['status' => '1']);                         // ✗ wrong type — int expected
+Customer::deleteAll(['statuss' => 1]);                        // ✗ typo — unknown attribute
+Customer::updateAll(['status' => 1], ['idd' => 5]);           // ✗ typo — unknown attribute in the condition
 ```
 
 #### Active Record relations validation
@@ -296,21 +298,23 @@ final class OrderItem extends ActiveRecord { /* ... */ }
 
 #### Active Record update values validation
 
-`updateAll()`'s attribute values and `updateAllCounters()`'s counter values are the other plain array these two methods take — the values written into the row, as opposed to the WHERE condition `activeRecordConditionValidation` checks. This rule checks that every attribute name exists on the ActiveRecord model and that its value's type is compatible with the attribute's declared type; unlike a condition, these values are written as-is, so (unlike `activeRecordConditionValidation`) an array value is not treated as an `IN (...)` shorthand and is always a type mismatch.
+`updateAll()`'s attribute values and `updateAllCounters()`'s counter values are the other plain array these two methods take — the values written into the row, as opposed to the WHERE condition `activeRecordConditionValidation` checks. This rule checks that every attribute name exists on the ActiveRecord model and that its value's type is compatible with the attribute's declared type; unlike a condition, these values are written as-is, so (unlike `activeRecordConditionValidation`) an array value is not treated as an `IN (...)` shorthand and is always a type mismatch. As with a condition, a value implementing `yii\db\ExpressionInterface` is accepted for any attribute regardless of its declared type — `yii\db\QueryBuilder::prepareUpdateSets()` builds it as raw SQL instead of type-casting it.
 
 ```php
 /**
  * @property int $id
  * @property int $status
  * @property int $age
+ * @property string $updated_at
  */
 final class Customer extends ActiveRecord { /* ... */ }
 
-Customer::updateAll(['status' => 1], ['id' => 5]);   // ✓
-Customer::updateAll(['statuss' => 1]);               // ✗ typo — unknown attribute
-Customer::updateAll(['status' => 'active']);         // ✗ wrong type — int expected
-Customer::updateAllCounters(['age' => 1]);           // ✓
-Customer::updateAllCounters(['agee' => 1]);          // ✗ typo — unknown attribute
+Customer::updateAll(['status' => 1], ['id' => 5]);              // ✓
+Customer::updateAll(['updated_at' => new Expression('NOW()')]); // ✓ raw SQL, not type-checked
+Customer::updateAll(['statuss' => 1]);                          // ✗ typo — unknown attribute
+Customer::updateAll(['status' => 'active']);                    // ✗ wrong type — int expected
+Customer::updateAllCounters(['age' => 1]);                      // ✓
+Customer::updateAllCounters(['agee' => 1]);                     // ✗ typo — unknown attribute
 ```
 
 #### `BaseObject` instantiation validation

src/Analyzers/ActiveRecordAttributeArrayAnalyzer.php

@@ -13,7 +13,10 @@
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Rules\IdentifierRuleError;
 use PHPStan\Type\MixedType;
+use PHPStan\Type\ObjectType;
+use PHPStan\Type\TypeCombinator;
 use PHPStan\Type\VerbosityLevel;
+use yii\db\ExpressionInterface;
 
 final class ActiveRecordAttributeArrayAnalyzer
 {
@@ -121,7 +124,9 @@ private function validateAttributeValueType(
             return [];
         }
 
-        if (!$expectedType->accepts($actualType, true)->no()) {
+        $acceptedType = TypeCombinator::union($expectedType, new ObjectType(ExpressionInterface::class));
+
+        if (!$acceptedType->accepts($actualType, true)->no()) {
             return [];
         }
 

tests/Rules/ActiveRecordConditionValidationRuleTest.php

@@ -19,13 +19,13 @@ public function testRule(): void
         $this->analyse(
             [self::getDataFilePath('code')],
             [
-                [sprintf('Unknown attribute "statuss" for ActiveRecord %s in findOne() condition.', $customerClass), 25],
-                [sprintf('Value for attribute "status" on ActiveRecord %s in findOne() condition must be int, string given.', $customerClass), 26],
+                [sprintf('Unknown attribute "statuss" for ActiveRecord %s in findOne() condition.', $customerClass), 26],
                 [sprintf('Value for attribute "status" on ActiveRecord %s in findOne() condition must be int, string given.', $customerClass), 27],
-                [sprintf('Unknown attribute "emial" for ActiveRecord %s in findAll() condition.', $customerClass), 28],
-                [sprintf('Unknown attribute "statuss" for ActiveRecord %s in deleteAll() condition.', $customerClass), 29],
-                [sprintf('Unknown attribute "idd" for ActiveRecord %s in updateAll() condition.', $customerClass), 30],
-                [sprintf('Unknown attribute "statuss" for ActiveRecord %s in updateAllCounters() condition.', $customerClass), 31],
+                [sprintf('Value for attribute "status" on ActiveRecord %s in findOne() condition must be int, string given.', $customerClass), 28],
+                [sprintf('Unknown attribute "emial" for ActiveRecord %s in findAll() condition.', $customerClass), 29],
+                [sprintf('Unknown attribute "statuss" for ActiveRecord %s in deleteAll() condition.', $customerClass), 30],
+                [sprintf('Unknown attribute "idd" for ActiveRecord %s in updateAll() condition.', $customerClass), 31],
+                [sprintf('Unknown attribute "statuss" for ActiveRecord %s in updateAllCounters() condition.', $customerClass), 32],
             ],
         );
     }

tests/Rules/ActiveRecordUpdateValuesValidationRuleTest.php

@@ -19,11 +19,11 @@ public function testRule(): void
         $this->analyse(
             [self::getDataFilePath('code')],
             [
-                [sprintf('Unknown attribute "statuss" for ActiveRecord %s in updateAll() attributes.', $customerClass), 21],
-                [sprintf('Value for attribute "status" on ActiveRecord %s in updateAll() attributes must be int, string given.', $customerClass), 22],
-                [sprintf('Value for attribute "status" on ActiveRecord %s in updateAll() attributes must be int, array<int, int> given.', $customerClass), 23],
-                [sprintf('Unknown attribute "agee" for ActiveRecord %s in updateAllCounters() counters.', $customerClass), 24],
-                [sprintf('Value for attribute "age" on ActiveRecord %s in updateAllCounters() counters must be int, string given.', $customerClass), 25],
+                [sprintf('Unknown attribute "statuss" for ActiveRecord %s in updateAll() attributes.', $customerClass), 22],
+                [sprintf('Value for attribute "status" on ActiveRecord %s in updateAll() attributes must be int, string given.', $customerClass), 23],
+                [sprintf('Value for attribute "status" on ActiveRecord %s in updateAll() attributes must be int, array<int, int> given.', $customerClass), 24],
+                [sprintf('Unknown attribute "agee" for ActiveRecord %s in updateAllCounters() counters.', $customerClass), 25],
+                [sprintf('Value for attribute "age" on ActiveRecord %s in updateAllCounters() counters must be int, string given.', $customerClass), 26],
             ],
         );
     }

tests/Rules/Data/ActiveRecordConditionValidation/code.php

@@ -4,6 +4,7 @@
 
 use MSpirkov\Yii2\PHPStan\Tests\Rules\Source\ActiveRecordConditionValidation\Customer;
 use MSpirkov\Yii2\PHPStan\Tests\Rules\Source\ActiveRecordConditionValidation\NotActiveRecord;
+use yii\db\Expression;
 
 final class ValidCustomerUsage
 {
@@ -75,6 +76,14 @@ public function run(): void
 
         // Attribute type is mixed — nothing meaningful to compare against.
         Customer::findOne(['extra' => 5]);
+
+        // ExpressionInterface bypasses dbTypecast in HashConditionBuilder::build(), so it's
+        // valid for any attribute regardless of its declared type.
+        Customer::findOne(['updated_at' => new Expression('NOW()')]);
+
+        // Same bypass applies per-value inside an IN condition — e.g. matching either
+        // a dynamically configured default status or one of two explicit ones.
+        Customer::findOne(['status' => [new Expression('(SELECT default_status FROM settings)'), 1, 2]]);
     }
 
     private function findByDynamicKey(string $dynamicKey): void

tests/Rules/Data/ActiveRecordUpdateValuesValidation/code.php

@@ -4,6 +4,7 @@
 
 use MSpirkov\Yii2\PHPStan\Tests\Rules\Source\ActiveRecordUpdateValuesValidation\Customer;
 use MSpirkov\Yii2\PHPStan\Tests\Rules\Source\ActiveRecordUpdateValuesValidation\NotActiveRecord;
+use yii\db\Expression;
 
 final class ValidCustomerUsage
 {
@@ -48,5 +49,9 @@ public function run(): void
         $className::updateAll(['statuss' => 1]);
         $methodName = 'updateAll';
         Customer::$methodName(['statuss' => 1]);
+
+        // ExpressionInterface bypasses dbTypecast in QueryBuilder::prepareUpdateSets(), so
+        // it's valid for any attribute regardless of its declared type.
+        Customer::updateAll(['updated_at' => new Expression('NOW()')]);
     }
 }

tests/Rules/Source/ActiveRecordConditionValidation/Customer.php

@@ -11,6 +11,7 @@
  * @property string $email
  * @property int $status
  * @property int $age
+ * @property string $updated_at
  * @property-read string $displayName
  * @property mixed $extra
  */

tests/Rules/Source/ActiveRecordUpdateValuesValidation/Customer.php

@@ -10,6 +10,7 @@
  * @property int $id
  * @property int $status
  * @property int $age
+ * @property string $updated_at
  */
 final class Customer extends ActiveRecord
 {