From 9fbc746945f242d9da7cebae399f5108fb007e26 Mon Sep 17 00:00:00 2001 From: Frank Weigel Date: Sun, 1 Jul 2018 22:25:07 +0200 Subject: [PATCH] [INTERNAL] JSModuleAnalyzer: update language metadata The JSModuleAnalyzer internally uses some metadata describing for each type of ESTree node what properties represent conditionally executed code branches and what properties represent unconditionally executed branches. The metadata is used while visiting an AST to classify dependencies as 'static' ord 'conditional'. During the migration of the analyzer from Java to JavaScript, metadata was only maintained for rudimentary set of nodes (basically ES5) and all other nodes have been marked as 'toBeDone'. This changes closes this gap and maintaines metadata for all ES6 node types, only the types planned for ES7 are kept as 'toBeDone' --- lib/lbt/analyzer/JSModuleAnalyzer.js | 114 +++++++++++++++++----- test/fixtures/lbt/modules/es6-syntax.js | 38 ++++++++ test/lib/lbt/analyzer/JSModuleAnalyzer.js | 27 +++++ 3 files changed, 154 insertions(+), 25 deletions(-) create mode 100644 test/fixtures/lbt/modules/es6-syntax.js diff --git a/lib/lbt/analyzer/JSModuleAnalyzer.js b/lib/lbt/analyzer/JSModuleAnalyzer.js index 768c0e445..70e85f4eb 100644 --- a/lib/lbt/analyzer/JSModuleAnalyzer.js +++ b/lib/lbt/analyzer/JSModuleAnalyzer.js @@ -9,7 +9,7 @@ const UI5ClientConstants = require("../UI5ClientConstants"); const {findOwnProperty, getLocation, getPropertyKey, isMethodCall, isString} = require("../utils/ASTUtils"); const log = require("@ui5/logger").getLogger("lbt:analyzer:JSModuleAnalyzer"); -// -------------------------------------------------------------------------------------------------------------------- +// ------------------------------------------------------------------------------------------------------------------ const EnrichedVisitorKeys = (function() { const VisitorKeys = require("estraverse").VisitorKeys; @@ -24,17 +24,44 @@ const EnrichedVisitorKeys = (function() { * E.g. in an IfExpression, the 'test' is always executed, whereas 'consequent' * and 'alternate' are only executed under certain conditions. * - * The object is checked against the 'official' list of node types - * and node keys as defined by 'estraverse' This helps to ensure that no new - * syntax addition is missed and that the configured keys are valid. + * While visiting the AST of a JavaSCript file, the JSModuleAnalyzer uses this information + * to decide whether a code block is executed conditionally or unconditionally. + * Besides this information which is inherent to the language, the analyzer uses + * additional knowledge about special APIS / constructs (e.g. the factory function of + * an AMD module is known to be executed when the module is executed, an IIFE is known to + * be executed etc.) + * + * To be more robust against the evolution of the language, the object below is checked + * against the 'official' list of node types and node keys as defined by 'estraverse'. + * This helps to ensure that no new syntax addition is missed and that the configured + * keys are valid. */ const TempKeys = { AssignmentExpression: [], - AssignmentPattern: toBeDone(["left", "right"]), + /* + * function( >>>a=3<<<, b) {...} + * var [>>>a=3<<<, b] = [...]; + * + * The default value expression (right) is only evaluated when there's no other value in + * the context of the pattern (e.g. destructuring or function call don't provide a value), + * so it's a conditional branch. + */ + AssignmentPattern: ["right"], ArrayExpression: [], - ArrayPattern: toBeDone(["elements"]), + /* + * var >>>[a=3, b]<<< = [...]; + * All elements in an array pattern are unconditional. + */ + ArrayPattern: [], // elements + /* + * The body of an arrow function is only executed when the arrow function is executed + */ ArrowFunctionExpression: ["body"], - AwaitExpression: toBeDone(["argument"]), // CAUTION: It's deferred to ES7. + /* + * The argument of await is always executed + * TODO how to handle code after the await expression? + */ + AwaitExpression: [], // argument BlockStatement: [], BinaryExpression: [], BreakStatement: [], @@ -49,12 +76,16 @@ const EnrichedVisitorKeys = (function() { ContinueStatement: [], DebuggerStatement: [], DirectiveStatement: [], - DoWhileStatement: [], // condition is executed on the same conditions as the surrounding block, potentially repeated, block is always entered and might be repeated + /* + * 'condition' is executed on the same conditions as the surrounding block, potentially repeated, + * 'block' is always entered and might be repeated + */ + DoWhileStatement: [], EmptyStatement: [], - ExportAllDeclaration: toBeDone(["source"]), - ExportDefaultDeclaration: toBeDone(["declaration"]), - ExportNamedDeclaration: toBeDone(["declaration", "specifiers", "source"]), - ExportSpecifier: toBeDone(["exported", "local"]), + ExportAllDeclaration: [], // no parts of an export are conditional - source + ExportDefaultDeclaration: [], // no parts of an export are conditional - declaration + ExportNamedDeclaration: [], // no parts of an export are conditional - declaration, specifiers, source + ExportSpecifier: [], // no parts of an export are conditional exported, local ExpressionStatement: [], ForStatement: ["update", "body"], ForInStatement: ["body"], @@ -64,10 +95,22 @@ const EnrichedVisitorKeys = (function() { GeneratorExpression: toBeDone(["blocks", "filter", "body"]), // CAUTION: It's deferred to ES7. Identifier: [], IfStatement: ["consequent", "alternate"], - ImportDeclaration: toBeDone(["specifiers", "source"]), - ImportDefaultSpecifier: toBeDone(["local"]), - ImportNamespaceSpecifier: toBeDone(["local"]), - ImportSpecifier: toBeDone(["imported", "local"]), + /* + * all parts of an import declaration are executed unconditionally + */ + ImportDeclaration: [], // specifiers, source + /* + * import >>>a<<< from 'module'; + */ + ImportDefaultSpecifier: [], // local + /* + * import >>>* as b<<< from 'module'; + */ + ImportNamespaceSpecifier: [], // local + /* + * import {>>>a as c<<<,b} from 'module'; + */ + ImportSpecifier: [], // imported, local Literal: [], LabeledStatement: [], LogicalExpression: [], @@ -77,19 +120,33 @@ const EnrichedVisitorKeys = (function() { ModuleSpecifier: [], NewExpression: [], ObjectExpression: [], - ObjectPattern: toBeDone(["properties"]), + /* + * >>>{a,b,c}<<< = {...} + * + * All properties in an object pattern are executed. + */ + ObjectPattern: [], // properties Program: [], Property: [], - RestElement: toBeDone(["argument"]), + /* + * argument of the rest element is always executed under the same condition as the rest element itself + */ + RestElement: [], // argument ReturnStatement: [], SequenceExpression: [], - SpreadElement: toBeDone(["argument"]), + SpreadElement: [], // the argument of the spread operator always needs to be evaluated - argument Super: [], SwitchStatement: [], SwitchCase: ["test", "consequent"], // test and consequent are executed only conditionally - TaggedTemplateExpression: toBeDone(["tag", "quasi"]), + /* + * all parts of a tagged template literal are executed under the same condition as the context + */ + TaggedTemplateExpression: [], // tag, quasi TemplateElement: [], - TemplateLiteral: toBeDone(["quasis", "expressions"]), + /* + * all parts of a template literal are executed under the same condition as the context + */ + TemplateLiteral: [], // quasis, expressions ThisExpression: [], ThrowStatement: [], TryStatement: ["handler"], // handler is called conditionally @@ -97,7 +154,11 @@ const EnrichedVisitorKeys = (function() { UpdateExpression: [], VariableDeclaration: [], VariableDeclarator: [], - WhileStatement: ["body"], // condition is executed on the same conditions as the surrounding block and potentially repeated, block maybe entered only conditionally but can be repeated + /* + * 'condition' is executed on the same conditions as the surrounding block and potentially repeated, + * 'block' maybe entered only conditionally but can be repeated + */ + WhileStatement: ["body"], WithStatement: [], YieldExpression: [] }; @@ -151,6 +212,9 @@ const CALL_JQUERY_SAP_IS_DECLARED = [["jQuery", "$"], "sap", "isDeclared"]; const CALL_JQUERY_SAP_REQUIRE = [["jQuery", "$"], "sap", "require"]; const CALL_JQUERY_SAP_REGISTER_PRELOADED_MODULES = [["jQuery", "$"], "sap", "registerPreloadedModules"]; +function isCallableExpression(node) { + return node.type == Syntax.FunctionExpression || node.type == Syntax.ArrowFunctionExpression; +} /** * Analyzes an already parsed JSDocument to collect information about the contained module(s). @@ -233,7 +297,7 @@ class JSModuleAnalyzer { if ( iArg < args.length && args[iArg].type == Syntax.ArrayExpression ) { iArg++; } - if ( iArg < args.length && args[iArg].type == Syntax.FunctionExpression ) { + if ( iArg < args.length && isCallableExpression(args[iArg]) ) { // unconditionally execute the factory function visit(args[iArg].body, conditional); } @@ -256,7 +320,7 @@ class JSModuleAnalyzer { analyzeDependencyArray(args[iArg].elements, conditional, null); iArg++; } - if ( iArg < args.length && args[iArg].type == Syntax.FunctionExpression ) { + if ( iArg < args.length && isCallableExpression(args[iArg]) ) { // analyze the callback function visit(args[iArg].body, conditional); } @@ -275,7 +339,7 @@ class JSModuleAnalyzer { let legacyCall = isMethodCall(node, CALL_JQUERY_SAP_REGISTER_PRELOADED_MODULES); info.setFormat( legacyCall ? ModuleFormat.UI5_LEGACY : ModuleFormat.UI5_DEFINE); onRegisterPreloadedModules(node, legacyCall); - } else if ( node.callee.type === Syntax.FunctionExpression ) { + } else if ( isCallableExpression(node.callee) ) { // recognizes a scope function declaration + argument visit(node.arguments, conditional); // NODE-TODO defaults of callee? diff --git a/test/fixtures/lbt/modules/es6-syntax.js b/test/fixtures/lbt/modules/es6-syntax.js new file mode 100644 index 000000000..72c325c71 --- /dev/null +++ b/test/fixtures/lbt/modules/es6-syntax.js @@ -0,0 +1,38 @@ +sap.ui.define([ + 'static/module1' +], (m1) => { // using an arrow function for the module factory + + sap.ui.require(['static/module2'], function() { + sap.ui.require(['static/module3'], function() {}); + sap.ui.require('no-dependency/module1'); // probing API does not introduce a dependency + }); + + // using an arrow function for the require callback + sap.ui.require([], () => { + sap.ui.require(['static/module4'], function() { + }); + }); + + // default value in array destructuring + let [exp1 = sap.ui.require(['conditional/module1'], function(){})] = []; + + // default value in object destructuring + let {exp2 = sap.ui.require(['conditional/module2'], function(){})} = {}; + + // dependency embedded in a template + let exp3 = `Some text with an embedded dependency ${sap.ui.require(['static/module5'], function(){})} and further text`; + + // dependency embedded in a tagged template + let exp4 = html`Some text with an embedded dependency ${sap.ui.require(['static/module6'], function(){})} and further text`; + + // IIAFE (an immediately invoked arrow function expression) + ((() => { + sap.ui.require(['static/module7'], function(){}); + })()); + + // a not immediately executed arrow function + let helper = (() => { + sap.ui.require(['conditional/module3'], function(){}); + }); + +}); \ No newline at end of file diff --git a/test/lib/lbt/analyzer/JSModuleAnalyzer.js b/test/lib/lbt/analyzer/JSModuleAnalyzer.js index e7b25361f..b581bdcb6 100644 --- a/test/lib/lbt/analyzer/JSModuleAnalyzer.js +++ b/test/lib/lbt/analyzer/JSModuleAnalyzer.js @@ -52,6 +52,7 @@ function analyzeModule(t, file, name, expectedDependencies, expectDocumentation) }); } + test.cb("DeclareToplevel", analyzeModule, "modules/declare_toplevel.js", EXPECTED_MODULE_NAME, EXPECTED_DECLARE_DEPENDENCIES); test.cb("DeclareFunctionExprScope", analyzeModule, "modules/declare_function_expr_scope.js", EXPECTED_MODULE_NAME, EXPECTED_DECLARE_DEPENDENCIES); @@ -81,3 +82,29 @@ test("Bundle", (t) => { t.truthy(info.dependencies.every((dep) => !info.isConditionalDependency(dep)), "none of the dependencies must be 'conditional'"); }); }); + +test("ES6 Syntax", (t) => { + return analyze("modules/es6-syntax.js", "modules/es6-syntax.js").then( (info) => { + const expected = [ + "conditional/module1.js", + "conditional/module2.js", + "conditional/module3.js", + "static/module1.js", + "static/module2.js", + "static/module3.js", + "static/module4.js", + "static/module5.js", + "static/module6.js", + "static/module7.js", + "ui5loader-autoconfig.js" + ]; + const actual = info.dependencies.sort(); + t.deepEqual(actual, expected, "module dependencies should match"); + expected.forEach((dep) => { + t.is(info.isConditionalDependency(dep), /^conditional\//.test(dep), + "only dependencies to 'conditional/*' modules should be conditional"); + t.is(info.isImplicitDependency(dep), !/^(?:conditional|static)\//.test(dep), + "all dependencies other than 'conditional/*' and 'static/*' should be implicit"); + }); + }); +});