Skip to content

Commit e0f0c80

Browse files
ymc9ymc9
committed
fix: @@validate should ignore fields that are not present
1 parent 2b12e09 commit e0f0c80

File tree

7 files changed

+340
-81
lines changed

7 files changed

+340
-81
lines changed

‎packages/schema/src/language-server/validator/attribute-application-validator.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
isEnum,
1616
isReferenceExpr,
1717
} from '@zenstackhq/language/ast';
18-
import { isFutureExpr, isRelationshipField, resolved } from '@zenstackhq/sdk';
18+
import { isDataModelFieldReference, isFutureExpr, isRelationshipField, resolved } from '@zenstackhq/sdk';
1919
import { ValidationAcceptor, streamAst } from 'langium';
2020
import pluralize from 'pluralize';
2121
import { AstValidator } from '../types';
@@ -151,6 +151,19 @@ export default class AttributeApplicationValidator implements AstValidator<Attri
151151
}
152152
}
153153

154+
@check('@@validate')
155+
private _checkValidate(attr: AttributeApplication, accept: ValidationAcceptor) {
156+
const condition = attr.args[0]?.value;
157+
if (
158+
condition &&
159+
streamAst(condition).some(
160+
(node) => isDataModelFieldReference(node) && isDataModel(node.$resolvedType?.decl)
161+
)
162+
) {
163+
accept('error', `\`@@validate\` condition cannot use relation fields`, { node: condition });
164+
}
165+
}
166+
154167
private validatePolicyKinds(
155168
kind: string,
156169
candidates: string[],

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

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,17 @@ import {
33
Expression,
44
ExpressionType,
55
isDataModel,
6+
isDataModelAttribute,
7+
isDataModelField,
68
isEnum,
9+
isLiteralExpr,
710
isMemberAccessExpr,
811
isNullExpr,
912
isThisExpr,
10-
isDataModelField,
11-
isLiteralExpr,
1213
} from '@zenstackhq/language/ast';
1314
import { isDataModelFieldReference, isEnumFieldReference } from '@zenstackhq/sdk';
14-
import { ValidationAcceptor } from 'langium';
15-
import { getContainingDataModel, isAuthInvocation, isCollectionPredicate } from '../../utils/ast-utils';
15+
import { AstNode, ValidationAcceptor } from 'langium';
16+
import { findUpAst, getContainingDataModel, isAuthInvocation, isCollectionPredicate } from '../../utils/ast-utils';
1617
import { AstValidator } from '../types';
1718
import { typeAssignable } from './utils';
1819

@@ -123,6 +124,17 @@ export default class ExpressionValidator implements AstValidator<Expression> {
123124

124125
case '==':
125126
case '!=': {
127+
if (this.isInValidationContext(expr)) {
128+
// in validation context, all fields are optional, so we should allow
129+
// comparing any field against null
130+
if (
131+
(isDataModelFieldReference(expr.left) && isNullExpr(expr.right)) ||
132+
(isDataModelFieldReference(expr.right) && isNullExpr(expr.left))
133+
) {
134+
return;
135+
}
136+
}
137+
126138
if (!!expr.left.$resolvedType?.array !== !!expr.right.$resolvedType?.array) {
127139
accept('error', 'incompatible operand types', { node: expr });
128140
break;
@@ -132,18 +144,24 @@ export default class ExpressionValidator implements AstValidator<Expression> {
132144
// - foo.user.id == userId
133145
// except:
134146
// - future().userId == userId
135-
if(isMemberAccessExpr(expr.left) && isDataModelField(expr.left.member.ref) && expr.left.member.ref.$container != getContainingDataModel(expr)
136-
|| isMemberAccessExpr(expr.right) && isDataModelField(expr.right.member.ref) && expr.right.member.ref.$container != getContainingDataModel(expr))
137-
{
147+
if (
148+
(isMemberAccessExpr(expr.left) &&
149+
isDataModelField(expr.left.member.ref) &&
150+
expr.left.member.ref.$container != getContainingDataModel(expr)) ||
151+
(isMemberAccessExpr(expr.right) &&
152+
isDataModelField(expr.right.member.ref) &&
153+
expr.right.member.ref.$container != getContainingDataModel(expr))
154+
) {
138155
// foo.user.id == auth().id
139156
// foo.user.id == "123"
140157
// foo.user.id == null
141158
// foo.user.id == EnumValue
142-
if(!(this.isNotModelFieldExpr(expr.left) || this.isNotModelFieldExpr(expr.right)))
143-
{
144-
accept('error', 'comparison between fields of different models are not supported', { node: expr });
145-
break;
146-
}
159+
if (!(this.isNotModelFieldExpr(expr.left) || this.isNotModelFieldExpr(expr.right))) {
160+
accept('error', 'comparison between fields of different models are not supported', {
161+
node: expr,
162+
});
163+
break;
164+
}
147165
}
148166

149167
if (
@@ -205,14 +223,17 @@ export default class ExpressionValidator implements AstValidator<Expression> {
205223
}
206224
}
207225

226+
private isInValidationContext(node: AstNode) {
227+
return findUpAst(node, (n) => isDataModelAttribute(n) && n.decl.$refText === '@@validate');
228+
}
208229

209230
private isNotModelFieldExpr(expr: Expression) {
210-
return isLiteralExpr(expr) || isEnumFieldReference(expr) || isNullExpr(expr) || this.isAuthOrAuthMemberAccess(expr)
231+
return (
232+
isLiteralExpr(expr) || isEnumFieldReference(expr) || isNullExpr(expr) || this.isAuthOrAuthMemberAccess(expr)
233+
);
211234
}
212235

213236
private isAuthOrAuthMemberAccess(expr: Expression) {
214237
return isAuthInvocation(expr) || (isMemberAccessExpr(expr) && isAuthInvocation(expr.operand));
215238
}
216-
217239
}
218-

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,6 @@ export function isCollectionPredicate(node: AstNode): node is BinaryExpr {
157157
return isBinaryExpr(node) && ['?', '!', '^'].includes(node.operator);
158158
}
159159

160-
161160
export function getContainingDataModel(node: Expression): DataModel | undefined {
162161
let curr: AstNode | undefined = node.$container;
163162
while (curr) {
@@ -167,4 +166,18 @@ export function getContainingDataModel(node: Expression): DataModel | undefined
167166
curr = curr.$container;
168167
}
169168
return undefined;
170-
}
169+
}
170+
171+
/**
172+
* Walk upward from the current AST node to find the first node that satisfies the predicate.
173+
*/
174+
export function findUpAst(node: AstNode, predicate: (node: AstNode) => boolean): AstNode | undefined {
175+
let curr: AstNode | undefined = node;
176+
while (curr) {
177+
if (predicate(curr)) {
178+
return curr;
179+
}
180+
curr = curr.$container;
181+
}
182+
return undefined;
183+
}

‎packages/schema/src/utils/typescript-expression-transformer.ts

Lines changed: 73 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
InvocationExpr,
88
isDataModel,
99
isEnumField,
10+
isNullExpr,
1011
isThisExpr,
1112
LiteralExpr,
1213
MemberAccessExpr,
@@ -168,13 +169,13 @@ export class TypeScriptExpressionTransformer {
168169
const max = getLiteral<number>(args[2]);
169170
let result: string;
170171
if (min === undefined) {
171-
result = `(${field}?.length > 0)`;
172+
result = this.ensureBooleanTernary(field, `${field}?.length > 0`);
172173
} else if (max === undefined) {
173-
result = `(${field}?.length >= ${min})`;
174+
result = this.ensureBooleanTernary(field, `${field}?.length >= ${min}`);
174175
} else {
175-
result = `(${field}?.length >= ${min} && ${field}?.length <= ${max})`;
176+
result = this.ensureBooleanTernary(field, `${field}?.length >= ${min} && ${field}?.length <= ${max}`);
176177
}
177-
return this.ensureBoolean(result);
178+
return result;
178179
}
179180

180181
@func('contains')
@@ -208,25 +209,25 @@ export class TypeScriptExpressionTransformer {
208209
private _regex(args: Expression[]) {
209210
const field = this.transform(args[0], false);
210211
const pattern = getLiteral<string>(args[1]);
211-
return `new RegExp(${JSON.stringify(pattern)}).test(${field})`;
212+
return this.ensureBoolean(`${field}?.match(new RegExp(${JSON.stringify(pattern)}))`);
212213
}
213214

214215
@func('email')
215216
private _email(args: Expression[]) {
216217
const field = this.transform(args[0], false);
217-
return `z.string().email().safeParse(${field}).success`;
218+
return this.ensureBooleanTernary(field, `z.string().email().safeParse(${field}).success`);
218219
}
219220

220221
@func('datetime')
221222
private _datetime(args: Expression[]) {
222223
const field = this.transform(args[0], false);
223-
return `z.string().datetime({ offset: true }).safeParse(${field}).success`;
224+
return this.ensureBooleanTernary(field, `z.string().datetime({ offset: true }).safeParse(${field}).success`);
224225
}
225226

226227
@func('url')
227228
private _url(args: Expression[]) {
228229
const field = this.transform(args[0], false);
229-
return `z.string().url().safeParse(${field}).success`;
230+
return this.ensureBooleanTernary(field, `z.string().url().safeParse(${field}).success`);
230231
}
231232

232233
@func('has')
@@ -239,22 +240,25 @@ export class TypeScriptExpressionTransformer {
239240
@func('hasEvery')
240241
private _hasEvery(args: Expression[], normalizeUndefined: boolean) {
241242
const field = this.transform(args[0], false);
242-
const result = `${this.transform(args[1], normalizeUndefined)}?.every((item) => ${field}?.includes(item))`;
243-
return this.ensureBoolean(result);
243+
return this.ensureBooleanTernary(
244+
field,
245+
`${this.transform(args[1], normalizeUndefined)}?.every((item) => ${field}?.includes(item))`
246+
);
244247
}
245248

246249
@func('hasSome')
247250
private _hasSome(args: Expression[], normalizeUndefined: boolean) {
248251
const field = this.transform(args[0], false);
249-
const result = `${this.transform(args[1], normalizeUndefined)}?.some((item) => ${field}?.includes(item))`;
250-
return this.ensureBoolean(result);
252+
return this.ensureBooleanTernary(
253+
field,
254+
`${this.transform(args[1], normalizeUndefined)}?.some((item) => ${field}?.includes(item))`
255+
);
251256
}
252257

253258
@func('isEmpty')
254259
private _isEmpty(args: Expression[]) {
255260
const field = this.transform(args[0], false);
256-
const result = `(!${field} || ${field}?.length === 0)`;
257-
return this.ensureBoolean(result);
261+
return `(!${field} || ${field}?.length === 0)`;
258262
}
259263

260264
private ensureBoolean(expr: string) {
@@ -263,7 +267,17 @@ export class TypeScriptExpressionTransformer {
263267
// as boolean true
264268
return `(${expr} ?? true)`;
265269
} else {
266-
return `(${expr} ?? false)`;
270+
return `((${expr}) ?? false)`;
271+
}
272+
}
273+
274+
private ensureBooleanTernary(predicate: string, value: string) {
275+
if (this.options.context === ExpressionContext.ValidationRule) {
276+
// all fields are optional in a validation context, so we treat undefined
277+
// as boolean true
278+
return `((${predicate}) !== undefined ? (${value}): true)`;
279+
} else {
280+
return `((${predicate}) !== undefined ? (${value}): false)`;
267281
}
268282
}
269283

@@ -315,7 +329,7 @@ export class TypeScriptExpressionTransformer {
315329
isDataModelFieldReference(expr.operand)
316330
) {
317331
// in a validation context, we treat unary involving undefined as boolean true
318-
result = `(${operand} !== undefined ? (${result}): true)`;
332+
result = this.ensureBooleanTernary(operand, result);
319333
}
320334
return result;
321335
}
@@ -336,21 +350,39 @@ export class TypeScriptExpressionTransformer {
336350
let _default = `(${left} ${expr.operator} ${right})`;
337351

338352
if (this.options.context === ExpressionContext.ValidationRule) {
339-
// in a validation context, we treat binary involving undefined as boolean true
340-
if (isDataModelFieldReference(expr.left)) {
341-
_default = `(${left} !== undefined ? (${_default}): true)`;
342-
}
343-
if (isDataModelFieldReference(expr.right)) {
344-
_default = `(${right} !== undefined ? (${_default}): true)`;
353+
const nullComparison = this.extractNullComparison(expr);
354+
if (nullComparison) {
355+
// null comparison covers both null and undefined
356+
const { fieldRef } = nullComparison;
357+
const field = this.transform(fieldRef, normalizeUndefined);
358+
if (expr.operator === '==') {
359+
_default = `(${field} === null || ${field} === undefined)`;
360+
} else if (expr.operator === '!=') {
361+
_default = `(${field} !== null && ${field} !== undefined)`;
362+
}
363+
} else {
364+
// for other comparisons, in a validation context,
365+
// we treat binary involving undefined as boolean true
366+
if (isDataModelFieldReference(expr.left)) {
367+
_default = this.ensureBooleanTernary(left, _default);
368+
}
369+
if (isDataModelFieldReference(expr.right)) {
370+
_default = this.ensureBooleanTernary(right, _default);
371+
}
345372
}
346373
}
347374

348375
return match(expr.operator)
349-
.with('in', () =>
350-
this.ensureBoolean(
351-
`${this.transform(expr.right, false)}?.includes(${this.transform(expr.left, normalizeUndefined)})`
352-
)
353-
)
376+
.with('in', () => {
377+
const left = `${this.transform(expr.left, normalizeUndefined)}`;
378+
let result = this.ensureBoolean(`${this.transform(expr.right, false)}?.includes(${left})`);
379+
380+
if (this.options.context === ExpressionContext.ValidationRule) {
381+
// in a validation context, we treat binary involving undefined as boolean true
382+
result = this.ensureBooleanTernary(left, result);
383+
}
384+
return result;
385+
})
354386
.with(P.union('==', '!='), () => {
355387
if (isThisExpr(expr.left) || isThisExpr(expr.right)) {
356388
// map equality comparison with `this` to id comparison
@@ -376,6 +408,20 @@ export class TypeScriptExpressionTransformer {
376408
.otherwise(() => _default);
377409
}
378410

411+
private extractNullComparison(expr: BinaryExpr) {
412+
if (expr.operator !== '==' && expr.operator !== '!=') {
413+
return undefined;
414+
}
415+
416+
if (isDataModelFieldReference(expr.left) && isNullExpr(expr.right)) {
417+
return { fieldRef: expr.left, nullExpr: expr.right };
418+
} else if (isDataModelFieldReference(expr.right) && isNullExpr(expr.left)) {
419+
return { fieldRef: expr.right, nullExpr: expr.left };
420+
} else {
421+
return undefined;
422+
}
423+
}
424+
379425
private collectionPredicate(expr: BinaryExpr, operator: '?' | '!' | '^', normalizeUndefined: boolean) {
380426
const operand = this.transform(expr.left, normalizeUndefined);
381427
const innerTransformer = new TypeScriptExpressionTransformer({

0 commit comments

Comments
 (0)