Skip to content

feat: detect and report short imports used in application #4372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions lib/common/declarations.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1168,6 +1168,13 @@ interface IDoctorService {
* @returns {Promise<boolean>} true if the environment is properly configured for local builds
*/
canExecuteLocalBuild(platform?: string, projectDir?: string, runtimeVersion?: string): Promise<boolean>;

/**
* Checks and notifies users for deprecated short imports in their applications.
* @param {string} projectDir Path to the application.
* @returns {void}
*/
checkForDeprecatedShortImportsInAppDir(projectDir: string): void;
}

interface IUtils {
Expand Down
7 changes: 6 additions & 1 deletion lib/common/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ import * as crypto from "crypto";
import * as _ from "lodash";

const Table = require("cli-table");
const STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg;

export function stripComments(content: string): string {
const newContent = content.replace(STRIP_COMMENTS, "");
return newContent;
}

export function doesCurrentNpmCommandMatch(patterns?: RegExp[]): boolean {
const currentNpmCommandArgv = getCurrentNpmCommandArgv();
Expand Down Expand Up @@ -682,7 +688,6 @@ const CONSTRUCTOR_ARGS = /constructor\s*([^\(]*)\(\s*([^\)]*)\)/m;
const FN_NAME_AND_ARGS = /^(?:function)?\s*([^\(]*)\(\s*([^\)]*)\)\s*(=>)?\s*[{_]/m;
const FN_ARG_SPLIT = /,/;
const FN_ARG = /^\s*(_?)(\S+?)\1\s*$/;
const STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg;

export function annotate(fn: any) {
let $inject: any,
Expand Down
36 changes: 36 additions & 0 deletions lib/common/test/unit-tests/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -825,4 +825,40 @@ describe("helpers", () => {
});
});
});

describe("stripComments", () => {
const testData: ITestData[] = [
{
input: `// this is comment,
const test = require("./test");`,
expectedResult: `\nconst test = require("./test");`
},
{
input: `/* this is multiline
comment */
const test = require("./test");`,
expectedResult: `\nconst test = require("./test");`
},
{
input: `/* this is multiline
comment
// with inner one line comment inside it
the multiline comment finishes here
*/
const test = require("./test");`,
expectedResult: `\nconst test = require("./test");`
},

{
input: `const test /*inline comment*/ = require("./test");`,
expectedResult: `const test = require("./test");`
},
];

it("strips comments correctly", () => {
testData.forEach(testCase => {
assertTestData(testCase, helpers.stripComments);
});
});
});
});
6 changes: 6 additions & 0 deletions lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,12 @@ export const NgFlavorName = "Angular";
export const VueFlavorName = "Vue.js";
export const TsFlavorName = "Plain TypeScript";
export const JsFlavorName = "Plain JavaScript";
export class ProjectTypes {
public static NgFlavorName = NgFlavorName;
public static VueFlavorName = VueFlavorName;
public static TsFlavorName = "Pure TypeScript";
public static JsFlavorName = "Pure JavaScript";
}
export const BUILD_OUTPUT_EVENT_NAME = "buildOutput";
export const CONNECTION_ERROR_EVENT_NAME = "connectionError";
export const USER_INTERACTION_NEEDED_EVENT_NAME = "userInteractionNeeded";
Expand Down
13 changes: 10 additions & 3 deletions lib/definitions/project.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,13 @@ interface IProjectDataService {
* @returns {Promise<IAssetGroup>} An object describing the current asset structure for Android.
*/
getAndroidAssetsStructure(opts: IProjectDir): Promise<IAssetGroup>;

/**
* Returns array with paths to all `.js` or `.ts` files in application's app directory.
* @param {string} projectDir Path to application.
* @returns {string[]} Array of paths to `.js` or `.ts` files.
*/
getAppExecutableFiles(projectDir: string): string[];
}

