Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Commit

Permalink
Add feature to import-name rule ignoring node_modules and etc ( #541 ) (
Browse files Browse the repository at this point in the history
#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.
  • Loading branch information
lonyele authored and Josh Goldberg committed Oct 24, 2018
1 parent 8d3988a commit e076605
Show file tree
Hide file tree
Showing 3 changed files with 183 additions and 18 deletions.
142 changes: 126 additions & 16 deletions src/importNameRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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<string, ts.ResolvedModuleFull>;
}
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[<ConfigKey>key] = opt[key];
return accum;
}
return accum;
}, { ignoreExternalModule: true });
}
return result;
}

Expand Down Expand Up @@ -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
? (<ts.ImportEqualsDeclaration>node).name
Expand All @@ -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 = <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;
}
}
57 changes: 56 additions & 1 deletion src/tests/ImportNameRuleTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
]);
});
});
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@
"include": [
"src"
]
}
}

0 comments on commit e076605

Please sign in to comment.