Skip to content

Commit

Permalink
better errors for issues with clauses (#387)
Browse files Browse the repository at this point in the history
  • Loading branch information
bakkot authored Dec 10, 2021
1 parent cfd4f94 commit 79db94d
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 53 deletions.
2 changes: 1 addition & 1 deletion src/Clause.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ export default class Clause extends Builder {

let nextNumber = '';
if (node.nodeName !== 'EMU-INTRO') {
nextNumber = clauseNumberer.next(clauseStack.length, node.nodeName === 'EMU-ANNEX').value;
nextNumber = clauseNumberer.next(clauseStack, node);
}
const parent = clauseStack[clauseStack.length - 1] || null;

Expand Down
2 changes: 1 addition & 1 deletion src/Spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ export default class Spec {
importStack: [],
clauseStack: [],
tagStack: [],
clauseNumberer: ClauseNumbers(),
clauseNumberer: ClauseNumbers(this),
inNoAutolink: false,
inAlg: false,
inNoEmd: false,
Expand Down
53 changes: 40 additions & 13 deletions src/clauseNums.ts
Original file line number Diff line number Diff line change
@@ -1,39 +1,66 @@
import type Clause from './Clause';
import type { Spec } from './ecmarkup';

/*@internal*/
export interface ClauseNumberIterator {
next(level: number, annex: boolean): IteratorResult<string>;
next(clauseStack: Clause[], node: HTMLElement): string;
}

/*@internal*/
export default function iterator(): ClauseNumberIterator {
export default function iterator(spec: Spec): ClauseNumberIterator {
const ids: (string | number)[] = [];
let inAnnex = false;
let currentLevel = 0;

return {
next(level: number, annex: boolean) {
if (inAnnex && !annex) throw new Error('Clauses cannot follow annexes');
if (level - currentLevel > 1) throw new Error('Skipped clause');
next(clauseStack: Clause[], node: HTMLElement) {
const annex = node.nodeName === 'EMU-ANNEX';
const level = clauseStack.length;
if (inAnnex && !annex) {
spec.warn({
type: 'node',
node,
ruleId: 'clause-after-annex',
message: 'clauses cannot follow annexes',
});
}
if (level - currentLevel > 1) {
spec.warn({
type: 'node',
node,
ruleId: 'skipped-caluse',
message: 'clause is being numbered without numbering its parent clause',
});
}

const nextNum = annex ? nextAnnexNum : nextClauseNum;

if (level === currentLevel) {
ids[currentLevel] = nextNum(level);
ids[currentLevel] = nextNum(clauseStack, node);
} else if (level > currentLevel) {
ids.push(nextNum(level));
ids.push(nextNum(clauseStack, node));
} else {
ids.length = level + 1;
ids[level] = nextNum(level);
ids[level] = nextNum(clauseStack, node);
}

currentLevel = level;

return { value: ids.join('.'), done: false };
return ids.join('.');
},
};

function nextAnnexNum(level: number): string | number {
function nextAnnexNum(clauseStack: Clause[], node: HTMLElement): string | number {
const level = clauseStack.length;
if (!inAnnex) {
if (level > 0) throw new Error('First annex must be at depth 0');
if (level > 0) {
spec.warn({
type: 'node',
node,
ruleId: 'annex-depth',
message: 'first annex must be at depth 0',
});
}
inAnnex = true;

return 'A';
Expand All @@ -43,10 +70,10 @@ export default function iterator(): ClauseNumberIterator {
return String.fromCharCode((<string>ids[0]).charCodeAt(0) + 1);
}

return nextClauseNum(level);
return nextClauseNum(clauseStack);
}

function nextClauseNum(level: number) {
function nextClauseNum({ length: level }: { length: number }) {
if (ids[level] === undefined) return 1;
return <number>ids[level] + 1;
}
Expand Down
9 changes: 2 additions & 7 deletions test/baselines/sources/malformed.bad.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,2 @@
<emu-annex id="sec-annex">
<h1>Annex</h1>
</emu-annex>
<emu-clause id="sec-clause">
<h1>Clause</h1>
<p>A clause cannot follow an annex and therefore this is malformed.</p>
</emu-clause>
<!-- emu-import requires href -->
<emu-import></emu-import>
43 changes: 12 additions & 31 deletions test/clauseIds.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,36 +11,17 @@ describe('clause id generation', () => {
});

specify('generating clause ids', () => {
assert.strictEqual(iter.next(0).value, '1');
assert.strictEqual(iter.next(1).value, '1.1');
assert.strictEqual(iter.next(1).value, '1.2');
assert.strictEqual(iter.next(2).value, '1.2.1');
assert.strictEqual(iter.next(0).value, '2');
assert.strictEqual(iter.next(0, true).value, 'A');
assert.strictEqual(iter.next(1, true).value, 'A.1');
assert.strictEqual(iter.next(1, true).value, 'A.2');
assert.strictEqual(iter.next(2, true).value, 'A.2.1');
assert.strictEqual(iter.next(0, true).value, 'B');
});

specify('error thrown for skipping clauses', () => {
assert.throws(() => {
iter.next(2);
}, /Skipped clause/);
});

specify('error thrown for non-annex following annex', () => {
assert.throws(() => {
iter.next(0);
iter.next(0, true);
iter.next(0);
}, /Clauses cannot follow annexes/);
});

specify('error thrown for annex not starting at depth 0', () => {
assert.throws(() => {
iter.next(0);
iter.next(1, true);
}, /First annex must be at depth 0/);
const CLAUSE = { nodeName: 'EMU-CLAUSE' };
const ANNEX = { nodeName: 'EMU-ANNEX' };
assert.strictEqual(iter.next([], CLAUSE), '1');
assert.strictEqual(iter.next([{}], CLAUSE), '1.1');
assert.strictEqual(iter.next([{}], CLAUSE), '1.2');
assert.strictEqual(iter.next([{}, {}], CLAUSE), '1.2.1');
assert.strictEqual(iter.next([], CLAUSE), '2');
assert.strictEqual(iter.next([], ANNEX), 'A');
assert.strictEqual(iter.next([{}], ANNEX), 'A.1');
assert.strictEqual(iter.next([{}], ANNEX), 'A.2');
assert.strictEqual(iter.next([{}, {}], ANNEX), 'A.2.1');
assert.strictEqual(iter.next([], ANNEX), 'B');
});
});
36 changes: 36 additions & 0 deletions test/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -812,4 +812,40 @@ ${M} </pre>
{ lintSpec: true }
);
});

it('clause after annex', async () => {
await assertError(
positioned`
<emu-annex id="sec-a">
<h1>Annex</h1>
</emu-annex>
${M}<emu-clause id="sec-c">
<h1>Clause</h1>
</emu-clause>
`,
{
ruleId: 'clause-after-annex',
nodeType: 'emu-clause',
message: 'clauses cannot follow annexes',
}
);
});

it('nested annex', async () => {
await assertError(
positioned`
<emu-clause id="sec-c">
<h1>Clause</h1>
${M}<emu-annex id="sec-a">
<h1>Annex</h1>
</emu-annex>
</emu-clause>
`,
{
ruleId: 'annex-depth',
nodeType: 'emu-annex',
message: 'first annex must be at depth 0',
}
);
});
});

0 comments on commit 79db94d

Please sign in to comment.