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

#451 : Add logic to support more specifiers for import-name #522

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
26 changes: 17 additions & 9 deletions src/importNameRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,15 @@ class ImportNameRuleWalker extends ErrorTolerantWalker {
}

private validateImport(node: ts.ImportEqualsDeclaration | ts.ImportDeclaration, importedName: string, moduleName: string): void {
moduleName = moduleName.replace(/.*\//, ''); // chop off the path
moduleName = this.makeCamelCase(moduleName);
if (this.isImportNameValid(importedName, moduleName) === false) {
const message: string = `Misnamed import. Import should be named '${moduleName}' but found '${importedName}'`;
let expectedImportedName = moduleName.replace(/.*\//, ''); // chop off the path
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the variable name moduleName to expectedModuleName as it was confusing.
Also the primary reason for new variable was to make sure that we aren't changing the input we get which the code was doing earlier.

expectedImportedName = this.makeCamelCase(expectedImportedName);
if (this.isImportNameValid(importedName, expectedImportedName, moduleName) === 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
: (<ts.ImportDeclaration>node).importClause!.name;
const nameNodeStartPos = nameNode!.getStart();
const fix = new Lint.Replacement(nameNodeStartPos, nameNode!.end - nameNodeStartPos, moduleName);
const fix = new Lint.Replacement(nameNodeStartPos, nameNode!.end - nameNodeStartPos, expectedImportedName);
this.addFailureAt(node.getStart(), node.getWidth(), message, fix);
}
}
Expand All @@ -104,14 +104,22 @@ class ImportNameRuleWalker extends ErrorTolerantWalker {
});
}

private isImportNameValid(importedName: string, moduleName: string): boolean {
if (moduleName === importedName) {
private isImportNameValid(importedName: string, expectedImportedName: string, moduleName: string): boolean {
if (expectedImportedName === importedName) {
return true;
}

// 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 => {
if (new RegExp(replacementKey).test(moduleName)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I replaced this Regex check with exact string match because in scenarios like below, the regex would match for both the import-names

import abc from 'fs/my-package'
import pqr from 'super/my-package'

return importedName === this.replacements[replacementKey];
for (let index = 0; allowedReplacementKeys.length > index; index = index + 1) {
if (replacementKey === allowedReplacementKeys[index]) {
return importedName === this.replacements[replacementKey];
}
}
return false;
});
Expand Down
15 changes: 15 additions & 0 deletions src/tests/ImportNameRuleTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe('importNameRule', () : void => {
const script : string = `
import App from 'App';
import App from 'x/y/z/App';
import graphqlTag from 'graphql-tag'
`;

TestHelper.assertViolations(ruleName, script, [ ]);
Expand Down Expand Up @@ -152,4 +153,18 @@ describe('importNameRule', () : void => {
}];
TestHelper.assertViolationsWithOptions(ruleName, options, script, []);
});

it('should pass on differing names when rule is configured with replacements for ES6', () : void => {
const script : string = `
import pkg from 'fs/package-name',
import abc from 'abc-tag',
import pqr from 'my-module'
`;
const options: any[] = [ true, {
'fs/package-name': 'pkg',
'abc-tag': 'abc',
'myModule': 'pqr'
}];
TestHelper.assertViolationsWithOptions(ruleName, options, script, []);
})
});
2 changes: 1 addition & 1 deletion tslint-warnings.csv
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ CWE 710 - Coding Standards Violation"
no-void-expression,Requires expressions of type `void` to appear in statement position.,TSLINT8CA59S,tslint,Non-SDL,Warning,Important,Opportunity for Excellence,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,,
no-with-statement,Do not use with statements. Assign the item to a new variable instead,TSLINT185D93L,tslint,Non-SDL,Warning,Important,Opportunity for Excellence,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,"398, 710","CWE 398 - Indicator of Poor Code Quality
CWE 710 - Coding Standards Violation"
non-literal-fs-path,Detect calls to fs functions with a non literal filepath,TSLINTJNBF48,tslint,SDL,Error,Critical,Mandatory,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,22,"CWE 22 - undefined"
non-literal-fs-path,Detect calls to fs functions with a non literal filepath,TSLINTJNBF48,tslint,SDL,Error,Critical,Mandatory,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,22,"CWE 22 - Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')"
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
non-literal-require,Detect require includes that are not for string literals,TSLINT1OU5CPO,tslint,SDL,Error,Critical,Mandatory,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,"95,676","CWE 95 - Improper Neutralization of Directives in Dynamically Evaluated Code ('Eval Injection')
CWE 676 - Use of Potentially Dangerous Function"
number-literal-format,"Checks that decimal literals should begin with '0.' instead of just '.', and should not end with a trailing '0'.",TSLINT1KQPG76,tslint,Non-SDL,Warning,Low,Opportunity for Excellence,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,710,"CWE 710 - Coding Standards Violation"
Expand Down