fix: Show validation error for the field comparison not in the same model (zenstackhq#912)

Author: jiashengguo

packages/schema/src/language-server/validator/expression-validator.ts

@@ -4,12 +4,15 @@ import {
     ExpressionType,
     isDataModel,
     isEnum,
+    isMemberAccessExpr,
     isNullExpr,
     isThisExpr,
+    isDataModelField,
+    isLiteralExpr,
 } from '@zenstackhq/language/ast';
-import { isDataModelFieldReference } from '@zenstackhq/sdk';
+import { isDataModelFieldReference, isEnumFieldReference } from '@zenstackhq/sdk';
 import { ValidationAcceptor } from 'langium';
-import { isAuthInvocation, isCollectionPredicate } from '../../utils/ast-utils';
+import { getContainingDataModel, isAuthInvocation, isCollectionPredicate } from '../../utils/ast-utils';
 import { AstValidator } from '../types';
 import { typeAssignable } from './utils';
 
@@ -124,6 +127,24 @@ export default class ExpressionValidator implements AstValidator<Expression> {
                     accept('error', 'incompatible operand types', { node: expr });
                     break;
                 }
+                // not supported:
+                //   - foo.a == bar
+                //   - foo.user.id == userId
+                // except:
+                //   - future().userId == userId
+                if(isMemberAccessExpr(expr.left) &&  isDataModelField(expr.left.member.ref) && expr.left.member.ref.$container != getContainingDataModel(expr)
+                || isMemberAccessExpr(expr.right) &&  isDataModelField(expr.right.member.ref) && expr.right.member.ref.$container != getContainingDataModel(expr))
+                {
+                    // foo.user.id == auth().id
+                    // foo.user.id == "123"
+                    // foo.user.id == null
+                    // foo.user.id == EnumValue
+                    if(!(this.isNotModelFieldExpr(expr.left) || this.isNotModelFieldExpr(expr.right)))
+                    {
+                        accept('error', 'comparison between fields of different models are not supported', { node: expr }); 
+                        break;  
+                    }                   
+                }
 
                 if (
                     (expr.left.$resolvedType?.nullable && isNullExpr(expr.right)) ||
@@ -183,4 +204,15 @@ export default class ExpressionValidator implements AstValidator<Expression> {
             }
         }
     }
+
+
+    private isNotModelFieldExpr(expr: Expression) {
+        return isLiteralExpr(expr) || isEnumFieldReference(expr) || isNullExpr(expr) || this.isAuthOrAuthMemberAccess(expr)
+    }
+
+    private isAuthOrAuthMemberAccess(expr: Expression) {
+        return isAuthInvocation(expr) || (isMemberAccessExpr(expr) && isAuthInvocation(expr.operand));
+    }
+
 }
+

packages/schema/src/language-server/zmodel-linker.ts

@@ -9,7 +9,6 @@ import {
     DataModelFieldType,
     Enum,
     EnumField,
-    Expression,
     ExpressionType,
     FunctionDecl,
     FunctionParam,
@@ -53,7 +52,7 @@ import {
 } from 'langium';
 import { match } from 'ts-pattern';
 import { CancellationToken } from 'vscode-jsonrpc';
-import { getAllDeclarationsFromImports, isAuthInvocation } from '../utils/ast-utils';
+import { getAllDeclarationsFromImports, isAuthInvocation, getContainingDataModel } from '../utils/ast-utils';
 import { mapBuiltinTypeToExpressionType } from './validator/utils';
 
 interface DefaultReference extends Reference {
@@ -292,24 +291,13 @@ export class ZModelLinker extends DefaultLinker {
                 }
             } else if (funcDecl.name === 'future' && isFromStdlib(funcDecl)) {
                 // future() function is resolved to current model
-                node.$resolvedType = { decl: this.getContainingDataModel(node) };
+                node.$resolvedType = { decl: getContainingDataModel(node) };
             } else {
                 this.resolveToDeclaredType(node, funcDecl.returnType);
             }
         }
     }
 
-    private getContainingDataModel(node: Expression): DataModel | undefined {
-        let curr: AstNode | undefined = node.$container;
-        while (curr) {
-            if (isDataModel(curr)) {
-                return curr;
-            }
-            curr = curr.$container;
-        }
-        return undefined;
-    }
-
     private resolveLiteral(node: LiteralExpr) {
         const type = match<LiteralExpr, ExpressionType>(node)
             .when(isStringLiteral, () => 'String')

packages/schema/src/utils/ast-utils.ts

@@ -156,3 +156,15 @@ export function getAllDeclarationsFromImports(documents: LangiumDocuments, model
 export function isCollectionPredicate(node: AstNode): node is BinaryExpr {
     return isBinaryExpr(node) && ['?', '!', '^'].includes(node.operator);
 }
+
+
+export function getContainingDataModel(node: Expression): DataModel | undefined {
+    let curr: AstNode | undefined = node.$container;
+    while (curr) {
+        if (isDataModel(curr)) {
+            return curr;
+        }
+        curr = curr.$container;
+    }
+    return undefined;
+}

packages/schema/tests/schema/validation/attribute-validation.test.ts

@@ -642,6 +642,35 @@ describe('Attribute tests', () => {
         `)
         ).toContain('comparison between model-typed fields are not supported');
 
+        expect(
+            await loadModelWithError(`
+            ${prelude}
+            model User {
+                id Int @id
+                lists List[]
+                todos Todo[]
+            }
+              
+            model List {
+                id Int @id
+                user User @relation(fields: [userId], references: [id])
+                userId Int
+                todos Todo[]
+            }
+              
+            model Todo {
+                id Int @id
+                user User @relation(fields: [userId], references: [id])
+                userId Int
+                list List @relation(fields: [listId], references: [id])
+                listId Int
+              
+                @@allow('read', list.user.id == userId)
+            }
+            
+        `)
+        ).toContain('comparison between fields of different models are not supported');
+
         expect(
             await loadModelWithError(`
             ${prelude}