reject AO headers with duplicate parameter names (#689)

Author: michaelficarra

src/header-parser.ts

@@ -105,6 +105,16 @@ export function parseHeader(headerText: string): ParsedHeaderOrFailure {
   let type: 'single-line' | 'multi-line';
   const params: Param[] = [];
   const optionalParams: Param[] = [];
+
+  function checkDuplicateParam(paramName: string, paramNameOffset: number) {
+    if (params.some(p => p.name === paramName) || optionalParams.some(p => p.name === paramName)) {
+      errors.push({
+        message: `duplicate parameter ${JSON.stringify(paramName)}`,
+        offset: paramNameOffset,
+      });
+    }
+  }
+
   if (text[0] === '\n') {
     // multiline: parse for parameter types
     type = 'multi-line';
@@ -142,8 +152,9 @@ export function parseHeader(headerText: string): ParsedHeaderOrFailure {
         errors.push({ message: 'expected parameter name', offset });
         return { type: 'failure', errors };
       }
-      offset += match[0].length;
       const paramName = match[0].trimRight();
+      checkDuplicateParam(paramName, offset);
+      offset += match[0].length;
 
       ({ match, text } = eat(text, /^:+ */i));
       if (!match) {
@@ -216,8 +227,9 @@ export function parseHeader(headerText: string): ParsedHeaderOrFailure {
         errors.push({ message: 'expected parameter name', offset });
         return { type: 'failure', errors };
       }
-      offset += match[0].length;
       const paramName = match[0].trimRight();
+      checkDuplicateParam(paramName, offset);
+      offset += match[0].length;
 
       (optional ? optionalParams : params).push({
         name: paramName,

src/lint/rules/variable-use-def.ts

@@ -144,8 +144,12 @@ export function checkVariableUsage(
       preceding.textContent != null
     ) {
       const isParameter = preceding.nodeName === 'H1';
+      const seen = new Set<string>();
       // `__` is for <del>_x_</del><ins>_y_</ins>, which has textContent `_x__y_`
       for (const name of preceding.textContent.matchAll(/(?<=\b|_)_([a-zA-Z0-9]+)_(?=\b|_)/g)) {
+        // We avoid dealing with parameter re-declaration here because tracking location information is annoying. It's handled in `checkDuplicateParam` in header-parser instead.
+        if (seen.has(name[1])) continue;
+        seen.add(name[1]);
         scope.declare(name[1], null, isParameter ? 'parameter' : undefined, !isParameter);
       }
     }

test/lint.ts

@@ -424,6 +424,30 @@ describe('linting whole program', () => {
           </emu-clause>
       `);
     });
+
+    it('duplicate parameters', async () => {
+      await assertLint(
+        positioned`
+          <emu-clause id="example" type="abstract operation">
+            <h1>
+              Example (
+                _x_: ~foo~,
+                ${M}_x_: ~bar~,
+              ): ~foo~ or ~bar~
+            </h1>
+            <dl class="header"></dl>
+            <emu-alg>
+              1. Return _x_.
+            </emu-alg>
+          </emu-clause>
+        `,
+        {
+          ruleId: 'header-format',
+          nodeType: 'h1',
+          message: 'duplicate parameter "_x_"',
+        },
+      );
+    });
   });
 
   describe('closing tags', () => {