Skip to content

Commit bfc3670

Browse files
committed
Multi-errors: Capture all errors in each compilation stage
1 parent 9843ebf commit bfc3670

File tree

8 files changed

+137
-5
lines changed

8 files changed

+137
-5
lines changed

lib/compiler/index.js

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

8484
pass(ast, options, session);
8585
});
86+
87+
// Collect all errors by stage
88+
session.checkErrors();
8689
});
8790

8891
switch (options.output) {

lib/compiler/session.js

+29-2
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ class Defaults {
1010
if (typeof options.warn === "function") { this.warn = options.warn; }
1111
if (typeof options.info === "function") { this.info = options.info; }
1212
}
13-
error(stage, ...args) {
14-
throw new GrammarError(...args);
13+
error() {
14+
// intentionally empty placeholder
1515
}
1616
warn() {
1717
// intentionally empty placeholder
@@ -24,12 +24,33 @@ class Defaults {
2424
class Session {
2525
constructor(options) {
2626
this._callbacks = new Defaults(options);
27+
this._firstError = null;
2728
this.errors = 0;
2829
this.problems = [];
2930
this.stage = null;
3031
}
3132
error(...args) {
3233
++this.errors;
34+
// In order to preserve backward compatibility we cannot change `GrammarError`
35+
// constructor, nor throw another class of error:
36+
// - if we change `GrammarError` constructor, this will break plugins that
37+
// throws `GrammarError`
38+
// - if we throw another Error class, this will break parser clients that
39+
// catches GrammarError
40+
//
41+
// So we use a compromise: we throw an `GrammarError` with all found problems
42+
// in the `problems` property, but the thrown error itself is the first
43+
// registered error.
44+
//
45+
// Thus when the old client catches the error it can find all properties on
46+
// the Grammar error that it want. On the other hand the new client can
47+
// inspect the `problems` property to get all problems.
48+
if (this._firstError === null) {
49+
this._firstError = new GrammarError(...args);
50+
this._firstError.stage = this.stage;
51+
this._firstError.problems = this.problems;
52+
}
53+
3354
this.problems.push(["error", ...args]);
3455
this._callbacks.error(this.stage, ...args);
3556
}
@@ -41,6 +62,12 @@ class Session {
4162
this.problems.push(["info", ...args]);
4263
this._callbacks.info(this.stage, ...args);
4364
}
65+
66+
checkErrors() {
67+
if (this.errors !== 0) {
68+
throw this._firstError;
69+
}
70+
}
4471
}
4572

4673
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

+61
Original file line numberDiff line numberDiff line change
@@ -230,13 +230,37 @@ export interface DiagnosticNote {
230230
location: LocationRange;
231231
}
232232

233+
/** Possible compilation stage name. */
234+
type Stage = keyof Stages;
235+
/** Severity level of problems that can be registered in compilation session. */
236+
type Severity = "error" | "warn" | "info";
237+
238+
type Problem = [
239+
/** Problem severity. */
240+
Severity,
241+
/** Diagnostic message. */
242+
string,
243+
/** Location where message is generated, if applicable. */
244+
LocationRange?,
245+
/** List of additional messages with their locations, if applicable. */
246+
DiagnosticNote[]?,
247+
];
248+
233249
/** Thrown if the grammar contains a semantic error. */
234250
export class GrammarError extends Error {
235251
/** Location of the error in the source. */
236252
location?: LocationRange;
237253
/** Additional messages with context information. */
238254
diagnostics: DiagnosticNote[];
239255

256+
/** Compilation stage during which error was generated. */
257+
stage: Stage | null;
258+
/**
259+
* List of diagnostics containing all errors, warnings and information
260+
* messages generated during compilation stage `stage`.
261+
*/
262+
problems: Problem[];
263+
240264
constructor(message: string, location?: LocationRange, diagnostics?: DiagnosticNote[]);
241265

242266
/**
@@ -849,6 +873,36 @@ export interface Session {
849873
info(message: string, location?: LocationRange, notes?: DiagnosticNote[]): void;
850874
}
851875

876+
export interface DiagnosticCallback {
877+
/**
878+
* Called when compiler reports an error.
879+
*
880+
* @param stage Stage in which this diagnostic was originated
881+
* @param message Main message, which should describe error objectives
882+
* @param location If defined, this is location described in the `message`
883+
* @param notes Additional messages with context information
884+
*/
885+
error?(stage: Stage, message: string, location?: LocationRange, notes?: DiagnosticNote[]): void;
886+
/**
887+
* Called when compiler reports a warning.
888+
*
889+
* @param stage Stage in which this diagnostic was originated
890+
* @param message Main message, which should describe warning objectives
891+
* @param location If defined, this is location described in the `message`
892+
* @param notes Additional messages with context information
893+
*/
894+
warn?(stage: Stage, message: string, location?: LocationRange, notes?: DiagnosticNote[]): void;
895+
/**
896+
* Called when compiler reports an informational message.
897+
*
898+
* @param stage Stage in which this diagnostic was originated
899+
* @param message Main message, which gives information about an event
900+
* @param location If defined, this is location described in the `message`
901+
* @param notes Additional messages with context information
902+
*/
903+
info?(stage: Stage, message: string, location?: LocationRange, notes?: DiagnosticNote[]): void;
904+
}
905+
852906
export interface BuildOptionsBase {
853907
/** rules the parser will be allowed to start parsing from (default: the first rule in the grammar) */
854908
allowedStartRules?: string[];
@@ -873,6 +927,13 @@ export interface BuildOptionsBase {
873927
plugins?: Plugin[];
874928
/** makes the parser trace its progress (default: `false`) */
875929
trace?: boolean;
930+
931+
/** Called when a semantic error during build was detected. */
932+
error?: DiagnosticCallback;
933+
/** Called when a warning during build was registered. */
934+
warn?: DiagnosticCallback;
935+
/** Called when an informational message during build was registered. */
936+
info?: DiagnosticCallback;
876937
}
877938

878939
export interface ParserBuildOptions extends BuildOptionsBase {

package.json

+1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
"@rollup/plugin-multi-entry": "^4.0.0",
5050
"@rollup/plugin-node-resolve": "^13.0.0",
5151
"@rollup/plugin-typescript": "^8.2.1",
52+
"@types/chai": "^4.2.18",
5253
"@types/jest": "^26.0.23",
5354
"chai": "^4.3.4",
5455
"eslint": "^7.26.0",

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

+31
Original file line numberDiff line numberDiff line change
@@ -1682,5 +1682,36 @@ Error: Expected "{", code block, comment, end of line, identifier, or whitespace
16821682
}
16831683
}).to.throw(peg.parser.SyntaxError);
16841684
});
1685+
1686+
it("reports multiply errors in each compilation stage", function() {
1687+
try {
1688+
peg.generate(`
1689+
start = leftRecursion
1690+
leftRecursion = duplicatedLabel:duplicatedRule duplicatedLabel:missingRule
1691+
duplicatedRule = missingRule
1692+
duplicatedRule = start
1693+
`);
1694+
} catch (e) {
1695+
expect(e).with.property("stage", "check");
1696+
expect(e).with.property("problems").to.be.an("array");
1697+
1698+
// Check that each problem is an array with at least two members and the first is a severity
1699+
e.problems.forEach(problem => {
1700+
expect(problem).to.be.an("array").lengthOf.gte(2);
1701+
expect(problem[0]).to.be.oneOf(["error", "warn", "info"]);
1702+
});
1703+
1704+
// Get second elements of errors (error messages)
1705+
const messages = e.problems.filter(p => p[0] === "error").map(p => p[1]);
1706+
1707+
// Check that all messages present in the list
1708+
expect(messages).to.include.members([
1709+
"Rule \"missingRule\" is not defined",
1710+
"Rule \"duplicatedRule\" is already defined",
1711+
"Label \"duplicatedLabel\" is already defined",
1712+
"Possible infinite loop when parsing (left recursion: duplicatedRule -> start -> leftRecursion -> duplicatedRule)",
1713+
]);
1714+
}
1715+
});
16851716
});
16861717
});

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)