Skip to content

Commit

Permalink
[FIX][WIP] Switch from esprima to espree
Browse files Browse the repository at this point in the history
This is work in progress and can't be submitted as the used espree
version (8.0.0-beta) is not a release version and it requires a newer
NodeJS min version than the tooling.

espree 8.0.0-beta was chosen as it switches the default for the
ecmaVersion option from 5 (7.3.1) to latest.

Before submitting, it also should be decided whether all parsing code
should be routed through some helper utility, to simplify future updates
/ changes of the parser. At the same time, we should decide about the
use of the sourceType and the ecmaVersion options. Should we expose them
in ui5.yaml? Or should we rather leave that to the linting step?

Fixes SAP/ui5-tooling#354 .
  • Loading branch information
codeworrior committed Jun 13, 2021
1 parent b86f0fa commit e7bef3b
Show file tree
Hide file tree
Showing 17 changed files with 145 additions and 106 deletions.
6 changes: 3 additions & 3 deletions lib/lbt/analyzer/FioriElementsAnalyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@

const ModuleName = require("../utils/ModuleName");
const SapUiDefine = require("../calls/SapUiDefine");
const esprima = require("esprima");
const {Syntax} = esprima;
const espree = require("espree");
const {Syntax} = espree;
const {getValue, isMethodCall, isString} = require("../utils/ASTUtils");
const log = require("@ui5/logger").getLogger("lbt:analyzer:FioriElementAnalyzer");

