From 3146d12f0721b38743c003c64a4f7cee84d5c072 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Wed, 19 Jul 2023 13:47:49 +0800 Subject: [PATCH 1/6] fix: improve binary & unary expression applicability check --- .../validator/expression-validator.ts | 69 +++++++++- .../validation/attribute-validation.test.ts | 121 ++++++++++++++++++ 2 files changed, 189 insertions(+), 1 deletion(-) diff --git a/packages/schema/src/language-server/validator/expression-validator.ts b/packages/schema/src/language-server/validator/expression-validator.ts index 18c826158..428a9ab69 100644 --- a/packages/schema/src/language-server/validator/expression-validator.ts +++ b/packages/schema/src/language-server/validator/expression-validator.ts @@ -1,7 +1,8 @@ -import { BinaryExpr, Expression, isBinaryExpr, isEnum } from '@zenstackhq/language/ast'; +import { BinaryExpr, Expression, ExpressionType, UnaryExpr, isBinaryExpr, isEnum } from '@zenstackhq/language/ast'; import { ValidationAcceptor } from 'langium'; import { isAuthInvocation } from '../../utils/ast-utils'; import { AstValidator } from '../types'; +import { typeAssignable } from './utils'; /** * Validates expressions. @@ -31,6 +32,10 @@ export default class ExpressionValidator implements AstValidator { case 'BinaryExpr': this.validateBinaryExpr(expr, accept); break; + + case 'UnaryExpr': + this.validateUnaryExpr(expr, accept); + break; } } @@ -48,6 +53,68 @@ export default class ExpressionValidator implements AstValidator { } 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; + } + } + } + + private validateUnaryExpr(expr: UnaryExpr, accept: ValidationAcceptor) { + switch (expr.operator) { + case '!': { + const supportedShapes: ExpressionType[] = ['Boolean', 'Any']; + if ( + !expr.operand.$resolvedType?.decl || + !supportedShapes.includes(expr.operand.$resolvedType.decl as ExpressionType) + ) { + accept('error', `invalid operand type for "${expr.operator}" operator`, { + node: expr.operand, + }); + } + break; + } } } diff --git a/packages/schema/tests/schema/validation/attribute-validation.test.ts b/packages/schema/tests/schema/validation/attribute-validation.test.ts index d5c5f488c..bd44f0820 100644 --- a/packages/schema/tests/schema/validation/attribute-validation.test.ts +++ b/packages/schema/tests/schema/validation/attribute-validation.test.ts @@ -388,6 +388,127 @@ 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'); + + expect( + await loadModelWithError(` + ${prelude} + model A { + id String @id + x String + @@allow('all', !x) + } + `) + ).toContain('invalid operand type for "!" operator'); + }); + it('policy filter function check', async () => { await loadModel(` ${prelude} From 62825546b71229c27314770079235efb41dc7317 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Wed, 19 Jul 2023 13:50:22 +0800 Subject: [PATCH 2/6] fixes --- .../src/language-server/validator/expression-validator.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/schema/src/language-server/validator/expression-validator.ts b/packages/schema/src/language-server/validator/expression-validator.ts index 428a9ab69..8e2cf1700 100644 --- a/packages/schema/src/language-server/validator/expression-validator.ts +++ b/packages/schema/src/language-server/validator/expression-validator.ts @@ -106,8 +106,8 @@ export default class ExpressionValidator implements AstValidator { case '!': { const supportedShapes: ExpressionType[] = ['Boolean', 'Any']; if ( - !expr.operand.$resolvedType?.decl || - !supportedShapes.includes(expr.operand.$resolvedType.decl as ExpressionType) + typeof expr.operand.$resolvedType?.decl !== 'string' || + !supportedShapes.includes(expr.operand.$resolvedType.decl) ) { accept('error', `invalid operand type for "${expr.operator}" operator`, { node: expr.operand, From 9e32fca05f8e80e7d3ad48ad28d858777bbcbac8 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Wed, 19 Jul 2023 14:01:22 +0800 Subject: [PATCH 3/6] fix build --- .../schema/src/language-server/validator/expression-validator.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/schema/src/language-server/validator/expression-validator.ts b/packages/schema/src/language-server/validator/expression-validator.ts index 8e2cf1700..bde68ea9e 100644 --- a/packages/schema/src/language-server/validator/expression-validator.ts +++ b/packages/schema/src/language-server/validator/expression-validator.ts @@ -2,7 +2,6 @@ import { BinaryExpr, Expression, ExpressionType, UnaryExpr, isBinaryExpr, isEnum import { ValidationAcceptor } from 'langium'; import { isAuthInvocation } from '../../utils/ast-utils'; import { AstValidator } from '../types'; -import { typeAssignable } from './utils'; /** * Validates expressions. From 6f29bb828c473b8a378ce42977df01fbb7f03e96 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Wed, 19 Jul 2023 14:25:50 +0800 Subject: [PATCH 4/6] remove unary check --- .../validator/expression-validator.ts | 23 +------------------ 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/packages/schema/src/language-server/validator/expression-validator.ts b/packages/schema/src/language-server/validator/expression-validator.ts index bde68ea9e..63960996d 100644 --- 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, ExpressionType, UnaryExpr, 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'; @@ -31,10 +31,6 @@ export default class ExpressionValidator implements AstValidator { case 'BinaryExpr': this.validateBinaryExpr(expr, accept); break; - - case 'UnaryExpr': - this.validateUnaryExpr(expr, accept); - break; } } @@ -100,23 +96,6 @@ export default class ExpressionValidator implements AstValidator { } } - private validateUnaryExpr(expr: UnaryExpr, accept: ValidationAcceptor) { - switch (expr.operator) { - case '!': { - const supportedShapes: ExpressionType[] = ['Boolean', 'Any']; - if ( - typeof expr.operand.$resolvedType?.decl !== 'string' || - !supportedShapes.includes(expr.operand.$resolvedType.decl) - ) { - accept('error', `invalid operand type for "${expr.operator}" operator`, { - node: expr.operand, - }); - } - break; - } - } - } - private isCollectionPredicate(expr: Expression) { return isBinaryExpr(expr) && ['?', '!', '^'].includes(expr.operator); } From 52dec9cdbe392f1cf0440d4183d9c01fa4a326d3 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Wed, 19 Jul 2023 14:29:30 +0800 Subject: [PATCH 5/6] resolve unary expression' type correctly --- packages/schema/src/language-server/zmodel-linker.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/schema/src/language-server/zmodel-linker.ts b/packages/schema/src/language-server/zmodel-linker.ts index 5326ff6cc..4a8557bdd 100644 --- 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, 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, extraScopes: ScopeProvider[]) { From 0bc3f9b6eeb07defee041d9a34a340f70f92a05c Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Wed, 19 Jul 2023 14:36:20 +0800 Subject: [PATCH 6/6] fix test --- .../schema/validation/attribute-validation.test.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/packages/schema/tests/schema/validation/attribute-validation.test.ts b/packages/schema/tests/schema/validation/attribute-validation.test.ts index bd44f0820..448331fd7 100644 --- a/packages/schema/tests/schema/validation/attribute-validation.test.ts +++ b/packages/schema/tests/schema/validation/attribute-validation.test.ts @@ -496,17 +496,6 @@ describe('Attribute tests', () => { } `) ).toContain('invalid operand type for "||" operator'); - - expect( - await loadModelWithError(` - ${prelude} - model A { - id String @id - x String - @@allow('all', !x) - } - `) - ).toContain('invalid operand type for "!" operator'); }); it('policy filter function check', async () => {