Skip to content

Commit

Permalink
fix(zmodel): member access from auth() is not properly resolved whe…
Browse files Browse the repository at this point in the history
…n the auth model is imported (#1260)
  • Loading branch information
ymc9 authored Apr 15, 2024
1 parent 8044a54 commit 5af5b69
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,28 @@
//////////////////////////////////////////////////////////////////////////////////////////////

datasource db {
provider = "sqlite"
url = "file:./dev.db"
provider = "sqlite"
url = "file:./dev.db"
}

generator client {
provider = "prisma-client-js"
provider = "prisma-client-js"
}

model User {
id Int @id() @default(autoincrement())
email String @unique()
posts Post[]
id Int @id() @default(autoincrement())
email String @unique()
posts Post[]
}

model Post {
id Int @id() @default(autoincrement())
name String
createdAt DateTime @default(now())
updatedAt DateTime @updatedAt()
published Boolean @default(false)
author User @relation(fields: [authorId], references: [id])
authorId Int
id Int @id() @default(autoincrement())
name String
createdAt DateTime @default(now())
updatedAt DateTime @updatedAt()
published Boolean @default(false)
author User @relation(fields: [authorId], references: [id])
authorId Int
@@index([name])
}
@@index([name])
}
35 changes: 21 additions & 14 deletions packages/schema/src/cli/cli-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,23 +64,30 @@ export async function loadDocument(fileName: string): Promise<Model> {
}
);

const validationErrors = langiumDocuments.all
.flatMap((d) => d.diagnostics ?? [])
.filter((e) => e.severity === 1)
const diagnostics = langiumDocuments.all
.flatMap((doc) => (doc.diagnostics ?? []).map((diag) => ({ doc, diag })))
.filter(({ diag }) => diag.severity === 1 || diag.severity === 2)
.toArray();

