Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #43 - Customizable operation parse error #174

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
157 changes: 95 additions & 62 deletions dist/ohm.js

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions dist/ohm.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/Interval.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Interval.prototype = {

getLineAndColumnMessage: function() {
var range = [this.startIdx, this.endIdx];
return util.getLineAndColumnMessage(this.inputStream.source, this.startIdx, range);
return util.describeSourceLocation(this.inputStream.source, this.startIdx, true, range);
},

// Returns an array of 0, 1, or 2 intervals that represents the result of the
Expand Down
22 changes: 22 additions & 0 deletions src/MatchResult.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,28 @@ MatchFailure.prototype.getRightmostFailures = function() {
return this._failures;
};

// optConfig has two options:
// includeSource=true shows the problematic source code.
// includeLineNumbers=true shows the problematic line numbers.
MatchFailure.prototype.getMessage = function(optConfig) {
var config = optConfig || {};
var source = this.state.inputStream.source;
var detail = 'Expected ' + this.getExpectedText();

// use default values if not defined
var includeSource = config.includeSource == null ? true : config.includeSource;
var includeLineNumbers = config.includeLineNumbers == null ? true : config.includeLineNumbers;

if (!includeSource) {
return getShortMatchErrorMessage(
this.getRightmostFailurePosition(),
this.state.inputStream.source,
detail);
}
return util.describeSourceLocation(
source, this.getRightmostFailurePosition(), includeLineNumbers) + detail;
};

// Return a string summarizing the expected contents of the input stream when
// the match failure occurred.
MatchFailure.prototype.getExpectedText = function() {
Expand Down
2 changes: 1 addition & 1 deletion src/Semantics.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ function parseSignature(signature, type) {
signature,
type === 'operation' ? 'OperationSignature' : 'AttributeSignature');
if (r.failed()) {
throw new Error(r.message);
throw new Error(r.getMessage({includeSource: true, includeLineNumbers: false}));
}

return prototypeGrammarSemantics(r).parse();
Expand Down
22 changes: 18 additions & 4 deletions src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,11 @@ exports.getLineAndColumn = function(str, offset) {

// Return a nicely-formatted string describing the line and column for the
// given offset in `str`.
exports.getLineAndColumnMessage = function(str, offset /* ...ranges */) {

exports.describeSourceLocation = function(str, offset, optIncludeLineNumbers /* ranges */) {

var includeLineNumbers = typeof optIncludeLineNumbers !== 'undefined' ?
optIncludeLineNumbers : true;
var repeatStr = common.repeatStr;

var lineAndCol = exports.getLineAndColumn(str, offset);
Expand All @@ -109,7 +113,11 @@ exports.getLineAndColumnMessage = function(str, offset /* ...ranges */) {

// Helper for appending formatting input lines to the buffer.
function appendLine(num, content, prefix) {
sb.append(prefix + lineNumbers[num] + ' | ' + content + '\n');
if (includeLineNumbers) {
sb.append(prefix + lineNumbers[num] + ' | ' + content + '\n');
} else {
sb.append(' ' + content + '\n');
}
}

// Include the previous line for context if possible.
Expand All @@ -123,7 +131,7 @@ exports.getLineAndColumnMessage = function(str, offset /* ...ranges */) {
// Start with a blank line, and indicate each range by overlaying a string of `~` chars.
var lineLen = lineAndCol.line.length;
var indicationLine = repeatStr(' ', lineLen + 1);
var ranges = Array.prototype.slice.call(arguments, 2);
var ranges = Array.prototype.slice.call(arguments, 3);
for (var i = 0; i < ranges.length; ++i) {
var startIdx = ranges[i][0];
var endIdx = ranges[i][1];
Expand All @@ -135,7 +143,7 @@ exports.getLineAndColumnMessage = function(str, offset /* ...ranges */) {

indicationLine = strcpy(indicationLine, repeatStr('~', endIdx - startIdx), startIdx);
}
var gutterWidth = 2 + lineNumbers[1].length + 3;
var gutterWidth = includeLineNumbers ? 2 + lineNumbers[1].length + 3 : 2;
sb.append(repeatStr(' ', gutterWidth));
indicationLine = strcpy(indicationLine, '^', lineAndCol.colNum - 1);
sb.append(indicationLine.replace(/ +$/, '') + '\n');
Expand All @@ -146,3 +154,9 @@ exports.getLineAndColumnMessage = function(str, offset /* ...ranges */) {
}
return sb.contents();
};

exports.getLineAndColumnMessage = function(str, offset) {
var args = Array.prototype.slice.call(arguments);
args.splice(2, 0, true);
return exports.describeSourceLocation.apply(this, args);
};
16 changes: 16 additions & 0 deletions test/test-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,22 @@ test('match failure', function(t) {
t.end();
});

test('syntax error in added operation', function(t) {
var g = ohm.grammar('G { start = "a" "b" "c" "d" }');

try {
g.createSemantics().addOperation('myOperation(x, y', {});
t.fail('Expected an exception to be thrown');
} catch (e) {
t.equal(e.message, [
'Line 1, col 17:',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't appear that this test passes.

' myOperation(x, y',
' ^',
'Expected ")"'].join('\n'));
}
t.end();
});

test('undeclared rules', function(t) {
t.throws(
function() { makeRuleWithBody('undeclaredRule'); },
Expand Down
17 changes: 9 additions & 8 deletions test/test-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ var util = require('../src/util');
// --------------------------------------------------------------------

var getLineAndColumnMessage = util.getLineAndColumnMessage;
var describeSourceLocation = util.describeSourceLocation;

test('getLineAndColumnMessage', function(t) {
t.equal(getLineAndColumnMessage('', 0), [
Expand Down Expand Up @@ -90,49 +91,49 @@ test('getLineAndColumnMessage', function(t) {
});

test('getLineAndColumnMessage with ranges', function(t) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should rename this test now, since it's not testing getLineAndColumnMessage anymore :-)

t.equal(getLineAndColumnMessage('3 + 4', 2, [0, 1]), [
t.equal(describeSourceLocation('3 + 4', 2, true, [0, 1]), [
'Line 1, col 3:',
'> 1 | 3 + 4',
' ~ ^',
''].join('\n'), 'a simple range');

t.equal(getLineAndColumnMessage('3 + 4', 2, [0, 1], [4, 5]), [
t.equal(describeSourceLocation('3 + 4', 2, true, [0, 1], [4, 5]), [
'Line 1, col 3:',
'> 1 | 3 + 4',
' ~ ^ ~',
''].join('\n'), 'more than one range');

t.equal(getLineAndColumnMessage('3 + 4', 2, [0, 100]), [
t.equal(describeSourceLocation('3 + 4', 2, true, [0, 100]), [
'Line 1, col 3:',
'> 1 | 3 + 4',
' ~~^~~',
''].join('\n'), 'end index out of bounds');

t.equal(getLineAndColumnMessage('3 + 4', 2, [0, 0]),
t.equal(describeSourceLocation('3 + 4', 2, true, [0, 0]),
getLineAndColumnMessage('3 + 4', 2),
'empty range');

t.equal(getLineAndColumnMessage('3 + 4', 0, [0, 3], [1, 5]), [
t.equal(describeSourceLocation('3 + 4', 0, true, [0, 3], [1, 5]), [
'Line 1, col 1:',
'> 1 | 3 + 4',
' ^~~~~',
''].join('\n'), 'overlapping ranges');

t.equal(getLineAndColumnMessage('blah\n3 + 4', 7, [5, 6]), [
t.equal(describeSourceLocation('blah\n3 + 4', 7, true, [5, 6]), [
'Line 2, col 3:',
' 1 | blah',
'> 2 | 3 + 4',
' ~ ^',
''].join('\n'), 'range on second line');

t.equal(getLineAndColumnMessage('blah\n3 + 4', 7, [0, 6]), [
t.equal(describeSourceLocation('blah\n3 + 4', 7, true, [0, 6]), [
'Line 2, col 3:',
' 1 | blah',
'> 2 | 3 + 4',
' ~ ^',
''].join('\n'), 'range crossing lines');

t.equal(getLineAndColumnMessage('blah\n3 + 4', 7, [0, 50]), [
t.equal(describeSourceLocation('blah\n3 + 4', 7, true, [0, 50]), [
'Line 2, col 3:',
' 1 | blah',
'> 2 | 3 + 4',
Expand Down