Skip to content

Commit

Permalink
Fix #363 by removing imports of ohm-js from extras (it’s not required) (
Browse files Browse the repository at this point in the history
#366)

The problem described in #363 occurs because the import of `ohm-js` pulls in the ES module, while `ohm-js/extras` is a CommonJS module. (This is the [dual-package hazard](https://nodejs.org/api/packages.html#dual-package-hazard).)

This PR eliminates the import of `ohm-js` from the extras module. It's not really required: it was only used for `instanceof` checks and we can skip those and instead check that the methods we need actually exist.
  • Loading branch information
pdubroy authored Feb 28, 2022
1 parent 9a6c27f commit 4dae27a
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 21 deletions.
19 changes: 3 additions & 16 deletions packages/ohm-js/extras/semantics-toAST.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
'use strict';

// --------------------------------------------------------------------
// Imports
// --------------------------------------------------------------------

const pexprs = require('../src/pexprs');
const MatchResult = require('../src/MatchResult');
const Grammar = require('../src/Grammar');

// --------------------------------------------------------------------
// Operations
// --------------------------------------------------------------------
Expand All @@ -23,11 +15,6 @@ const defaultOperation = {

// without customization
if (!Object.prototype.hasOwnProperty.call(mapping, ctorName)) {
// intermediate node
if (this._node instanceof pexprs.Alt || this._node instanceof pexprs.Apply) {
return children[0].toAST(mapping);
}

// lexical rule
if (this.isLexical()) {
return this.sourceString;
Expand Down Expand Up @@ -111,8 +98,8 @@ const defaultOperation = {
// The optional `mapping` parameter can be used to customize how the nodes of the CST
// are mapped to the AST (see /doc/extras.md#toastmatchresult-mapping).
function toAST(res, mapping) {
if (!(res instanceof MatchResult) || res.failed()) {
throw new Error('toAST() expects a succesfull MatchResult as first parameter');
if (typeof res.failed !== 'function' || res.failed()) {
throw new Error('toAST() expects a succesful MatchResult as first parameter');
}

mapping = Object.assign({}, mapping);
Expand All @@ -130,7 +117,7 @@ function toAST(res, mapping) {

// Returns a semantics containg the toAST(mapping) operation for the given grammar g.
function semanticsForToAST(g) {
if (!(g instanceof Grammar)) {
if (typeof g.createSemantics !== 'function') {
throw new Error('semanticsToAST() expects a Grammar as parameter');
}

Expand Down
5 changes: 4 additions & 1 deletion packages/ohm-js/src/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,10 @@ function invalidCodePoint(applyWrapper) {
// Get an interval that covers all of the hex digits.
const digitIntervals = applyWrapper.children.slice(1, -1).map(d => d.source);
const fullInterval = digitIntervals[0].coverageWith(...digitIntervals.slice(1));
return createError(`U+${fullInterval.contents} is not a valid Unicode code point`, fullInterval);
return createError(
`U+${fullInterval.contents} is not a valid Unicode code point`,
fullInterval
);
}

// ----------------- Kleene operators -----------------
Expand Down
15 changes: 13 additions & 2 deletions packages/ohm-js/test/extras/test-toAST.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ const fs = require('fs');
const test = require('ava');

const ohm = require('../..');
const {toAST} = require('../../extras');
const {semanticsForToAST} = require('../../extras');
const {semanticsForToAST, toAST} = require('../../extras');

const g = ohm.grammar(fs.readFileSync('test/data/arithmetic.ohm'));

Expand Down Expand Up @@ -253,3 +252,15 @@ test('real examples (combinations)', t => {
};
t.deepEqual(ast, expected, 'proper AST for arithmetic example #2');
});

test('usage errors', t => {
t.throws(() => toAST(g.match('doesnotmatch')), {
message: /toAST\(\) expects a succesful MatchResult as first parameter/,
});
t.throws(() => toAST({}), {
message: /toAST\(\) expects a succesful MatchResult as first parameter/,
});
t.throws(() => semanticsForToAST({}), {
message: /semanticsToAST\(\) expects a Grammar as parameter/,
});
});
3 changes: 1 addition & 2 deletions packages/ohm-js/test/test-recipes.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,5 @@ test('semantics recipes w/ method shorthand', t => {

test('recipes with astral plane code units', t => {
const g = ohm.grammar(String.raw`G { start = "\u{1F920}" }`);
t.truthy(
ohm.makeRecipe(g.toRecipe()).match('🤠').succeeded());
t.truthy(ohm.makeRecipe(g.toRecipe()).match('🤠').succeeded());
});

0 comments on commit 4dae27a

Please sign in to comment.