Skip to content

Commit 5e2384f

Browse files
authored
suppress "unknown x" errors from productions defined in external biblios (#434)
1 parent e4fb2ad commit 5e2384f

File tree

4 files changed

+108
-24
lines changed

4 files changed

+108
-24
lines changed

src/lint/collect-grammar-diagnostics.ts

+17-2
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,18 @@ export async function collectGrammarDiagnostics(
7373
}
7474
const paramToError = unusedParameterErrors.get(nodeName)!;
7575
paramToError.set(paramName, error);
76-
} else {
77-
report(error);
76+
return;
7877
}
78+
if (
79+
m.code === Diagnostics.Cannot_find_name_0_.code &&
80+
spec.biblio.byProductionName(m.messageArguments?.[0]) != null
81+
) {
82+
// grammarkdown assumes it has the whole grammar in scope
83+
// but some productions might be defined in an external biblio
84+
// TODO: thread the actual grammar through to grammarkdown so it has appropriate context, instead of surpressing this error
85+
return;
86+
}
87+
report(error);
7988
});
8089
}
8190

@@ -118,6 +127,12 @@ export async function collectGrammarDiagnostics(
118127
for (const [name, { production, rhses }] of productions) {
119128
const originalRhses = actualGrammarProductions.get(name)?.rhses;
120129
if (originalRhses === undefined) {
130+
if (spec.biblio.byProductionName(name) != null) {
131+
// in an ideal world we'd keep the full grammar in the biblio so we could check for a matching RHS, not just a matching LHS
132+
// but, we're not in that world
133+
// https://github.com/tc39/ecmarkup/issues/431
134+
continue;
135+
}
121136
const { line, column } = getLocationInGrammar(grammar, production.pos);
122137
report({
123138
type: 'contents',

test/errors.js

+13-19
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@
33
let assert = require('assert');
44
let emu = require('../lib/ecmarkup');
55

6-
let { lintLocationMarker: M, positioned, assertError, assertErrorFree } = require('./utils.js');
6+
let {
7+
lintLocationMarker: M,
8+
positioned,
9+
assertError,
10+
assertErrorFree,
11+
getBiblio,
12+
} = require('./utils.js');
713

814
describe('errors', () => {
915
it('no contributors', async () => {
@@ -1002,23 +1008,11 @@ ${M} </pre>
10021008
});
10031009

10041010
it('negative: external biblio', async () => {
1005-
let upstream = await emu.build(
1006-
'root.html',
1007-
() => `
1008-
<emu-grammar type="definition">
1009-
Foo : \`a\`
1010-
</emu-grammar>
1011-
`,
1012-
{
1013-
copyright: false,
1014-
location: 'https://example.com/spec/',
1015-
warn: e => {
1016-
console.error('Error:', e);
1017-
throw new Error(e.message);
1018-
},
1019-
}
1020-
);
1021-
let upstreamBiblio = upstream.exportBiblio();
1011+
let upstream = await getBiblio(`
1012+
<emu-grammar type="definition">
1013+
Foo : \`a\`
1014+
</emu-grammar>
1015+
`);
10221016

10231017
await assertErrorFree(
10241018
`
@@ -1034,7 +1028,7 @@ ${M} </pre>
10341028
</emu-clause>
10351029
`,
10361030
{
1037-
extraBiblios: [upstreamBiblio],
1031+
extraBiblios: [upstream],
10381032
}
10391033
);
10401034
});

test/lint.js

+63-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
'use strict';
22

3-
let { assertLint, assertLintFree, positioned, lintLocationMarker: M } = require('./utils.js');
3+
let {
4+
assertLint,
5+
assertLintFree,
6+
positioned,
7+
lintLocationMarker: M,
8+
getBiblio,
9+
} = require('./utils.js');
410

511
describe('linting whole program', () => {
612
describe('grammar validity', () => {
@@ -67,6 +73,40 @@ describe('linting whole program', () => {
6773
}
6874
);
6975
});
76+
77+
it('unknown nonterminal', async () => {
78+
await assertLint(
79+
positioned`
80+
<emu-grammar type="definition">
81+
Foo: ${M}Bar
82+
</emu-grammar>
83+
`,
84+
{
85+
ruleId: 'grammarkdown:2000',
86+
nodeType: 'emu-grammar',
87+
message: "Cannot find name: 'Bar'.",
88+
}
89+
);
90+
});
91+
92+
it('negative: nonterminal defined upstream', async () => {
93+
let upstream = await getBiblio(`
94+
<emu-grammar type="definition">
95+
Bar: \`a\`
96+
</emu-grammar>
97+
`);
98+
99+
await assertLintFree(
100+
`
101+
<emu-grammar type="definition">
102+
Foo: Bar
103+
</emu-grammar>
104+
`,
105+
{
106+
extraBiblios: [upstream],
107+
}
108+
);
109+
});
70110
});
71111

72112
describe('early error shape', () => {
@@ -210,6 +250,28 @@ describe('linting whole program', () => {
210250
}
211251
);
212252
});
253+
254+
it('negative: productions defined in biblio', async () => {
255+
let upstream = await getBiblio(`
256+
<emu-grammar type="definition">
257+
Statement: EmptyStatement
258+
</emu-grammar>
259+
`);
260+
261+
await assertLintFree(
262+
`
263+
<emu-grammar>
264+
Statement: EmptyStatement
265+
</emu-grammar>
266+
<emu-alg>
267+
1. Return Foo.
268+
</emu-alg>
269+
`,
270+
{
271+
extraBiblios: [upstream],
272+
}
273+
);
274+
});
213275
});
214276

215277
describe('header format', () => {

test/utils.js

+15-2
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,11 @@ async function assertError(obj, { ruleId, nodeType, message }, opts) {
105105
}
106106

107107
async function assertErrorFree(html, opts) {
108+
if (typeof html !== 'string') {
109+
throw new Error(
110+
"assertErrorFree expects a string; did you forget to remove the 'positioned' tag?"
111+
);
112+
}
108113
let warnings = [];
109114

110115
await emu.build('test-example.emu', async () => html, {
@@ -121,8 +126,15 @@ async function assertLint(a, b) {
121126
await assertError(a, b, { lintSpec: true });
122127
}
123128

124-
async function assertLintFree(html) {
125-
await assertErrorFree(html, { lintSpec: true });
129+
async function assertLintFree(html, opts = {}) {
130+
await assertErrorFree(html, { lintSpec: true, ...opts });
131+
}
132+
133+
async function getBiblio(html) {
134+
let upstream = await emu.build('root.html', () => html, {
135+
location: 'https://example.com/spec/',
136+
});
137+
return upstream.exportBiblio();
126138
}
127139

128140
module.exports = {
@@ -132,4 +144,5 @@ module.exports = {
132144
assertErrorFree,
133145
assertLint,
134146
assertLintFree,
147+
getBiblio,
135148
};

0 commit comments

Comments
 (0)