Expand Down Expand Up @@ -147,7 +147,7 @@ class FioriElementsAnalyzer {
// console.log("analyzing template component %s", moduleName);
const resource = await this._pool.findResource(moduleName);
const code = await resource.buffer();
const ast = esprima.parseScript(code.toString());
const ast = espree.parse(code.toString(), {sourceType: "script"});
const defaultTemplateName = this._analyzeAST(moduleName, ast);
const templateName = (pageConfig.component && pageConfig.component.settings &&
pageConfig.component.settings.templateName) || defaultTemplateName;
Expand Down
4 changes: 2 additions & 2 deletions lib/lbt/analyzer/JSModuleAnalyzer.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"use strict";

const esprima = require("esprima");
const {Syntax} = esprima;
const espree = require("espree");
const {Syntax} = espree;
const escope = require("escope");
const ModuleName = require("../utils/ModuleName");
const {Format: ModuleFormat} = require("../resources/ModuleInfo");
Expand Down
6 changes: 3 additions & 3 deletions lib/lbt/analyzer/SmartTemplateAnalyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@

const ModuleName = require("../utils/ModuleName");
const SapUiDefine = require("../calls/SapUiDefine");
const esprima = require("esprima");
const {Syntax} = esprima;
const espree = require("espree");
const {Syntax} = espree;
const {getValue, isMethodCall, isString} = require("../utils/ASTUtils");
const log = require("@ui5/logger").getLogger("lbt:analyzer:SmartTemplateAnalyzer");

Expand Down Expand Up @@ -109,7 +109,7 @@ class TemplateComponentAnalyzer {
try {
const resource = await this._pool.findResource(moduleName);
const code = await resource.buffer();
const ast = esprima.parseScript(code.toString());
const ast = espree.parse(code.toString(), {sourceType: "script"});
const defaultTemplateName = this._analyzeAST(moduleName, ast);
const templateName = (pageConfig.component && pageConfig.component.settings &&
pageConfig.component.settings.templateName) || defaultTemplateName;
Expand Down
4 changes: 2 additions & 2 deletions lib/lbt/analyzer/XMLCompositeAnalyzer.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"use strict";

const esprima = require("esprima");
const {Syntax} = esprima;
const espree = require("espree");
const {Syntax} = espree;
const SapUiDefine = require("../calls/SapUiDefine");
const {getValue, isMethodCall, isString} = require("../utils/ASTUtils");
const ModuleName = require("../utils/ModuleName");
Expand Down
6 changes: 3 additions & 3 deletions lib/lbt/analyzer/analyzeLibraryJS.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"use strict";
const esprima = require("esprima");
const {Syntax} = esprima;
const espree = require("espree");
const {Syntax} = espree;
const {getPropertyKey, isMethodCall, isIdentifier, getStringArray} = require("../utils/ASTUtils");
const VisitorKeys = require("estraverse").VisitorKeys;

Expand Down Expand Up @@ -63,7 +63,7 @@ async function analyze(resource) {
}

const code = await resource.getBuffer();
visit( esprima.parseScript(code.toString()) );
visit( espree.parse(code.toString(), {sourceType: "script"}) );

return libInfo;
}
Expand Down
6 changes: 3 additions & 3 deletions lib/lbt/bundle/Builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

const terser = require("terser");
const {pd} = require("pretty-data");
const esprima = require("esprima");
const {Syntax} = esprima;
const espree = require("espree");
const {Syntax} = espree;
// const MOZ_SourceMap = require("source-map");

const {isMethodCall} = require("../utils/ASTUtils");
Expand Down Expand Up @@ -508,7 +508,7 @@ function rewriteDefine(targetBundleFormat, code, moduleName) {
let ast;
const codeStr = code.toString();
try {
ast = esprima.parseScript(codeStr, {range: true});
ast = espree.parse(codeStr, {sourceType: "script", range: true});
} catch (e) {
log.error("error while parsing %s: %s", moduleName, e.message);
return;
Expand Down
2 changes: 1 addition & 1 deletion lib/lbt/calls/SapUiDefine.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"use strict";

const {Syntax} = require("esprima");
const {Syntax} = require("espree");
const ModuleName = require("../utils/ModuleName");
const {isString, isBoolean} = require("../utils/ASTUtils");

Expand Down
4 changes: 2 additions & 2 deletions lib/lbt/resources/ResourcePool.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
const fs = require("fs");
const path = require("path");
*/
const esprima = require("esprima");
const espree = require("espree");
const ComponentAnalyzer = require("../analyzer/ComponentAnalyzer");
const SmartTemplateAnalyzer = require("../analyzer/SmartTemplateAnalyzer");
const FioriElementsAnalyzer = require("../analyzer/FioriElementsAnalyzer");
Expand Down Expand Up @@ -67,7 +67,7 @@ async function determineDependencyInfo(resource, rawInfo, pool) {
const promises = [];
let ast;
try {
ast = esprima.parseScript(code.toString(), {comment: true});
ast = espree.parse(code.toString(), {comment: true});
} catch (err) {
log.error("failed to parse %s: %s", resource.name, err.message);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/lbt/utils/ASTUtils.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"use strict";

const {Syntax} = require("esprima");
const {Syntax} = require("espree");

/**
* Checks whether the given node is a string literal.
Expand Down
85 changes: 58 additions & 27 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@
"cheerio": "1.0.0-rc.9",
"escape-unicode": "^0.2.0",
"escope": "^3.6.0",
"esprima": "^4.0.1",
"espree": "^8.0.0-beta",
"estraverse": "5.1.0",
"globby": "^11.0.3",
"graceful-fs": "^4.2.6",
Expand Down
16 changes: 8 additions & 8 deletions test/lib/lbt/analyzer/FioriElementsAnalyzer.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const test = require("ava");
const FioriElementsAnalyzer = require("../../../../lib/lbt/analyzer/FioriElementsAnalyzer");
const sinon = require("sinon");
const esprima = require("esprima");
const espree = require("espree");

test("analyze: with Component.js", async (t) => {
const emptyPool = {};
Expand Down Expand Up @@ -143,7 +143,7 @@ test.serial("_analyzeTemplateComponent: Manifest with TemplateAssembler code", a
const analyzer = new FioriElementsAnalyzer(mockPool);

const stubAnalyzeAST = sinon.stub(analyzer, "_analyzeAST").returns("mytpl");
const stubParse = sinon.stub(esprima, "parse").returns("");
const stubParse = sinon.stub(espree, "parse").returns("");

await analyzer._analyzeTemplateComponent("pony",
{}, moduleInfo);
Expand Down Expand Up @@ -177,7 +177,7 @@ test.serial("_analyzeTemplateComponent: no default template name", async (t) =>
const analyzer = new FioriElementsAnalyzer(mockPool);

const stubAnalyzeAST = sinon.stub(analyzer, "_analyzeAST").returns("");
const stubParse = sinon.stub(esprima, "parse").returns("");
const stubParse = sinon.stub(espree, "parse").returns("");

await analyzer._analyzeTemplateComponent("pony",
{}, moduleInfo);
Expand Down Expand Up @@ -206,7 +206,7 @@ test.serial("_analyzeTemplateComponent: with template name from pageConfig", asy
const analyzer = new FioriElementsAnalyzer(mockPool);

const stubAnalyzeAST = sinon.stub(analyzer, "_analyzeAST").returns("");
const stubParse = sinon.stub(esprima, "parse").returns("");
const stubParse = sinon.stub(espree, "parse").returns("");

await analyzer._analyzeTemplateComponent("pony", {
component: {
Expand Down Expand Up @@ -239,7 +239,7 @@ test("_analyzeAST: get template name from ast", async (t) => {
"manifest": "json"
}
});});`;
const ast = esprima.parse(code);
const ast = espree.parse(code);

const analyzer = new FioriElementsAnalyzer();

Expand Down Expand Up @@ -269,7 +269,7 @@ test("_analyzeAST: no template name from ast", async (t) => {
"manifest": "json"
}
});});`;
const ast = esprima.parse(code);
const ast = espree.parse(code);

const analyzer = new FioriElementsAnalyzer();

Expand Down Expand Up @@ -297,7 +297,7 @@ test("_analyzeTemplateClassDefinition: get template name from metadata", async (
"manifest": "json"
}
};`;
const ast = esprima.parse(code);
const ast = espree.parse(code);
const expression = ast.body[0].declarations[0].init;

const analyzer = new FioriElementsAnalyzer();
Expand All @@ -319,7 +319,7 @@ test("_analyzeTemplateClassDefinition: no string template name from metadata", a
"manifest": "json"
}
};`;
const ast = esprima.parse(code);
const ast = espree.parse(code);
const expression = ast.body[0].declarations[0].init;

const analyzer = new FioriElementsAnalyzer();
Expand Down
6 changes: 3 additions & 3 deletions test/lib/lbt/analyzer/JSModuleAnalyzer.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const test = require("ava");
const fs = require("fs");
const path = require("path");
const esprima = require("esprima");
const espree = require("espree");
const ModuleInfo = require("../../../../lib/lbt/resources/ModuleInfo");
const JSModuleAnalyzer = require("../../../../lib/lbt/analyzer/JSModuleAnalyzer");

Expand Down Expand Up @@ -51,7 +51,7 @@ function analyze(file, name) {
}

function analyzeString(content, name) {
const ast = esprima.parseScript(content, {comment: true});
const ast = espree.parse(content, {sourceType: "script", comment: true});
const info = new ModuleInfo(name);
new JSModuleAnalyzer().analyze(ast, name, info);
return info;
Expand Down Expand Up @@ -594,7 +594,7 @@ test("Toplevel define", (t) => {
});

test("Invalid ui5 bundle comment", (t) => {
const content = `/@ui5-bundles sap/ui/thirdparty/xxx.js
const content = `//@ui5-bundles sap/ui/thirdparty/xxx.js
if(!('xxx'in Node.prototype)){}
//@ui5-bundle-raw-includes sap/ui/thirdparty/aaa.js
(function(g,f){g.AAA=f();}(this,(function(){})));
Expand Down
Loading

0 comments on commit e7bef3b

Please sign in to comment.