From f22313c38f3eb4006fb3c22c6aa838607f7593d3 Mon Sep 17 00:00:00 2001
From: rosen-vladimirov <rosen.vladimirov@progress.com>
Date: Wed, 20 Mar 2019 13:26:05 +0200
Subject: [PATCH] fix: warnings for short imports should not be shown for
 minified code

In some cases when there's minified code, CLI prints warning for short imports, while in fact there's no such thing in the code.
Fix the regular exepression to be more strict.
Also, instead of iterrating over each regular expression for core-modules directories, join them in a single RegExp. This improves the worst case scenario (i.e. when there are short imports for `xml` of `tns-core-modules`) with around 40%.
---
 lib/services/doctor-service.ts  | 30 +++++++++++++--------
 test/services/doctor-service.ts | 46 ++++++++++++++++++++++++++++++---
 2 files changed, 62 insertions(+), 14 deletions(-)

diff --git a/lib/services/doctor-service.ts b/lib/services/doctor-service.ts
index b45311af04..e408876713 100644
--- a/lib/services/doctor-service.ts
+++ b/lib/services/doctor-service.ts
@@ -135,7 +135,7 @@ export class DoctorService implements IDoctorService {
 	}
 
 	protected getDeprecatedShortImportsInFiles(files: string[], projectDir: string): { file: string, line: string }[] {
-		const shortImportRegExps = this.getShortImportRegExps(projectDir);
+		const shortImportRegExp = this.getShortImportRegExp(projectDir);
 		const shortImports: { file: string, line: string }[] = [];
 
 		for (const file of files) {
@@ -144,17 +144,15 @@ export class DoctorService implements IDoctorService {
 			const linesToCheck = _.flatten(strippedComments
 				.split(/\r?\n/)
 				.map(line => line.split(";")));
+
 			const linesWithRequireStatements = linesToCheck
 				.filter(line => /\btns-core-modules\b/.exec(line) === null && (/\bimport\b/.exec(line) || /\brequire\b/.exec(line)));
 
 			for (const line of linesWithRequireStatements) {
-				for (const regExp of shortImportRegExps) {
-					const matches = line.match(regExp);
+				const matches = line.match(shortImportRegExp);
 
-					if (matches && matches.length) {
-						shortImports.push({ file, line });
-						break;
-					}
+				if (matches && matches.length) {
+					shortImports.push({ file, line });
 				}
 			}
 		}
@@ -162,14 +160,24 @@ export class DoctorService implements IDoctorService {
 		return shortImports;
 	}
 
-	private getShortImportRegExps(projectDir: string): RegExp[] {
+	private getShortImportRegExp(projectDir: string): RegExp {
 		const pathToTnsCoreModules = path.join(projectDir, NODE_MODULES_FOLDER_NAME, TNS_CORE_MODULES_NAME);
-		const contents = this.$fs.readDirectory(pathToTnsCoreModules)
+		const coreModulesSubDirs = this.$fs.readDirectory(pathToTnsCoreModules)
 			.filter(entry => this.$fs.getFsStats(path.join(pathToTnsCoreModules, entry)).isDirectory());
 
-		const regExps = contents.map(c => new RegExp(`[\"\']${c}[\"\'/]`, "g"));
+		const stringRegularExpressionsPerDir = coreModulesSubDirs.map(c => {
+			// require("text");
+			// require("text/smth");
+			// require(   "text/smth");
+			// require(   "text/smth"   );
+			// import * as text from "text";
+			// import { a } from "text";
+			// import {a } from "text/abc"
+			const subDirPart = `[\"\']${c}[\"\'/]`;
+			return `(\\brequire\\s*?\\(\\s*?${subDirPart})|(\\bimport\\b.*?from\\s*?${subDirPart})`;
+		});
 
-		return regExps;
+		return new RegExp(stringRegularExpressionsPerDir.join("|"), "g");
 	}
 
 	private async runSetupScriptCore(executablePath: string, setupScriptArgs: string[]): Promise<ISpawnResult> {
diff --git a/test/services/doctor-service.ts b/test/services/doctor-service.ts
index 0074c806cd..aeb29fba4c 100644
--- a/test/services/doctor-service.ts
+++ b/test/services/doctor-service.ts
@@ -41,7 +41,8 @@ describe("doctorService", () => {
 	describe("checkForDeprecatedShortImportsInAppDir", () => {
 		const tnsCoreModulesDirs = [
 			"application",
-			"data"
+			"data",
+			"text"
 		];
 
 		const testData: { filesContents: IStringDictionary, expectedShortImports: any[] }[] = [
@@ -179,15 +180,54 @@ const Observable = require("tns-core-modules-widgets/data/observable").Observabl
 				},
 				expectedShortImports: []
 			},
+			{
+				filesContents: {
+					// minified code that has both require and some of the tns-core-modules subdirs (i.e. text)
+					file1: `o.cache&&(r+="&cache="+u(o.cache)),t=["<!DOCTYPE html>","<html>","<head>",'<meta charset="UTF-8" />','<script type="text/javascript">',"   var UWA = {hosts:"+n(e.hosts)+"},",'       curl = {apiName: "require"};`
+				},
+				expectedShortImports: []
+			},
+			{
+				filesContents: {
+					// spaces around require
+					file1: 'const application   = require     (  "application"   ); console.log("application");',
+				},
+				expectedShortImports: [
+					{ file: "file1", line: 'const application   = require     (  "application"   )' }
+				]
+			},
+			{
+				filesContents: {
+					// spaces around require
+					file1: 'const application   = require     (  "tns-core-modules/application"   ); console.log("application");',
+				},
+				expectedShortImports: []
+			},
+			{
+				filesContents: {
+					// spaces in import line
+					file1: "import     { run }       from    'application'       ;",
+				},
+				expectedShortImports: [
+					{ file: "file1", line: "import     { run }       from    'application'       " }
+				]
+			},
+			{
+				filesContents: {
+					// spaces in import line
+					file1: "import     { run }       from    'tns-core-modules/application'       ;",
+				},
+				expectedShortImports: []
+			},
 			{
 				// Incorrect behavior, currently by design
 				// In case you have a multiline string and one of the lines matches our RegExp we'll detect it as short import
 				filesContents: {
-					file1: 'const _ = require("lodash");const application = require("application");console.log("application");console.log(`this is line\nyou should import some long words here "application" module and other words here`)',
+					file1: 'const _ = require("lodash");const application = require("application");console.log("application");console.log(`this is line\nyou should import some long words here require("application") module and other words here`)',
 				},
 				expectedShortImports: [
 					{ file: "file1", line: 'const application = require("application")' },
-					{ file: "file1", line: 'you should import some long words here "application" module and other words here`)' },
+					{ file: "file1", line: 'you should import some long words here require("application") module and other words here`)' },
 				]
 			},
 		];