Detect duplicate declaration (#9)

src/validator.ts

@@ -8,43 +8,99 @@ import {
   RecognitionException,
 } from 'antlr4ts';
 import { YorkieSchemaLexer } from '../antlr/YorkieSchemaLexer';
-import { YorkieSchemaParser } from '../antlr/YorkieSchemaParser';
+import {
+  ObjectTypeContext,
+  PropertyNameContext,
+  YorkieSchemaParser,
+} from '../antlr/YorkieSchemaParser';
 import { YorkieSchemaListener } from '../antlr/YorkieSchemaListener';
 import {
   TypeAliasDeclarationContext,
   TypeReferenceContext,
 } from '../antlr/YorkieSchemaParser';
 import { ParseTreeWalker } from 'antlr4ts/tree';
 
+/**
+ * `TypeSymbol` represents a type alias declaration.
+ */
 type TypeSymbol = {
   name: string;
   line: number;
   column: number;
 };
 
+/**
+ * `TypeReference` represents a type reference in a type alias declaration.
+ */
 type TypeReference = {
   name: string;
   parent: string;
   line: number;
   column: number;
 };
 
+/**
+ * `Diagnostic` represents a diagnostic message.
+ */
+export type Diagnostic = {
+  severity: 'error' | 'warning' | 'info';
+  message: string;
+  range: {
+    start: { column: number; line: number };
+    end: { column: number; line: number };
+  };
+};
+
 export class TypeCollectorListener implements YorkieSchemaListener {
-  public symbolTable: Map<string, TypeSymbol> = new Map();
-  public errors: Array<{ message: string; line: number; column: number }> = [];
+  private symbol: string | null = null;
+  private properties: Set<string> | null = null;
 
-  public parent: string | null = null;
+  public symbolMap: Map<string, TypeSymbol> = new Map();
   public referenceMap: Map<string, TypeReference> = new Map();
+  public errors: Array<{ message: string; line: number; column: number }> = [];
 
   enterTypeAliasDeclaration(ctx: TypeAliasDeclarationContext) {
     const typeName = ctx.Identifier().text;
     const { line, charPositionInLine } = ctx.Identifier().symbol;
-    this.symbolTable.set(typeName, {
+
+    if (this.symbolMap.has(typeName)) {
+      this.errors.push({
+        message: `Duplicate type declaration: ${typeName}`,
+        line: line,
+        column: charPositionInLine,
+      });
+    }
+
+    this.symbolMap.set(typeName, {
       name: typeName,
       line: line,
       column: charPositionInLine,
     });
-    this.parent = typeName;
+
+    this.symbol = typeName;
+  }
+
+  enterObjectType(_: ObjectTypeContext) {
+    this.properties = new Set();
+  }
+
+  exitObjectType(_: ObjectTypeContext) {
+    this.properties = null;
+  }
+
+  enterPropertyName(ctx: PropertyNameContext) {
+    const typeName = ctx.Identifier()!.text;
+    const { line, charPositionInLine } = ctx.Identifier()!.symbol;
+
+    if (this.properties?.has(typeName)) {
+      this.errors.push({
+        message: `Duplicate property name: ${typeName}`,
+        line: line,
+        column: charPositionInLine,
+      });
+    }
+
+    this.properties?.add(typeName);
   }
 
   enterTypeReference(ctx: TypeReferenceContext) {
@@ -53,22 +109,13 @@ export class TypeCollectorListener implements YorkieSchemaListener {
 
     this.referenceMap.set(typeName, {
       name: typeName,
-      parent: this.parent!,
+      parent: this.symbol!,
       line: line,
       column: charPositionInLine,
     });
   }
 }
 
-export type Diagnostic = {
-  severity: 'error' | 'warning' | 'info';
-  message: string;
-  range: {
-    start: { column: number; line: number };
-    end: { column: number; line: number };
-  };
-};
-
 class LexerErrorListener implements ANTLRErrorListener<number> {
   constructor(private errorList: Diagnostic[]) {}
 
@@ -144,7 +191,7 @@ export function validate(data: string): { errors: Array<Diagnostic> } {
 
   // TODO(hackerwins): This is a naive implementation and performance can be improved.
   for (const [, ref] of listener.referenceMap) {
-    if (!listener.symbolTable.has(ref.name)) {
+    if (!listener.symbolMap.has(ref.name)) {
       listener.errors.push({
         message: `Type '${ref.name}' is not defined.`,
         line: ref.line,
@@ -153,7 +200,7 @@ export function validate(data: string): { errors: Array<Diagnostic> } {
     }
   }
 
-  for (const [, symbol] of listener.symbolTable) {
+  for (const [, symbol] of listener.symbolMap) {
     const visited = new Set();
     let current: string | undefined = symbol.name;
     while (current) {
@@ -166,16 +213,13 @@ export function validate(data: string): { errors: Array<Diagnostic> } {
         });
         break;
       }
-      if (current === 'Document') {
-        break;
-      }
 
       visited.add(current);
       current = listener.referenceMap.get(current)?.parent;
     }
 
     // 03. Check if there is a type that is not in the document.
-    if (!current || current !== 'Document') {
+    if (!visited.has('Document')) {
       listener.errors.push({
         message: `Type '${symbol.name}' is not in the document.`,
         line: symbol.line,

test/schema.test.ts

@@ -97,6 +97,34 @@ describe('Schema:TypeScript', () => {
     `;
     expect(validate(schema).errors.length).toBe(0);
   });
+
+  it('should detect duplicate type alias declarations', () => {
+    const schema = `
+      type Document = {
+      };
+      type Document = {
+      };
+    `;
+    expect(validate(schema).errors.length).toBeGreaterThan(0);
+  });
+
+  it('should detect duplicate keys in object', () => {
+    const schema = `
+      type Document = {
+        field: string;
+        field: number;
+      };
+    `;
+    expect(validate(schema).errors.length).toBeGreaterThan(0);
+
+    const schema2 = `
+      type Document = {
+        key: string;
+        obj: { key: string; };
+      };
+    `;
+    expect(validate(schema2).errors.length).toBe(0);
+  });
 });
 
 describe('Schema:Yorkie', () => {
@@ -259,4 +287,14 @@ describe('Schema:Semantic', () => {
     `;
     expect(validate(schema).errors.length).toBeGreaterThan(0);
   });
+
+  it('should detect circular references with Document type', () => {
+    const schema = `
+      type Document = {
+        field1: string;
+        field2: Document;
+      };
+    `;
+    expect(validate(schema).errors.length).toBeGreaterThan(0);
+  });
 });