From 0db223ede88f61cd8681160a8783f3b0ae3012fd Mon Sep 17 00:00:00 2001
From: Tony Garnock-Jones <tonyg@leastfixedpoint.com>
Date: Mon, 24 May 2021 11:53:25 +0200
Subject: [PATCH] More tests, and variant label duplicate check

---
 .../javascript/packages/schema/src/checker.ts | 17 +++--
 .../packages/schema/test/checker.test.ts      | 74 +++++++++++++++++++
 2 files changed, 85 insertions(+), 6 deletions(-)

diff --git a/implementations/javascript/packages/schema/src/checker.ts b/implementations/javascript/packages/schema/src/checker.ts
index f9a4043..a184fbc 100644
--- a/implementations/javascript/packages/schema/src/checker.ts
+++ b/implementations/javascript/packages/schema/src/checker.ts
@@ -32,13 +32,18 @@ class Checker {
 
     checkDefinition(def: M.Definition, name: symbol): void {
         switch (def._variant) {
-            case 'or':
-                [def.pattern0, def.pattern1, ... def.patternN].forEach(({ variantLabel, pattern }) =>
-                    this.checkPattern(new Set(),
-                                      pattern,
-                                      `variant ${variantLabel} of ${name.description!}`,
-                                      typeFor(_ref => ANY_TYPE, pattern)));
+            case 'or': {
+                const labels = new Set<string>();
+                [def.pattern0, def.pattern1, ... def.patternN].forEach(({ variantLabel, pattern }) => {
+                    const context = `variant ${variantLabel} of ${name.description!}`;
+                    if (labels.has(variantLabel)) {
+                        this.recordProblem(context, `duplicate variant label`);
+                    }
+                    labels.add(variantLabel);
+                    this.checkPattern(new Set(), pattern, context, typeFor(_ref => ANY_TYPE, pattern));
+                });
                 break;
+            }
             case 'and': {
                 const ps = [def.pattern0, def.pattern1, ... def.patternN];
                 const scope = new Set<string>();
diff --git a/implementations/javascript/packages/schema/test/checker.test.ts b/implementations/javascript/packages/schema/test/checker.test.ts
index 6deb73d..583355f 100644
--- a/implementations/javascript/packages/schema/test/checker.test.ts
+++ b/implementations/javascript/packages/schema/test/checker.test.ts
@@ -59,5 +59,79 @@ describe('checker', () => {
             expect(() => readSchema('version 1 . A = [@a string @b string @c int @a any ...].'))
                 .toThrow(/duplicate binding named "a" in tail of A/);
         });
+        describe('in records', () => {
+            it('complains about duplicates in recs (1)', () => {
+                expect(() => readSchema('version 1 . A = <a @a string @a int>.'))
+                    .toThrow(/duplicate binding named "a" in item 1 of fields of A/);
+            });
+            it('complains about duplicates in recs (2)', () => {
+                expect(() => readSchema('version 1 . A = <a @a string <x @y int @a int>>.'))
+                    .toThrow(/duplicate binding named "a" in item 1 of fields of item 1 of fields of A/);
+            });
+            it('complains about duplicates in recs (3)', () => {
+                expect(() => readSchema('version 1 . A = <a @a string <<rec> =x [@y int @a int]>>.'))
+                    .toThrow(/duplicate binding named "a" in item 1 of fields of item 1 of fields of A/);
+            });
+            it('complains about duplicates in recs (4)', () => {
+                expect(() => readSchema('version 1 . A = <a @a string <<rec> @a =x [@y int @z int]>>.'))
+                    .toThrow(/duplicate binding named "a" in label of item 1 of fields of A/);
+            });
+            it('complains about duplicates in recs (5)', () => {
+                expect(() => readSchema('version 1 . A = <a @a string <<rec> @a any [@y int @z int]>>.'))
+                    .toThrow(/duplicate binding named "a" in label of item 1 of fields of A/);
+            });
+        });
+        describe('in unions', () => {
+            it('is OK with non-duplicate but duplicate-seeming bindings across branches', () => {
+                expect(readSchema('version 1 . A = <a @a string> / <b @a string>.')).not.toBeNull();
+            });
+            it('complains about duplicates within branches', () => {
+                expect(() => readSchema('version 1 . A = <a @a string @a int> / <b @a string>.'))
+                    .toThrow(/in item 1 of fields of variant a of A/);
+            });
+            it('complains about duplicate branch names', () => {
+                expect(() => readSchema('version 1 . A = @x <a @a string> / @x <b @a string>.'))
+                    .toThrow(/duplicate variant label in variant x of A/);
+            });
+        });
+        describe('in intersections', () => {
+            it('is OK with non-duplicate bindings across branches', () => {
+                expect(readSchema('version 1 . A = <a @a string> & <a @b string>.')).not.toBeNull();
+            });
+            it('complains about duplicates within branches', () => {
+                expect(() => readSchema(
+                    `version 1 . A = <a @a string @a int> & <a @b string @c int>.`))
+                    .toThrow(/in item 1 of fields of A/);
+            });
+            it('complains about duplicates across branches', () => {
+                expect(() => readSchema(
+                    `version 1 . A = <a @a string @b int> & <a @a string @c int>.`))
+                    .toThrow(/in item 0 of fields of A/);
+            });
+            it('complains about duplicates within named branches', () => {
+                expect(() => readSchema(
+                    `version 1 .
+                     AAA = <a @a string @a int>.
+                     ABC = <a @b string @c int>.
+                     A = @x AAA & @y ABC.`))
+                    .toThrow(/in item 1 of fields of AAA/);
+            });
+            it('is OK with seeming- but non-duplicates across named branches', () => {
+                expect(readSchema(
+                    `version 1 .
+                     AAA = <a @a string @b int>.
+                     ABC = <a @a string @c int>.
+                     A = @x AAA & @y ABC.`))
+                    .not.toBeNull();
+            });
+            it('complains about duplicate branch names', () => {
+                expect(() => readSchema(
+                    `version 1 .
+                     AAB = <a @a string @b int>.
+                     ACD = <a @c string @d int>.
+                     A = @x AAB & @x ACD.`))
+                    .toThrow(/in A/);
+            });
+        });
     });
 });