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"); + }); + }); +});