Skip to content

fix(zmodel): member access from auth() is not properly resolved when the auth model is imported #1260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 15, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
@@ -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;
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@ import {
isLiteralExpr,
isMemberAccessExpr,
isNullExpr,
isReferenceExpr,
isThisExpr,
} from '@zenstackhq/language/ast';
import { isAuthInvocation, isDataModelFieldReference, isEnumFieldReference } from '@zenstackhq/sdk';
@@ -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,
});
}
}
}

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';
@@ -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] });
}
14 changes: 4 additions & 10 deletions packages/schema/src/language-server/zmodel-linker.ts
Original file line number Diff line number Diff line change
@@ -36,9 +36,9 @@ import {
isStringLiteral,
} from '@zenstackhq/language/ast';
import {
getAuthModel,
getContainingModel,
getModelFieldsWithBases,
hasAttribute,
isAuthInvocation,
isFutureExpr,
} from '@zenstackhq/sdk';
@@ -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 {
@@ -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 };
}
27 changes: 15 additions & 12 deletions packages/schema/src/language-server/zmodel-scope.ts
Original file line number Diff line number Diff line change
@@ -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,
@@ -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';

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

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

@@ -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);
}
@@ -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);
}
6 changes: 5 additions & 1 deletion packages/schema/src/utils/ast-utils.ts
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -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 () => {
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
@@ -28,6 +28,6 @@ describe('Regression: issue 756', () => {
}
`
)
).toContain('expression cannot be resolved');
).toContain(`Could not resolve reference to DataModelField named 'authorId'.`);
});
});