From ed1cc9d45e517b3b38815483cc60fa7182ffd067 Mon Sep 17 00:00:00 2001
From: Frank Weigel <frank.weigel@sap.com>
Date: Tue, 7 Jan 2020 14:45:30 +0100
Subject: [PATCH] [FIX] Detect dynamic dependencies also when newer APIs are
 used (#391)

[FIX] Detect dynamic dependencies also when newer APIs are used

So far, dynamic dependencies only had been detected for calls to jQuery.sap.require. Now, calls to sap.ui.require and sap.ui.requireSync are also checked.
---
 lib/lbt/analyzer/JSModuleAnalyzer.js          | 17 ++++++++----
 .../lbt/modules/amd_dynamic_require.js        | 11 ++++++++
 .../lbt/modules/amd_dynamic_require_sync.js   |  7 +++++
 .../lbt/modules/declare_dynamic_require.js    |  5 ++++
 test/lib/lbt/analyzer/JSModuleAnalyzer.js     | 26 +++++++++++++++++++
 5 files changed, 61 insertions(+), 5 deletions(-)
 create mode 100644 test/fixtures/lbt/modules/amd_dynamic_require.js
 create mode 100644 test/fixtures/lbt/modules/amd_dynamic_require_sync.js
 create mode 100644 test/fixtures/lbt/modules/declare_dynamic_require.js

diff --git a/lib/lbt/analyzer/JSModuleAnalyzer.js b/lib/lbt/analyzer/JSModuleAnalyzer.js
index 489a74841..927723cc4 100644
--- a/lib/lbt/analyzer/JSModuleAnalyzer.js
+++ b/lib/lbt/analyzer/JSModuleAnalyzer.js
@@ -546,7 +546,7 @@ class JSModuleAnalyzer {
 						const requiredModuleName2 = ModuleName.fromUI5LegacyName( arg.alternate.value );
 						info.addDependency(requiredModuleName2, true);
 					} else {
-						log.verbose("jQuery.sap.require: cannot evaluate dynamic arguments: ", arg);
+						log.verbose("jQuery.sap.require: cannot evaluate dynamic arguments: ", arg && arg.type);
 						info.dynamicDependencies = true;
 					}
 				}
@@ -558,13 +558,17 @@ class JSModuleAnalyzer {
 			const nArgs = args.length;
 			const i = 0;
 
-			if ( i < nArgs && isString(args[i]) ) {
-				const moduleName = ModuleName.fromRequireJSName( args[i].value );
-				info.addDependency(moduleName, conditional);
+			if ( i < nArgs ) {
+				if ( isString(args[i]) ) {
+					const moduleName = ModuleName.fromRequireJSName( args[i].value );
+					info.addDependency(moduleName, conditional);
+				} else {
+					log.verbose("sap.ui.requireSync: cannot evaluate dynamic arguments: ", args[i] && args[i].type);
+					info.dynamicDependencies = true;
+				}
 			}
 		}
 
-
 		function onSapUiPredefine(predefineCall) {
 			const args = predefineCall.arguments;
 			const nArgs = args.length;
@@ -622,6 +626,9 @@ class JSModuleAnalyzer {
 						requiredModule = ModuleName.resolveRelativeRequireJSName(name, item.value);
 					}
 					info.addDependency( requiredModule, conditional );
+				} else {
+					log.verbose("sap.ui.require/sap.ui.define: cannot evaluate dynamic argument: ", item && item.type);
+					info.dynamicDependencies = true;
 				}
 			});
 		}
diff --git a/test/fixtures/lbt/modules/amd_dynamic_require.js b/test/fixtures/lbt/modules/amd_dynamic_require.js
new file mode 100644
index 000000000..ac90f9261
--- /dev/null
+++ b/test/fixtures/lbt/modules/amd_dynamic_require.js
@@ -0,0 +1,11 @@
+sap.ui.define([], function() {
+	return {
+		load: function(modName) {
+			sap.ui.require([modName], function(modExport) {
+				// module was loaded
+			}, function(err) {
+				// error occurred
+			});
+		}
+	}
+});
\ No newline at end of file
diff --git a/test/fixtures/lbt/modules/amd_dynamic_require_sync.js b/test/fixtures/lbt/modules/amd_dynamic_require_sync.js
new file mode 100644
index 000000000..fdd4a3729
--- /dev/null
+++ b/test/fixtures/lbt/modules/amd_dynamic_require_sync.js
@@ -0,0 +1,7 @@
+sap.ui.define([], function() {
+	return {
+		load: function(modName) {
+			return sap.ui.requireSync(modName);
+		}
+	}
+});
diff --git a/test/fixtures/lbt/modules/declare_dynamic_require.js b/test/fixtures/lbt/modules/declare_dynamic_require.js
new file mode 100644
index 000000000..d29d573ad
--- /dev/null
+++ b/test/fixtures/lbt/modules/declare_dynamic_require.js
@@ -0,0 +1,5 @@
+jQuery.sap.declare("sap.ui.testmodule");
+
+sap.ui.testmodule.load = function(modName) {
+	jQuery.sap.require(modName);
+};
diff --git a/test/lib/lbt/analyzer/JSModuleAnalyzer.js b/test/lib/lbt/analyzer/JSModuleAnalyzer.js
index 4f2b06041..23a1b4be1 100644
--- a/test/lib/lbt/analyzer/JSModuleAnalyzer.js
+++ b/test/lib/lbt/analyzer/JSModuleAnalyzer.js
@@ -94,6 +94,8 @@ function analyzeModule(
 				expectedSubmodules,
 				"submodules should match");
 		}
+		t.false(info.dynamicDependencies,
+			"no use of dynamic dependencies should have been detected");
 	}).then(() => t.end(), (e) => t.fail(`failed to analyze module with error: ${e.message}`));
 }
 
@@ -356,7 +358,31 @@ test("ES6 Syntax", (t) => {
 				"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");
+			t.false(info.dynamicDependencies,
+				"no use of dynamic dependencies should have been detected");
 		});
 	});
 });
 
+test("Dynamic import (declare/require)", (t) => {
+	return analyze("modules/declare_dynamic_require.js").then((info) => {
+		t.true(info.dynamicDependencies,
+			"the use of dynamic dependencies should have been detected");
+	});
+});
+
+test("Dynamic import (define/require)", (t) => {
+	return analyze("modules/amd_dynamic_require.js").then((info) => {
+		t.true(info.dynamicDependencies,
+			"the use of dynamic dependencies should have been detected");
+	});
+});
+
+test("Dynamic import (define/requireSync)", (t) => {
+	return analyze("modules/amd_dynamic_require_sync.js").then((info) => {
+		t.true(info.dynamicDependencies,
+			"the use of dynamic dependencies should have been detected");
+	});
+});
+
+