fix: add a few missing zmodel validation checks

ymc9 · ymc9 · commit aa1b5bfe7fe5 · 2025-07-17T22:08:10.000+08:00
diff --git a/TODO.md b/TODO.md
@@ -7,6 +7,9 @@
     - [x] migrate
     - [x] info
     - [x] init
+    - [ ] validate
+    - [ ] format
+    - [ ] db seed
 - [ ] ORM
     - [x] Create
         - [x] Input validation
diff --git a/packages/language/res/stdlib.zmodel b/packages/language/res/stdlib.zmodel
@@ -224,6 +224,11 @@ attribute @@@prisma()
  */
 attribute @@@completionHint(_ values: String[])
 
+/**
+ * Indicates that the attribute can only be applied once to a declaration.
+ */
+attribute @@@once()
+
 /**
  * Defines a single-field ID on the model.
  *
@@ -232,7 +237,7 @@ attribute @@@completionHint(_ values: String[])
  * @param sort: Allows you to specify in what order the entries of the ID are stored in the database. The available options are Asc and Desc.
  * @param clustered: Defines whether the ID is clustered or non-clustered. Defaults to true.
  */
-attribute @id(map: String?, length: Int?, sort: SortOrder?, clustered: Boolean?) @@@prisma @@@supportTypeDef
+attribute @id(map: String?, length: Int?, sort: SortOrder?, clustered: Boolean?) @@@prisma @@@supportTypeDef @@@once
 
 /**
  * Defines a default value for a field.
@@ -247,7 +252,7 @@ attribute @default(_ value: ContextType, map: String?) @@@prisma @@@supportTypeD
  * @param sort: Allows you to specify in what order the entries of the constraint are stored in the database. The available options are Asc and Desc.
  * @param clustered: Boolean	Defines whether the constraint is clustered or non-clustered. Defaults to false.
  */
