Skip to content

Commit

Permalink
Change parser from Esprima to Babel
Browse files Browse the repository at this point in the history
This commit changes the parser from esprima (the default parser that comes with recast) to @babel/parser. It uses the pre-configured Babel parser shipped with recast (recast/parsers/babel) that ensures the AST returned by Babel is compatible with recast.

The goal of this change is to support newer JavaScript syntax that is not yet supported by Esprima, so this commit also includes a new test where the input source includes class syntax, class fields, and a decorator.

I also replaced jshint with eslint, based on the current ember-cli configuration, but adjusted the rules to be looser to avoid noise in the diff. We can tighten up the rules and update the source in a separate change.

This also adds a prettier config pulled from ember-cli, but does not enforce the rules in the test suite at this time (again, to avoid a noisy diff making it hard to understand the Babel/Esprima parser change).
  • Loading branch information
tomdale committed Jul 1, 2019
1 parent 1900e8e commit 4187722
Show file tree
Hide file tree
Showing 12 changed files with 1,781 additions and 18 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/tests/fixtures/
67 changes: 67 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
module.exports = {
root: true,
parserOptions: {
ecmaVersion: 2018,
},
plugins: ['mocha', 'node'],
extends: ['eslint:recommended', 'plugin:node/recommended', 'prettier'],
env: {
browser: false,
node: true,
es6: true,
},
globals: {},
rules: {
/*** For compatibility with existing source */
'no-var': 0,
'no-undef': 0,
'strict': 0,
'object-shorthand': 0,
'no-lonely-if': 0,

/*** Possible Errors ***/

'no-console': 0,
'no-template-curly-in-string': 2,
'no-unsafe-negation': 2,

/*** Best Practices ***/

curly: 2,
eqeqeq: 2,
'guard-for-in': 0,
'no-caller': 2,
'no-eq-null': 2,
'no-eval': 2,
'no-new': 0,
'no-unused-expressions': [
2,
{
allowShortCircuit: true,
allowTernary: true,
},
],
'wrap-iife': 0,
yoda: 2,

/*** Variables ***/

'no-unused-vars': 2,
'no-use-before-define': [2, 'nofunc'],

/*** Stylistic Issues ***/

camelcase: 2,
'new-cap': [2, { properties: false }],
'no-array-constructor': 2,
'no-bitwise': 2,
'no-plusplus': 0,
'no-unneeded-ternary': 2,

/*** ECMAScript 6 ***/

'no-useless-computed-key': 2,
'prefer-template': 2,
'symbol-description': 2,
},
};
5 changes: 0 additions & 5 deletions .jshintrc

This file was deleted.

7 changes: 7 additions & 0 deletions .prettierrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';

module.exports = {
singleQuote: true,
trailingComma: 'es5',
printWidth: 120,
};
3 changes: 2 additions & 1 deletion lib/ember-router-generator.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module.exports = EmberRouterGenerator;

var recast = require('recast');
var parser = require('recast/parsers/babel');

var findFunctionExpression = require('./helpers/find-function-expression');
var findRoutes = require('./helpers/find-routes');
Expand All @@ -21,7 +22,7 @@ EmberRouterGenerator.prototype.clone = function() {
};

EmberRouterGenerator.prototype._ast = function() {
return (this.ast = this.ast || recast.parse(this.source));
return (this.ast = this.ast || recast.parse(this.source, { parser }));
};

EmberRouterGenerator.prototype._walk = function() {
Expand Down
10 changes: 8 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,20 @@
"url": "https://github.com/ember-cli/ember-router-generator.git"
},
"dependencies": {
"@babel/parser": "^7.4.5",
"@babel/traverse": "^7.4.5",
"recast": "^0.18.1"
},
"devDependencies": {
"assertion-error": "^1.1.0",
"chai": "^4.2.0",
"esprima": "^2.7.2",
"eslint": "^6.0.1",
"eslint-config-prettier": "^6.0.0",
"eslint-plugin-mocha": "^5.3.0",
"eslint-plugin-node": "^9.1.0",
"mocha": "^6.1.4",
"mocha-jshint": "2.3.1"
"mocha-eslint": "^5.0.0",
"prettier": "^1.18.2"
},
"author": "Adolfo Builes",
"license": "MIT",
Expand Down
8 changes: 8 additions & 0 deletions tests/fixtures/class-syntax-foos-route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class AppRouter extends Router {
@tracked location = config.locationType;
rootURL = config.rootURL;
}

Router.map(function() {
this.route('foos');
});
7 changes: 7 additions & 0 deletions tests/fixtures/class-syntax.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class AppRouter extends Router {
@tracked location = config.locationType;
rootURL = config.rootURL;
}

Router.map(function() {
});
15 changes: 10 additions & 5 deletions tests/generator-test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
var EmberRouterGenerator = require('../index.js');
var assert = require('assert');
var fs = require('fs');
var astEquality = require('./helpers/esprima-ast-equality');
var recast = require('recast');
var expect = require('chai').expect;

var astEquality = require('./helpers/ast-equality');

describe('Adding routes', function() {
it('adds routes', function() {
Expand All @@ -16,6 +12,15 @@ describe('Adding routes', function() {
astEquality(newRoutes.code(), fs.readFileSync('./tests/fixtures/foos-route.js'));
});

it('handles files with class syntax', function() {
var source = fs.readFileSync('./tests/fixtures/class-syntax.js');

var routes = new EmberRouterGenerator(source);
var newRoutes = routes.add('foos');

astEquality(newRoutes.code(), fs.readFileSync('./tests/fixtures/class-syntax-foos-route.js'));
});

it('leaves untouched existing routes', function() {
var source = fs.readFileSync('./tests/fixtures/foos-route.js');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@ EqualityError.prototype = Object.create(Error.prototype);
EqualityError.prototype.name = 'EqualityError';
EqualityError.prototype.constructor = EqualityError;

var assert = require('assert');
var esprima = require('esprima');
var recast = require('recast');
var parser = require('recast/parsers/babel');
var traverse = require('@babel/traverse').default;

module.exports = function(actual, expected, message) {
var parsedActual = esprima.parse(actual);
var parsedExpected = esprima.parse(expected);
var parsedActual = recast.parse(actual.toString(), { parser });
var parsedExpected = recast.parse(expected.toString(), { parser });

// Removes source-specific metadata from AST nodes
stripSourceInfo(parsedActual, parsedExpected);

var seemEqual = JSON.stringify(parsedActual) === JSON.stringify(parsedExpected);

Expand All @@ -27,3 +30,15 @@ module.exports = function(actual, expected, message) {
);
}
};

function stripSourceInfo(ast1, ast2) {
const stripNode = node => {
delete node.tokens;
delete node.loc;
delete node.start;
delete node.end;
}

traverse.cheap(ast1, stripNode);
traverse.cheap(ast2, stripNode);
}
2 changes: 1 addition & 1 deletion tests/jshint-test.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@
require('mocha-jshint')();
require('mocha-eslint')(['lib', 'index.js', 'tests/*.js', 'tests/helpers']);
Loading

0 comments on commit 4187722

Please sign in to comment.