From c3d3cc8193f3b164cb9b4c7aaca9ad427d6931cb Mon Sep 17 00:00:00 2001 From: Addison Keizer Date: Sun, 13 Nov 2016 23:12:37 -0500 Subject: [PATCH] Resolve issue #56. Add new error that fires on compile of left/right associativity ambiguousness. Includes test to check change --- src/GrammarDecl.js | 15 +++++++ src/errors.js | 11 +++++ src/pexprs-assertProperAssociativity.js | 60 +++++++++++++++++++++++++ src/pexprs.js | 1 + test/test-errors.js | 25 +++++++++++ 5 files changed, 112 insertions(+) create mode 100644 src/pexprs-assertProperAssociativity.js diff --git a/src/GrammarDecl.js b/src/GrammarDecl.js index 13a155e7..713c6946 100644 --- a/src/GrammarDecl.js +++ b/src/GrammarDecl.js @@ -105,6 +105,7 @@ GrammarDecl.prototype.build = function() { // the part of the source that caused it. var grammarErrors = []; var grammarHasInvalidApplications = false; + var parents = {}; Object.keys(grammar.rules).forEach(function(ruleName) { var body = grammar.rules[ruleName].body; try { @@ -118,6 +119,11 @@ GrammarDecl.prototype.build = function() { grammarErrors.push(e); grammarHasInvalidApplications = true; } + if (body instanceof pexprs.Alt) { + for (var idx = 0; idx < body.terms.length; idx++) { + parents[body.terms[idx].ruleName] = ruleName; + } + } }); if (!grammarHasInvalidApplications) { // The following check can only be done if the grammar has no invalid applications. @@ -130,6 +136,15 @@ GrammarDecl.prototype.build = function() { } }); } + + Object.keys(grammar.rules).forEach(function(ruleName) { + var body = grammar.rules[ruleName].body; + try { + body.assertProperAssociativity(ruleName, parents[ruleName], body.source); + } catch (e) { + grammarErrors.push(e); + } + }); if (grammarErrors.length > 0) { errors.throwErrors(grammarErrors); } diff --git a/src/errors.js b/src/errors.js index 71b32bcc..b79631bd 100644 --- a/src/errors.js +++ b/src/errors.js @@ -186,6 +186,16 @@ function missingSemanticAction(ctorName, name, type) { return e; } +// --------- left-right associativity ambiguousness ----------- + +function associativityAmbiguousness(parentName, expr) { + return createError( + 'Left or Right associativity is unclear. Ensure ' + parentName + + ' is only on either the right or left.', + expr + ); +} + // -------------------------------------------------------------------- // Exports // -------------------------------------------------------------------- @@ -209,6 +219,7 @@ module.exports = { undeclaredRule: undeclaredRule, wrongNumberOfArguments: wrongNumberOfArguments, wrongNumberOfParameters: wrongNumberOfParameters, + associativityAmbiguousness: associativityAmbiguousness, throwErrors: function(errors) { if (errors.length === 1) { diff --git a/src/pexprs-assertProperAssociativity.js b/src/pexprs-assertProperAssociativity.js new file mode 100644 index 00000000..95b1a937 --- /dev/null +++ b/src/pexprs-assertProperAssociativity.js @@ -0,0 +1,60 @@ +'use strict'; + +// -------------------------------------------------------------------- +// Imports +// -------------------------------------------------------------------- + +var common = require('./common'); +var errors = require('./errors'); +var pexprs = require('./pexprs'); + +// -------------------------------------------------------------------- +// Operations +// -------------------------------------------------------------------- + +pexprs.PExpr.prototype.assertProperAssociativity = common.abstract( + 'assertProperAssociativity' +); + +pexprs.any.assertProperAssociativity = +pexprs.end.assertProperAssociativity = +pexprs.Terminal.prototype.assertProperAssociativity = +pexprs.Range.prototype.assertProperAssociativity = +pexprs.Param.prototype.assertProperAssociativity = +pexprs.Lex.prototype.assertProperAssociativity = +pexprs.UnicodeChar.prototype.assertProperAssociativity = function(ruleName) { + // no-op +}; + +pexprs.Alt.prototype.assertProperAssociativity = function(ruleName) { + // no-op +}; + +pexprs.Seq.prototype.assertProperAssociativity = function(ruleName, trueParentName, src) { + if (this.factors.length < 2) { + return; + } + if (this.factors[0] instanceof pexprs.Apply && + this.factors[this.factors.length - 1] instanceof pexprs.Apply) { + if (this.factors[0].ruleName === this.factors[this.factors.length - 1].ruleName && + this.factors[0].ruleName === trueParentName) { + throw errors.associativityAmbiguousness(trueParentName, src); + } + } +}; + +pexprs.Iter.prototype.assertProperAssociativity = function(ruleName) { + this.expr.assertProperAssociativity(ruleName); +}; + +pexprs.Not.prototype.assertProperAssociativity = function(ruleName) { + // no-op +}; + +pexprs.Lookahead.prototype.assertProperAssociativity = function(ruleName) { + this.expr.assertProperAssociativity(ruleName); +}; + +pexprs.Apply.prototype.assertProperAssociativity = function(ruleName) { + // no-op +}; diff --git a/src/pexprs.js b/src/pexprs.js index b8b8a876..de1d3540 100644 --- a/src/pexprs.js +++ b/src/pexprs.js @@ -237,3 +237,4 @@ require('./pexprs-toDisplayString'); require('./pexprs-toArgumentNameList'); require('./pexprs-toFailure'); require('./pexprs-toString'); +require('./pexprs-assertProperAssociativity'); diff --git a/test/test-errors.js b/test/test-errors.js index f0c83e8b..cae24054 100644 --- a/test/test-errors.js +++ b/test/test-errors.js @@ -251,3 +251,28 @@ test('errors for Not-of-', function(t) { t.end(); }); + +test('Error where left-right associativity is unclear', function(t) { + try { + ohm.grammar( + 'G {\n' + + ' A = A "+" A -- add \n' + + ' | B\n' + + ' B = "117"\n' + + '}'); + t.fail('Expected an exception to be thrown'); + } catch (e) { + t.equal(e.message, 'Line 2, col 7:\n' + + ' 1 | G {\n> 2 | A = A "+" A -- add \n' + + ' ^~~~~~~\n 3 | | B\n' + + 'Left or Right associativity is unclear. Ensure A is only on either the right or left.'); + } + ohm.grammar( + 'G {\n' + + ' A = A "+" B -- add \n' + + ' | B\n' + + ' B = "117"\n' + + '}'); + t.end(); + +});