[FIX] Encapsulate JS parsing and switch from esprima to espree

A new module lib/lbt/utils/parseJS is introduced that exposes a parseJS function and a Syntax object with all AST node types. The new module is used in all places where JavaSCript code is parsed (incl. tests). The module uses espree v6 for parsing instead of esprima. This is the maximum version that can be used without introducing breaking changes to the tooling (higher versions of espree require higher node versions than documented in our package.json). The set of allowed parser options is limited to ease a later migration to another parser. Fixes SAP/ui5-tooling#354 .
SAP · Jun 15, 2021 · 68557a0 · 68557a0
1 parent b86f0fa
commit 68557a0
Show file tree

Hide file tree

Showing 18 changed files with 167 additions and 93 deletions.
diff --git a/lib/lbt/analyzer/FioriElementsAnalyzer.js b/lib/lbt/analyzer/FioriElementsAnalyzer.js
@@ -60,8 +60,7 @@
 
 const ModuleName = require("../utils/ModuleName");
 const SapUiDefine = require("../calls/SapUiDefine");
-const esprima = require("esprima");
-const {Syntax} = esprima;
+const {parseJS, Syntax} = require("../utils/parseJS");
 const {getValue, isMethodCall, isString} = require("../utils/ASTUtils");
 const log = require("@ui5/logger").getLogger("lbt:analyzer:FioriElementAnalyzer");
 
@@ -147,7 +146,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 = parseJS(code);
 		const defaultTemplateName = this._analyzeAST(moduleName, ast);
 		const templateName = (pageConfig.component && pageConfig.component.settings &&
 								pageConfig.component.settings.templateName) || defaultTemplateName;

diff --git a/lib/lbt/analyzer/JSModuleAnalyzer.js b/lib/lbt/analyzer/JSModuleAnalyzer.js
@@ -1,7 +1,6 @@
 "use strict";
 
-const esprima = require("esprima");
-const {Syntax} = esprima;
+const {Syntax} = require("../utils/parseJS");
 const escope = require("escope");
 const ModuleName = require("../utils/ModuleName");
 const {Format: ModuleFormat} = require("../resources/ModuleInfo");

diff --git a/lib/lbt/analyzer/SmartTemplateAnalyzer.js b/lib/lbt/analyzer/SmartTemplateAnalyzer.js
@@ -27,8 +27,7 @@
 
 const ModuleName = require("../utils/ModuleName");
 const SapUiDefine = require("../calls/SapUiDefine");
-const esprima = require("esprima");
-const {Syntax} = esprima;
+const {parseJS, Syntax} = require("../utils/parseJS");
 const {getValue, isMethodCall, isString} = require("../utils/ASTUtils");
 const log = require("@ui5/logger").getLogger("lbt:analyzer:SmartTemplateAnalyzer");
 
