fix: improve binary & unary expression applicability check (zenstackh…

…q#589)
Eventiva · Jul 19, 2023 · eb2d896 · eb2d896
1 parent 87be443
commit eb2d896
Show file tree

Hide file tree

Showing 3 changed files with 163 additions and 2 deletions.
diff --git a/packages/schema/src/language-server/validator/expression-validator.ts b/packages/schema/src/language-server/validator/expression-validator.ts
@@ -1,4 +1,4 @@
-import { BinaryExpr, Expression, isBinaryExpr, isEnum } from '@zenstackhq/language/ast';
+import { BinaryExpr, Expression, ExpressionType, isBinaryExpr, isEnum } from '@zenstackhq/language/ast';
 import { ValidationAcceptor } from 'langium';
 import { isAuthInvocation } from '../../utils/ast-utils';
 import { AstValidator } from '../types';
@@ -48,6 +48,51 @@ export default class ExpressionValidator implements AstValidator<Expression> {
                 }
                 break;
             }
+
+            case '>':
+            case '>=':
+            case '<':
+            case '<=':
+            case '&&':
+            case '||': {
+                let supportedShapes: ExpressionType[];
+                if (['>', '>=', '<', '<='].includes(expr.operator)) {
+                    supportedShapes = ['Int', 'Float', 'DateTime', 'Any'];
+                } else {
+                    supportedShapes = ['Boolean', 'Any'];
+                }
+
+                if (
+                    typeof expr.left.$resolvedType?.decl !== 'string' ||
+                    !supportedShapes.includes(expr.left.$resolvedType.decl)
+                ) {
+                    accept('error', `invalid operand type for "${expr.operator}" operator`, {
+                        node: expr.left,
+                    });
+                    return;
+                }
+                if (
+                    typeof expr.right.$resolvedType?.decl !== 'string' ||
+                    !supportedShapes.includes(expr.right.$resolvedType.decl)
+                ) {
+                    accept('error', `invalid operand type for "${expr.operator}" operator`, {
+                        node: expr.right,
+                    });
+                    return;
+                }
+
+                // DateTime comparison is only allowed between two DateTime values
+                if (expr.left.$resolvedType.decl === 'DateTime' && expr.right.$resolvedType.decl !== 'DateTime') {
+                    accept('error', 'incompatible operand types', { node: expr });
+                } else if (
+                    expr.right.$resolvedType.decl === 'DateTime' &&
+                    expr.left.$resolvedType.decl !== 'DateTime'
+                ) {
+                    accept('error', 'incompatible operand types', { node: expr });
+                }
+
+                break;
+            }
         }
     }
 

diff --git a/packages/schema/src/language-server/zmodel-linker.ts b/packages/schema/src/language-server/zmodel-linker.ts
@@ -215,7 +215,13 @@ export class ZModelLinker extends DefaultLinker {
 
     private resolveUnary(node: UnaryExpr, document: LangiumDocument<AstNode>, extraScopes: ScopeProvider[]) {
         this.resolve(node.operand, document, extraScopes);
-        node.$resolvedType = node.operand.$resolvedType;
+        switch (node.operator) {
+            case '!':
+                this.resolveToBuiltinTypeOrDecl(node, 'Boolean');
+                break;
+            default:
+                throw Error(`Unsupported unary operator: ${node.operator}`);
+        }
     }
 
     private resolveObject(node: ObjectExpr, document: LangiumDocument<AstNode>, extraScopes: ScopeProvider[]) {

diff --git a/packages/schema/tests/schema/validation/attribute-validation.test.ts b/packages/schema/tests/schema/validation/attribute-validation.test.ts
@@ -388,6 +388,116 @@ describe('Attribute tests', () => {
         ).toContain(`Value is not assignable to parameter`);
     });
 
+    it('policy expressions', async () => {
+        await loadModel(`
+           ${prelude}
+           model A {
+               id String @id
+               x Int
+               x1 Int
+               y DateTime
+               y1 DateTime
+               z Float
+               z1 Decimal
+               foo Boolean
+               bar Boolean
+
+               @@allow('all', x > 0)
+               @@allow('all', x > x1)
+               @@allow('all', y >= y1)
+               @@allow('all', z < z1)
+               @@allow('all', z1 < z)
+               @@allow('all', x < z)
+               @@allow('all', x < z1)
+               @@allow('all', foo && bar)
+               @@allow('all', foo || bar)
+               @@allow('all', !foo)
+           }
+        `);
+
+        expect(
+            await loadModelWithError(`
+            ${prelude}
+            model A {
+                id String @id
+                x String
+                @@allow('all', x > 0)
+            }
+        `)
+        ).toContain('invalid operand type for ">" operator');
+
+        expect(
+            await loadModelWithError(`
+            ${prelude}
+            model A {
+                id String @id
+                x String
+                @@allow('all', x < 0)
+            }
+        `)
+        ).toContain('invalid operand type for "<" operator');
+
+        expect(
+            await loadModelWithError(`
+            ${prelude}
+            model A {
+                id String @id
+                x String
+                y String
+                @@allow('all', x < y)
+            }
+        `)
+        ).toContain('invalid operand type for "<" operator');
+
+        expect(
+            await loadModelWithError(`
+            ${prelude}
+            model A {
+                id String @id
+                x String
+                y String
+                @@allow('all', x <= y)
+            }
+        `)
+        ).toContain('invalid operand type for "<=" operator');
+
+        expect(
+            await loadModelWithError(`
+            ${prelude}
+            model A {
+                id String @id
+                x Int
+                y DateTime
+                @@allow('all', x <= y)
+            }
+        `)
+        ).toContain('incompatible operand types');
+
+        expect(
+            await loadModelWithError(`
+            ${prelude}
+            model A {
+                id String @id
+                x String
+                y String
+                @@allow('all', x && y)
+            }
+        `)
+        ).toContain('invalid operand type for "&&" operator');
+
+        expect(
+            await loadModelWithError(`
+            ${prelude}
+            model A {
+                id String @id
+                x String
+                y String
+                @@allow('all', x || y)
+            }
+        `)
+        ).toContain('invalid operand type for "||" operator');
+    });
+
     it('policy filter function check', async () => {
         await loadModel(`
             ${prelude}