-attribute @unique(map: String?, length: Int?, sort: SortOrder?, clustered: Boolean?) @@@prisma
+attribute @unique(map: String?, length: Int?, sort: SortOrder?, clustered: Boolean?) @@@prisma @@@once
 
 /**
  * Defines a multi-field ID (composite ID) on the model.
diff --git a/packages/language/src/validators/attribute-application-validator.ts b/packages/language/src/validators/attribute-application-validator.ts
@@ -75,6 +75,8 @@ export default class AttributeApplicationValidator implements AstValidator<Attri
             accept('error', `attribute "${decl.name}" cannot be used on type declarations`, { node: attr });
         }
 
+        this.checkDuplicatedAttributes(attr, accept);
+
         const filledParams = new Set<AttributeParam>();
 
         for (const arg of attr.args) {
@@ -131,6 +133,18 @@ export default class AttributeApplicationValidator implements AstValidator<Attri
         }
     }
 
+    private checkDuplicatedAttributes(attr: AttributeApplication, accept: ValidationAcceptor) {
+        const attrDecl = attr.decl.ref;
+        if (!attrDecl?.attributes.some((a) => a.decl.ref?.name === '@@@once')) {
+            return;
+        }
+
+        const duplicates = attr.$container.attributes.filter((a) => a.decl.ref === attrDecl && a !== attr);
+        if (duplicates.length > 0) {
+            accept('error', `Attribute "${attrDecl.name}" can only be applied once`, { node: attr });
+        }
+    }
+
     @check('@@allow')
     @check('@@deny')
     // @ts-expect-error
@@ -197,13 +211,21 @@ export default class AttributeApplicationValidator implements AstValidator<Attri
     }
 
     @check('@@unique')
+    @check('@@id')
     // @ts-expect-error
     private _checkUnique(attr: AttributeApplication, accept: ValidationAcceptor) {
         const fields = attr.args[0]?.value;
         if (!fields) {
+            accept('error', `expects an array of field references`, {
+                node: attr.args[0]!,
+            });
             return;
         }
         if (isArrayExpr(fields)) {
+            if (fields.items.length === 0) {
+                accept('error', `\`@@unique\` expects at least one field reference`, { node: fields });
+                return;
+            }
             fields.items.forEach((item) => {
                 if (!isReferenceExpr(item)) {
                     accept('error', `Expecting a field reference`, {
diff --git a/packages/language/src/validators/datamodel-validator.ts b/packages/language/src/validators/datamodel-validator.ts
@@ -259,11 +259,20 @@ export default class DataModelValidator implements AstValidator<DataModel> {
             return;
         }
 
+        if (this.isSelfRelation(field)) {
+            if (!thisRelation.name) {
+                accept('error', 'Self-relation field must have a name in @relation attribute', {
+                    node: field,
+                });
+                return;
+            }
+        }
+
         const oppositeModel = field.type.reference!.ref! as DataModel;
 
         // Use name because the current document might be updated
         let oppositeFields = getModelFieldsWithBases(oppositeModel, false).filter(
-            (f) => f.type.reference?.ref?.name === contextModel.name,
+            (f) => f !== field && f.type.reference?.ref?.name === contextModel.name,
         );
         oppositeFields = oppositeFields.filter((f) => {
             const fieldRel = this.parseRelation(f);
@@ -322,27 +331,41 @@ export default class DataModelValidator implements AstValidator<DataModel> {
 
         let relationOwner: DataModelField;
 
-        if (thisRelation?.references?.length && thisRelation.fields?.length) {
-            if (oppositeRelation?.references || oppositeRelation?.fields) {
-                accept('error', '"fields" and "references" must be provided only on one side of relation field', {
-                    node: oppositeField,
-                });
-                return;
-            } else {
-                relationOwner = oppositeField;
-            }
-        } else if (oppositeRelation?.references?.length && oppositeRelation.fields?.length) {
-            if (thisRelation?.references || thisRelation?.fields) {
-                accept('error', '"fields" and "references" must be provided only on one side of relation field', {
-                    node: field,
-                });
-                return;
-            } else {
-                relationOwner = field;
+        if (field.type.array && oppositeField.type.array) {
+            // if both the field is array, then it's an implicit many-to-many relation,
+            // neither side should have fields/references
+            for (const r of [thisRelation, oppositeRelation]) {
+                if (r.fields?.length || r.references?.length) {
+                    accept(
+                        'error',
+                        'Implicit many-to-many relation cannot have "fields" or "references" in @relation attribute',
+                        {
+                            node: r === thisRelation ? field : oppositeField,
+                        },
+                    );
+                }
             }
         } else {
-            // if both the field is array, then it's an implicit many-to-many relation
-            if (!(field.type.array && oppositeField.type.array)) {
+            if (thisRelation?.references?.length && thisRelation.fields?.length) {
+                if (oppositeRelation?.references || oppositeRelation?.fields) {
+                    accept('error', '"fields" and "references" must be provided only on one side of relation field', {
+                        node: oppositeField,
+                    });
+                    return;
+                } else {
+                    relationOwner = oppositeField;
+                }
+            } else if (oppositeRelation?.references?.length && oppositeRelation.fields?.length) {
+                if (thisRelation?.references || thisRelation?.fields) {
+                    accept('error', '"fields" and "references" must be provided only on one side of relation field', {
+                        node: field,
+                    });
+                    return;
+                } else {
+                    relationOwner = field;
+                }
+            } else {
+                // for non-M2M relations, one side must have fields/references
                 [field, oppositeField].forEach((f) => {
                     if (!this.isSelfRelation(f)) {
                         accept(
@@ -352,56 +375,60 @@ export default class DataModelValidator implements AstValidator<DataModel> {
                         );
                     }
                 });
+                return;
             }
-            return;
-        }
-
-        if (!relationOwner.type.array && !relationOwner.type.optional) {
-            accept('error', 'Relation field needs to be list or optional', {
-                node: relationOwner,
-            });
-            return;
-        }
 
-        if (relationOwner !== field && !relationOwner.type.array) {
-            // one-to-one relation requires defining side's reference field to be @unique
-            // e.g.:
-            //     model User {
-            //         id String @id @default(cuid())
-            //         data UserData?
-            //     }
-            //     model UserData {
-            //         id String @id @default(cuid())
-            //         user User  @relation(fields: [userId], references: [id])
-            //         userId String
-            //     }
-            //
-            // UserData.userId field needs to be @unique
-
-            const containingModel = field.$container as DataModel;
-            const uniqueFieldList = getUniqueFields(containingModel);
-
-            // field is defined in the abstract base model
-            if (containingModel !== contextModel) {
-                uniqueFieldList.push(...getUniqueFields(contextModel));
+            if (!relationOwner.type.array && !relationOwner.type.optional) {
+                accept('error', 'Relation field needs to be list or optional', {
+                    node: relationOwner,
+                });
+                return;
             }
 
-            thisRelation.fields?.forEach((ref) => {
-                const refField = ref.target.ref as DataModelField;
-                if (refField) {
-                    if (refField.attributes.find((a) => a.decl.ref?.name === '@id' || a.decl.ref?.name === '@unique')) {
-                        return;
-                    }
-                    if (uniqueFieldList.some((list) => list.includes(refField))) {
-                        return;
-                    }
-                    accept(
-                        'error',
-                        `Field "${refField.name}" on model "${containingModel.name}" is part of a one-to-one relation and must be marked as @unique or be part of a model-level @@unique attribute`,
-                        { node: refField },
-                    );
+            if (relationOwner !== field && !relationOwner.type.array) {
+                // one-to-one relation requires defining side's reference field to be @unique
+                // e.g.:
+                //     model User {
+                //         id String @id @default(cuid())
+                //         data UserData?
+                //     }
+                //     model UserData {
+                //         id String @id @default(cuid())
+                //         user User  @relation(fields: [userId], references: [id])
+                //         userId String
+                //     }
+                //
+                // UserData.userId field needs to be @unique
+
+                const containingModel = field.$container as DataModel;
+                const uniqueFieldList = getUniqueFields(containingModel);
+
+                // field is defined in the abstract base model
+                if (containingModel !== contextModel) {
+                    uniqueFieldList.push(...getUniqueFields(contextModel));
                 }
-            });
+
+                thisRelation.fields?.forEach((ref) => {
+                    const refField = ref.target.ref as DataModelField;
+                    if (refField) {
+                        if (
+                            refField.attributes.find(
+                                (a) => a.decl.ref?.name === '@id' || a.decl.ref?.name === '@unique',
+                            )
+                        ) {
+                            return;
+                        }
+                        if (uniqueFieldList.some((list) => list.includes(refField))) {
+                            return;
+                        }
+                        accept(
+                            'error',
+                            `Field "${refField.name}" on model "${containingModel.name}" is part of a one-to-one relation and must be marked as @unique or be part of a model-level @@unique attribute`,
+                            { node: refField },
+                        );
+                    }
+                });
+            }
         }
     }
 
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/tests/e2e/package.json b/tests/e2e/package.json
@@ -7,5 +7,8 @@
     },
     "dependencies": {
         "@zenstackhq/testtools": "workspace:*"
+    },
+    "devDependencies": {
+        "@zenstackhq/cli": "workspace:*"
     }
 }
diff --git a/tests/e2e/prisma-consistency/zmodel-validation.test.ts b/tests/e2e/prisma-consistency/zmodel-validation.test.ts

Original file line number	Diff line number	Diff line change
`@@ -7,5 +7,8 @@`
`7`	`7`	`},`
`8`	`8`	`"dependencies": {`
`9`	`9`	`"@zenstackhq/testtools": "workspace:*"`
	`10`	`+ },`
	`11`	`+ "devDependencies": {`
	`12`	`+ "@zenstackhq/cli": "workspace:*"`
`10`	`13`	`}`
`11`	`14`	`}`