Merge pull request #4434 from NativeScript/vladimirov/fix-short-imports

fix: warnings for short imports should be shown correctly

Author: Fatme

lib/services/doctor-service.ts

@@ -141,8 +141,10 @@ export class DoctorService implements IDoctorService {
 		for (const file of files) {
 			const fileContent = this.$fs.readText(file);
 			const strippedComments = helpers.stripComments(fileContent);
-			const linesWithRequireStatements = strippedComments
+			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) {

lib/services/project-data-service.ts

@@ -1,7 +1,15 @@
 import * as path from "path";
 import { ProjectData } from "../project-data";
 import { exported } from "../common/decorators";
-import { NATIVESCRIPT_PROPS_INTERNAL_DELIMITER, AssetConstants, SRC_DIR, RESOURCES_DIR, MAIN_DIR, CLI_RESOURCES_DIR_NAME, ProjectTypes } from "../constants";
+import {
+	NATIVESCRIPT_PROPS_INTERNAL_DELIMITER,
+	AssetConstants, SRC_DIR,
+	RESOURCES_DIR,
+	MAIN_DIR,
+	CLI_RESOURCES_DIR_NAME,
+	ProjectTypes,
+	NODE_MODULES_FOLDER_NAME
+} from "../constants";
 
 interface IProjectFileData {
 	projectData: any;
@@ -124,6 +132,7 @@ export class ProjectDataService implements IProjectDataService {
 			supportedFileExtension = ".ts";
 		}
 
+		const pathToProjectNodeModules = path.join(projectDir, NODE_MODULES_FOLDER_NAME);
 		const files = this.$fs.enumerateFilesInDirectorySync(
 			projectData.appDirectoryPath,
 			(filePath, fstat) => {
@@ -132,6 +141,12 @@ export class ProjectDataService implements IProjectDataService {
 				}
 
 				if (fstat.isDirectory()) {
+					if (filePath === pathToProjectNodeModules) {
+						// we do not want to get the files from node_modules directory of the project.
+						// We'll get here only when you have nsconfig.json with appDirectoryPath set to "."
+						return false;
+					}
+
 					return true;
 				}
 

package.json

@@ -1,7 +1,7 @@
 {
   "name": "nativescript",
   "preferGlobal": true,
-  "version": "5.2.3",
+  "version": "5.2.4",
   "author": "Telerik <support@telerik.com>",
   "description": "Command-line interface for building NativeScript projects",
   "bin": {

test/services/doctor-service.ts

@@ -49,7 +49,7 @@ describe("doctorService", () => {
 				filesContents: {
 					file1: 'const application = require("application");'
 				},
-				expectedShortImports: [{ file: "file1", line: 'const application = require("application");' }]
+				expectedShortImports: [{ file: "file1", line: 'const application = require("application")' }]
 			},
 			{
 				filesContents: {
@@ -61,7 +61,7 @@ describe("doctorService", () => {
 				filesContents: {
 					file1: 'const Observable = require("data/observable").Observable;'
 				},
-				expectedShortImports: [{ file: "file1", line: 'const Observable = require("data/observable").Observable;' }]
+				expectedShortImports: [{ file: "file1", line: 'const Observable = require("data/observable").Observable' }]
 			},
 			{
 				filesContents: {
@@ -73,7 +73,7 @@ describe("doctorService", () => {
 				filesContents: {
 					file1: 'import * as application from "application";'
 				},
-				expectedShortImports: [{ file: "file1", line: 'import * as application from "application";' }]
+				expectedShortImports: [{ file: "file1", line: 'import * as application from "application"' }]
 			},
 			{
 				filesContents: {
@@ -85,7 +85,7 @@ describe("doctorService", () => {
 				filesContents: {
 					file1: 'import { run } from "application";'
 				},
-				expectedShortImports: [{ file: "file1", line: 'import { run } from "application";' }]
+				expectedShortImports: [{ file: "file1", line: 'import { run } from "application"' }]
 			},
 			{
 				filesContents: {
@@ -98,7 +98,7 @@ describe("doctorService", () => {
 				filesContents: {
 					file1: "import { run } from 'application';"
 				},
-				expectedShortImports: [{ file: "file1", line: "import { run } from 'application';" }]
+				expectedShortImports: [{ file: "file1", line: "import { run } from 'application'" }]
 			},
 			{
 				// Using single quotes
@@ -114,8 +114,8 @@ const Observable = require("data/observable").Observable;
 `
 				},
 				expectedShortImports: [
-					{ file: "file1", line: 'const application = require("application");' },
-					{ file: "file1", line: 'const Observable = require("data/observable").Observable;' },
+					{ file: "file1", line: 'const application = require("application")' },
+					{ file: "file1", line: 'const Observable = require("data/observable").Observable' },
 				]
 			},
 			{
@@ -125,7 +125,7 @@ const Observable = require("tns-core-modules/data/observable").Observable;
 `
 				},
 				expectedShortImports: [
-					{ file: "file1", line: 'const application = require("application");' },
+					{ file: "file1", line: 'const application = require("application")' },
 				]
 			},
 			{
@@ -137,8 +137,8 @@ const Observable = require("tns-core-modules/data/observable").Observable;
 const Observable = require("data/observable").Observable;`
 				},
 				expectedShortImports: [
-					{ file: "file1", line: 'const application = require("application");' },
-					{ file: "file2", line: 'const Observable = require("data/observable").Observable;'  },
+					{ file: "file1", line: 'const application = require("application")' },
+					{ file: "file2", line: 'const Observable = require("data/observable").Observable' },
 				]
 			},
 			{
@@ -150,7 +150,45 @@ const Observable = require("tns-core-modules/data/observable").Observable;
 					file2: `const application = require("some-name-tns-core-modules-widgets/application");
 const Observable = require("tns-core-modules-widgets/data/observable").Observable;`
 				},
-				expectedShortImports: [ ]
+				expectedShortImports: []
+			},
+			{
+				filesContents: {
+					// several statements on one line
+					file1: 'const _ = require("lodash");console.log("application");'
+				},
+				expectedShortImports: []
+			},
+			{
+				filesContents: {
+					// several statements on one line with actual short imports
+					file1: 'const _ = require("lodash");const application = require("application");console.log("application");',
+					file2: 'const _ = require("lodash");const application = require("application");const Observable = require("data/observable").Observable;'
+				},
+				expectedShortImports: [
+					{ file: "file1", line: 'const application = require("application")' },
+					{ file: "file2", line: 'const application = require("application")' },
+					{ file: "file2", line: 'const Observable = require("data/observable").Observable' },
+				]
+			},
+			{
+				filesContents: {
+					// several statements on one line withoutshort imports
+					file1: 'const _ = require("lodash");const application = require("tns-core-modules/application");console.log("application");',
+					file2: 'const _ = require("lodash");const application = require("tns-core-modules/application");const Observable = require("tns-core-modules/data/observable").Observable;'
+				},
+				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`)',
+				},
+				expectedShortImports: [
+					{ file: "file1", line: 'const application = require("application")' },
+					{ file: "file1", line: 'you should import some long words here "application" module and other words here`)' },
+				]
 			},
 		];