From aaeafd4a1fd194dd08e5ae47c29d90f0b4c7d197 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Matthias=20O=C3=9Fwald?=
 <1410947+matz3@users.noreply.github.com>
Date: Mon, 25 Apr 2022 17:03:04 +0200
Subject: [PATCH] [FIX] JSModuleAnalyzer: Fix detection of bundle name (#705)

JIRA: CPOUI5FOUNDATION-485
---
 lib/lbt/analyzer/JSModuleAnalyzer.js      | 15 ++++-
 test/lib/lbt/analyzer/JSModuleAnalyzer.js | 75 +++++++++++++++++++++++
 2 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/lib/lbt/analyzer/JSModuleAnalyzer.js b/lib/lbt/analyzer/JSModuleAnalyzer.js
index da1c494ad..c7053b3b0 100644
--- a/lib/lbt/analyzer/JSModuleAnalyzer.js
+++ b/lib/lbt/analyzer/JSModuleAnalyzer.js
@@ -301,6 +301,9 @@ class JSModuleAnalyzer {
 		 */
 		let bIsUi5Module = false;
 
+		// Module name via @ui5-bundle comment in first line. Overrides all other main module candidates.
+		let firstLineBundleName;
+
 		// first analyze the whole AST...
 		visit(ast, false);
 
@@ -314,7 +317,12 @@ class JSModuleAnalyzer {
 						log.verbose(`bundle include directive ${subModule}`);
 					} else if ( comment.value.startsWith("@ui5-bundle ") ) {
 						const bundleName = comment.value.slice("@ui5-bundle ".length);
-						setMainModuleInfo(bundleName, null);
+						if (comment.start === 0) {
+							// Remember the name from the first line to use it as final name
+							firstLineBundleName = bundleName;
+						} else {
+							setMainModuleInfo(bundleName, null);
+						}
 						log.verbose(`bundle name directive ${bundleName}`);
 					} else {
 						log.warn(`unrecognized bundle directive ${comment.value}`);
@@ -324,7 +332,10 @@ class JSModuleAnalyzer {
 		}
 
 		// ...and finally take conclusions about the file's content
-		if ( !mainModuleFound ) {
+		if ( firstLineBundleName ) {
+			// If the first line has a bundle name, use it and override all other found names
+			info.name = firstLineBundleName;
+		} else if ( !mainModuleFound ) {
 			// if there's exactly one module definition in this file but it didn't
 			// immediately qualify as main module, make it now the main module
 			if ( candidateName != null && nModuleDeclarations == 1 ) {
diff --git a/test/lib/lbt/analyzer/JSModuleAnalyzer.js b/test/lib/lbt/analyzer/JSModuleAnalyzer.js
index 10b5da6e5..139431eed 100644
--- a/test/lib/lbt/analyzer/JSModuleAnalyzer.js
+++ b/test/lib/lbt/analyzer/JSModuleAnalyzer.js
@@ -684,3 +684,78 @@ jQuery.sap.registerPreloadedModules({
 	t.deepEqual(info.subModules, ["foo/bar.js"],
 		"submodule from jQuery.sap.registerPreloadedModules");
 });
+
+test("Module that contains jQuery.sap.declare should be derived as subModule", (t) => {
+	const content = `
+sap.ui.define([], function() {
+	jQuery.sap.declare("foo.bar");
+});
+`;
+	const info = analyzeString(content, "modules/module-with-jquery-sap-declare.js");
+	t.is(info.name, "modules/module-with-jquery-sap-declare.js");
+	t.is(info.rawModule, false);
+	t.is(info.format, "ui5-declare"); // FIXME: Format should actually be ui5-define
+	t.is(info.requiresTopLevelScope, false);
+	t.deepEqual(info.subModules, ["foo/bar.js"],
+		"jQuery.sap.declare subModule should be detected");
+	t.deepEqual(info.dependencies, ["jquery.sap.global.js"], "Implicit dependency");
+});
+
+test("Bundle that contains jQuery.sap.declare (sap.ui.predefine) should not be derived as module name", (t) => {
+	const content = `//@ui5-bundle test1/library-preload.js
+sap.ui.predefine("test1/module1", [], function() {
+	jQuery.sap.declare("foo.bar");
+});
+`;
+	const info = analyzeString(content, "modules/bundle-with-jquery-sap-declare.js");
+	t.is(info.name, "test1/library-preload.js", "Module name should be taken from @ui5-bundle comment");
+	t.is(info.rawModule, false);
+	t.is(info.format, "ui5-declare"); // FIXME: Format should actually be ui5-define
+	t.is(info.requiresTopLevelScope, false);
+	// Note: foo/bar.js is not listed as the predefine body is not analyzed
+	t.deepEqual(info.subModules, ["test1/module1.js"],
+		"subModule via sap.ui.predefine should be detected");
+	t.deepEqual(info.dependencies, ["jquery.sap.global.js"], "Implicit dependency");
+});
+
+test("Bundle that contains jQuery.sap.declare (sap.ui.require.preload) should not be derived as module name", (t) => {
+	const content = `//@ui5-bundle test1/library-preload.js
+sap.ui.require.preload({
+	"test1/module1.js": function() {
+		sap.ui.define([], function() {
+			jQuery.sap.declare("foo.bar");
+		});
+	}
+});
+
+`;
+	const info = analyzeString(content, "modules/bundle-with-jquery-sap-declare.js");
+	t.is(info.name, "test1/library-preload.js", "Module name should be taken from @ui5-bundle comment");
+	t.is(info.rawModule, false);
+	t.is(info.format, "ui5-define");
+	t.is(info.requiresTopLevelScope, false);
+	// Note: foo/bar.js is not listed as the sap.ui.define body is not analyzed
+	t.deepEqual(info.subModules, ["test1/module1.js"],
+		"subModule via sap.ui.predefine should be detected");
+	t.deepEqual(info.dependencies, ["ui5loader-autoconfig.js"], "Implicit dependency");
+});
+
+test("@ui5-bundle comment: Multiple comments", (t) => {
+	const content = `//@ui5-bundle test/bundle1.js
+//@ui5-bundle test/bundle2.js
+`;
+	const info = analyzeString(content, "modules/ui5-bundle-comments.js");
+	t.is(info.name, "test/bundle1.js", "Comment from first line should be used");
+	t.deepEqual(info.subModules, []);
+	t.deepEqual(info.dependencies, []);
+});
+
+test("@ui5-bundle comment: Multiple comments (Not in first line)", (t) => {
+	const content = `console.log('Foo');
+//@ui5-bundle test/bundle1.js
+//@ui5-bundle test/bundle2.js
+`;
+	t.throws(() => analyzeString(content, "modules/ui5-bundle-comments.js"), {
+		message: "conflicting main modules found (unnamed + named)"
+	});
+});