From e0766055dca762d2511e59ea73ea74f1f0d47973 Mon Sep 17 00:00:00 2001 From: Taeheon Kim Date: Wed, 24 Oct 2018 11:29:41 +0900 Subject: [PATCH] Add feature to import-name rule ignoring node_modules and etc ( #541 ) (#569) * Make more arguments can be passed to options by using index flag and make more state for the later * Extract checking replacements logic to function for more checking for the later * Extract ignoredList from third argument * Make "isImportNameValid()" return true when ignoredList has moduleName that is currently in a validation process Add "es7" to "lib" at tsconfig.json for using Array.includes() with string[] type * Extra config from fourth argument * Ignore if isExternalLibraryImport flag is true from runtime node object * Add Tests that can be tested with "grunt all" - Need Help I want to write a test about ignoring node_moduels feature, but "node" object is only given at runtime. I don't know how to write a test of it. * for.. of doesn't support Map at es5 target. It just changes to for loop and silently doesn't go into for loop. I changed it to forEach. Now It's working. * 1. Remove "es7" from tsconfig 2. Without es7 support, Array.includes doesnt work. I changed it using filter function. (This is the reason why I added es7 support) I think Array.includes() is not that 'absolutely needed' I just removed it. * comment added? * comment for interfaces * Add optionExamples * Change optionsExamples --> naming of previous one can be confusing. --- src/importNameRule.ts | 142 +++++++++++++++++++++++++++---- src/tests/ImportNameRuleTests.ts | 57 ++++++++++++- tsconfig.json | 2 +- 3 files changed, 183 insertions(+), 18 deletions(-) diff --git a/src/importNameRule.ts b/src/importNameRule.ts index b52314577..f295a6524 100644 --- a/src/importNameRule.ts +++ b/src/importNameRule.ts @@ -15,6 +15,12 @@ export class Rule extends Lint.Rules.AbstractRule { hasFix: true, options: null, // tslint:disable-line:no-null-keyword optionsDescription: '', + optionExamples: [ + [true], + [true, { moduleName: 'importedName' }], + [true, { moduleName: 'importedName' }, ['moduleName1', 'moduleName2']], + [true, { moduleName: 'importedName' }, ['moduleName1', 'moduleName2'], { ignoreExternalModule: false }] + ], typescriptOnly: true, issueClass: 'Ignored', issueType: 'Warning', @@ -29,27 +35,90 @@ export class Rule extends Lint.Rules.AbstractRule { } } +type Replacement = { [index: string]: string; }; +type IgnoredList = string[]; +type ConfigKey = 'ignoreExternalModule'; +type Config = { [index in ConfigKey]: unknown; }; + +// This is for temporarily reolving type errors. Actual runtime Node, SourceFile object +// has those properties. +interface RuntimeSourceFile extends ts.SourceFile { + resolvedModules: Map; +} +interface RuntimeNode extends ts.Node { + parent: RuntimeSourceFile; +} + +type Option = { + replacements: Replacement + ignoredList: IgnoredList + config: Config +}; + class ImportNameRuleWalker extends ErrorTolerantWalker { - private replacements: { [index: string]: string; }; + private option: Option; constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { super(sourceFile, options); - this.replacements = this.extractOptions(); + this.option = this.extractOptions(); } - private extractOptions(): { [index: string]: string; } { - const result : { [index: string]: string; } = {}; - this.getOptions().forEach((opt: unknown) => { - if (isObject(opt)) { - Object.keys(opt).forEach((key: string): void => { - const value: unknown = opt[key]; - if (typeof value === 'string') { - result[key] = value; - } - }); + private extractOptions(): Option { + const result : Option = { + replacements: {}, + ignoredList: [], + config: { + ignoreExternalModule: true + } + }; + + this.getOptions().forEach((opt: unknown, index: number) => { + if (index === 1 && isObject(opt)) { + result.replacements = this.extractReplacements(opt); + } + + if (index === 2 && Array.isArray(opt)) { + result.ignoredList = this.extractIgnoredList(opt); + } + + if (index === 3 && isObject(opt)) { + result.config = this.extractConfig(opt); } }); + + return result; + } + + private extractReplacements(opt: Replacement | unknown): Replacement { + const result: Replacement = {}; + if (isObject(opt)) { + Object.keys(opt).forEach((key: string): void => { + const value: unknown = opt[key]; + if (typeof value === 'string') { + result[key] = value; + } + }); + } + return result; + } + + private extractIgnoredList(opt: IgnoredList): IgnoredList { + return opt.filter((moduleName: string) => typeof moduleName === 'string'); + } + + private extractConfig(opt: unknown): Config { + const result: Config = { ignoreExternalModule: true }; + const configKeyLlist: ConfigKey[] = ['ignoreExternalModule']; + if (isObject(opt)) { + return Object.keys(opt).reduce((accum: Config, key: string) => { + if (configKeyLlist.filter((configKey: string) => configKey === key).length >= 1) { + accum[key] = opt[key]; + return accum; + } + return accum; + }, { ignoreExternalModule: true }); + } return result; } @@ -84,7 +153,7 @@ class ImportNameRuleWalker extends ErrorTolerantWalker { private validateImport(node: ts.ImportEqualsDeclaration | ts.ImportDeclaration, importedName: string, moduleName: string): void { let expectedImportedName = moduleName.replace(/.*\//, ''); // chop off the path expectedImportedName = this.makeCamelCase(expectedImportedName); - if (this.isImportNameValid(importedName, expectedImportedName, moduleName) === false) { + if (this.isImportNameValid(importedName, expectedImportedName, moduleName, node) === false) { const message: string = `Misnamed import. Import should be named '${expectedImportedName}' but found '${importedName}'`; const nameNode = node.kind === ts.SyntaxKind.ImportEqualsDeclaration ? (node).name @@ -102,24 +171,65 @@ class ImportNameRuleWalker extends ErrorTolerantWalker { }); } - private isImportNameValid(importedName: string, expectedImportedName: string, moduleName: string): boolean { + private isImportNameValid(importedName: string, expectedImportedName: string, moduleName: string, + node: ts.ImportEqualsDeclaration | ts.ImportDeclaration): boolean { if (expectedImportedName === importedName) { return true; } + const isReplacementsExist = this.checkReplacementsExist(importedName, expectedImportedName, moduleName, this.option.replacements); + if (isReplacementsExist) { + return true; + } + + const isIgnoredModuleExist = this.checkIgnoredListExists(moduleName, this.option.ignoredList); + if (isIgnoredModuleExist) { + return true; + } + + const ignoreThisExternalModule = this.checkIgnoreExternalModule(moduleName, node, this.option.config); + if (ignoreThisExternalModule) { + return true; + } + + return false; + } + + private checkReplacementsExist(importedName: string, expectedImportedName: string, moduleName: string, replacements: Replacement) + : boolean { // Allowed Replacement keys are specifiers that are allowed when overriding or adding exceptions // to import-name rule. // Example: for below import statement // `import cgi from 'fs-util/cgi-common'` // The Valid specifiers are: [cgiCommon, fs-util/cgi-common, cgi-common] const allowedReplacementKeys: string[] = [expectedImportedName, moduleName, moduleName.replace(/.*\//, '')]; - return Utils.exists(Object.keys(this.replacements), (replacementKey: string): boolean => { + return Utils.exists(Object.keys(replacements), (replacementKey: string): boolean => { for (let index = 0; allowedReplacementKeys.length > index; index = index + 1) { if (replacementKey === allowedReplacementKeys[index]) { - return importedName === this.replacements[replacementKey]; + return importedName === replacements[replacementKey]; } } return false; }); } + + // Ignore array of strings that comes from third argument. + private checkIgnoredListExists(moduleName: string, ignoredList: IgnoredList): boolean { + return ignoredList.filter((ignoredModule: string) => ignoredModule === moduleName).length >= 1; + } + + // Ignore NPM installed modules by checking its module path at runtime + private checkIgnoreExternalModule(moduleName: string, node: unknown, opt: Config): boolean { + const runtimeNode: RuntimeNode = node; + if (opt.ignoreExternalModule === true && runtimeNode.parent !== undefined && runtimeNode.parent.resolvedModules !== undefined) { + let ignoreThisExternalModule = false; + runtimeNode.parent.resolvedModules.forEach((value: ts.ResolvedModuleFull, key: string) => { + if (key === moduleName && value.isExternalLibraryImport === true) { + ignoreThisExternalModule = true; + } + }); + return ignoreThisExternalModule; + } + return false; + } } diff --git a/src/tests/ImportNameRuleTests.ts b/src/tests/ImportNameRuleTests.ts index c50383485..c294fbeb5 100644 --- a/src/tests/ImportNameRuleTests.ts +++ b/src/tests/ImportNameRuleTests.ts @@ -164,5 +164,60 @@ describe('importNameRule', () : void => { 'myModule': 'pqr' }]; TestHelper.assertViolationsWithOptions(ruleName, options, script, []); - }) + }); + + it('should pass on ignoring modules from ignoredList(string[] from third argument)', () : void => { + const script : string = ` + import pkg from 'fs/package-name', + import abc from 'abc-tag', + import pqr from 'my-module' + import what from 'what-module' + import Up from 'up-module' + `; + const options: any[] = [ true, { + 'fs/package-name': 'pkg', + 'abc-tag': 'abc', + 'myModule': 'pqr' + }, ['what-module', 'up-module']]; + TestHelper.assertViolationsWithOptions(ruleName, options, script, []); + }); + + it('should pass on ignoring third argument value other than string[]', () : void => { + const script : string = ` + import pkg from 'fs/package-name', + import abc from 'abc-tag', + import pqr from 'my-module' + import what from 'what-module' + import Up from 'up-module' + `; + const options: any[] = [ true, { + 'fs/package-name': 'pkg', + 'abc-tag': 'abc', + 'myModule': 'pqr' + }, [123, { 'whatever': 'object' }]]; + TestHelper.assertViolationsWithOptions(ruleName, options, script, [ + { + "failure": "Misnamed import. Import should be named 'whatModule' but found 'what'", + "name": Utils.absolutePath("file.ts"), + "ruleName": "import-name", + "startPosition": { "character": 9, "line": 5 }, + "fix": { + "innerStart": 130, + "innerLength": 4, + "innerText": "whatModule" + } + }, + { + "failure": "Misnamed import. Import should be named 'upModule' but found 'Up'", + "name": Utils.absolutePath("file.ts"), + "ruleName": "import-name", + "startPosition": { "character": 9, "line": 6 }, + "fix": { + "innerStart": 169, + "innerLength": 2, + "innerText": "upModule" + } + } + ]); + }); }); diff --git a/tsconfig.json b/tsconfig.json index dcbac87e4..5b9d58327 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -28,4 +28,4 @@ "include": [ "src" ] -} +} \ No newline at end of file