Skip to content

Commit 3a2f1f9

Browse files
ymc9ymc9
authored
fix(language): ref resolution failure causes exception in validator (#269)
* fix(language): ref resolution failure causes exception in validator * fix lint * update * fix tests
1 parent 9bf6d7f commit 3a2f1f9

File tree

6 files changed

+54
-40
lines changed

6 files changed

+54
-40
lines changed

‎packages/ide/vscode/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "zenstack-v3",
33
"publisher": "zenstack",
4-
"version": "3.0.6",
4+
"version": "3.0.8",
55
"displayName": "ZenStack V3 Language Tools",
66
"description": "VSCode extension for ZenStack (v3) ZModel language",
77
"private": true,

‎packages/language/src/module.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
} from 'langium/lsp';
1010
import { ZModelGeneratedModule, ZModelGeneratedSharedModule, ZModelLanguageMetaData } from './generated/module';
1111
import { ZModelValidator, registerValidationChecks } from './validator';
12+
import { ZModelDocumentBuilder } from './zmodel-document-builder';
1213
import { ZModelLinker } from './zmodel-linker';
1314
import { ZModelScopeComputation, ZModelScopeProvider } from './zmodel-scope';
1415
import { ZModelWorkspaceManager } from './zmodel-workspace-manager';
@@ -49,6 +50,7 @@ export type ZModelSharedServices = LangiumSharedServices;
4950

5051
export const ZModelSharedModule: Module<ZModelSharedServices, DeepPartial<ZModelSharedServices>> = {
5152
workspace: {
53+
DocumentBuilder: (services) => new ZModelDocumentBuilder(services),
5254
WorkspaceManager: (services) => new ZModelWorkspaceManager(services),
5355
},
5456
};

