Skip to content

Commit 79ef57a

Browse files
ymc9ymc9
authored
fix: @@validate should ignore fields that are not present (#1104)
1 parent 2b12e09 commit 79ef57a

File tree

7 files changed

+367
-85
lines changed

7 files changed

+367
-85
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: 100 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,6 @@ import {
55
DataModel,
66
Expression,
77
InvocationExpr,
8-
isDataModel,
9-
isEnumField,
10-
isThisExpr,
118
LiteralExpr,
129
MemberAccessExpr,
1310
NullExpr,
@@ -16,9 +13,15 @@ import {
1613
StringLiteral,
1714
ThisExpr,
1815
UnaryExpr,
16+
isArrayExpr,
17+
isDataModel,
18+
isEnumField,
19+
isLiteralExpr,
20+
isNullExpr,
21+
isThisExpr,
1922
} from '@zenstackhq/language/ast';
2023
import { ExpressionContext, getLiteral, isDataModelFieldReference, isFromStdlib, isFutureExpr } from '@zenstackhq/sdk';
21-
import { match, P } from 'ts-pattern';
24+
import { P, match } from 'ts-pattern';
2225
import { getIdFields } from './ast-utils';
2326

2427
export class TypeScriptExpressionTransformerError extends Error {
@@ -168,13 +171,17 @@ export class TypeScriptExpressionTransformer {
168171
const max = getLiteral<number>(args[2]);
169172
let result: string;
170173
if (min === undefined) {
171-
result = `(${field}?.length > 0)`;
174+
result = this.ensureBooleanTernary(args[0], field, `${field}?.length > 0`);
172175
} else if (max === undefined) {
173-
result = `(${field}?.length >= ${min})`;
176+
result = this.ensureBooleanTernary(args[0], field, `${field}?.length >= ${min}`);
174177
} else {
175-
result = `(${field}?.length >= ${min} && ${field}?.length <= ${max})`;
178+
result = this.ensureBooleanTernary(
179+
args[0],
180+
field,
181+
`${field}?.length >= ${min} && ${field}?.length <= ${max}`
182+
);
176183
}
177-
return this.ensureBoolean(result);
184+
return result;
178185
}
179186

180187
@func('contains')
@@ -208,25 +215,29 @@ export class TypeScriptExpressionTransformer {
208215
private _regex(args: Expression[]) {
209216
const field = this.transform(args[0], false);
210217
const pattern = getLiteral<string>(args[1]);
211-
return `new RegExp(${JSON.stringify(pattern)}).test(${field})`;
218+
return this.ensureBooleanTernary(args[0], field, `new RegExp(${JSON.stringify(pattern)}).test(${field})`);
212219
}
213220

214221
@func('email')
215222
private _email(args: Expression[]) {
216223
const field = this.transform(args[0], false);
217-
return `z.string().email().safeParse(${field}).success`;
224+
return this.ensureBooleanTernary(args[0], field, `z.string().email().safeParse(${field}).success`);
218225
}
219226

220227
@func('datetime')
221228
private _datetime(args: Expression[]) {
222229
const field = this.transform(args[0], false);
223-
return `z.string().datetime({ offset: true }).safeParse(${field}).success`;
230+
return this.ensureBooleanTernary(
231+
args[0],
232+
field,
233+
`z.string().datetime({ offset: true }).safeParse(${field}).success`
234+
);
224235
}
225236

226237
@func('url')
227238
private _url(args: Expression[]) {
228239
const field = this.transform(args[0], false);
229-
return `z.string().url().safeParse(${field}).success`;
240+
return this.ensureBooleanTernary(args[0], field, `z.string().url().safeParse(${field}).success`);
230241
}
231242

232243
@func('has')
@@ -239,22 +250,27 @@ export class TypeScriptExpressionTransformer {
239250
@func('hasEvery')
240251
private _hasEvery(args: Expression[], normalizeUndefined: boolean) {
241252
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);
253+
return this.ensureBooleanTernary(
254+
args[0],
255+
field,
256+
`${this.transform(args[1], normalizeUndefined)}?.every((item) => ${field}?.includes(item))`
257+
);
244258
}
245259

246260
@func('hasSome')
247261
private _hasSome(args: Expression[], normalizeUndefined: boolean) {
248262
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);
263+
return this.ensureBooleanTernary(
264+
args[0],
265+
field,
266+
`${this.transform(args[1], normalizeUndefined)}?.some((item) => ${field}?.includes(item))`
267+
);
251268
}
252269

253270
@func('isEmpty')
254271
private _isEmpty(args: Expression[]) {
255272
const field = this.transform(args[0], false);
256-
const result = `(!${field} || ${field}?.length === 0)`;
257-
return this.ensureBoolean(result);
273+
return `(!${field} || ${field}?.length === 0)`;
258274
}
259275

260276
private ensureBoolean(expr: string) {
@@ -263,7 +279,22 @@ export class TypeScriptExpressionTransformer {
263279
// as boolean true
264280
return `(${expr} ?? true)`;
265281
} else {
266-
return `(${expr} ?? false)`;
282+
return `((${expr}) ?? false)`;
283+
}
284+
}
285+
286+
private ensureBooleanTernary(predicate: Expression, transformedPredicate: string, value: string) {
287+
if (isLiteralExpr(predicate) || isArrayExpr(predicate)) {
288+
// these are never undefined
289+
return value;
290+
}
291+
292+
if (this.options.context === ExpressionContext.ValidationRule) {
293+
// all fields are optional in a validation context, so we treat undefined
294+
// as boolean true
295+
return `((${transformedPredicate}) !== undefined ? (${value}): true)`;
296+
} else {
297+
return `((${transformedPredicate}) !== undefined ? (${value}): false)`;
267298
}
268299
}
269300

@@ -315,7 +346,7 @@ export class TypeScriptExpressionTransformer {
315346
isDataModelFieldReference(expr.operand)
316347
) {
317348
// in a validation context, we treat unary involving undefined as boolean true
318-
result = `(${operand} !== undefined ? (${result}): true)`;
349+
result = this.ensureBooleanTernary(expr.operand, operand, result);
319350
}
320351
return result;
321352
}
@@ -336,21 +367,45 @@ export class TypeScriptExpressionTransformer {
336367
let _default = `(${left} ${expr.operator} ${right})`;
337368

338369
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)`;
370+
const nullComparison = this.extractNullComparison(expr);
371+
if (nullComparison) {
372+
// null comparison covers both null and undefined
373+
const { fieldRef } = nullComparison;
374+
const field = this.transform(fieldRef, normalizeUndefined);
375+
if (expr.operator === '==') {
376+
_default = `(${field} === null || ${field} === undefined)`;
377+
} else if (expr.operator === '!=') {
378+
_default = `(${field} !== null && ${field} !== undefined)`;
379+
}
380+
} else {
381+
// for other comparisons, in a validation context,
382+
// we treat binary involving undefined as boolean true
383+
if (isDataModelFieldReference(expr.left)) {
384+
_default = this.ensureBooleanTernary(expr.left, left, _default);
385+
}
386+
if (isDataModelFieldReference(expr.right)) {
387+
_default = this.ensureBooleanTernary(expr.right, right, _default);
388+
}
345389
}
346390
}
347391

348392
return match(expr.operator)
349-
.with('in', () =>
350-
this.ensureBoolean(
351-
`${this.transform(expr.right, false)}?.includes(${this.transform(expr.left, normalizeUndefined)})`
352-
)
353-
)
393+
.with('in', () => {
394+
const left = `${this.transform(expr.left, normalizeUndefined)}`;
395+
const right = `${this.transform(expr.right, false)}`;
396+
let result = `${right}?.includes(${left})`;
397+
if (this.options.context === ExpressionContext.ValidationRule) {
398+
// in a validation context, we treat binary involving undefined as boolean true
399+
result = this.ensureBooleanTernary(
400+
expr.left,
401+
left,
402+
this.ensureBooleanTernary(expr.right, right, result)
403+
);
404+
} else {
405+
result = this.ensureBoolean(result);
406+
}
407+
return result;
408+
})
354409
.with(P.union('==', '!='), () => {
355410
if (isThisExpr(expr.left) || isThisExpr(expr.right)) {
356411
// map equality comparison with `this` to id comparison
@@ -376,6 +431,20 @@ export class TypeScriptExpressionTransformer {
376431
.otherwise(() => _default);
377432
}
378433

434+
private extractNullComparison(expr: BinaryExpr) {
435+
if (expr.operator !== '==' && expr.operator !== '!=') {
436+
return undefined;
437+
}
438+
439+
if (isDataModelFieldReference(expr.left) && isNullExpr(expr.right)) {
440+
return { fieldRef: expr.left, nullExpr: expr.right };
441+
} else if (isDataModelFieldReference(expr.right) && isNullExpr(expr.left)) {
442+
return { fieldRef: expr.right, nullExpr: expr.left };
443+
} else {
444+
return undefined;
445+
}
446+
}
447+
379448
private collectionPredicate(expr: BinaryExpr, operator: '?' | '!' | '^', normalizeUndefined: boolean) {
380449
const operand = this.transform(expr.left, normalizeUndefined);
381450
const innerTransformer = new TypeScriptExpressionTransformer({

0 commit comments

Comments
 (0)