From 090be1324ccedd5ca6b703dcb822402902f3eff7 Mon Sep 17 00:00:00 2001 From: Kevin Lau Date: Thu, 18 Jul 2019 22:33:41 -0700 Subject: [PATCH 1/3] Fix `import-name` options indexing off-by-one The options begin at index 0. The previous developer might have gotten it mixed up with initial `true` in the config array which controls `ruleSeverity`, not `ruleArguments`. --- package-lock.json | 41 +++++++++----------------------- src/importNameRule.ts | 6 ++--- src/tests/ImportNameRuleTests.ts | 12 ++-------- 3 files changed, 16 insertions(+), 43 deletions(-) diff --git a/package-lock.json b/package-lock.json index 107a30b1a..fd4963af3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1299,8 +1299,7 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "aproba": { "version": "1.2.0", @@ -1321,14 +1320,12 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -1343,20 +1340,17 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -1473,8 +1467,7 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -1486,7 +1479,6 @@ "version": "1.0.0", "bundled": true, "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -1501,7 +1493,6 @@ "version": "3.0.4", "bundled": true, "dev": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -1509,14 +1500,12 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "minipass": { "version": "2.2.4", "bundled": true, "dev": true, - "optional": true, "requires": { "safe-buffer": "^5.1.1", "yallist": "^3.0.0" @@ -1535,7 +1524,6 @@ "version": "0.5.1", "bundled": true, "dev": true, - "optional": true, "requires": { "minimist": "0.0.8" } @@ -1616,8 +1604,7 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -1629,7 +1616,6 @@ "version": "1.4.0", "bundled": true, "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -1715,8 +1701,7 @@ "safe-buffer": { "version": "5.1.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "safer-buffer": { "version": "2.1.2", @@ -1752,7 +1737,6 @@ "version": "1.0.2", "bundled": true, "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -1772,7 +1756,6 @@ "version": "3.0.1", "bundled": true, "dev": true, - "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -1816,14 +1799,12 @@ "wrappy": { "version": "1.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "yallist": { "version": "3.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true } } }, diff --git a/src/importNameRule.ts b/src/importNameRule.ts index f37bfc342..604a02aee 100644 --- a/src/importNameRule.ts +++ b/src/importNameRule.ts @@ -45,15 +45,15 @@ export class Rule extends Lint.Rules.AbstractRule { if (options.ruleArguments instanceof Array) { options.ruleArguments.forEach((opt: unknown, index: number) => { - if (index === 1 && isObject(opt)) { + if (index === 0 && isObject(opt)) { result.replacements = this.extractReplacements(opt); } - if (index === 2 && Array.isArray(opt)) { + if (index === 1 && Array.isArray(opt)) { result.ignoredList = this.extractIgnoredList(opt); } - if (index === 3 && isObject(opt)) { + if (index === 2 && isObject(opt)) { result.config = this.extractConfig(opt); } }); diff --git a/src/tests/ImportNameRuleTests.ts b/src/tests/ImportNameRuleTests.ts index 632107ff7..f0f488280 100644 --- a/src/tests/ImportNameRuleTests.ts +++ b/src/tests/ImportNameRuleTests.ts @@ -114,7 +114,6 @@ describe('importNameRule', (): void => { `; const options = [ - true, { backbone: 'Backbone', react: 'React', @@ -132,7 +131,6 @@ describe('importNameRule', (): void => { import pqr from 'my-module' `; const options = [ - true, { 'fs/package-name': 'pkg', 'abc-tag': 'abc', @@ -151,7 +149,6 @@ describe('importNameRule', (): void => { import Up from 'up-module' `; const options = [ - true, { 'fs/package-name': 'pkg', 'abc-tag': 'abc', @@ -171,7 +168,6 @@ describe('importNameRule', (): void => { import Up from 'up-module' `; const options = [ - true, { 'fs/package-name': 'pkg', 'abc-tag': 'abc', @@ -251,7 +247,6 @@ describe('importNameRule', (): void => { `; const options = [ - true, {}, {}, { @@ -268,7 +263,6 @@ describe('importNameRule', (): void => { `; const options = [ - true, {}, {}, { @@ -301,7 +295,6 @@ describe('importNameRule', (): void => { `; const options = [ - true, {}, {}, { @@ -318,7 +311,6 @@ describe('importNameRule', (): void => { `; const options = [ - true, {}, {}, { @@ -351,7 +343,7 @@ describe('importNameRule', (): void => { import GraphqlTag from 'x/y/z/graphql-tag'; import graphqlTag from 'x/y/z/graphql-tag'; `; - const options = [true, {}, {}, { case: 'any-case' }]; + const options = [{}, {}, { case: 'any-case' }]; TestHelper.assertViolationsWithOptions(ruleName, options, script, []); }); @@ -359,7 +351,7 @@ describe('importNameRule', (): void => { const script: string = ` import gTag from 'x/y/z/graphql-tag'; `; - const options = [true, {}, {}, { case: 'any-case' }]; + const options = [{}, {}, { case: 'any-case' }]; TestHelper.assertViolationsWithOptions(ruleName, options, script, [ { failure: "Misnamed import. Import should be named 'graphqlTag' or 'GraphqlTag' but found 'gTag'", From b26592ee930c51f60f1c9da26868d83aa4191b70 Mon Sep 17 00:00:00 2001 From: Kevin Lau Date: Thu, 18 Jul 2019 22:33:43 -0700 Subject: [PATCH 2/3] Update `import-name` README.md Add documentation and examples of all the `import-name` options. --- README.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6c0909eb8..4570010b1 100644 --- a/README.md +++ b/README.md @@ -184,11 +184,16 @@ We recommend you specify exact versions of lint libraries, including `tslint-mic The name of the imported module must match the name of the thing being imported. For special characters (-, ., _) remove them and make the following character uppercase. - For example, it is valid to name imported modules the same as the module name: import Service = require('x/y/z/Service') and import Service from 'x/y/z/Service'. + For example, it is valid to name imported modules the same as the module name: import Service = require('./x/y/z/Service') and import Service from './x/y/z/Service'. But it is invalid to change the name being imported, such as: import MyCoolService = require('x/y/z/Service') and import MyCoolService from 'x/y/z/Service'. When containing special characters such as import $$ from 'my-awesome_library'; the corresponding configuration would look like 'import-name': [true, { 'myAwesomeLibrary': '$$' }]. Since version 2.0.9 it is possible to configure this rule with a list of exceptions. - For example, to allow underscore to be imported as _, add this configuration: 'import-name': [ true, { 'underscore': '_' }] + The default is to ignore this rule for external modules ignoreExternalModule: true and to use camelCase case: 'camelCase'. + To not ignore this rule for external modules, add this configuration: 'import-name': [true, null, null, { ignoreExternalModule: false }]. + To allow underscore to be imported as _, add this configuration: 'import-name': [true, { 'underscore': '_' }, null, { ignoreExternalModule: false }]. + To ignore this rule for react and express, add this configuration: 'import-name': [true, null, ['react', 'express'], { ignoreExternalModule: false }]. + To use PascalCase: 'import-name': [true, null, null, { case: 'PascalCase' }]. + To use camelCase or PascalCase: 'import-name': [true, null, null, { case: 'any-case' }]. 2.0.5 From 5e298324cb17c96f218222f4a1b9f4888a418f57 Mon Sep 17 00:00:00 2001 From: Kevin Lau Date: Thu, 18 Jul 2019 22:33:45 -0700 Subject: [PATCH 3/3] Make `import-name` camelCase begin with lowercase `camelCase` currently allows the first letter to be uppercase by enforcing that it matches the casing of the file name first letter. Enforce the first letter to be lowercase. --- src/importNameRule.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/importNameRule.ts b/src/importNameRule.ts index 604a02aee..01bb4fc37 100644 --- a/src/importNameRule.ts +++ b/src/importNameRule.ts @@ -233,12 +233,14 @@ function walk(ctx: Lint.WalkContext