Skip to content

Commit ad6d03e

Browse files
committed
Multi-errors: Capture all errors in each compilation stage
1 parent 0ace49f commit ad6d03e

File tree

8 files changed

+153
-5
lines changed

8 files changed

+153
-5
lines changed

lib/compiler/index.js

+3
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,9 @@ const compiler = {
9898

9999
pass(ast, options, session);
100100
});
101+
102+
// Collect all errors by stage
103+
session.checkErrors();
101104
});
102105

103106
switch (options.output) {

lib/compiler/session.js

+29-2
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ class Defaults {
1111
if (typeof options.info === "function") { this.info = options.info; }
1212
}
1313

14-
error(stage, ...args) {
15-
throw new GrammarError(...args);
14+
error() {
15+
// Intentionally empty placeholder
1616
}
1717

1818
warn() {
@@ -27,13 +27,34 @@ class Defaults {
2727
class Session {
2828
constructor(options) {
2929
this._callbacks = new Defaults(options);
30+
this._firstError = null;
3031
this.errors = 0;
3132
this.problems = [];
3233
this.stage = null;
3334
}
3435

3536
error(...args) {
3637
++this.errors;
38+
// In order to preserve backward compatibility we cannot change `GrammarError`
39+
// constructor, nor throw another class of error:
40+
// - if we change `GrammarError` constructor, this will break plugins that
41+
// throws `GrammarError`
42+
// - if we throw another Error class, this will break parser clients that
43+
// catches GrammarError
44+
//
45+
// So we use a compromise: we throw an `GrammarError` with all found problems
46+
// in the `problems` property, but the thrown error itself is the first
47+
// registered error.
48+
//
49+
// Thus when the old client catches the error it can find all properties on
50+
// the Grammar error that it want. On the other hand the new client can
51+
// inspect the `problems` property to get all problems.
52+
if (this._firstError === null) {
53+
this._firstError = new GrammarError(...args);
54+
this._firstError.stage = this.stage;
55+
this._firstError.problems = this.problems;
56+
}
57+
3758
this.problems.push(["error", ...args]);
3859
this._callbacks.error(this.stage, ...args);
3960
}
@@ -47,6 +68,12 @@ class Session {
4768
this.problems.push(["info", ...args]);
4869
this._callbacks.info(this.stage, ...args);
4970
}
71+
72+
checkErrors() {
73+
if (this.errors !== 0) {
74+
throw this._firstError;
75+
}
76+
}
5077
}
5178

5279
module.exports = Session;

lib/grammar-error.js

+4
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ class GrammarError extends Error {
2727
diagnostics = [];
2828
}
2929
this.diagnostics = diagnostics;
30+
// All problems if this error is thrown by the plugin and not at stage
31+
// checking phase
32+
this.stage = null;
33+
this.problems = [["error", message, location, diagnostics]];
3034
}
3135

3236
toString() {

lib/peg.d.ts

+77
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,22 @@ export interface DiagnosticNote {
266266
location: LocationRange;
267267
}
268268

269+
/** Possible compilation stage name. */
270+
type Stage = keyof Stages;
271+
/** Severity level of problems that can be registered in compilation session. */
272+
type Severity = "error" | "warn" | "info";
273+
274+
type Problem = [
275+
/** Problem severity. */
276+
Severity,
277+
/** Diagnostic message. */
278+
string,
279+
/** Location where message is generated, if applicable. */
280+
LocationRange?,
281+
/** List of additional messages with their locations, if applicable. */
282+
DiagnosticNote[]?,
283+
];
284+
269285
/** Thrown if the grammar contains a semantic error. */
270286
export class GrammarError extends Error {
271287
/** Location of the error in the source. */
@@ -274,6 +290,15 @@ export class GrammarError extends Error {
274290
/** Additional messages with context information. */
275291
diagnostics: DiagnosticNote[];
276292

293+
/** Compilation stage during which error was generated. */
294+
stage: Stage | null;
295+
296+
/**
297+
* List of diagnostics containing all errors, warnings and information
298+
* messages generated during compilation stage `stage`.
299+
*/
300+
problems: Problem[];
301+
277302
constructor(
278303
message: string,
279304
location?: LocationRange,
@@ -919,6 +944,51 @@ export interface Session {
919944
): void;
920945
}
921946

947+
export interface DiagnosticCallback {
948+
/**
949+
* Called when compiler reports an error.
950+
*
951+
* @param stage Stage in which this diagnostic was originated
952+
* @param message Main message, which should describe error objectives
953+
* @param location If defined, this is location described in the `message`
954+
* @param notes Additional messages with context information
955+
*/
956+
error?(
957+
stage: Stage,
958+
message: string,
959+
location?: LocationRange,
960+
notes?: DiagnosticNote[]
961+
): void;
962+
/**
963+
* Called when compiler reports a warning.
964+
*
965+
* @param stage Stage in which this diagnostic was originated
966+
* @param message Main message, which should describe warning objectives
967+
* @param location If defined, this is location described in the `message`
968+
* @param notes Additional messages with context information
969+
*/
970+
warn?(
971+
stage: Stage,
972+
message: string,
973+
location?: LocationRange,
974+
notes?: DiagnosticNote[]
975+
): void;
976+
/**
977+
* Called when compiler reports an informational message.
978+
*
979+
* @param stage Stage in which this diagnostic was originated
980+
* @param message Main message, which gives information about an event
981+
* @param location If defined, this is location described in the `message`
982+
* @param notes Additional messages with context information
983+
*/
984+
info?(
985+
stage: Stage,
986+
message: string,
987+
location?: LocationRange,
988+
notes?: DiagnosticNote[]
989+
): void;
990+
}
991+
922992
/**
923993
* Parser dependencies, is an object which maps variables used to access the
924994
* dependencies in the parser to module IDs used to load them
@@ -956,6 +1026,13 @@ export interface BuildOptionsBase {
9561026

9571027
/** Makes the parser trace its progress (default: `false`) */
9581028
trace?: boolean;
1029+
1030+
/** Called when a semantic error during build was detected. */
1031+
error?: DiagnosticCallback;
1032+
/** Called when a warning during build was registered. */
1033+
warn?: DiagnosticCallback;
1034+
/** Called when an informational message during build was registered. */
1035+
info?: DiagnosticCallback;
9591036
}
9601037

9611038
export interface ParserBuildOptions extends BuildOptionsBase {

package.json

+1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
"@rollup/plugin-multi-entry": "^4.1.0",
5353
"@rollup/plugin-node-resolve": "^13.0.4",
5454
"@rollup/plugin-typescript": "^8.2.5",
55+
"@types/chai": "^4.2.18",
5556
"@types/jest": "^26.0.24",
5657
"@types/node": "^16.6.1",
5758
"@typescript-eslint/eslint-plugin": "^4.29.1",

test/behavior/generated-parser-behavior.spec.js

+31
Original file line numberDiff line numberDiff line change
@@ -1780,5 +1780,36 @@ Error: Expected "{", code block, comment, end of line, identifier, or whitespace
17801780
}
17811781
}).to.throw(peg.parser.SyntaxError);
17821782
});
1783+
1784+
it("reports multiply errors in each compilation stage", () => {
1785+
try {
1786+
peg.generate(`
1787+
start = leftRecursion
1788+
leftRecursion = duplicatedLabel:duplicatedRule duplicatedLabel:missingRule
1789+
duplicatedRule = missingRule
1790+
duplicatedRule = start
1791+
`);
1792+
} catch (e) {
1793+
expect(e).with.property("stage", "check");
1794+
expect(e).with.property("problems").to.be.an("array");
1795+
1796+
// Check that each problem is an array with at least two members and the first is a severity
1797+
e.problems.forEach(problem => {
1798+
expect(problem).to.be.an("array").lengthOf.gte(2);
1799+
expect(problem[0]).to.be.oneOf(["error", "warn", "info"]);
1800+
});
1801+
1802+
// Get second elements of errors (error messages)
1803+
const messages = e.problems.filter(p => p[0] === "error").map(p => p[1]);
1804+
1805+
// Check that all messages present in the list
1806+
expect(messages).to.include.members([
1807+
"Rule \"missingRule\" is not defined",
1808+
"Rule \"duplicatedRule\" is already defined",
1809+
"Label \"duplicatedLabel\" is already defined",
1810+
"Possible infinite loop when parsing (left recursion: duplicatedRule -> start -> leftRecursion -> duplicatedRule)",
1811+
]);
1812+
}
1813+
});
17831814
});
17841815
});