if (validationErrors.length > 0) {
console.error(colors.red('Validation errors:'));
for (const validationError of validationErrors) {
console.error(
colors.red(
`line ${validationError.range.start.line + 1}: ${
validationError.message
} [${document.textDocument.getText(validationError.range)}]`
)
);
let hasErrors = false;

if (diagnostics.length > 0) {
for (const { doc, diag } of diagnostics) {
const message = `${path.relative(process.cwd(), doc.uri.fsPath)}:${diag.range.start.line + 1}:${
diag.range.start.character + 1
} - ${diag.message}`;

if (diag.severity === 1) {
console.error(colors.red(message));
hasErrors = true;
} else {
console.warn(colors.yellow(message));
}
}

if (hasErrors) {
throw new CliError('Schema contains validation errors');
}
throw new CliError('schema validation errors');
}

const model = document.parseResult.value as Model;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
isLiteralExpr,
isMemberAccessExpr,
isNullExpr,
isReferenceExpr,
isThisExpr,
} from '@zenstackhq/language/ast';
import { isAuthInvocation, isDataModelFieldReference, isEnumFieldReference } from '@zenstackhq/sdk';
Expand All @@ -33,9 +34,21 @@ export default class ExpressionValidator implements AstValidator<Expression> {
{ node: expr }
);
} else {
accept('error', 'expression cannot be resolved', {
node: expr,
const hasReferenceResolutionError = streamAst(expr).some((node) => {
if (isMemberAccessExpr(node)) {
return !!node.member.error;
}
if (isReferenceExpr(node)) {
return !!node.target.error;
}
return false;
});
if (!hasReferenceResolutionError) {
// report silent errors not involving linker errors
accept('error', 'Expression cannot be resolved', {
node: expr,
});
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Model, isDataModel, isDataSource } from '@zenstackhq/language/ast';
import { hasAttribute } from '@zenstackhq/sdk';
import { LangiumDocuments, ValidationAcceptor } from 'langium';
import { getAllDeclarationsFromImports, resolveImport, resolveTransitiveImports } from '../../utils/ast-utils';
import { getAllDeclarationsIncludingImports, resolveImport, resolveTransitiveImports } from '../../utils/ast-utils';
import { PLUGIN_MODULE_NAME, STD_LIB_MODULE_NAME } from '../constants';
import { AstValidator } from '../types';
import { validateDuplicatedDeclarations } from './utils';
Expand Down Expand Up @@ -43,7 +43,7 @@ export default class SchemaValidator implements AstValidator<Model> {
}

private validateDataSources(model: Model, accept: ValidationAcceptor) {
const dataSources = getAllDeclarationsFromImports(this.documents, model).filter((d) => isDataSource(d));
const dataSources = getAllDeclarationsIncludingImports(this.documents, model).filter((d) => isDataSource(d));
if (dataSources.length > 1) {
accept('error', 'Multiple datasource declarations are not allowed', { node: dataSources[1] });
}
Expand Down
14 changes: 4 additions & 10 deletions packages/schema/src/language-server/zmodel-linker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ import {
isStringLiteral,
} from '@zenstackhq/language/ast';
import {
getAuthModel,
getContainingModel,
getModelFieldsWithBases,
hasAttribute,
isAuthInvocation,
isFutureExpr,
} from '@zenstackhq/sdk';
Expand All @@ -58,7 +58,7 @@ import {
} from 'langium';
import { match } from 'ts-pattern';
import { CancellationToken } from 'vscode-jsonrpc';
import { getAllDeclarationsFromImports, getContainingDataModel } from '../utils/ast-utils';
import { getAllDataModelsIncludingImports, getContainingDataModel } from '../utils/ast-utils';
import { mapBuiltinTypeToExpressionType } from './validator/utils';

interface DefaultReference extends Reference {
Expand Down Expand Up @@ -287,14 +287,8 @@ export class ZModelLinker extends DefaultLinker {
const model = getContainingModel(node);

if (model) {
let authModel = getAllDeclarationsFromImports(this.langiumDocuments(), model).find((d) => {
return isDataModel(d) && hasAttribute(d, '@@auth');
});
if (!authModel) {
authModel = getAllDeclarationsFromImports(this.langiumDocuments(), model).find((d) => {
return isDataModel(d) && d.name === 'User';
});
}
const allDataModels = getAllDataModelsIncludingImports(this.langiumDocuments(), model);
const authModel = getAuthModel(allDataModels);
if (authModel) {
node.$resolvedType = { decl: authModel, nullable: true };
}
Expand Down
27 changes: 15 additions & 12 deletions packages/schema/src/language-server/zmodel-scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,7 @@ import {
isReferenceExpr,
isThisExpr,
} from '@zenstackhq/language/ast';
import {
getAuthModel,
getDataModels,
getModelFieldsWithBases,
getRecursiveBases,
isAuthInvocation,
} from '@zenstackhq/sdk';
import { getAuthModel, getModelFieldsWithBases, getRecursiveBases, isAuthInvocation } from '@zenstackhq/sdk';
import {
AstNode,
AstNodeDescription,
Expand All @@ -37,7 +31,12 @@ import {
} from 'langium';
import { match } from 'ts-pattern';
import { CancellationToken } from 'vscode-jsonrpc';
import { isCollectionPredicate, isFutureInvocation, resolveImportUri } from '../utils/ast-utils';
import {
getAllDataModelsIncludingImports,
isCollectionPredicate,
isFutureInvocation,
resolveImportUri,
} from '../utils/ast-utils';
import { PLUGIN_MODULE_NAME, STD_LIB_MODULE_NAME } from './constants';

/**
Expand Down Expand Up @@ -88,7 +87,7 @@ export class ZModelScopeComputation extends DefaultScopeComputation {
}

export class ZModelScopeProvider extends DefaultScopeProvider {
constructor(services: LangiumServices) {
constructor(private readonly services: LangiumServices) {
super(services);
}

Expand Down Expand Up @@ -145,9 +144,9 @@ export class ZModelScopeProvider extends DefaultScopeProvider {
return EMPTY_SCOPE;
})
.when(isMemberAccessExpr, (operand) => {
// operand is a member access, it must be resolved to a
// operand is a member access, it must be resolved to a non-array data model type
const ref = operand.member.ref;
if (isDataModelField(ref)) {
if (isDataModelField(ref) && !ref.type.array) {
const targetModel = ref.type.reference?.ref;
return this.createScopeForModel(targetModel, globalScope);
}
Expand Down Expand Up @@ -222,7 +221,11 @@ export class ZModelScopeProvider extends DefaultScopeProvider {
private createScopeForAuthModel(node: AstNode, globalScope: Scope) {
const model = getContainerOfType(node, isModel);
if (model) {
const authModel = getAuthModel(getDataModels(model, true));
const allDataModels = getAllDataModelsIncludingImports(
this.services.shared.workspace.LangiumDocuments,
model
);
const authModel = getAuthModel(allDataModels);
if (authModel) {
return this.createScopeForModel(authModel, globalScope);
}
Expand Down
6 changes: 5 additions & 1 deletion packages/schema/src/utils/ast-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,11 +216,15 @@ export function resolveImport(documents: LangiumDocuments, imp: ModelImport): Mo
return undefined;
}

export function getAllDeclarationsFromImports(documents: LangiumDocuments, model: Model) {
export function getAllDeclarationsIncludingImports(documents: LangiumDocuments, model: Model) {
const imports = resolveTransitiveImports(documents, model);
return model.declarations.concat(...imports.map((imp) => imp.declarations));
}

export function getAllDataModelsIncludingImports(documents: LangiumDocuments, model: Model) {
return getAllDeclarationsIncludingImports(documents, model).filter(isDataModel);
}

export function isCollectionPredicate(node: AstNode): node is BinaryExpr {
return isBinaryExpr(node) && ['?', '!', '^'].includes(node.operator);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1081,7 +1081,7 @@ describe('Attribute tests', () => {
@@allow('all', auth().email != null)
}
`)
).toContain(`expression cannot be resolved`);
).toContain(`Could not resolve reference to DataModelField named 'email'.`);
});

it('collection predicate expression check', async () => {
Expand Down
53 changes: 53 additions & 0 deletions tests/integration/tests/regression/issue-1257.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { FILE_SPLITTER, loadSchema } from '@zenstackhq/testtools';

describe('issue 1210', () => {
it('regression', async () => {
await loadSchema(
`schema.zmodel
import "./user"
import "./image"
generator client {
provider = "prisma-client-js"
}
datasource db {
provider = "postgresql"
url = env("DATABASE_URL")
}
${FILE_SPLITTER}base.zmodel
abstract model Base {
id Int @id @default(autoincrement())
}
${FILE_SPLITTER}user.zmodel
import "./base"
import "./image"
enum Role {
Admin
}
model User extends Base {
email String @unique
role Role
@@auth
}
${FILE_SPLITTER}image.zmodel
import "./user"
import "./base"
model Image extends Base {
width Int @default(0)
height Int @default(0)
@@allow('read', true)
@@allow('all', auth().role == Admin)
}
`,
{ addPrelude: false, pushDb: false }
);
});
});
2 changes: 1 addition & 1 deletion tests/integration/tests/regression/issue-756.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ describe('Regression: issue 756', () => {
}
`
)
).toContain('expression cannot be resolved');
).toContain(`Could not resolve reference to DataModelField named 'authorId'.`);
});
});

0 comments on commit 5af5b69

Please sign in to comment.