From aad188d7650602e607463fbe6840c6702b82ff9b Mon Sep 17 00:00:00 2001
From: Kevin Gibbons <bakkot@gmail.com>
Date: Thu, 24 Dec 2020 10:53:45 -0800
Subject: [PATCH] distinguish between attribute errors and attribute value
 errors

---
 src/Builder.ts    |  2 +-
 src/Note.ts       |  2 +-
 src/Spec.ts       | 21 +++++++++++++++++++--
 src/Xref.ts       |  8 ++++----
 src/utils.ts      | 13 +++++++++++++
 test/lint-tags.js |  2 +-
 6 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/src/Builder.ts b/src/Builder.ts
index 0515c986..f45d18be 100644
--- a/src/Builder.ts
+++ b/src/Builder.ts
@@ -23,7 +23,7 @@ export default class Builder {
     if (nodeId !== null) {
       if (spec.nodeIds.has(nodeId)) {
         spec.warn({
-          type: 'attr',
+          type: 'attr-value',
           attr: 'id',
           ruleId: 'duplicate-id',
           message: `<${node.tagName.toLowerCase()}> has duplicate id ${JSON.stringify(nodeId)}`,
diff --git a/src/Note.ts b/src/Note.ts
index 4a530bf7..a00e4add 100644
--- a/src/Note.ts
+++ b/src/Note.ts
@@ -72,7 +72,7 @@ export default class Note extends Builder {
       label = "Editor's Note";
     } else {
       this.spec.warn({
-        type: 'attr',
+        type: 'attr-value',
         attr: 'type',
         ruleId: 'invalid-note',
         message: `unknown note type ${JSON.stringify(this.type)}`,
diff --git a/src/Spec.ts b/src/Spec.ts
index 9af8893e..a79b1a47 100644
--- a/src/Spec.ts
+++ b/src/Spec.ts
@@ -99,6 +99,13 @@ export type Warning =
       ruleId: string;
       message: string;
     }
+  | {
+      type: 'attr-value';
+      node: Element;
+      attr: string;
+      ruleId: string;
+      message: string;
+    }
   | {
       type: 'contents';
       node: Text | Element;
@@ -158,6 +165,16 @@ function wrapWarn(source: string, spec: Spec, warn: (err: EcmarkupError) => void
         }
         break;
       case 'attr': {
+        let loc = spec.locate(e.node);
+        if (loc) {
+          file = loc.file;
+          source = loc.source;
+          ({ line, column } = utils.attrLocation(source, loc, e.attr));
+        }
+        nodeType = e.node.tagName.toLowerCase();
+        break;
+      }
+      case 'attr-value': {
         let loc = spec.locate(e.node);
         if (loc) {
           file = loc.file;
@@ -924,7 +941,7 @@ export default class Spec {
         let targetEntry = this.biblio.byId(target);
         if (targetEntry == null) {
           this.warn({
-            type: 'attr',
+            type: 'attr-value',
             attr: 'replaces-step',
             ruleId: 'invalid-replacement',
             message: `could not find step ${JSON.stringify(target)}`,
@@ -932,7 +949,7 @@ export default class Spec {
           });
         } else if (targetEntry.type !== 'step') {
           this.warn({
-            type: 'attr',
+            type: 'attr-value',
             attr: 'replaces-step',
             ruleId: 'invalid-replacement',
             message: `expected algorithm to replace a step, not a ${targetEntry.type}`,
diff --git a/src/Xref.ts b/src/Xref.ts
index a7811abc..cdd7b96a 100644
--- a/src/Xref.ts
+++ b/src/Xref.ts
@@ -78,7 +78,7 @@ export default class Xref extends Builder {
     if (href) {
       if (href[0] !== '#') {
         spec.warn({
-          type: 'attr',
+          type: 'attr-value',
           attr: 'href',
           ruleId: 'invalid-xref',
           message: `xref to anything other than a fragment id is not supported (is ${JSON.stringify(
@@ -94,7 +94,7 @@ export default class Xref extends Builder {
       this.entry = spec.biblio.byId(id);
       if (!this.entry) {
         spec.warn({
-          type: 'attr',
+          type: 'attr-value',
           attr: 'href',
           ruleId: 'xref-not-found',
           message: `can't find clause, production, note or example with id ${JSON.stringify(id)}`,
@@ -148,7 +148,7 @@ export default class Xref extends Builder {
       let namespaceSuffix =
         namespace === '<no location>' ? '' : ` in namespace ${JSON.stringify(namespace)}`;
       spec.warn({
-        type: 'attr',
+        type: 'attr-value',
         attr: 'aoid',
         ruleId: 'xref-not-found',
         message: `can't find abstract op with aoid ${JSON.stringify(aoid)}` + namespaceSuffix,
@@ -262,7 +262,7 @@ function buildStepLink(spec: Spec, xref: Element, entry: Biblio.StepBiblioEntry)
     let applicable = bullets[Math.min(i, 5)];
     if (s > applicable.length) {
       spec.warn({
-        type: 'attr',
+        type: 'attr-value',
         ruleId: 'high-step-number',
         message: `ecmarkup does not know how to deal with step numbers as high as ${s}; if you need this, open an issue on ecmarkup`,
         node: xref,
diff --git a/src/utils.ts b/src/utils.ts
index 0903996e..e1c5a89b 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -187,6 +187,19 @@ export function offsetToLineAndColumn(string: string, offset: number) {
   return { line: line + 1, column: column + 1 };
 }
 
+export function attrLocation(
+  source: string | undefined,
+  loc: MarkupData.ElementLocation,
+  attr: string
+) {
+  let attrLoc = loc.startTag.attrs[attr];
+  if (attrLoc == null) {
+    return { line: loc.startTag.line, column: loc.startTag.col };
+  } else {
+    return { line: attrLoc.line, column: attrLoc.col };
+  }
+}
+
 export function attrValueLocation(
   source: string | undefined,
   loc: MarkupData.ElementLocation,
diff --git a/test/lint-tags.js b/test/lint-tags.js
index 662188c4..5b9bbc47 100644
--- a/test/lint-tags.js
+++ b/test/lint-tags.js
@@ -19,7 +19,7 @@ describe('tags', () => {
   it('oldid', async () => {
     await assertLint(
       positioned`
-        <emu-clause id="foo" oldid="${M}bar">
+        <emu-clause id="foo" ${M}oldid="bar">
           <h1>Example</h1>
         </emu-clause>
       `,