From 717f2ec8e6b04e67966183d25cc0ae59db94f43b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Matthias=20O=C3=9Fwald?=
 <1410947+matz3@users.noreply.github.com>
Date: Wed, 3 Jun 2020 18:03:15 +0200
Subject: [PATCH] [FIX] ComponentAnalyzer: Properly handle sap.ui5/routing
 (#463)

Co-authored-by: Merlin Beutlberger <m.beutlberger@sap.com>
---
 lib/lbt/analyzer/ComponentAnalyzer.js      | 26 ++++-----
 test/lib/lbt/analyzer/ComponentAnalyzer.js | 62 ++++++++++++++++++++++
 2 files changed, 76 insertions(+), 12 deletions(-)

diff --git a/lib/lbt/analyzer/ComponentAnalyzer.js b/lib/lbt/analyzer/ComponentAnalyzer.js
index bc454cc70..fb2f47194 100644
--- a/lib/lbt/analyzer/ComponentAnalyzer.js
+++ b/lib/lbt/analyzer/ComponentAnalyzer.js
@@ -60,7 +60,7 @@ class ComponentAnalyzer {
 				log.verbose("No manifest found for '%s', skipping analysis", resource.name);
 			}
 		} catch (err) {
-			log.error("an error occurred while analyzing component %s (ignored)", resource.name, err);
+			log.error("an error occurred while analyzing component %s (ignored)", resource.name, err.stack);
 		}
 
 		return info;
@@ -188,17 +188,19 @@ class ComponentAnalyzer {
 				info.addDependency(viewsModule);
 			}
 
-			for (const targetName of Object.keys(routing.targets)) {
-				const target = routing.targets[targetName];
-				if (target && target.viewName) {
-					// merge target config with default config
-					const config = Object.assign({}, routing.config, target);
-					const module = ModuleName.fromUI5LegacyName(
-						(config.viewPath ? config.viewPath + "." : "") +
-						config.viewName, ".view." + config.viewType.toLowerCase() );
-					log.verbose("converting routing target '%s' to view dependency '%s'", targetName, module);
-					// TODO make this a conditional dependency, depending on the route pattern?
-					info.addDependency(module);
+			if (routing.targets) {
+				for (const targetName of Object.keys(routing.targets)) {
+					const target = routing.targets[targetName];
+					if (target && target.viewName) {
+						// merge target config with default config
+						const config = Object.assign({}, routing.config, target);
+						const module = ModuleName.fromUI5LegacyName(
+							(config.viewPath ? config.viewPath + "." : "") +
+							config.viewName, ".view." + config.viewType.toLowerCase() );
+						log.verbose("converting routing target '%s' to view dependency '%s'", targetName, module);
+						// TODO make this a conditional dependency, depending on the route pattern?
+						info.addDependency(module);
+					}
 				}
 			}
 		}
diff --git a/test/lib/lbt/analyzer/ComponentAnalyzer.js b/test/lib/lbt/analyzer/ComponentAnalyzer.js
index 15ea083ee..db9852dcc 100644
--- a/test/lib/lbt/analyzer/ComponentAnalyzer.js
+++ b/test/lib/lbt/analyzer/ComponentAnalyzer.js
@@ -480,6 +480,68 @@ test("_analyzeManifest: Manifest with routing and routes object", async (t) => {
 		"addDependency should be called with the app dependency name");
 });
 
+test("_analyzeManifest: Manifest with legacy routes object", async (t) => {
+	const manifest = {
+		"sap.ui5": {
+			routing: {
+				config: {
+					viewPath: "test.view",
+					viewType: "XML"
+				},
+				routes: {
+					test: {
+						pattern: "",
+						view: "App",
+						targetAggregation: "pages",
+						targetControl: "app",
+					}
+				}
+			}
+		}
+	};
+
+	const moduleInfo = {
+		addDependency: function() {}
+	};
+	const stubAddDependency = sinon.spy(moduleInfo, "addDependency");
+
+	const analyzer = new ComponentAnalyzer();
+
+	await analyzer._analyzeManifest(manifest, moduleInfo);
+
+	// Note: Dependencies to views within legacy routes are not collected
+	t.true(stubAddDependency.calledOnce, "addDependency was called once");
+	t.deepEqual(stubAddDependency.getCall(0).args[0], "sap/ui/core/routing/Router.js",
+		"addDependency should be called with the router dependency name");
+});
+
+test("_analyzeManifest: Manifest with empty routes array", async (t) => {
+	const manifest = {
+		"sap.ui5": {
+			routing: {
+				config: {
+					viewPath: "test.view",
+					viewType: "XML"
+				},
+				routes: []
+			}
+		}
+	};
+
+	const moduleInfo = {
+		addDependency: function() {}
+	};
+	const stubAddDependency = sinon.spy(moduleInfo, "addDependency");
+
+	const analyzer = new ComponentAnalyzer();
+
+	await analyzer._analyzeManifest(manifest, moduleInfo);
+
+	t.true(stubAddDependency.calledOnce, "addDependency was called once");
+	t.deepEqual(stubAddDependency.getCall(0).args[0], "sap/ui/core/routing/Router.js",
+		"addDependency should be called with the router dependency name");
+});
+
 test("_analyzeManifest: Manifest with rootview object", async (t) => {
 	const manifest = {
 		"sap.ui5": {