test/unit/compiler/passes/helpers.js

+7-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"use strict";
22

3+
const { GrammarError } = require("../../../../lib/peg");
34
const parser = require("../../../../lib/parser");
45
const Session = require("../../../../lib/compiler/session");
56

@@ -47,7 +48,9 @@ module.exports = function(chai, utils) {
4748

4849
const ast = parser.parse(grammar);
4950

50-
utils.flag(this, "object")(ast, options, new Session());
51+
utils.flag(this, "object")(ast, options, new Session({
52+
error(stage, ...args) { throw new GrammarError(...args); },
53+
}));
5154

5255
this.assert(
5356
matchProps(ast, props),
@@ -64,7 +67,9 @@ module.exports = function(chai, utils) {
6467
let passed, result;
6568

6669
try {
67-
utils.flag(this, "object")(ast, {}, new Session());
70+
utils.flag(this, "object")(ast, {}, new Session({
71+
error(stage, ...args) { throw new GrammarError(...args); },
72+
}));
6873
passed = true;
6974
} catch (e) {
7075
result = e;

test/unit/grammar-error.spec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"use strict";
22

33
const chai = require("chai");
4-
const GrammarError = require("../../lib/grammar-error");
4+
const { GrammarError } = require("../../lib/peg");
55

66
const expect = chai.expect;
77

0 commit comments

Comments
 (0)