interface IAssetItem {
Expand Down Expand Up @@ -535,9 +542,9 @@ interface ICocoaPodsService {

/**
* Merges pod's xcconfig file into project's xcconfig file
* @param projectData
* @param platformData
* @param opts
* @param projectData
* @param platformData
* @param opts
*/
mergePodXcconfigFile(projectData: IProjectData, platformData: IPlatformData, opts: IRelease): Promise<void>;
}
Expand Down
8 changes: 4 additions & 4 deletions lib/project-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,19 @@ export class ProjectData implements IProjectData {
*/
private static PROJECT_TYPES: IProjectType[] = [
{
type: "Pure JavaScript",
type: constants.ProjectTypes.JsFlavorName,
isDefaultProjectType: true
},
{
type: constants.NgFlavorName,
type: constants.ProjectTypes.NgFlavorName,
requiredDependencies: ["@angular/core", "nativescript-angular"]
},
{
type: constants.VueFlavorName,
type: constants.ProjectTypes.VueFlavorName,
requiredDependencies: ["nativescript-vue"]
},
{
type: "Pure TypeScript",
type: constants.ProjectTypes.TsFlavorName,
requiredDependencies: ["typescript", "nativescript-dev-typescript"]
}
];
Expand Down
63 changes: 60 additions & 3 deletions lib/services/doctor-service.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { EOL } from "os";
import * as path from "path";
import * as helpers from "../common/helpers";
import { TrackActionNames } from "../constants";
import { TrackActionNames, NODE_MODULES_FOLDER_NAME, TNS_CORE_MODULES_NAME } from "../constants";
import { doctor, constants } from "nativescript-doctor";

class DoctorService implements IDoctorService {
export class DoctorService implements IDoctorService {
private static DarwinSetupScriptLocation = path.join(__dirname, "..", "..", "setup", "mac-startup-shell-script.sh");
private static WindowsSetupScriptExecutable = "powershell.exe";
private static WindowsSetupScriptArguments = ["start-process", "-FilePath", "PowerShell.exe", "-NoNewWindow", "-Wait", "-ArgumentList", '"-NoProfile -ExecutionPolicy Bypass -Command iex ((new-object net.webclient).DownloadString(\'https://www.nativescript.org/setup/win\'))"'];
Expand All @@ -14,10 +14,12 @@ class DoctorService implements IDoctorService {
private $logger: ILogger,
private $childProcess: IChildProcess,
private $injector: IInjector,
private $projectDataService: IProjectDataService,
private $fs: IFileSystem,
private $terminalSpinnerService: ITerminalSpinnerService,
private $versionsService: IVersionsService) { }

public async printWarnings(configOptions?: { trackResult: boolean , projectDir?: string, runtimeVersion?: string, options?: IOptions }): Promise<void> {
public async printWarnings(configOptions?: { trackResult: boolean, projectDir?: string, runtimeVersion?: string, options?: IOptions }): Promise<void> {
const infos = await this.$terminalSpinnerService.execute<NativeScriptDoctor.IInfo[]>({
text: `Getting environment information ${EOL}`
}, () => doctor.getInfos({ projectDir: configOptions && configOptions.projectDir, androidRuntimeVersion: configOptions && configOptions.runtimeVersion }));
Expand Down Expand Up @@ -48,6 +50,8 @@ class DoctorService implements IDoctorService {
this.$logger.error("Cannot get the latest versions information from npm. Please try again later.");
}

this.checkForDeprecatedShortImportsInAppDir(configOptions.projectDir);

await this.$injector.resolve<IPlatformEnvironmentRequirements>("platformEnvironmentRequirements").checkEnvironmentRequirements({
platform: null,
projectDir: configOptions && configOptions.projectDir,
Expand Down Expand Up @@ -113,6 +117,59 @@ class DoctorService implements IDoctorService {
return !hasWarnings;
}

public checkForDeprecatedShortImportsInAppDir(projectDir: string): void {
if (projectDir) {
try {
const files = this.$projectDataService.getAppExecutableFiles(projectDir);
const shortImports = this.getDeprecatedShortImportsInFiles(files, projectDir);
if (shortImports.length) {
this.$logger.printMarkdown("Detected short imports in your application. Please note that `short imports are deprecated` since NativeScript 5.2.0. More information can be found in this blogpost https://www.nativescript.org/blog/say-goodbye-to-short-imports-in-nativescript");
shortImports.forEach(shortImport => {
this.$logger.printMarkdown(`In file \`${shortImport.file}\` line \`${shortImport.line}\` is short import. Add \`tns-core-modules/\` in front of the required/imported module.`);
});
}
} catch (err) {
this.$logger.trace(`Unable to validate if project has short imports. Error is`, err);
}
}
}

protected getDeprecatedShortImportsInFiles(files: string[], projectDir: string): { file: string, line: string }[] {
const shortImportRegExps = this.getShortImportRegExps(projectDir);
const shortImports: { file: string, line: string }[] = [];

for (const file of files) {
const fileContent = this.$fs.readText(file);
const strippedComments = helpers.stripComments(fileContent);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fileContentWithoutComments?

const linesWithRequireStatements = strippedComments
.split(/\r?\n/)
.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);

if (matches && matches.length) {
shortImports.push({ file, line });
break;
}
}
}
}

return shortImports;
}

private getShortImportRegExps(projectDir: string): RegExp[] {
const pathToTnsCoreModules = path.join(projectDir, NODE_MODULES_FOLDER_NAME, TNS_CORE_MODULES_NAME);
const contents = this.$fs.readDirectory(pathToTnsCoreModules)
.filter(entry => this.$fs.getFsStats(path.join(pathToTnsCoreModules, entry)).isDirectory());

const regExps = contents.map(c => new RegExp(`[\"\']${c}[\"\'/]`, "g"));

return regExps;
}

private async runSetupScriptCore(executablePath: string, setupScriptArgs: string[]): Promise<ISpawnResult> {
return this.$childProcess.spawnFromEvent(executablePath, setupScriptArgs, "close", { stdio: "inherit" });
}
Expand Down
3 changes: 3 additions & 0 deletions lib/services/platform-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export class PlatformService extends EventEmitter implements IPlatformService {
private $errors: IErrors,
private $fs: IFileSystem,
private $logger: ILogger,
private $doctorService: IDoctorService,
private $packageInstallationManager: IPackageInstallationManager,
private $platformsData: IPlatformsData,
private $projectDataService: IProjectDataService,
Expand Down Expand Up @@ -237,6 +238,8 @@ export class PlatformService extends EventEmitter implements IPlatformService {
await this.cleanDestinationApp(platformInfo);
}

this.$doctorService.checkForDeprecatedShortImportsInAppDir(platformInfo.projectData.projectDir);

await this.preparePlatformCore(
platformInfo.platform,
platformInfo.appFilesUpdaterOptions,
Expand Down
28 changes: 27 additions & 1 deletion lib/services/project-data-service.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
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 } from "../constants";
import { NATIVESCRIPT_PROPS_INTERNAL_DELIMITER, AssetConstants, SRC_DIR, RESOURCES_DIR, MAIN_DIR, CLI_RESOURCES_DIR_NAME, ProjectTypes } from "../constants";

interface IProjectFileData {
projectData: any;
Expand Down Expand Up @@ -116,6 +116,32 @@ export class ProjectDataService implements IProjectDataService {
};
}

public getAppExecutableFiles(projectDir: string): string[] {
const projectData = this.getProjectData(projectDir);

let supportedFileExtension = ".js";
if (projectData.projectType === ProjectTypes.NgFlavorName || projectData.projectType === ProjectTypes.TsFlavorName) {
supportedFileExtension = ".ts";
}

const files = this.$fs.enumerateFilesInDirectorySync(
projectData.appDirectoryPath,
(filePath, fstat) => {
if (filePath.indexOf(projectData.appResourcesDirectoryPath) !== -1) {
return false;
}

if (fstat.isDirectory()) {
return true;
}

return path.extname(filePath) === supportedFileExtension;
}
);

return files;
}

private getImageDefinitions(): IImageDefinitionsStructure {
const pathToImageDefinitions = path.join(__dirname, "..", "..", CLI_RESOURCES_DIR_NAME, AssetConstants.assets, AssetConstants.imageDefinitionsFileName);
const imageDefinitions = this.$fs.readJson(pathToImageDefinitions);
Expand Down
3 changes: 3 additions & 0 deletions test/npm-support.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ function createTestInjector(): IInjector {
extractPackage: async (packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise<void> => undefined
});
testInjector.register("usbLiveSyncService", () => ({}));
testInjector.register("doctorService", {
checkForDeprecatedShortImportsInAppDir: (projectDir: string): void => undefined
});

return testInjector;
}
Expand Down
3 changes: 3 additions & 0 deletions test/platform-commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ function createTestInjector() {
trackOptions: () => Promise.resolve(null)
});
testInjector.register("usbLiveSyncService", ({}));
testInjector.register("doctorService", {
checkForDeprecatedShortImportsInAppDir: (projectDir: string): void => undefined
});

return testInjector;
}
Expand Down
3 changes: 3 additions & 0 deletions test/platform-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ function createTestInjector() {
}
});
testInjector.register("usbLiveSyncService", () => ({}));
testInjector.register("doctorService", {
checkForDeprecatedShortImportsInAppDir: (projectDir: string): void => undefined
});

return testInjector;
}
Expand Down
Loading