Skip to content

Commit d8dce13

Browse files
ymc9ymc9
authored
fix: reference resolution issue inside collection predicate expressions (#927)
1 parent 6f470f7 commit d8dce13

File tree

3 files changed

+148
-22
lines changed

3 files changed

+148
-22
lines changed

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

Lines changed: 74 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,12 @@ import {
5252
} from 'langium';
5353
import { match } from 'ts-pattern';
5454
import { CancellationToken } from 'vscode-jsonrpc';
55-
import { getAllDeclarationsFromImports, isAuthInvocation, getContainingDataModel } from '../utils/ast-utils';
55+
import {
56+
getAllDeclarationsFromImports,
57+
getContainingDataModel,
58+
isAuthInvocation,
59+
isCollectionPredicate,
60+
} from '../utils/ast-utils';
5661
import { mapBuiltinTypeToExpressionType } from './validator/utils';
5762

5863
interface DefaultReference extends Reference {
@@ -94,9 +99,21 @@ export class ZModelLinker extends DefaultLinker {
9499
extraScopes: ScopeProvider[],
95100
onlyFromExtraScopes = false
96101
) {
97-
if (!this.resolveFromScopeProviders(container, property, document, extraScopes) && !onlyFromExtraScopes) {
98-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
99-
const reference: Reference<AstNode> = (container as any)[property];
102+
if (this.resolveFromScopeProviders(container, property, document, extraScopes)) {
103+
return;
104+
}
105+
106+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
107+
const reference: DefaultReference = (container as any)[property];
108+
109+
if (onlyFromExtraScopes) {
110+
// if reference is not resolved from explicit scope providers and automatic linking is not allowed,
111+
// we should explicitly create a linking error
112+
reference._ref = this.createLinkingError({ reference, container, property });
113+
114+
// Add the reference to the document's array of references
115+
document.references.push(reference);
116+
} else {
100117
this.doLink({ reference, container, property }, document);
101118
}
102119
}
@@ -118,6 +135,10 @@ export class ZModelLinker extends DefaultLinker {
118135
if (target) {
119136
reference._ref = target;
120137
reference._nodeDescription = this.descriptions.createDescription(target, target.name, document);
138+
139+
// Add the reference to the document's array of references
140+
document.references.push(reference);
141+
121142
return target;
122143
}
123144
}
@@ -244,6 +265,22 @@ export class ZModelLinker extends DefaultLinker {
244265
node.args.forEach((arg) => this.resolve(arg, document, extraScopes));
245266

246267
if (node.target.ref) {
268+
// if the reference is inside the RHS of a collection predicate, it cannot be resolve to a field
269+
// not belonging to the collection's model type
270+
271+
const collectionPredicateContext = this.getCollectionPredicateContextDataModel(node);
272+
if (
273+
// inside a collection predicate RHS
274+
collectionPredicateContext &&
275+
// current ref expr is resolved to a field
276+
isDataModelField(node.target.ref) &&
277+
// the resolved field doesn't belong to the collection predicate's operand's type
278+
node.target.ref.$container !== collectionPredicateContext
279+
) {
280+
this.unresolvableRefExpr(node);
281+
return;
282+
}
283+
247284
// resolve type
248285
if (node.target.ref.$type === EnumField) {
249286
this.resolveToBuiltinTypeOrDecl(node, node.target.ref.$container);
@@ -253,6 +290,26 @@ export class ZModelLinker extends DefaultLinker {
253290
}
254291
}
255292

293+
private getCollectionPredicateContextDataModel(node: ReferenceExpr) {
294+
let curr: AstNode | undefined = node;
295+
while (curr) {
296+
if (
297+
curr.$container &&
298+
// parent is a collection predicate
299+
isCollectionPredicate(curr.$container) &&
300+
// the collection predicate's LHS is resolved to a DataModel
301+
isDataModel(curr.$container.left.$resolvedType?.decl) &&
302+
// current node is the RHS
303+
curr.$containerProperty === 'right'
304+
) {
305+
// return the resolved type of LHS
306+
return curr.$container.left.$resolvedType?.decl;
307+
}
308+
curr = curr.$container;
309+
}
310+
return undefined;
311+
}
312+
256313
private resolveArray(node: ArrayExpr, document: LangiumDocument<AstNode>, extraScopes: ScopeProvider[]) {
257314
node.items.forEach((item) => this.resolve(item, document, extraScopes));
258315

@@ -414,13 +471,8 @@ export class ZModelLinker extends DefaultLinker {
414471
if (resolved) {
415472
this.resolveToDeclaredType(item, (resolved as DataModelField).type);
416473
} else {
417-
// need to clear linked reference, because it's resolved in default scope by default
418-
const ref = item.target as DefaultReference;
419-
ref._ref = this.createLinkingError({
420-
reference: ref,
421-
container: item,
422-
property: 'target',
423-
});
474+
// mark unresolvable
475+
this.unresolvableRefExpr(item);
424476
}
425477
}
426478
});
@@ -432,13 +484,8 @@ export class ZModelLinker extends DefaultLinker {
432484
if (resolved) {
433485
this.resolveToDeclaredType(node.value, (resolved as DataModelField).type);
434486
} else {
435-
// need to clear linked reference, because it's resolved in default scope by default
436-
const ref = node.value.target as DefaultReference;
437-
ref._ref = this.createLinkingError({
438-
reference: ref,
439-
container: node.value,
440-
property: 'target',
441-
});
487+
// mark unresolvable
488+
this.unresolvableRefExpr(node.value);
442489
}
443490
}
444491
}
@@ -448,6 +495,15 @@ export class ZModelLinker extends DefaultLinker {
448495
node.$resolvedType = node.value.$resolvedType;
449496
}
450497

498+
private unresolvableRefExpr(item: ReferenceExpr) {
499+
const ref = item.target as DefaultReference;
500+
ref._ref = this.createLinkingError({
501+
reference: ref,
502+
container: item,
503+
property: 'target',
504+
});
505+
}
506+
451507
private findAttrParamForArg(arg: AttributeArg): AttributeParam | undefined {
452508
const attr = arg.$container.decl.ref;
453509
if (!attr) {

‎packages/schema/tests/schema/zmodel-generator.test.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,22 +86,21 @@ describe('ZModel Generator Tests', () => {
8686
name String
8787
role UserRole
8888
deleted Boolean
89-
level Int
90-
9189
9290
posts Post[]
9391
9492
@@allow('read', posts ? [author == auth()])
9593
9694
@@deny('read', name == '123' && (role == USER || name == '456'))
9795
98-
@@allow('delete', posts?[author == auth() && ( level <10 || author.role == USER) && !author.deleted])
96+
@@allow('delete', posts?[author == auth() && ( length <10 || author.role == USER) && !author.deleted])
9997
}
10098
10199
model Post {
102100
id String @id
103101
author User? @relation(fields: [authorId], references: [id])
104102
authorId String?
103+
length Int
105104
}
106105
`,
107106
'User'
@@ -111,7 +110,7 @@ describe('ZModel Generator Tests', () => {
111110
checkAttribute(model.attributes[1], `@@deny('read', name == '123' && (role == USER || name == '456'))`);
112111
checkAttribute(
113112
model.attributes[2],
114-
`@@allow('delete', posts?[author == auth() && (level < 10 || author.role == USER) && !author.deleted])`
113+
`@@allow('delete', posts?[author == auth() && (length < 10 || author.role == USER) && !author.deleted])`
115114
);
116115
});
117116
});

‎tests/integration/tests/regression/issue-925.test.ts‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { loadModelWithError } from '@zenstackhq/testtools';
2+
3+
describe('Regression: issue 925', () => {
4+
it('member reference from this', async () => {
5+
await expect(
6+
loadModelWithError(
7+
`
8+
model User {
9+
id Int @id @default(autoincrement())
10+
company Company[]
11+
test Int
12+
13+
@@allow('read', auth().company?[staff?[companyId == this.test]])
14+
}
15+
16+
model Company {
17+
id Int @id @default(autoincrement())
18+
user User @relation(fields: [userId], references: [id])
19+
userId Int
20+
21+
staff Staff[]
22+
@@allow('read', true)
23+
}
24+
25+
model Staff {
26+
id Int @id @default(autoincrement())
27+
28+
company Company @relation(fields: [companyId], references: [id])
29+
companyId Int
30+
31+
@@allow('read', true)
32+
}
33+
`
34+
)
35+
).resolves.toContain("Could not resolve reference to DataModelField named 'test'.");
36+
});
37+
38+
it('simple reference', async () => {
39+
await expect(
40+
loadModelWithError(
41+
`
42+
model User {
43+
id Int @id @default(autoincrement())
44+
company Company[]
45+
test Int
46+
47+
@@allow('read', auth().company?[staff?[companyId == test]])
48+
}
49+
50+
model Company {
51+
id Int @id @default(autoincrement())
52+
user User @relation(fields: [userId], references: [id])
53+
userId Int
54+
55+
staff Staff[]
56+
@@allow('read', true)
57+
}
58+
59+
model Staff {
60+
id Int @id @default(autoincrement())
61+
62+
company Company @relation(fields: [companyId], references: [id])
63+
companyId Int
64+
65+
@@allow('read', true)
66+
}
67+
`
68+
)
69+
).resolves.toContain("Could not resolve reference to ReferenceTarget named 'test'.");
70+
});
71+
});

0 commit comments

Comments
 (0)