‎packages/language/src/utils.ts‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { invariant } from '@zenstackhq/common-helpers';
21
import { AstUtils, URI, type AstNode, type LangiumDocument, type LangiumDocuments, type Reference } from 'langium';
32
import fs from 'node:fs';
43
import path from 'path';
@@ -172,7 +171,10 @@ export function getRecursiveBases(
172171
}
173172
seen.add(decl);
174173
decl.mixins.forEach((mixin) => {
175-
const baseDecl = mixin.ref;
174+
// avoid using mixin.ref since this function can be called before linking
175+
const baseDecl = decl.$container.declarations.find(
176+
(d): d is TypeDef => isTypeDef(d) && d.name === mixin.$refText,
177+
);
176178
if (baseDecl) {
177179
if (!includeDelegate && isDelegateModel(baseDecl)) {
178180
return;
@@ -517,13 +519,15 @@ export function getAllFields(
517519

518520
const fields: DataField[] = [];
519521
for (const mixin of decl.mixins) {
520-
invariant(mixin.ref, `Mixin ${mixin.$refText} is not resolved`);
521-
fields.push(...getAllFields(mixin.ref, includeIgnored, seen));
522+
if (mixin.ref) {
523+
fields.push(...getAllFields(mixin.ref, includeIgnored, seen));
524+
}
522525
}
523526

524527
if (isDataModel(decl) && decl.baseModel) {
525-
invariant(decl.baseModel.ref, `Base model ${decl.baseModel.$refText} is not resolved`);
526-
fields.push(...getAllFields(decl.baseModel.ref, includeIgnored, seen));
528+
if (decl.baseModel.ref) {
529+
fields.push(...getAllFields(decl.baseModel.ref, includeIgnored, seen));
530+
}
527531
}
528532

529533
fields.push(...decl.fields.filter((f) => includeIgnored || !hasAttribute(f, '@ignore')));
@@ -541,13 +545,15 @@ export function getAllAttributes(
541545

542546
const attributes: DataModelAttribute[] = [];
543547
for (const mixin of decl.mixins) {
544-
invariant(mixin.ref, `Mixin ${mixin.$refText} is not resolved`);
545-
attributes.push(...getAllAttributes(mixin.ref, seen));
548+
if (mixin.ref) {
549+
attributes.push(...getAllAttributes(mixin.ref, seen));
550+
}
546551
}
547552

548553
if (isDataModel(decl) && decl.baseModel) {
549-
invariant(decl.baseModel.ref, `Base model ${decl.baseModel.$refText} is not resolved`);
550-
attributes.push(...getAllAttributes(decl.baseModel.ref, seen));
554+
if (decl.baseModel.ref) {
555+
attributes.push(...getAllAttributes(decl.baseModel.ref, seen));
556+
}
551557
}
552558

553559
attributes.push(...decl.attributes);

‎packages/language/src/validator.ts‎

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
/* eslint-disable @typescript-eslint/no-unused-expressions */
2-
3-
import type { AstNode, LangiumDocument, ValidationAcceptor, ValidationChecks } from 'langium';
1+
import type { ValidationAcceptor, ValidationChecks } from 'langium';
42
import type {
53
Attribute,
64
DataModel,
@@ -50,54 +48,39 @@ export function registerValidationChecks(services: ZModelServices) {
5048
export class ZModelValidator {
5149
constructor(protected readonly services: ZModelServices) {}
5250

53-
private shouldCheck(node: AstNode) {
54-
let doc: LangiumDocument | undefined;
55-
let currNode: AstNode | undefined = node;
56-
while (currNode) {
57-
if (currNode.$document) {
58-
doc = currNode.$document;
59-
break;
60-
}
61-
currNode = currNode.$container;
62-
}
63-
64-
return doc?.parseResult.lexerErrors.length === 0 && doc?.parseResult.parserErrors.length === 0;
65-
}
66-
6751
checkModel(node: Model, accept: ValidationAcceptor): void {
68-
this.shouldCheck(node) &&
69-
new SchemaValidator(this.services.shared.workspace.LangiumDocuments).validate(node, accept);
52+
new SchemaValidator(this.services.shared.workspace.LangiumDocuments).validate(node, accept);
7053
}
7154

7255
checkDataSource(node: DataSource, accept: ValidationAcceptor): void {
73-
this.shouldCheck(node) && new DataSourceValidator().validate(node, accept);
56+
new DataSourceValidator().validate(node, accept);
7457
}
7558

7659
checkDataModel(node: DataModel, accept: ValidationAcceptor): void {
77-
this.shouldCheck(node) && new DataModelValidator().validate(node, accept);
60+
new DataModelValidator().validate(node, accept);
7861
}
7962

8063
checkTypeDef(node: TypeDef, accept: ValidationAcceptor): void {
81-
this.shouldCheck(node) && new TypeDefValidator().validate(node, accept);
64+
new TypeDefValidator().validate(node, accept);
8265
}
8366

8467
checkEnum(node: Enum, accept: ValidationAcceptor): void {
85-
this.shouldCheck(node) && new EnumValidator().validate(node, accept);
68+
new EnumValidator().validate(node, accept);
8669
}
8770

8871
checkAttribute(node: Attribute, accept: ValidationAcceptor): void {
89-
this.shouldCheck(node) && new AttributeValidator().validate(node, accept);
72+
new AttributeValidator().validate(node, accept);
9073
}
9174

9275
checkExpression(node: Expression, accept: ValidationAcceptor): void {
93-
this.shouldCheck(node) && new ExpressionValidator().validate(node, accept);
76+
new ExpressionValidator().validate(node, accept);
9477
}
9578

9679
checkFunctionInvocation(node: InvocationExpr, accept: ValidationAcceptor): void {
97-
this.shouldCheck(node) && new FunctionInvocationValidator().validate(node, accept);
80+
new FunctionInvocationValidator().validate(node, accept);
9881
}
9982

10083
checkFunctionDecl(node: FunctionDecl, accept: ValidationAcceptor): void {
101-
this.shouldCheck(node) && new FunctionDeclValidator().validate(node, accept);
84+
new FunctionDeclValidator().validate(node, accept);
10285
}
10386
}

‎packages/language/src/zmodel-document-builder.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import { DefaultDocumentBuilder, type BuildOptions, type LangiumDocument } from 'langium';
2+
3+
export class ZModelDocumentBuilder extends DefaultDocumentBuilder {
4+
override buildDocuments(documents: LangiumDocument[], options: BuildOptions, cancelToken: any): Promise<void> {
5+
return super.buildDocuments(
6+
documents,
7+
{
8+
...options,
9+
validation:
10+
// force overriding validation options
11+
options.validation === false || options.validation === undefined
12+
? options.validation
13+
: {
14+
stopAfterLexingErrors: true,
15+
stopAfterParsingErrors: true,
16+
stopAfterLinkingErrors: true,
17+
},
18+
},
19+
cancelToken,
20+
);
21+
}
22+
}

‎packages/language/test/expression-validation.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, it } from 'vitest';
22
import { loadSchema, loadSchemaWithError } from './utils';
33

44
describe('Expression Validation Tests', () => {
5-
it('should reject model comparison', async () => {
5+
it('should reject model comparison1', async () => {
66
await loadSchemaWithError(
77
`
88
model User {
@@ -15,14 +15,15 @@ describe('Expression Validation Tests', () => {
1515
id Int @id
1616
title String
1717
author User @relation(fields: [authorId], references: [id])
18+
authorId Int
1819
@@allow('all', author == this)
1920
}
2021
`,
2122
'comparison between models is not supported',
2223
);
2324
});
2425

25-
it('should reject model comparison', async () => {
26+
it('should reject model comparison2', async () => {
2627
await loadSchemaWithError(
2728
`
2829
model User {

0 commit comments

Comments
 (0)