@@ -109,7 +108,7 @@ class TemplateComponentAnalyzer {
 		try {
 			const resource = await this._pool.findResource(moduleName);
 			const code = await resource.buffer();
-			const ast = esprima.parseScript(code.toString());
+			const ast = parseJS(code);
 			const defaultTemplateName = this._analyzeAST(moduleName, ast);
 			const templateName = (pageConfig.component && pageConfig.component.settings &&
 									pageConfig.component.settings.templateName) || defaultTemplateName;

diff --git a/lib/lbt/analyzer/XMLCompositeAnalyzer.js b/lib/lbt/analyzer/XMLCompositeAnalyzer.js
@@ -1,7 +1,6 @@
 "use strict";
 
-const esprima = require("esprima");
-const {Syntax} = esprima;
+const {Syntax} = require("../utils/parseJS");
 const SapUiDefine = require("../calls/SapUiDefine");
 const {getValue, isMethodCall, isString} = require("../utils/ASTUtils");
 const ModuleName = require("../utils/ModuleName");

diff --git a/lib/lbt/analyzer/analyzeLibraryJS.js b/lib/lbt/analyzer/analyzeLibraryJS.js
@@ -1,6 +1,5 @@
 "use strict";
-const esprima = require("esprima");
-const {Syntax} = esprima;
+const {parseJS, Syntax} = require("../utils/parseJS");
 const {getPropertyKey, isMethodCall, isIdentifier, getStringArray} = require("../utils/ASTUtils");
 const VisitorKeys = require("estraverse").VisitorKeys;
 
@@ -63,7 +62,7 @@ async function analyze(resource) {
 	}
 
 	const code = await resource.getBuffer();
-	visit( esprima.parseScript(code.toString()) );
+	visit( parseJS(code) );
 
 	return libInfo;
 }

diff --git a/lib/lbt/bundle/Builder.js b/lib/lbt/bundle/Builder.js
@@ -4,8 +4,7 @@
 
 const terser = require("terser");
 const {pd} = require("pretty-data");
-const esprima = require("esprima");
-const {Syntax} = esprima;
+const {parseJS, Syntax} = require("../utils/parseJS");
 // const MOZ_SourceMap = require("source-map");
 
 const {isMethodCall} = require("../utils/ASTUtils");
@@ -508,7 +507,7 @@ function rewriteDefine(targetBundleFormat, code, moduleName) {
 	let ast;
 	const codeStr = code.toString();
 	try {
-		ast = esprima.parseScript(codeStr, {range: true});
+		ast = parseJS(codeStr, {range: true});
 	} catch (e) {
 		log.error("error while parsing %s: %s", moduleName, e.message);
 		return;

diff --git a/lib/lbt/calls/SapUiDefine.js b/lib/lbt/calls/SapUiDefine.js
@@ -1,6 +1,6 @@
 "use strict";
 
-const {Syntax} = require("esprima");
+const {Syntax} = require("../utils/parseJS");
 const ModuleName = require("../utils/ModuleName");
 const {isString, isBoolean} = require("../utils/ASTUtils");
 

diff --git a/lib/lbt/resources/ResourcePool.js b/lib/lbt/resources/ResourcePool.js
@@ -4,7 +4,7 @@
 const fs = require("fs");
 const path = require("path");
 */
-const esprima = require("esprima");
+const {parseJS} = require("../utils/parseJS");
 const ComponentAnalyzer = require("../analyzer/ComponentAnalyzer");
 const SmartTemplateAnalyzer = require("../analyzer/SmartTemplateAnalyzer");
 const FioriElementsAnalyzer = require("../analyzer/FioriElementsAnalyzer");
@@ -67,7 +67,7 @@ async function determineDependencyInfo(resource, rawInfo, pool) {
 		const promises = [];
 		let ast;
 		try {
-			ast = esprima.parseScript(code.toString(), {comment: true});
+			ast = parseJS(code, {comment: true});
 		} catch (err) {
 			log.error("failed to parse %s: %s", resource.name, err.message);
 		}

diff --git a/lib/lbt/utils/ASTUtils.js b/lib/lbt/utils/ASTUtils.js
@@ -1,6 +1,6 @@
 "use strict";
 
-const {Syntax} = require("esprima");
+const {Syntax} = require("../utils/parseJS");
 
 /**
  * Checks whether the given node is a string literal.

diff --git a/lib/lbt/utils/parseJS.js b/lib/lbt/utils/parseJS.js
@@ -0,0 +1,28 @@
+"use strict";
+
+const espree = require("espree");
+const {Syntax} = espree;
+
+const defaultOptions = {
+	comment: false,
+	ecmaVersion: 2020,
+	range: false,
+	sourceType: "script",
+};
+
+const allowedOptions = Object.keys(defaultOptions);
+
+function parseJS(code, options = {}) {
+	const firstUnsupportedOption =
+		Object.keys(options).find((name) => !allowedOptions.includes(name));
+	if (firstUnsupportedOption != null) {
+		throw new TypeError(`Option ${firstUnsupportedOption} is not one of ${allowedOptions})`);
+	}
+
+	return espree.parse(code, Object.assign({}, defaultOptions, options));
+}
+
+module.exports = {
+	parseJS,
+	Syntax
+};
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -109,7 +109,7 @@
 		"cheerio": "1.0.0-rc.9",
 		"escape-unicode": "^0.2.0",
 		"escope": "^3.6.0",
-		"esprima": "^4.0.1",
+		"espree": "^6.2.1",
 		"estraverse": "5.1.0",
 		"globby": "^11.0.3",
 		"graceful-fs": "^4.2.6",

diff --git a/test/lib/lbt/analyzer/FioriElementsAnalyzer.js b/test/lib/lbt/analyzer/FioriElementsAnalyzer.js
@@ -1,7 +1,7 @@
 const test = require("ava");
 const FioriElementsAnalyzer = require("../../../../lib/lbt/analyzer/FioriElementsAnalyzer");
 const sinon = require("sinon");
-const esprima = require("esprima");
+const parseJS = require("../../../../lib/lbt/utils/parseJS");
 
 test("analyze: with Component.js", async (t) => {
 	const emptyPool = {};
@@ -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(parseJS, "parseJS").returns("");
 
 	await analyzer._analyzeTemplateComponent("pony",
 		{}, moduleInfo);
@@ -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(parseJS, "parseJS").returns("");
 
 	await analyzer._analyzeTemplateComponent("pony",
 		{}, moduleInfo);
@@ -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(parseJS, "parseJS").returns("");
 
 	await analyzer._analyzeTemplateComponent("pony", {
 		component: {
@@ -239,7 +239,7 @@ test("_analyzeAST: get template name from ast", async (t) => {
 				"manifest": "json"
 			}
 		});});`;
-	const ast = esprima.parse(code);
+	const ast = parseJS.parseJS(code);
 
 	const analyzer = new FioriElementsAnalyzer();
 
@@ -269,7 +269,7 @@ test("_analyzeAST: no template name from ast", async (t) => {
 				"manifest": "json"
 			}
 		});});`;
-	const ast = esprima.parse(code);
+	const ast = parseJS.parseJS(code);
 
 	const analyzer = new FioriElementsAnalyzer();
 
@@ -297,7 +297,7 @@ test("_analyzeTemplateClassDefinition: get template name from metadata", async (
 				"manifest": "json"
 			}
 		};`;
-	const ast = esprima.parse(code);
+	const ast = parseJS.parseJS(code);
 	const expression = ast.body[0].declarations[0].init;
 
 	const analyzer = new FioriElementsAnalyzer();
@@ -319,7 +319,7 @@ test("_analyzeTemplateClassDefinition: no string template name from metadata", a
 				"manifest": "json"
 			}
 		};`;
-	const ast = esprima.parse(code);
+	const ast = parseJS.parseJS(code);
 	const expression = ast.body[0].declarations[0].init;
 
 	const analyzer = new FioriElementsAnalyzer();

diff --git a/test/lib/lbt/analyzer/JSModuleAnalyzer.js b/test/lib/lbt/analyzer/JSModuleAnalyzer.js
@@ -1,7 +1,7 @@
 const test = require("ava");
 const fs = require("fs");
 const path = require("path");
-const esprima = require("esprima");
+const {parseJS} = require("../../../../lib/lbt/utils/parseJS");
 const ModuleInfo = require("../../../../lib/lbt/resources/ModuleInfo");
 const JSModuleAnalyzer = require("../../../../lib/lbt/analyzer/JSModuleAnalyzer");
 
@@ -51,7 +51,7 @@ function analyze(file, name) {
 }
 
 function analyzeString(content, name) {
-	const ast = esprima.parseScript(content, {comment: true});
+	const ast = parseJS(content, {comment: true});
 	const info = new ModuleInfo(name);
 	new JSModuleAnalyzer().analyze(ast, name, info);
 	return info;
@@ -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(){})));