Skip to content

Commit 504bfa6

Browse files
authored
Fix <ins>/<del> with structured headers (#533)
1 parent 1802e24 commit 504bfa6

16 files changed

+136
-69
lines changed

src/Clause.ts

+26-18
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
parseH1,
1414
ParsedHeader,
1515
} from './header-parser';
16-
import { offsetToLineAndColumn } from './utils';
16+
import { offsetToLineAndColumn, traverseWhile } from './utils';
1717

1818
const aoidTypes = [
1919
'abstract operation',
@@ -32,7 +32,11 @@ export const SPECIAL_KINDS_MAP = new Map([
3232
export const SPECIAL_KINDS = [...SPECIAL_KINDS_MAP.keys()];
3333

3434
export function extractStructuredHeader(header: Element): Element | null {
35-
const dl = header.nextElementSibling;
35+
const dl = traverseWhile(
36+
header.nextElementSibling,
37+
'nextElementSibling',
38+
el => el.nodeName === 'DEL'
39+
);
3640
if (dl == null || dl.tagName !== 'DL' || !dl.classList.contains('header')) {
3741
return null;
3842
}
@@ -95,39 +99,43 @@ export default class Clause extends Builder {
9599
}
96100

97101
this.signature = null;
98-
let header = this.node.firstElementChild;
99-
while (header != null && header.tagName === 'SPAN' && header.children.length === 0) {
100-
// skip oldids
101-
header = header.nextElementSibling;
102-
}
103-
if (header == null) {
102+
const header = traverseWhile(
103+
this.node.firstElementChild,
104+
'nextElementSibling',
105+
// skip <del> and oldids
106+
el => el.nodeName === 'DEL' || (el.nodeName === 'SPAN' && el.children.length === 0)
107+
);
108+
let headerH1 = traverseWhile(header, 'firstElementChild', el => el.nodeName === 'INS', {
109+
once: true,
110+
});
111+
if (headerH1 == null) {
104112
this.spec.warn({
105113
type: 'node',
106114
ruleId: 'missing-header',
107115
message: `could not locate header element`,
108116
node: this.node,
109117
});
110-
header = null;
111-
} else if (header.tagName !== 'H1') {
118+
headerH1 = null;
119+
} else if (headerH1.tagName !== 'H1') {
112120
this.spec.warn({
113121
type: 'node',
114122
ruleId: 'missing-header',
115-
message: `could not locate header element; found <${header.tagName.toLowerCase()}> before any <h1>`,
116-
node: header,
123+
message: `could not locate header element; found <${header!.tagName.toLowerCase()}> before any <h1>`,
124+
node: header!,
117125
});
118-
header = null;
126+
headerH1 = null;
119127
} else {
120-
this.buildStructuredHeader(header);
128+
this.buildStructuredHeader(headerH1, header!);
121129
}
122-
this.header = header;
123-
if (header == null) {
130+
this.header = headerH1;
131+
if (headerH1 == null) {
124132
this.title = 'UNKNOWN';
125133
this.titleHTML = 'UNKNOWN';
126134
}
127135
}
128136

129-
buildStructuredHeader(header: Element) {
130-
const dl = extractStructuredHeader(header);
137+
buildStructuredHeader(header: Element, headerSurrogate: Element = header) {
138+
const dl = extractStructuredHeader(headerSurrogate);
131139
if (dl === null) {
132140
return;
133141
}

src/Production.ts

+7-11
Original file line numberDiff line numberDiff line change
@@ -52,18 +52,14 @@ export default class Production extends Builder {
5252
if (node.hasAttribute('primary')) {
5353
primary = true;
5454
} else {
55-
let parent = node.parentElement;
56-
if (parent != null) {
55+
const parent = utils.traverseWhile(
56+
node.parentElement,
57+
'parentElement',
5758
// highlighted nodes still count as primary unless they are being deleted (i.e. in a <del> tag)
58-
while (parent.tagName === 'INS' || parent.tagName === 'MARK') {
59-
parent = parent.parentElement;
60-
if (parent == null) {
61-
break;
62-
}
63-
}
64-
if (parent != null && parent.tagName === 'EMU-GRAMMAR') {
65-
primary = parent.hasAttribute('primary') || parent.getAttribute('type') === 'definition';
66-
}
59+
el => el.nodeName === 'INS' || el.nodeName === 'MARK'
60+
);
61+
if (parent != null && parent.tagName === 'EMU-GRAMMAR') {
62+
primary = parent.hasAttribute('primary') || parent.getAttribute('type') === 'definition';
6763
}
6864
}
6965
this.primary = primary;

src/clauseNums.ts

+15-11
Original file line numberDiff line numberDiff line change
@@ -108,17 +108,21 @@ export default function iterator(spec: Spec): ClauseNumberIterator {
108108
message:
109109
'multi-step explicit clause numbers should not be mixed with single-step clause numbers in the same parent clause',
110110
});
111-
} else if (
112-
nums.some((n, i) => n < ids[level][i]) ||
113-
nums.every((n, i) => n === ids[level][i])
114-
) {
115-
spec.warn({
116-
type: 'attr-value',
117-
node,
118-
attr: 'number',
119-
ruleId: 'invalid-clause-number',
120-
message: 'clause numbers should be strictly increasing',
121-
});
111+
} else {
112+
// Make sure that `nums` is strictly greater than `ids[level]` (i.e.,
113+
// that their items are not identical and that the item in `nums` is
114+
// strictly greater than the value in `ids[level]` at the first
115+
// index where they differ).
116+
const i = nums.findIndex((num, i) => num !== ids[level][i]);
117+
if (i < 0 || !(nums[i] > ids[level][i])) {
118+
spec.warn({
119+
type: 'attr-value',
120+
node,
121+
attr: 'number',
122+
ruleId: 'invalid-clause-number',
123+
message: 'clause numbers should be strictly increasing',
124+
});
125+
}
122126
}
123127
}
124128
return nums;

src/header-parser.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ export function parseH1(headerText: string): ParsedHeaderOrFailure {
237237
if (match) {
238238
offset += match[0].length;
239239
returnOffset = offset;
240-
({ match, text } = eat(text, /^(.*)(?!<\/(ins|del|mark)>)/i));
240+
({ match, text } = eat(text, /^(.*?)(?=<\/(ins|del|mark)>|$)/im));
241241
if (match) {
242242
returnType = match[1].trim();
243243
if (returnType === '') {

src/utils.ts

+19-12
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,21 @@ export function replaceTextNode(node: Node, frag: DocumentFragment) {
117117
return newXrefNodes;
118118
}
119119

120+
/*@internal*/
121+
export function traverseWhile<P extends string, T extends Record<P, T | null>>(
122+
node: T | null,
123+
relationship: P,
124+
predicate: (node: T) => boolean,
125+
options?: { once?: boolean }
126+
): T | null {
127+
const once = options?.once ?? false;
128+
while (node != null && predicate(node)) {
129+
node = node[relationship];
130+
if (once) break;
131+
}
132+
return node;
133+
}
134+
120135
/*@internal*/
121136
export function logVerbose(str: string) {
122137
const dateString = new Date().toISOString();
@@ -132,20 +147,12 @@ export function logWarning(str: string) {
132147
const CLAUSE_LIKE = ['EMU-ANNEX', 'EMU-CLAUSE', 'EMU-INTRO', 'EMU-NOTE', 'BODY'];
133148
/*@internal*/
134149
export function shouldInline(node: Node) {
135-
let parent = node.parentNode;
150+
const surrogateParentTags = ['EMU-GRAMMAR', 'EMU-IMPORT', 'INS', 'DEL'];
151+
const parent = traverseWhile(node.parentNode, 'parentNode', node =>
152+
surrogateParentTags.includes(node?.nodeName ?? '')
153+
);
136154
if (!parent) return false;
137155

138-
while (
139-
parent &&
140-
parent.parentNode &&
141-
(parent.nodeName === 'EMU-GRAMMAR' ||
142-
parent.nodeName === 'EMU-IMPORT' ||
143-
parent.nodeName === 'INS' ||
144-
parent.nodeName === 'DEL')
145-
) {
146-
parent = parent.parentNode;
147-
}
148-
149156
const clauseLikeParent =
150157
CLAUSE_LIKE.includes(parent.nodeName) ||
151158
CLAUSE_LIKE.includes((parent as Element).getAttribute('data-simulate-tagname')?.toUpperCase()!);

test/baselines.js

+29-3
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ describe('baselines', () => {
3535
let optionsSets = [{ lintSpec: false }];
3636
for (let file of files) {
3737
const reference = REFERENCE_DIR + file;
38-
it(SOURCES_DIR + file, async () => {
38+
const sourcePath = SOURCES_DIR + file;
39+
it(sourcePath, async () => {
3940
let expectedFiles = new Map();
4041

4142
(function walk(f) {
@@ -51,7 +52,7 @@ describe('baselines', () => {
5152
}
5253
})(reference);
5354

54-
let spec = await build(SOURCES_DIR + file, {});
55+
let spec = await build(sourcePath, {});
5556

5657
let actualFiles = handleSingleFileOutput(spec.generatedFiles);
5758

@@ -81,14 +82,39 @@ describe('baselines', () => {
8182
if (rebaseline) {
8283
return;
8384
}
85+
86+
let contents = fs.readFileSync(sourcePath, 'utf8');
87+
let expectedWarnings = [...contents.matchAll(/<!--\s+EXPECT_WARNING(.*?)-->/g)].map(m =>
88+
JSON.parse(m[1])
89+
);
90+
let warningProps = new Set(expectedWarnings.flatMap(obj => Object.keys(obj)));
91+
function pickFromWarning(warning) {
92+
if (warningProps.size === 0) {
93+
// No warnings are expected, so "pick" the entire object.
94+
return warning;
95+
}
96+
let picks = {};
97+
for (let [key, value] of Object.entries(warning)) {
98+
if (warningProps.has(key)) picks[key] = value;
99+
}
100+
return picks;
101+
}
102+
84103
for (let options of optionsSets) {
85-
let spec = await build(SOURCES_DIR + file, options);
104+
let warnings = [];
105+
let warn = warning => warnings.push(warning);
106+
let spec = await build(sourcePath, { ...options, warn });
86107
let actualFiles = handleSingleFileOutput(spec.generatedFiles);
87108
assert.deepStrictEqual(
88109
actualFiles,
89110
expectedFiles,
90111
`output differed when using option ${JSON.stringify(options)}`
91112
);
113+
assert.deepStrictEqual(
114+
warnings.map(warning => pickFromWarning(warning)),
115+
expectedWarnings,
116+
`warnings differed when using option ${JSON.stringify(options)}`
117+
);
92118
}
93119
});
94120
}

test/baselines/generated-reference/clauses.html

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
<li><span>Jump to search box</span><code>/</code></li>
88
</ul></div><div id="spec-container">
9-
<div><h2>Table of Contents</h2><ol class="toc"><li><a href="#sec-intro" title="Intro">Intro</a><ol class="toc"><li><a href="#sec-intro2" title="Sub Intro">Sub Intro</a></li></ol></li><li><a href="#sec-clause" title="Clause Foo(a, b)"><span class="secnum">1</span> Clause Foo(<var>a</var>, <var>b</var>)</a><ol class="toc"><li><a href="#Foo" title="Sub Clause"><span class="secnum">1.1</span> Sub Clause</a></li></ol></li><li><a href="#sec-number" title="Explicit number"><span class="secnum">7</span> Explicit number</a><ol class="toc"><li><a href="#sec-number1" title="Automatic number inside explicit number"><span class="secnum">7.1</span> Automatic number inside explicit number</a></li><li><a href="#sec-number2" title="Nested explicit number"><span class="secnum">7.4</span> Nested explicit number</a></li><li><a href="#sec-number3" title="Automatic number after explicit number"><span class="secnum">7.5</span> Automatic number after explicit number</a></li><li><a href="#sec-number-nested" title="Multi-step explicit numbers"><span class="secnum">7.6</span> Multi-step explicit numbers</a><ol class="toc"><li><a href="#sec-number-multi-step" title="Multi-step explicit numbers"><span class="secnum">7.6.1.1</span> Multi-step explicit numbers</a></li><li><a href="#sec-number-multi-step-1" title="Automatic number after explicit number"><span class="secnum">7.6.1.2</span> Automatic number after explicit number</a><ol class="toc"><li><a href="#sec-number-multi-step-inner" title="Nested clause after explicit multi-step number"><span class="secnum">7.6.1.2.1</span> Nested clause after explicit multi-step number</a></li></ol></li><li><a href="#sec-number-multi-step" title="Increasing multi-step explicit numbers"><span class="secnum">7.6.2.1</span> Increasing multi-step explicit numbers</a></li></ol></li></ol></li><li><a href="#sec-annex" title="Annex"><span class="secnum">A</span> Annex</a><ol class="toc"><li><a href="#sec-annex2" title="Sub-annex"><span class="secnum">A.1</span> Sub-annex</a></li></ol></li></ol></div><emu-intro id="sec-intro">
9+
<div><h2>Table of Contents</h2><ol class="toc"><li><a href="#sec-intro" title="Intro">Intro</a><ol class="toc"><li><a href="#sec-intro2" title="Sub Intro">Sub Intro</a></li></ol></li><li><a href="#sec-clause" title="Clause Foo(a, b)"><span class="secnum">1</span> Clause Foo(<var>a</var>, <var>b</var>)</a><ol class="toc"><li><a href="#Foo" title="Sub Clause"><span class="secnum">1.1</span> Sub Clause</a></li></ol></li><li><a href="#sec-number" title="Explicit number"><span class="secnum">7</span> Explicit number</a><ol class="toc"><li><a href="#sec-number1" title="Automatic number inside explicit number"><span class="secnum">7.1</span> Automatic number inside explicit number</a></li><li><a href="#sec-number2" title="Nested explicit number"><span class="secnum">7.4</span> Nested explicit number</a></li><li><a href="#sec-number3" title="Automatic number after explicit number"><span class="secnum">7.5</span> Automatic number after explicit number</a></li><li><a href="#sec-number-nested" title="Multi-step explicit numbers"><span class="secnum">7.6</span> Multi-step explicit numbers</a><ol class="toc"><li><a href="#sec-number-multi-step" title="Multi-step explicit numbers"><span class="secnum">7.6.1.1</span> Multi-step explicit numbers</a></li><li><a href="#sec-number-multi-step-1" title="Automatic number after explicit number"><span class="secnum">7.6.1.2</span> Automatic number after explicit number</a><ol class="toc"><li><a href="#sec-number-multi-step-inner" title="Nested clause after explicit multi-step number"><span class="secnum">7.6.1.2.1</span> Nested clause after explicit multi-step number</a></li></ol></li><li><a href="#sec-number-multi-step-2" title="Increasing multi-step explicit numbers"><span class="secnum">7.6.2.1</span> Increasing multi-step explicit numbers</a></li></ol></li></ol></li><li><a href="#sec-annex" title="Annex"><span class="secnum">A</span> Annex</a><ol class="toc"><li><a href="#sec-annex2" title="Sub-annex"><span class="secnum">A.1</span> Sub-annex</a></li></ol></li></ol></div><emu-intro id="sec-intro">
1010
<h1>Intro</h1>
1111
<emu-intro id="sec-intro2">
1212
<h1>Sub Intro</h1>
@@ -51,7 +51,7 @@ <h1><span class="secnum">7.6.1.2.1</span> Nested clause after explicit multi-ste
5151
</emu-clause>
5252
</emu-clause>
5353

54-
<emu-clause id="sec-number-multi-step" number="2.1">
54+
<emu-clause id="sec-number-multi-step-2" number="2.1">
5555
<h1><span class="secnum">7.6.2.1</span> Increasing multi-step explicit numbers</h1>
5656
</emu-clause>
5757
</emu-clause>

test/baselines/generated-reference/duplicate-ids.html

+5
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
</ul></div><div id="spec-container">
99
<emu-clause id="sec-a">
1010
<h1><span class="secnum">1</span> A</h1>
11+
<!-- EXPECT_WARNING { "ruleId": "duplicate-id", "message": "<emu-clause> has duplicate id \"sec-a\"" } -->
1112
<emu-clause id="sec-a">
1213
<h1><span class="secnum">1.1</span> Sub A</h1>
1314

@@ -16,6 +17,7 @@ <h1><span class="secnum">1.1</span> Sub A</h1>
1617
</figure></emu-example>
1718
</emu-clause>
1819
</emu-clause>
20+
<!-- EXPECT_WARNING { "ruleId": "duplicate-id", "message": "<emu-clause> has duplicate id \"sec-a\"" } -->
1921
<emu-clause id="sec-a">
2022
<h1><span class="secnum">2</span> Section A: Extras</h1>
2123
<emu-table id="table-of-stuff" caption="A Table Of Stuff" informative=""><figure><figcaption>Table 1 (Informative): A Table Of Stuff</figcaption>
@@ -25,14 +27,17 @@ <h1><span class="secnum">2</span> Section A: Extras</h1>
2527
</tbody></table>
2628
</figure></emu-table>
2729
</emu-clause>
30+
<!-- EXPECT_WARNING { "ruleId": "duplicate-id", "message": "<emu-clause> has duplicate id \"sec-a\"" } -->
2831
<emu-clause id="sec-a">
2932
<h1><span class="secnum">3</span> Section A: Extras</h1>
33+
<!-- EXPECT_WARNING { "ruleId": "duplicate-id", "message": "<emu-table> has duplicate id \"table-of-stuff\"" } -->
3034
<emu-table id="table-of-stuff" caption="A Table Of Stuff" informative=""><figure><figcaption>Table 2 (Informative): A Table Of Stuff</figcaption>
3135
<table>
3236
<tbody><tr><th>Column 1</th><th>Column 2</th></tr>
3337
<tr><td>Value</td><td>Value 2</td></tr>
3438
</tbody></table>
3539
</figure></emu-table>
40+
<!-- EXPECT_WARNING { "ruleId": "duplicate-id", "message": "<emu-example> has duplicate id \"an-example\"" } -->
3641
<emu-example id="an-example" caption="An example"><figure><figcaption>Example (Informative): An example</figcaption>
3742
Multiple examples are numbered similar to notes
3843
</figure></emu-example>

test/baselines/generated-reference/figure.html

+3-3
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@
3838
</tbody></table>
3939
</figure></emu-table>
4040

41-
<emu-figure id="figure-2" informative=""><figure><figcaption>Figure 3 (Informative): This is the caption</figcaption>
41+
<emu-figure id="figure-3" informative=""><figure><figcaption>Figure 3 (Informative): This is the caption</figcaption>
4242

4343
this is a figure!
4444
</figure></emu-figure>
4545

46-
<emu-table id="table-1"><figure><figcaption>Table 3: This is a table</figcaption>
46+
<emu-table id="table-3"><figure><figcaption>Table 3: This is a table</figcaption>
4747

4848
<table>
4949
<thead>
@@ -56,7 +56,7 @@
5656
</tbody></table>
5757
</figure></emu-table>
5858

59-
<emu-table id="table-2" informative=""><figure><figcaption>Table 4 (Informative): This is a <b>second</b> table</figcaption>
59+
<emu-table id="table-4" informative=""><figure><figcaption>Table 4 (Informative): This is a <b>second</b> table</figcaption>
6060

6161
<table>
6262
<tbody><tr><th>Column 1</th><th>Column 2</th></tr>

test/baselines/generated-reference/namespaces.html

+7
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ <h1><span class="secnum">1.1</span> Clause 1.1</h1>
4444
</emu-production>
4545
</emu-grammar>
4646

47+
<!-- FIXME: Shouldn't this be allowed by the preceding `namespace=clause`? -->
48+
<!-- EXPECT_WARNING { "ruleId": "duplicate-definition", "message": "duplicate definition \"SomeAlg\"" } -->
4749
<emu-clause id="c111" aoid="SomeAlg">
4850
<h1><span class="secnum">1.1.1</span> SomeAlg</h1>
4951
</emu-clause>
@@ -63,10 +65,14 @@ <h1><span class="secnum">A</span> Annex</h1>
6365
</emu-rhs>
6466
</emu-production>
6567
</emu-grammar>
68+
<!-- FIXME: Shouldn't this be allowed by the preceding `namespace=annex`? -->
69+
<!-- EXPECT_WARNING { "ruleId": "duplicate-definition", "message": "duplicate definition \"SomeAlg\"" } -->
6670
<emu-annex id="annex11" aoid="SomeAlg">
6771
<h1><span class="secnum">A.1</span> SomeAlg</h1>
6872
</emu-annex>
6973

74+
<!-- FIXME: Shouldn't this be allowed by `namespace=annex2`? -->
75+
<!-- EXPECT_WARNING { "ruleId": "duplicate-definition", "message": "duplicate definition \"SomeAlg\"" } -->
7076
<emu-annex id="annex12" aoid="SomeAlg" namespace="annex2">
7177
<h1><span class="secnum">A.2</span> Annex 1.2</h1>
7278
<p><emu-xref aoid="SomeAlg" id="_ref_3"><a href="#i1">SomeAlg</a></emu-xref> should link to #annex12. <emu-nt id="_ref_12"><a href="#prod-annex-Foo">Foo</a></emu-nt> should link to the production in #annex1.</p>
@@ -78,6 +84,7 @@ <h1><span class="secnum">A.2</span> Annex 1.2</h1>
7884

7985
<emu-annex id="annex2" namespace="annex">
8086
<h1><span class="secnum">B</span> Annex 2</h1>
87+
<!-- EXPECT_WARNING { "ruleId": "duplicate-definition", "message": "duplicate definition \"SomeAlg\"" } -->
8188
<emu-annex id="annex21" aoid="SomeAlg">
8289
<h1><span class="secnum">B.1</span> SomeAlg</h1>
8390
</emu-annex>

test/baselines/generated-reference/structured-headers.html

+4-3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ <h1><span class="secnum">1</span> ExampleAO ( param [ , param2 ] )</h1>
1717
<emu-alg><ol><li>Algorithm steps go here.</li></ol></emu-alg>
1818
</emu-clause>
1919

20+
<!-- EXPECT_WARNING { "ruleId": "duplicate-id", "message": "<emu-clause> has duplicate id \"sec-exampleao\"" } -->
2021
<emu-clause id="sec-exampleao" type="abstract operation">
2122
<h1><span class="secnum">2</span> ExampleAO ( )</h1>
2223
<p>The abstract operation ExampleAO takes no arguments. It redefines an existing algorithm. It performs the following steps when called:</p>
@@ -87,9 +88,9 @@ <h1><span class="secnum">6.2</span> IsThat</h1>
8788
</emu-clause>
8889
</emu-clause>
8990

90-
<emu-clause id="sec-example-return-type" type="abstract operation" aoid="ExampleAO3">
91-
<h1><span class="secnum">7</span> ExampleAO3 ( param )</h1>
92-
<p>The abstract operation <emu-xref aoid="ExampleAO3" id="_ref_0"><a href="#sec-exampleao3">ExampleAO3</a></emu-xref> takes argument param (an <emu-xref href="#integer"><a href="https://tc39.es/ecma262/#integer">integer</a></emu-xref>) and returns the return type. It performs the following steps when called:</p>
91+
<emu-clause id="sec-example-return-type" type="abstract operation" aoid="ExampleAO5">
92+
<h1><span class="secnum">7</span> ExampleAO5 ( param )</h1>
93+
<p>The abstract operation ExampleAO5 takes argument param (an <emu-xref href="#integer"><a href="https://tc39.es/ecma262/#integer">integer</a></emu-xref>) and returns the return type. It performs the following steps when called:</p>
9394
<emu-alg><ol><li>Algorithm steps go here.</li></ol></emu-alg>
9495
</emu-clause>
9596

test/baselines/sources/clauses.html

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ <h1>Nested clause after explicit multi-step number</h1>
4848
</emu-clause>
4949
</emu-clause>
5050

51-
<emu-clause id="sec-number-multi-step" number="2.1">
51+
<emu-clause id="sec-number-multi-step-2" number="2.1">
5252
<h1>Increasing multi-step explicit numbers</h1>
5353
</emu-clause>
5454
</emu-clause>

0 commit comments

Comments
 (0)