From 44766ae61d9dba3e3acb6434a0202a18f0db62a5 Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Thu, 9 Mar 2017 23:00:16 -0500 Subject: [PATCH 01/17] Added structured-imports rule :sparkles: --- src/customRules/structuredImportsRule.ts | 101 ++++++++++++++++++ .../ancestorCases-fix.ts.lint | 6 ++ .../structured-imports/ancestorCases.ts.lint | 11 ++ .../structured-imports/baseCase-fix.ts.lint | 2 + .../rules/structured-imports/baseCase.ts.lint | 3 + .../structured-imports/multiLine-fix.ts.lint | 6 ++ .../structured-imports/multiLine.ts.lint | 10 ++ test/rules/structured-imports/tslint.json | 5 + 8 files changed, 144 insertions(+) create mode 100644 src/customRules/structuredImportsRule.ts create mode 100644 test/rules/structured-imports/ancestorCases-fix.ts.lint create mode 100644 test/rules/structured-imports/ancestorCases.ts.lint create mode 100644 test/rules/structured-imports/baseCase-fix.ts.lint create mode 100644 test/rules/structured-imports/baseCase.ts.lint create mode 100644 test/rules/structured-imports/multiLine-fix.ts.lint create mode 100644 test/rules/structured-imports/multiLine.ts.lint create mode 100644 test/rules/structured-imports/tslint.json diff --git a/src/customRules/structuredImportsRule.ts b/src/customRules/structuredImportsRule.ts new file mode 100644 index 0000000..4a90b34 --- /dev/null +++ b/src/customRules/structuredImportsRule.ts @@ -0,0 +1,101 @@ +import * as ts from 'typescript'; +import * as Lint from 'tslint'; + +import * as path from 'path'; + +export class Rule extends Lint.Rules.AbstractRule { + /* tslint:disable:object-literal-sort-keys */ + public static metadata: Lint.IRuleMetadata = { + 'ruleName': 'structured-imports', + 'description': 'Enforce a structure to your imports. Absolute imports listed first, then relative.', + 'descriptionDetails': Lint.Utils.dedent``, + 'hasFix': true, + 'optionsDescription': Lint.Utils.dedent``, + 'options': null, + 'optionExamples': null, + 'type': 'style', + 'typescriptOnly': false, + }; + /* tslint:enable:object-literal-sort-keys */ + + public static STRUCTURED_IMPORTS_ABS_FIRST_ERROR = 'Import structure should be listed absolute imports first, then relative imports.'; + + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + const structuredImportsWalker = new StructuredImportsWalker(sourceFile, this.getOptions()); + return this.applyWithWalker(structuredImportsWalker); + } +} + +enum ImportType { + Absolute = 0, + RelativeAncestor = 1, // '../' + RelativeCurrent = 2, // './' +}; + +const importStuctureOrder = [ImportType.Absolute, ImportType.RelativeAncestor, ImportType.RelativeCurrent]; + +class StructuredImportsWalker extends Lint.RuleWalker { + private previousImport: ImportType; + private currentImport: ImportType; + + constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { + super(sourceFile, options); + } + + // e.g. 'import Foo from './foo';' + public visitImportDeclaration(node: ts.ImportDeclaration) { + const importPath = node.moduleSpecifier.getText(); + if (!this.previousImport) { + this.previousImport = getImportType(importPath); + this.currentImport = getImportType(importPath); + } else { + this.currentImport = getImportType(importPath); + if (!isCurrentImportValid(this.previousImport, this.currentImport)) { + this.addFailure(this.createFailure(node.moduleSpecifier.getStart(), node.moduleSpecifier.getFullWidth(), Rule.STRUCTURED_IMPORTS_ABS_FIRST_ERROR)); + } + } + if (isCurrentImportValid(this.previousImport, this.currentImport)) { + this.previousImport = this.currentImport; + } + } +} + + +function isCurrentImportValid(prevImport: ImportType, currImport: ImportType): boolean { + + if (prevImport === currImport) { + return true; + } + + if (prevImport === ImportType.Absolute && currImport === getNextOrderedImport(prevImport)) { + return true; + } + if (prevImport === ImportType.RelativeAncestor && currImport === getNextOrderedImport(prevImport)) { + return true; + } + +} + +function getNextOrderedImport(importType: ImportType) { + if (importType < importStuctureOrder.length - 1) { + return importStuctureOrder[importType + 1]; + } +} + + +function getImportType(path: string): ImportType { + if (isRelativeCurrent(path)) { + return ImportType.RelativeCurrent; + } else if (isRelativeAncestor(path)) { + return ImportType.RelativeAncestor; + } + return ImportType.Absolute; +} + +function isRelativeCurrent(path: string) { + return path.substr(1, 2) === './' ; +} + +function isRelativeAncestor(path: string) { + return path.substr(1, 4) === '../' ; +} diff --git a/test/rules/structured-imports/ancestorCases-fix.ts.lint b/test/rules/structured-imports/ancestorCases-fix.ts.lint new file mode 100644 index 0000000..c90f780 --- /dev/null +++ b/test/rules/structured-imports/ancestorCases-fix.ts.lint @@ -0,0 +1,6 @@ +import * as SomethingAbsolute from 'something-absolute'; +import {absolute1, absolute2} from 'library'; +import AnotherAbsolute from 'another-absolute'; +import { anotherRelativeImport } from "../ancestorImport"; +import * as relative from "./relativeImport1"; +import { relativeImport } from "./relativeImport2"; diff --git a/test/rules/structured-imports/ancestorCases.ts.lint b/test/rules/structured-imports/ancestorCases.ts.lint new file mode 100644 index 0000000..c967ab4 --- /dev/null +++ b/test/rules/structured-imports/ancestorCases.ts.lint @@ -0,0 +1,11 @@ +import * as relative from "./relativeImport1"; +import { relativeImport } from "./relativeImport2"; +import { anotherRelativeImport } from "../ancestorImport"; + ~~~~~~~~~~~~~~~~~~~~ [Import structure should be listed absolute imports first, then relative imports.] + +import AnotherAbsolute from 'another-absolute'; + ~~~~~~~~~~~~~~~~~~~ [Import structure should be listed absolute imports first, then relative imports.] +import * as SomethingAbsolute from 'something-absolute'; + ~~~~~~~~~~~~~~~~~~~~~ [Import structure should be listed absolute imports first, then relative imports.] +import {absolute1, absolute2} from 'library'; + ~~~~~~~~~~ [Import structure should be listed absolute imports first, then relative imports.] diff --git a/test/rules/structured-imports/baseCase-fix.ts.lint b/test/rules/structured-imports/baseCase-fix.ts.lint new file mode 100644 index 0000000..d8d33b5 --- /dev/null +++ b/test/rules/structured-imports/baseCase-fix.ts.lint @@ -0,0 +1,2 @@ +import BarModule from 'BarModule'; +import Foo from './foo'; diff --git a/test/rules/structured-imports/baseCase.ts.lint b/test/rules/structured-imports/baseCase.ts.lint new file mode 100644 index 0000000..af44fe4 --- /dev/null +++ b/test/rules/structured-imports/baseCase.ts.lint @@ -0,0 +1,3 @@ +import Foo from './foo'; +import BarModule from 'BarModule'; + ~~~~~~~~~~~~ [Import structure should be listed absolute imports first, then relative imports.] diff --git a/test/rules/structured-imports/multiLine-fix.ts.lint b/test/rules/structured-imports/multiLine-fix.ts.lint new file mode 100644 index 0000000..64962ed --- /dev/null +++ b/test/rules/structured-imports/multiLine-fix.ts.lint @@ -0,0 +1,6 @@ +import {Foo, Bar, Baz} from 'FooBarBaz'; +import BarModule from 'BarModule'; +import Foo from './foo'; +import Bar from './Bar'; +import Foo2 from './foo2'; +import Bar2 from './Bar2'; diff --git a/test/rules/structured-imports/multiLine.ts.lint b/test/rules/structured-imports/multiLine.ts.lint new file mode 100644 index 0000000..76c8de9 --- /dev/null +++ b/test/rules/structured-imports/multiLine.ts.lint @@ -0,0 +1,10 @@ +import Foo from './foo'; +import Bar from './Bar'; + +import {Foo, Bar, Baz} from 'FooBarBaz'; + ~~~~~~~~~~~~ [Import structure should be listed absolute imports first, then relative imports.] +import BarModule from 'BarModule'; + ~~~~~~~~~~~~ [Import structure should be listed absolute imports first, then relative imports.] + +import Foo2 from './foo2'; +import Bar2 from './Bar2'; diff --git a/test/rules/structured-imports/tslint.json b/test/rules/structured-imports/tslint.json new file mode 100644 index 0000000..659166b --- /dev/null +++ b/test/rules/structured-imports/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "structured-imports": true + } +} From 8553145ae4a94c617afc88a94ea485066d2c46ee Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Thu, 9 Mar 2017 23:32:08 -0500 Subject: [PATCH 02/17] Clean up and tweaks --- src/customRules/structuredImportsRule.ts | 50 ++++++++----------- .../structured-imports/ancestorCases.ts.lint | 8 +-- .../rules/structured-imports/baseCase.ts.lint | 2 +- .../structured-imports/multiLine.ts.lint | 4 +- 4 files changed, 29 insertions(+), 35 deletions(-) diff --git a/src/customRules/structuredImportsRule.ts b/src/customRules/structuredImportsRule.ts index 4a90b34..2dc717b 100644 --- a/src/customRules/structuredImportsRule.ts +++ b/src/customRules/structuredImportsRule.ts @@ -1,16 +1,13 @@ import * as ts from 'typescript'; import * as Lint from 'tslint'; -import * as path from 'path'; - export class Rule extends Lint.Rules.AbstractRule { /* tslint:disable:object-literal-sort-keys */ public static metadata: Lint.IRuleMetadata = { 'ruleName': 'structured-imports', - 'description': 'Enforce a structure to your imports. Absolute imports listed first, then relative.', - 'descriptionDetails': Lint.Utils.dedent``, - 'hasFix': true, - 'optionsDescription': Lint.Utils.dedent``, + 'description': 'Enforce structure to your imports. Import structure should be listed in the folloing order: modules, absolute imports, relative parent/ancestor directories, relative sibling directors.', + 'hasFix': false, + 'optionsDescription': 'Not configurable.', 'options': null, 'optionExamples': null, 'type': 'style', @@ -18,7 +15,7 @@ export class Rule extends Lint.Rules.AbstractRule { }; /* tslint:enable:object-literal-sort-keys */ - public static STRUCTURED_IMPORTS_ABS_FIRST_ERROR = 'Import structure should be listed absolute imports first, then relative imports.'; + public static STRUCTURED_IMPORTS_ABS_FIRST_ERROR = 'Import structure should be listed as absolute imports first, then relative imports.'; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { const structuredImportsWalker = new StructuredImportsWalker(sourceFile, this.getOptions()); @@ -27,12 +24,13 @@ export class Rule extends Lint.Rules.AbstractRule { } enum ImportType { - Absolute = 0, - RelativeAncestor = 1, // '../' - RelativeCurrent = 2, // './' + Module = 0, // 'module' + Absolute = 1, // '/some/folder/file' + RelativeAncestor = 2, // '../parentFolder' + RelativeSibling = 3, // './siblingFolder' }; -const importStuctureOrder = [ImportType.Absolute, ImportType.RelativeAncestor, ImportType.RelativeCurrent]; +const importStuctureOrder = [ImportType.Module, ImportType.Absolute, ImportType.RelativeAncestor, ImportType.RelativeSibling]; class StructuredImportsWalker extends Lint.RuleWalker { private previousImport: ImportType; @@ -60,20 +58,11 @@ class StructuredImportsWalker extends Lint.RuleWalker { } } - function isCurrentImportValid(prevImport: ImportType, currImport: ImportType): boolean { - if (prevImport === currImport) { return true; } - - if (prevImport === ImportType.Absolute && currImport === getNextOrderedImport(prevImport)) { - return true; - } - if (prevImport === ImportType.RelativeAncestor && currImport === getNextOrderedImport(prevImport)) { - return true; - } - + return getNextOrderedImport(prevImport) === currImport; } function getNextOrderedImport(importType: ImportType) { @@ -82,20 +71,25 @@ function getNextOrderedImport(importType: ImportType) { } } - function getImportType(path: string): ImportType { - if (isRelativeCurrent(path)) { - return ImportType.RelativeCurrent; + if (isRelativeSibling(path)) { + return ImportType.RelativeSibling; } else if (isRelativeAncestor(path)) { return ImportType.RelativeAncestor; + } else if (isAbsolute(path)) { + return ImportType.Absolute; } - return ImportType.Absolute; + return ImportType.Module; } -function isRelativeCurrent(path: string) { - return path.substr(1, 2) === './' ; +function isAbsolute(path: string) { + return path[1] === '/' ; // [0] is quote mark } function isRelativeAncestor(path: string) { - return path.substr(1, 4) === '../' ; + return path.substr(1, 4) === '../' ; // path[0] is quote mark +} + +function isRelativeSibling(path: string) { + return path.substr(1, 2) === './' ; // path[0] is quote mark } diff --git a/test/rules/structured-imports/ancestorCases.ts.lint b/test/rules/structured-imports/ancestorCases.ts.lint index c967ab4..fdd69a0 100644 --- a/test/rules/structured-imports/ancestorCases.ts.lint +++ b/test/rules/structured-imports/ancestorCases.ts.lint @@ -1,11 +1,11 @@ import * as relative from "./relativeImport1"; import { relativeImport } from "./relativeImport2"; import { anotherRelativeImport } from "../ancestorImport"; - ~~~~~~~~~~~~~~~~~~~~ [Import structure should be listed absolute imports first, then relative imports.] + ~~~~~~~~~~~~~~~~~~~~ [Import structure should be listed as absolute imports first, then relative imports.] import AnotherAbsolute from 'another-absolute'; - ~~~~~~~~~~~~~~~~~~~ [Import structure should be listed absolute imports first, then relative imports.] + ~~~~~~~~~~~~~~~~~~~ [Import structure should be listed as absolute imports first, then relative imports.] import * as SomethingAbsolute from 'something-absolute'; - ~~~~~~~~~~~~~~~~~~~~~ [Import structure should be listed absolute imports first, then relative imports.] + ~~~~~~~~~~~~~~~~~~~~~ [Import structure should be listed as absolute imports first, then relative imports.] import {absolute1, absolute2} from 'library'; - ~~~~~~~~~~ [Import structure should be listed absolute imports first, then relative imports.] + ~~~~~~~~~~ [Import structure should be listed as absolute imports first, then relative imports.] diff --git a/test/rules/structured-imports/baseCase.ts.lint b/test/rules/structured-imports/baseCase.ts.lint index af44fe4..2463992 100644 --- a/test/rules/structured-imports/baseCase.ts.lint +++ b/test/rules/structured-imports/baseCase.ts.lint @@ -1,3 +1,3 @@ import Foo from './foo'; import BarModule from 'BarModule'; - ~~~~~~~~~~~~ [Import structure should be listed absolute imports first, then relative imports.] + ~~~~~~~~~~~~ [Import structure should be listed as absolute imports first, then relative imports.] diff --git a/test/rules/structured-imports/multiLine.ts.lint b/test/rules/structured-imports/multiLine.ts.lint index 76c8de9..9edba7f 100644 --- a/test/rules/structured-imports/multiLine.ts.lint +++ b/test/rules/structured-imports/multiLine.ts.lint @@ -2,9 +2,9 @@ import Foo from './foo'; import Bar from './Bar'; import {Foo, Bar, Baz} from 'FooBarBaz'; - ~~~~~~~~~~~~ [Import structure should be listed absolute imports first, then relative imports.] + ~~~~~~~~~~~~ [Import structure should be listed as absolute imports first, then relative imports.] import BarModule from 'BarModule'; - ~~~~~~~~~~~~ [Import structure should be listed absolute imports first, then relative imports.] + ~~~~~~~~~~~~ [Import structure should be listed as absolute imports first, then relative imports.] import Foo2 from './foo2'; import Bar2 from './Bar2'; From 1923877e9a2d96e1662052cf3659fa0b163765ac Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Fri, 10 Mar 2017 08:59:37 -0500 Subject: [PATCH 03/17] Add rule docs :memo: --- docs/rules/structured-imports.md | 33 ++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 docs/rules/structured-imports.md diff --git a/docs/rules/structured-imports.md b/docs/rules/structured-imports.md new file mode 100644 index 0000000..be686e3 --- /dev/null +++ b/docs/rules/structured-imports.md @@ -0,0 +1,33 @@ +# Enforces a certain structure in your import statements + +## Rationale +- Consistency in import statement structure throughout the codebase + +## Rule Details +Enforce a certain structure in your imports. Import structure should be listed in the following order: + +* Module imports +* Absolute imports +* Relative parent/ancestor directory imports +* Relative sibling directory imports + +The following are considered warnings +```js +import Foo from './foo'; +import Baz = '../baz'; +import BarModule from 'BarModule'; +``` + +The following patterns are not warnings: + +```js +export interface TheThreeStooges { +import BarModule from 'BarModule'; +import Baz = '../baz'; +import Foo from './foo'; +} +``` + +## When Not To Use It + +If you do not with to enforce consistency in your import structuring. From 0033fc8a495755da36da52c1820b904a718e455b Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Fri, 10 Mar 2017 09:12:20 -0500 Subject: [PATCH 04/17] Change error messages --- src/customRules/structuredImportsRule.ts | 9 ++++----- test/rules/structured-imports/ancestorCases.ts.lint | 8 ++++---- test/rules/structured-imports/baseCase.ts.lint | 2 +- test/rules/structured-imports/multiLine.ts.lint | 4 ++-- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/customRules/structuredImportsRule.ts b/src/customRules/structuredImportsRule.ts index 2dc717b..02e436e 100644 --- a/src/customRules/structuredImportsRule.ts +++ b/src/customRules/structuredImportsRule.ts @@ -15,7 +15,7 @@ export class Rule extends Lint.Rules.AbstractRule { }; /* tslint:enable:object-literal-sort-keys */ - public static STRUCTURED_IMPORTS_ABS_FIRST_ERROR = 'Import structure should be listed as absolute imports first, then relative imports.'; + public static STRUCTURED_IMPORTS_ABS_FIRST_ERROR = 'Imports should be listed in the following order: module imports, absolute imports, relative imports.'; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { const structuredImportsWalker = new StructuredImportsWalker(sourceFile, this.getOptions()); @@ -48,13 +48,12 @@ class StructuredImportsWalker extends Lint.RuleWalker { this.currentImport = getImportType(importPath); } else { this.currentImport = getImportType(importPath); - if (!isCurrentImportValid(this.previousImport, this.currentImport)) { + if (isCurrentImportValid(this.previousImport, this.currentImport)) { + this.previousImport = this.currentImport; + } else { this.addFailure(this.createFailure(node.moduleSpecifier.getStart(), node.moduleSpecifier.getFullWidth(), Rule.STRUCTURED_IMPORTS_ABS_FIRST_ERROR)); } } - if (isCurrentImportValid(this.previousImport, this.currentImport)) { - this.previousImport = this.currentImport; - } } } diff --git a/test/rules/structured-imports/ancestorCases.ts.lint b/test/rules/structured-imports/ancestorCases.ts.lint index fdd69a0..c71e9a6 100644 --- a/test/rules/structured-imports/ancestorCases.ts.lint +++ b/test/rules/structured-imports/ancestorCases.ts.lint @@ -1,11 +1,11 @@ import * as relative from "./relativeImport1"; import { relativeImport } from "./relativeImport2"; import { anotherRelativeImport } from "../ancestorImport"; - ~~~~~~~~~~~~~~~~~~~~ [Import structure should be listed as absolute imports first, then relative imports.] + ~~~~~~~~~~~~~~~~~~~~ [Imports should be listed in the following order: module imports, absolute imports, relative imports.] import AnotherAbsolute from 'another-absolute'; - ~~~~~~~~~~~~~~~~~~~ [Import structure should be listed as absolute imports first, then relative imports.] + ~~~~~~~~~~~~~~~~~~~ [Imports should be listed in the following order: module imports, absolute imports, relative imports.] import * as SomethingAbsolute from 'something-absolute'; - ~~~~~~~~~~~~~~~~~~~~~ [Import structure should be listed as absolute imports first, then relative imports.] + ~~~~~~~~~~~~~~~~~~~~~ [Imports should be listed in the following order: module imports, absolute imports, relative imports.] import {absolute1, absolute2} from 'library'; - ~~~~~~~~~~ [Import structure should be listed as absolute imports first, then relative imports.] + ~~~~~~~~~~ [Imports should be listed in the following order: module imports, absolute imports, relative imports.] diff --git a/test/rules/structured-imports/baseCase.ts.lint b/test/rules/structured-imports/baseCase.ts.lint index 2463992..ff213e4 100644 --- a/test/rules/structured-imports/baseCase.ts.lint +++ b/test/rules/structured-imports/baseCase.ts.lint @@ -1,3 +1,3 @@ import Foo from './foo'; import BarModule from 'BarModule'; - ~~~~~~~~~~~~ [Import structure should be listed as absolute imports first, then relative imports.] + ~~~~~~~~~~~~ [Imports should be listed in the following order: module imports, absolute imports, relative imports.] diff --git a/test/rules/structured-imports/multiLine.ts.lint b/test/rules/structured-imports/multiLine.ts.lint index 9edba7f..99c2c20 100644 --- a/test/rules/structured-imports/multiLine.ts.lint +++ b/test/rules/structured-imports/multiLine.ts.lint @@ -2,9 +2,9 @@ import Foo from './foo'; import Bar from './Bar'; import {Foo, Bar, Baz} from 'FooBarBaz'; - ~~~~~~~~~~~~ [Import structure should be listed as absolute imports first, then relative imports.] + ~~~~~~~~~~~~ [Imports should be listed in the following order: module imports, absolute imports, relative imports.] import BarModule from 'BarModule'; - ~~~~~~~~~~~~ [Import structure should be listed as absolute imports first, then relative imports.] + ~~~~~~~~~~~~ [Imports should be listed in the following order: module imports, absolute imports, relative imports.] import Foo2 from './foo2'; import Bar2 from './Bar2'; From cac50df00b768e74f72e1d6d8ae81688fa76ed10 Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Fri, 10 Mar 2017 09:26:01 -0500 Subject: [PATCH 05/17] Fix tlint error :hammer: --- src/customRules/structuredImportsRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/customRules/structuredImportsRule.ts b/src/customRules/structuredImportsRule.ts index 02e436e..4b39ca6 100644 --- a/src/customRules/structuredImportsRule.ts +++ b/src/customRules/structuredImportsRule.ts @@ -66,7 +66,7 @@ function isCurrentImportValid(prevImport: ImportType, currImport: ImportType): b function getNextOrderedImport(importType: ImportType) { if (importType < importStuctureOrder.length - 1) { - return importStuctureOrder[importType + 1]; + return importStuctureOrder[(importType as number) + 1]; } } From bf31490df5682d7edac74f52d338deef2b853b37 Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Fri, 10 Mar 2017 09:28:43 -0500 Subject: [PATCH 06/17] Enable custom rule --- src/customRules.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/customRules.ts b/src/customRules.ts index e4a2d7e..952ac0d 100644 --- a/src/customRules.ts +++ b/src/customRules.ts @@ -2,5 +2,6 @@ module.exports = { 'rulesDirectory': ['./customRules'], 'rules': { 'trailing-comma-interface': true, + 'structured-imports': true, }, }; From 5e0a6ec5f98810a635b84be32fd127b96c4ad74b Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Sun, 12 Mar 2017 23:25:23 -0400 Subject: [PATCH 07/17] PR fixes :hammer: --- docs/rules/import-type-order.md | 33 +++++++++++++++++++ docs/rules/structured-imports.md | 33 ------------------- src/customRules.ts | 2 +- ...dImportsRule.ts => importTypeOrderRule.ts} | 20 +++++------ test/rules/structured-imports/tslint.json | 2 +- 5 files changed, 45 insertions(+), 45 deletions(-) create mode 100644 docs/rules/import-type-order.md delete mode 100644 docs/rules/structured-imports.md rename src/customRules/{structuredImportsRule.ts => importTypeOrderRule.ts} (82%) diff --git a/docs/rules/import-type-order.md b/docs/rules/import-type-order.md new file mode 100644 index 0000000..4a3c25b --- /dev/null +++ b/docs/rules/import-type-order.md @@ -0,0 +1,33 @@ +# Enforces order of import statement types. + +## Rationale +- Improves readability and organization by grouping naturally related items together. + +## Rule Details +Enforce order of import statement types. Import structure should be listed in the following order: + +* Externals +* Absolute paths +* Parent directories +* Siblings + +The following are considered warnings +```js +import Foo from './foo'; +import Baz = '../baz'; +import BarModule from 'BarModule'; +``` + +The following import order is valid: + +```js +export interface TheThreeStooges { +import BarModule from 'BarModule'; +import Baz = '../baz'; +import Foo from './foo'; +} +``` + +## When Not To Use It + +If you do not wish to enforce consistency in your import statements. diff --git a/docs/rules/structured-imports.md b/docs/rules/structured-imports.md deleted file mode 100644 index be686e3..0000000 --- a/docs/rules/structured-imports.md +++ /dev/null @@ -1,33 +0,0 @@ -# Enforces a certain structure in your import statements - -## Rationale -- Consistency in import statement structure throughout the codebase - -## Rule Details -Enforce a certain structure in your imports. Import structure should be listed in the following order: - -* Module imports -* Absolute imports -* Relative parent/ancestor directory imports -* Relative sibling directory imports - -The following are considered warnings -```js -import Foo from './foo'; -import Baz = '../baz'; -import BarModule from 'BarModule'; -``` - -The following patterns are not warnings: - -```js -export interface TheThreeStooges { -import BarModule from 'BarModule'; -import Baz = '../baz'; -import Foo from './foo'; -} -``` - -## When Not To Use It - -If you do not with to enforce consistency in your import structuring. diff --git a/src/customRules.ts b/src/customRules.ts index 952ac0d..732eded 100644 --- a/src/customRules.ts +++ b/src/customRules.ts @@ -2,6 +2,6 @@ module.exports = { 'rulesDirectory': ['./customRules'], 'rules': { 'trailing-comma-interface': true, - 'structured-imports': true, + 'import-type-order': true, }, }; diff --git a/src/customRules/structuredImportsRule.ts b/src/customRules/importTypeOrderRule.ts similarity index 82% rename from src/customRules/structuredImportsRule.ts rename to src/customRules/importTypeOrderRule.ts index 4b39ca6..23df515 100644 --- a/src/customRules/structuredImportsRule.ts +++ b/src/customRules/importTypeOrderRule.ts @@ -4,14 +4,14 @@ import * as Lint from 'tslint'; export class Rule extends Lint.Rules.AbstractRule { /* tslint:disable:object-literal-sort-keys */ public static metadata: Lint.IRuleMetadata = { - 'ruleName': 'structured-imports', - 'description': 'Enforce structure to your imports. Import structure should be listed in the folloing order: modules, absolute imports, relative parent/ancestor directories, relative sibling directors.', + 'ruleName': 'import-type-order', + 'description': 'Enforce structure to your imports. Import structure should be listed in the following order: modules, absolute imports, relative parent/ancestor directories, relative sibling directors.', 'hasFix': false, 'optionsDescription': 'Not configurable.', 'options': null, 'optionExamples': null, 'type': 'style', - 'typescriptOnly': false, + 'typescriptOnly': true, }; /* tslint:enable:object-literal-sort-keys */ @@ -24,13 +24,13 @@ export class Rule extends Lint.Rules.AbstractRule { } enum ImportType { - Module = 0, // 'module' + External = 0, // 'external' Absolute = 1, // '/some/folder/file' - RelativeAncestor = 2, // '../parentFolder' - RelativeSibling = 3, // './siblingFolder' + Ancestor = 2, // '../parentFolder' + Sibling = 3, // './siblingFolder' }; -const importStuctureOrder = [ImportType.Module, ImportType.Absolute, ImportType.RelativeAncestor, ImportType.RelativeSibling]; +const importStuctureOrder = [ImportType.External, ImportType.Absolute, ImportType.Ancestor, ImportType.Sibling]; class StructuredImportsWalker extends Lint.RuleWalker { private previousImport: ImportType; @@ -72,13 +72,13 @@ function getNextOrderedImport(importType: ImportType) { function getImportType(path: string): ImportType { if (isRelativeSibling(path)) { - return ImportType.RelativeSibling; + return ImportType.Sibling; } else if (isRelativeAncestor(path)) { - return ImportType.RelativeAncestor; + return ImportType.Ancestor; } else if (isAbsolute(path)) { return ImportType.Absolute; } - return ImportType.Module; + return ImportType.External; } function isAbsolute(path: string) { diff --git a/test/rules/structured-imports/tslint.json b/test/rules/structured-imports/tslint.json index 659166b..35bcc71 100644 --- a/test/rules/structured-imports/tslint.json +++ b/test/rules/structured-imports/tslint.json @@ -1,5 +1,5 @@ { "rules": { - "structured-imports": true + "import-type-order": true } } From 16aa5f68514e5fbdd2c527aebe039656629a0efa Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Mon, 13 Mar 2017 12:03:20 -0400 Subject: [PATCH 08/17] Clean up getImportType() --- src/customRules/importTypeOrderRule.ts | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/src/customRules/importTypeOrderRule.ts b/src/customRules/importTypeOrderRule.ts index 23df515..23148ad 100644 --- a/src/customRules/importTypeOrderRule.ts +++ b/src/customRules/importTypeOrderRule.ts @@ -71,24 +71,12 @@ function getNextOrderedImport(importType: ImportType) { } function getImportType(path: string): ImportType { - if (isRelativeSibling(path)) { + if (path.substr(1, 2) === './') { return ImportType.Sibling; - } else if (isRelativeAncestor(path)) { + } else if (path.substr(1, 4) === '../') { return ImportType.Ancestor; - } else if (isAbsolute(path)) { + } else if (path[1] === '/') { return ImportType.Absolute; } return ImportType.External; } - -function isAbsolute(path: string) { - return path[1] === '/' ; // [0] is quote mark -} - -function isRelativeAncestor(path: string) { - return path.substr(1, 4) === '../' ; // path[0] is quote mark -} - -function isRelativeSibling(path: string) { - return path.substr(1, 2) === './' ; // path[0] is quote mark -} From 81f58fd15976f4dff0eeecc42cfed879a25c3969 Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Mon, 13 Mar 2017 12:09:13 -0400 Subject: [PATCH 09/17] Clean error wording and use footnote definition in test cases --- src/customRules/importTypeOrderRule.ts | 2 +- test/rules/structured-imports/ancestorCases.ts.lint | 10 ++++++---- test/rules/structured-imports/baseCase.ts.lint | 4 +++- test/rules/structured-imports/multiLine.ts.lint | 6 ++++-- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/customRules/importTypeOrderRule.ts b/src/customRules/importTypeOrderRule.ts index 23148ad..8478b33 100644 --- a/src/customRules/importTypeOrderRule.ts +++ b/src/customRules/importTypeOrderRule.ts @@ -15,7 +15,7 @@ export class Rule extends Lint.Rules.AbstractRule { }; /* tslint:enable:object-literal-sort-keys */ - public static STRUCTURED_IMPORTS_ABS_FIRST_ERROR = 'Imports should be listed in the following order: module imports, absolute imports, relative imports.'; + public static STRUCTURED_IMPORTS_ABS_FIRST_ERROR = 'Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports.'; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { const structuredImportsWalker = new StructuredImportsWalker(sourceFile, this.getOptions()); diff --git a/test/rules/structured-imports/ancestorCases.ts.lint b/test/rules/structured-imports/ancestorCases.ts.lint index c71e9a6..442ecb6 100644 --- a/test/rules/structured-imports/ancestorCases.ts.lint +++ b/test/rules/structured-imports/ancestorCases.ts.lint @@ -1,11 +1,13 @@ import * as relative from "./relativeImport1"; import { relativeImport } from "./relativeImport2"; import { anotherRelativeImport } from "../ancestorImport"; - ~~~~~~~~~~~~~~~~~~~~ [Imports should be listed in the following order: module imports, absolute imports, relative imports.] + ~~~~~~~~~~~~~~~~~~~~ [0] import AnotherAbsolute from 'another-absolute'; - ~~~~~~~~~~~~~~~~~~~ [Imports should be listed in the following order: module imports, absolute imports, relative imports.] + ~~~~~~~~~~~~~~~~~~~ [0] import * as SomethingAbsolute from 'something-absolute'; - ~~~~~~~~~~~~~~~~~~~~~ [Imports should be listed in the following order: module imports, absolute imports, relative imports.] + ~~~~~~~~~~~~~~~~~~~~~ [0] import {absolute1, absolute2} from 'library'; - ~~~~~~~~~~ [Imports should be listed in the following order: module imports, absolute imports, relative imports.] + ~~~~~~~~~~ [0] + +[0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. diff --git a/test/rules/structured-imports/baseCase.ts.lint b/test/rules/structured-imports/baseCase.ts.lint index ff213e4..de8d90a 100644 --- a/test/rules/structured-imports/baseCase.ts.lint +++ b/test/rules/structured-imports/baseCase.ts.lint @@ -1,3 +1,5 @@ import Foo from './foo'; import BarModule from 'BarModule'; - ~~~~~~~~~~~~ [Imports should be listed in the following order: module imports, absolute imports, relative imports.] + ~~~~~~~~~~~~ [0] + +[0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. diff --git a/test/rules/structured-imports/multiLine.ts.lint b/test/rules/structured-imports/multiLine.ts.lint index 99c2c20..83fd5aa 100644 --- a/test/rules/structured-imports/multiLine.ts.lint +++ b/test/rules/structured-imports/multiLine.ts.lint @@ -2,9 +2,11 @@ import Foo from './foo'; import Bar from './Bar'; import {Foo, Bar, Baz} from 'FooBarBaz'; - ~~~~~~~~~~~~ [Imports should be listed in the following order: module imports, absolute imports, relative imports.] + ~~~~~~~~~~~~ [0] import BarModule from 'BarModule'; - ~~~~~~~~~~~~ [Imports should be listed in the following order: module imports, absolute imports, relative imports.] + ~~~~~~~~~~~~ [0] import Foo2 from './foo2'; import Bar2 from './Bar2'; + +[0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. From a737379bbbdc7c8d4872fbe12ab698f5e0f48977 Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Mon, 13 Mar 2017 20:32:52 -0400 Subject: [PATCH 10/17] Refactor rule for minor performance improvements with AbstractWalker --- src/customRules/importTypeOrderRule.ts | 51 +++++++------------ .../structured-imports/ancestorCases.ts.lint | 8 +-- .../rules/structured-imports/baseCase.ts.lint | 2 +- .../structured-imports/multiLine.ts.lint | 4 +- 4 files changed, 25 insertions(+), 40 deletions(-) diff --git a/src/customRules/importTypeOrderRule.ts b/src/customRules/importTypeOrderRule.ts index 8478b33..e1489b1 100644 --- a/src/customRules/importTypeOrderRule.ts +++ b/src/customRules/importTypeOrderRule.ts @@ -15,10 +15,10 @@ export class Rule extends Lint.Rules.AbstractRule { }; /* tslint:enable:object-literal-sort-keys */ - public static STRUCTURED_IMPORTS_ABS_FIRST_ERROR = 'Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports.'; + public static IMPORT_TYPE_ORDER_ERROR = 'Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports.'; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - const structuredImportsWalker = new StructuredImportsWalker(sourceFile, this.getOptions()); + const structuredImportsWalker = new StructuredImportsWalker(sourceFile, this.ruleName, undefined); return this.applyWithWalker(structuredImportsWalker); } } @@ -32,43 +32,28 @@ enum ImportType { const importStuctureOrder = [ImportType.External, ImportType.Absolute, ImportType.Ancestor, ImportType.Sibling]; -class StructuredImportsWalker extends Lint.RuleWalker { - private previousImport: ImportType; - private currentImport: ImportType; +class StructuredImportsWalker extends Lint.AbstractWalker { + public walk(sourceFile: ts.SourceFile) { + const importNodes = sourceFile.statements + .filter((child) => child.kind === ts.SyntaxKind.ImportDeclaration) + .map((child) => child as ts.ImportDeclaration); - constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { - super(sourceFile, options); - } + if (importNodes.length === 0) { return; } - // e.g. 'import Foo from './foo';' - public visitImportDeclaration(node: ts.ImportDeclaration) { - const importPath = node.moduleSpecifier.getText(); - if (!this.previousImport) { - this.previousImport = getImportType(importPath); - this.currentImport = getImportType(importPath); - } else { - this.currentImport = getImportType(importPath); - if (isCurrentImportValid(this.previousImport, this.currentImport)) { - this.previousImport = this.currentImport; + let previousImport = getImportType(importNodes.shift().moduleSpecifier.getText()); + while (importNodes.length) { + const currentImportNode = importNodes.shift().moduleSpecifier; + const currentImport = getImportType(currentImportNode.getText()); + if (previousImport > currentImport) { + const errorStart = currentImportNode.getStart(); + const errorWidth = currentImportNode.getEnd() - errorStart; + this.addFailureAt(errorStart, errorWidth, Rule.IMPORT_TYPE_ORDER_ERROR); } else { - this.addFailure(this.createFailure(node.moduleSpecifier.getStart(), node.moduleSpecifier.getFullWidth(), Rule.STRUCTURED_IMPORTS_ABS_FIRST_ERROR)); + previousImport = currentImport; } } } -} - -function isCurrentImportValid(prevImport: ImportType, currImport: ImportType): boolean { - if (prevImport === currImport) { - return true; - } - return getNextOrderedImport(prevImport) === currImport; -} - -function getNextOrderedImport(importType: ImportType) { - if (importType < importStuctureOrder.length - 1) { - return importStuctureOrder[(importType as number) + 1]; - } -} +}; function getImportType(path: string): ImportType { if (path.substr(1, 2) === './') { diff --git a/test/rules/structured-imports/ancestorCases.ts.lint b/test/rules/structured-imports/ancestorCases.ts.lint index 442ecb6..9cb2168 100644 --- a/test/rules/structured-imports/ancestorCases.ts.lint +++ b/test/rules/structured-imports/ancestorCases.ts.lint @@ -1,13 +1,13 @@ import * as relative from "./relativeImport1"; import { relativeImport } from "./relativeImport2"; import { anotherRelativeImport } from "../ancestorImport"; - ~~~~~~~~~~~~~~~~~~~~ [0] + ~~~~~~~~~~~~~~~~~~~ [0] import AnotherAbsolute from 'another-absolute'; - ~~~~~~~~~~~~~~~~~~~ [0] + ~~~~~~~~~~~~~~~~~~ [0] import * as SomethingAbsolute from 'something-absolute'; - ~~~~~~~~~~~~~~~~~~~~~ [0] + ~~~~~~~~~~~~~~~~~~~~ [0] import {absolute1, absolute2} from 'library'; - ~~~~~~~~~~ [0] + ~~~~~~~~~ [0] [0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. diff --git a/test/rules/structured-imports/baseCase.ts.lint b/test/rules/structured-imports/baseCase.ts.lint index de8d90a..17d860f 100644 --- a/test/rules/structured-imports/baseCase.ts.lint +++ b/test/rules/structured-imports/baseCase.ts.lint @@ -1,5 +1,5 @@ import Foo from './foo'; import BarModule from 'BarModule'; - ~~~~~~~~~~~~ [0] + ~~~~~~~~~~~ [0] [0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. diff --git a/test/rules/structured-imports/multiLine.ts.lint b/test/rules/structured-imports/multiLine.ts.lint index 83fd5aa..e1af03f 100644 --- a/test/rules/structured-imports/multiLine.ts.lint +++ b/test/rules/structured-imports/multiLine.ts.lint @@ -2,9 +2,9 @@ import Foo from './foo'; import Bar from './Bar'; import {Foo, Bar, Baz} from 'FooBarBaz'; - ~~~~~~~~~~~~ [0] + ~~~~~~~~~~~ [0] import BarModule from 'BarModule'; - ~~~~~~~~~~~~ [0] + ~~~~~~~~~~~ [0] import Foo2 from './foo2'; import Bar2 from './Bar2'; From 46c850efb7e313a82521e5b9ce8fbf52b9329810 Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Mon, 13 Mar 2017 22:12:03 -0400 Subject: [PATCH 11/17] Utilize applyWithFunction() instead of applyWithWalker() --- src/customRules/importTypeOrderRule.ts | 41 +++++++++++++------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/customRules/importTypeOrderRule.ts b/src/customRules/importTypeOrderRule.ts index e1489b1..6fc3b6c 100644 --- a/src/customRules/importTypeOrderRule.ts +++ b/src/customRules/importTypeOrderRule.ts @@ -1,6 +1,7 @@ import * as ts from 'typescript'; import * as Lint from 'tslint'; + export class Rule extends Lint.Rules.AbstractRule { /* tslint:disable:object-literal-sort-keys */ public static metadata: Lint.IRuleMetadata = { @@ -18,8 +19,7 @@ export class Rule extends Lint.Rules.AbstractRule { public static IMPORT_TYPE_ORDER_ERROR = 'Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports.'; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - const structuredImportsWalker = new StructuredImportsWalker(sourceFile, this.ruleName, undefined); - return this.applyWithWalker(structuredImportsWalker); + return this.applyWithFunction(sourceFile, walker); } } @@ -32,28 +32,27 @@ enum ImportType { const importStuctureOrder = [ImportType.External, ImportType.Absolute, ImportType.Ancestor, ImportType.Sibling]; -class StructuredImportsWalker extends Lint.AbstractWalker { - public walk(sourceFile: ts.SourceFile) { - const importNodes = sourceFile.statements - .filter((child) => child.kind === ts.SyntaxKind.ImportDeclaration) - .map((child) => child as ts.ImportDeclaration); +function walker(context: Lint.WalkContext): void { + const { sourceFile } = context; + const importNodes = sourceFile.statements + .filter((child) => child.kind === ts.SyntaxKind.ImportDeclaration) + .map((child) => child as ts.ImportDeclaration); - if (importNodes.length === 0) { return; } + if (importNodes.length === 0) { return; } - let previousImport = getImportType(importNodes.shift().moduleSpecifier.getText()); - while (importNodes.length) { - const currentImportNode = importNodes.shift().moduleSpecifier; - const currentImport = getImportType(currentImportNode.getText()); - if (previousImport > currentImport) { - const errorStart = currentImportNode.getStart(); - const errorWidth = currentImportNode.getEnd() - errorStart; - this.addFailureAt(errorStart, errorWidth, Rule.IMPORT_TYPE_ORDER_ERROR); - } else { - previousImport = currentImport; - } + let previousImport = getImportType(importNodes.shift().moduleSpecifier.getText()); + while (importNodes.length) { + const currentImportNode = importNodes.shift().moduleSpecifier; + const currentImport = getImportType(currentImportNode.getText()); + if (previousImport > currentImport) { + const errorStart = currentImportNode.getStart(); + const errorWidth = currentImportNode.getEnd() - errorStart; + context.addFailureAt(errorStart, errorWidth, Rule.IMPORT_TYPE_ORDER_ERROR); + } else { + previousImport = currentImport; } } -}; +} function getImportType(path: string): ImportType { if (path.substr(1, 2) === './') { @@ -64,4 +63,4 @@ function getImportType(path: string): ImportType { return ImportType.Absolute; } return ImportType.External; -} +}; From e5610a4027c38ea4f4d377b386243786a5a05a21 Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Mon, 13 Mar 2017 23:14:36 -0400 Subject: [PATCH 12/17] Add more speicific test cases --- src/customRules/importTypeOrderRule.ts | 2 +- .../structured-imports/ancestorCases-fix.ts.lint | 6 ------ test/rules/structured-imports/ancestorCases.ts.lint | 13 ------------- test/rules/structured-imports/baseCase-fix.ts.lint | 2 -- .../{multiLine.ts.lint => invalid-order.ts.lint} | 12 +++++------- test/rules/structured-imports/multiLine-fix.ts.lint | 6 ------ .../no-absolute-before-external.ts.lint | 5 +++++ .../no-parent-before-absolute.ts.lint | 5 +++++ ...ase.ts.lint => no-sibling-before-parent.ts.lint} | 4 ++-- test/rules/structured-imports/valid-order.ts.lint | 4 ++++ 10 files changed, 22 insertions(+), 37 deletions(-) delete mode 100644 test/rules/structured-imports/ancestorCases-fix.ts.lint delete mode 100644 test/rules/structured-imports/ancestorCases.ts.lint delete mode 100644 test/rules/structured-imports/baseCase-fix.ts.lint rename test/rules/structured-imports/{multiLine.ts.lint => invalid-order.ts.lint} (54%) delete mode 100644 test/rules/structured-imports/multiLine-fix.ts.lint create mode 100644 test/rules/structured-imports/no-absolute-before-external.ts.lint create mode 100644 test/rules/structured-imports/no-parent-before-absolute.ts.lint rename test/rules/structured-imports/{baseCase.ts.lint => no-sibling-before-parent.ts.lint} (67%) create mode 100644 test/rules/structured-imports/valid-order.ts.lint diff --git a/src/customRules/importTypeOrderRule.ts b/src/customRules/importTypeOrderRule.ts index 6fc3b6c..1fb4aea 100644 --- a/src/customRules/importTypeOrderRule.ts +++ b/src/customRules/importTypeOrderRule.ts @@ -57,7 +57,7 @@ function walker(context: Lint.WalkContext): void { function getImportType(path: string): ImportType { if (path.substr(1, 2) === './') { return ImportType.Sibling; - } else if (path.substr(1, 4) === '../') { + } else if (path.substr(1, 3) === '../') { return ImportType.Ancestor; } else if (path[1] === '/') { return ImportType.Absolute; diff --git a/test/rules/structured-imports/ancestorCases-fix.ts.lint b/test/rules/structured-imports/ancestorCases-fix.ts.lint deleted file mode 100644 index c90f780..0000000 --- a/test/rules/structured-imports/ancestorCases-fix.ts.lint +++ /dev/null @@ -1,6 +0,0 @@ -import * as SomethingAbsolute from 'something-absolute'; -import {absolute1, absolute2} from 'library'; -import AnotherAbsolute from 'another-absolute'; -import { anotherRelativeImport } from "../ancestorImport"; -import * as relative from "./relativeImport1"; -import { relativeImport } from "./relativeImport2"; diff --git a/test/rules/structured-imports/ancestorCases.ts.lint b/test/rules/structured-imports/ancestorCases.ts.lint deleted file mode 100644 index 9cb2168..0000000 --- a/test/rules/structured-imports/ancestorCases.ts.lint +++ /dev/null @@ -1,13 +0,0 @@ -import * as relative from "./relativeImport1"; -import { relativeImport } from "./relativeImport2"; -import { anotherRelativeImport } from "../ancestorImport"; - ~~~~~~~~~~~~~~~~~~~ [0] - -import AnotherAbsolute from 'another-absolute'; - ~~~~~~~~~~~~~~~~~~ [0] -import * as SomethingAbsolute from 'something-absolute'; - ~~~~~~~~~~~~~~~~~~~~ [0] -import {absolute1, absolute2} from 'library'; - ~~~~~~~~~ [0] - -[0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. diff --git a/test/rules/structured-imports/baseCase-fix.ts.lint b/test/rules/structured-imports/baseCase-fix.ts.lint deleted file mode 100644 index d8d33b5..0000000 --- a/test/rules/structured-imports/baseCase-fix.ts.lint +++ /dev/null @@ -1,2 +0,0 @@ -import BarModule from 'BarModule'; -import Foo from './foo'; diff --git a/test/rules/structured-imports/multiLine.ts.lint b/test/rules/structured-imports/invalid-order.ts.lint similarity index 54% rename from test/rules/structured-imports/multiLine.ts.lint rename to test/rules/structured-imports/invalid-order.ts.lint index e1af03f..f792891 100644 --- a/test/rules/structured-imports/multiLine.ts.lint +++ b/test/rules/structured-imports/invalid-order.ts.lint @@ -1,12 +1,10 @@ -import Foo from './foo'; -import Bar from './Bar'; +import BarTwo from './Bar'; +import FooTwo from '../foo'; + ~~~~~~~~ [0] +import BazTwo from '/Baz'; + ~~~~~~ [0] import {Foo, Bar, Baz} from 'FooBarBaz'; ~~~~~~~~~~~ [0] -import BarModule from 'BarModule'; - ~~~~~~~~~~~ [0] - -import Foo2 from './foo2'; -import Bar2 from './Bar2'; [0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. diff --git a/test/rules/structured-imports/multiLine-fix.ts.lint b/test/rules/structured-imports/multiLine-fix.ts.lint deleted file mode 100644 index 64962ed..0000000 --- a/test/rules/structured-imports/multiLine-fix.ts.lint +++ /dev/null @@ -1,6 +0,0 @@ -import {Foo, Bar, Baz} from 'FooBarBaz'; -import BarModule from 'BarModule'; -import Foo from './foo'; -import Bar from './Bar'; -import Foo2 from './foo2'; -import Bar2 from './Bar2'; diff --git a/test/rules/structured-imports/no-absolute-before-external.ts.lint b/test/rules/structured-imports/no-absolute-before-external.ts.lint new file mode 100644 index 0000000..a083ec3 --- /dev/null +++ b/test/rules/structured-imports/no-absolute-before-external.ts.lint @@ -0,0 +1,5 @@ +import Foo from '/foo'; +import {Bar, Baz} from 'BarBaz'; + ~~~~~~~~ [0] + +[0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. diff --git a/test/rules/structured-imports/no-parent-before-absolute.ts.lint b/test/rules/structured-imports/no-parent-before-absolute.ts.lint new file mode 100644 index 0000000..02707ec --- /dev/null +++ b/test/rules/structured-imports/no-parent-before-absolute.ts.lint @@ -0,0 +1,5 @@ +import Foo from '../foo'; +import BarModule from '/bar'; + ~~~~~~ [0] + +[0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. diff --git a/test/rules/structured-imports/baseCase.ts.lint b/test/rules/structured-imports/no-sibling-before-parent.ts.lint similarity index 67% rename from test/rules/structured-imports/baseCase.ts.lint rename to test/rules/structured-imports/no-sibling-before-parent.ts.lint index 17d860f..3737aba 100644 --- a/test/rules/structured-imports/baseCase.ts.lint +++ b/test/rules/structured-imports/no-sibling-before-parent.ts.lint @@ -1,5 +1,5 @@ import Foo from './foo'; -import BarModule from 'BarModule'; - ~~~~~~~~~~~ [0] +import Bar from '../bar'; + ~~~~~~~~ [0] [0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. diff --git a/test/rules/structured-imports/valid-order.ts.lint b/test/rules/structured-imports/valid-order.ts.lint new file mode 100644 index 0000000..8e7beb1 --- /dev/null +++ b/test/rules/structured-imports/valid-order.ts.lint @@ -0,0 +1,4 @@ +import {Foo, Bar, Baz} from 'FooBarBaz'; +import BazTwo from '/Baz'; +import FooTwo from '../foo'; +import BarTwo from './Bar'; From 2c125da3df5eaf3c1c08605a91d7b27751bf5800 Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Mon, 13 Mar 2017 23:30:33 -0400 Subject: [PATCH 13/17] Add test case side-effects imports --- .../invalid-order-side-effects.ts.lint | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 test/rules/structured-imports/invalid-order-side-effects.ts.lint diff --git a/test/rules/structured-imports/invalid-order-side-effects.ts.lint b/test/rules/structured-imports/invalid-order-side-effects.ts.lint new file mode 100644 index 0000000..d27b7fe --- /dev/null +++ b/test/rules/structured-imports/invalid-order-side-effects.ts.lint @@ -0,0 +1,9 @@ +import './Bar'; +import '../foo'; + ~~~~~~~~ [0] +import '/Baz'; + ~~~~~~ [0] +import 'FooBarBaz'; + ~~~~~~~~~~~ [0] + +[0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. From f9c4c4b1a16a96caed85076428e5b24f7f9bf722 Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Tue, 14 Mar 2017 09:57:30 -0400 Subject: [PATCH 14/17] PR Fixes, round 2 :hammer: --- docs/rules/import-type-order.md | 6 +++--- src/customRules/importTypeOrderRule.ts | 18 ++++++++++-------- .../structured-imports/valid-order.ts.lint | 1 + 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/docs/rules/import-type-order.md b/docs/rules/import-type-order.md index 4a3c25b..daad212 100644 --- a/docs/rules/import-type-order.md +++ b/docs/rules/import-type-order.md @@ -4,7 +4,7 @@ - Improves readability and organization by grouping naturally related items together. ## Rule Details -Enforce order of import statement types. Import structure should be listed in the following order: +Enforce order of import statement types. Imports should be listed in the following order: * Externals * Absolute paths @@ -13,8 +13,8 @@ Enforce order of import statement types. Import structure should be listed in th The following are considered warnings ```js -import Foo from './foo'; -import Baz = '../baz'; +import Foo from './foo'; // Should come after external and parent imports. +import Baz = '../baz'; // Should come after external imports import BarModule from 'BarModule'; ``` diff --git a/src/customRules/importTypeOrderRule.ts b/src/customRules/importTypeOrderRule.ts index 1fb4aea..6b3febf 100644 --- a/src/customRules/importTypeOrderRule.ts +++ b/src/customRules/importTypeOrderRule.ts @@ -40,21 +40,23 @@ function walker(context: Lint.WalkContext): void { if (importNodes.length === 0) { return; } - let previousImport = getImportType(importNodes.shift().moduleSpecifier.getText()); + let lastValidType = getImportType(importNodes.shift()); while (importNodes.length) { - const currentImportNode = importNodes.shift().moduleSpecifier; - const currentImport = getImportType(currentImportNode.getText()); - if (previousImport > currentImport) { - const errorStart = currentImportNode.getStart(); - const errorWidth = currentImportNode.getEnd() - errorStart; + const currentNode = importNodes.shift(); + const currentType = getImportType(currentNode); + if (lastValidType > currentType) { + const {moduleSpecifier} = currentNode; + const errorStart = moduleSpecifier.getStart(); + const errorWidth = moduleSpecifier.getEnd() - errorStart; context.addFailureAt(errorStart, errorWidth, Rule.IMPORT_TYPE_ORDER_ERROR); } else { - previousImport = currentImport; + lastValidType = currentType; } } } -function getImportType(path: string): ImportType { +function getImportType({moduleSpecifier}: ts.ImportDeclaration): ImportType { + const path = moduleSpecifier.getText(); if (path.substr(1, 2) === './') { return ImportType.Sibling; } else if (path.substr(1, 3) === '../') { diff --git a/test/rules/structured-imports/valid-order.ts.lint b/test/rules/structured-imports/valid-order.ts.lint index 8e7beb1..a29f00e 100644 --- a/test/rules/structured-imports/valid-order.ts.lint +++ b/test/rules/structured-imports/valid-order.ts.lint @@ -1,4 +1,5 @@ import {Foo, Bar, Baz} from 'FooBarBaz'; import BazTwo from '/Baz'; import FooTwo from '../foo'; +import './side-effect' import BarTwo from './Bar'; From 2f585aaa458e1598a4fade7dfa42d28a65c9073c Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Tue, 14 Mar 2017 09:59:18 -0400 Subject: [PATCH 15/17] Update CHANGELOG.md :memo: --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d927c58..d0de705 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ## [1.5.0] - 2017-03-14 ### Added - Update tslint react to v2.5.0 [#52](https://github.com/Shopify/tslint-config-shopify/pull/52) +- Created a custom rule for import-type-order [#48](https://github.com/Shopify/tslint-config-shopify/pull/48) + ## [1.4.0] - 2017-03-06 ### Added From 97f66690a9bc350ca5d07f0bdca45129ecd998ae Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Tue, 14 Mar 2017 22:06:26 -0400 Subject: [PATCH 16/17] PR fixes, round 3 :hammer: --- docs/rules/import-type-order.md | 9 +-------- src/customRules/importTypeOrderRule.ts | 2 +- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/docs/rules/import-type-order.md b/docs/rules/import-type-order.md index daad212..eb815cd 100644 --- a/docs/rules/import-type-order.md +++ b/docs/rules/import-type-order.md @@ -4,12 +4,7 @@ - Improves readability and organization by grouping naturally related items together. ## Rule Details -Enforce order of import statement types. Imports should be listed in the following order: - -* Externals -* Absolute paths -* Parent directories -* Siblings +Improves readability and organization by grouping related imports together. Imports should be listed in order of: external modules, absolute paths, relative paths, relative siblings. The following are considered warnings ```js @@ -21,11 +16,9 @@ import BarModule from 'BarModule'; The following import order is valid: ```js -export interface TheThreeStooges { import BarModule from 'BarModule'; import Baz = '../baz'; import Foo from './foo'; -} ``` ## When Not To Use It diff --git a/src/customRules/importTypeOrderRule.ts b/src/customRules/importTypeOrderRule.ts index 6b3febf..d9797ad 100644 --- a/src/customRules/importTypeOrderRule.ts +++ b/src/customRules/importTypeOrderRule.ts @@ -16,7 +16,7 @@ export class Rule extends Lint.Rules.AbstractRule { }; /* tslint:enable:object-literal-sort-keys */ - public static IMPORT_TYPE_ORDER_ERROR = 'Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports.'; + public static IMPORT_TYPE_ORDER_ERROR = 'Imports should be listed in the following order: external imports, absolute imports, ancestor imports, sibling imports.'; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { return this.applyWithFunction(sourceFile, walker); From 8bbccdc0d9919c4a5a558b5f7e19134f78347200 Mon Sep 17 00:00:00 2001 From: Ismail Syed Date: Tue, 14 Mar 2017 22:10:43 -0400 Subject: [PATCH 17/17] Fix test cases --- .../rules/structured-imports/invalid-order-side-effects.ts.lint | 2 +- test/rules/structured-imports/invalid-order.ts.lint | 2 +- .../structured-imports/no-absolute-before-external.ts.lint | 2 +- test/rules/structured-imports/no-parent-before-absolute.ts.lint | 2 +- test/rules/structured-imports/no-sibling-before-parent.ts.lint | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/rules/structured-imports/invalid-order-side-effects.ts.lint b/test/rules/structured-imports/invalid-order-side-effects.ts.lint index d27b7fe..a55beef 100644 --- a/test/rules/structured-imports/invalid-order-side-effects.ts.lint +++ b/test/rules/structured-imports/invalid-order-side-effects.ts.lint @@ -6,4 +6,4 @@ import '/Baz'; import 'FooBarBaz'; ~~~~~~~~~~~ [0] -[0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. +[0]: Imports should be listed in the following order: external imports, absolute imports, ancestor imports, sibling imports. diff --git a/test/rules/structured-imports/invalid-order.ts.lint b/test/rules/structured-imports/invalid-order.ts.lint index f792891..dc3a95c 100644 --- a/test/rules/structured-imports/invalid-order.ts.lint +++ b/test/rules/structured-imports/invalid-order.ts.lint @@ -7,4 +7,4 @@ import BazTwo from '/Baz'; import {Foo, Bar, Baz} from 'FooBarBaz'; ~~~~~~~~~~~ [0] -[0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. +[0]: Imports should be listed in the following order: external imports, absolute imports, ancestor imports, sibling imports. diff --git a/test/rules/structured-imports/no-absolute-before-external.ts.lint b/test/rules/structured-imports/no-absolute-before-external.ts.lint index a083ec3..e7dba54 100644 --- a/test/rules/structured-imports/no-absolute-before-external.ts.lint +++ b/test/rules/structured-imports/no-absolute-before-external.ts.lint @@ -2,4 +2,4 @@ import Foo from '/foo'; import {Bar, Baz} from 'BarBaz'; ~~~~~~~~ [0] -[0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. +[0]: Imports should be listed in the following order: external imports, absolute imports, ancestor imports, sibling imports. diff --git a/test/rules/structured-imports/no-parent-before-absolute.ts.lint b/test/rules/structured-imports/no-parent-before-absolute.ts.lint index 02707ec..ca95b72 100644 --- a/test/rules/structured-imports/no-parent-before-absolute.ts.lint +++ b/test/rules/structured-imports/no-parent-before-absolute.ts.lint @@ -2,4 +2,4 @@ import Foo from '../foo'; import BarModule from '/bar'; ~~~~~~ [0] -[0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. +[0]: Imports should be listed in the following order: external imports, absolute imports, ancestor imports, sibling imports. diff --git a/test/rules/structured-imports/no-sibling-before-parent.ts.lint b/test/rules/structured-imports/no-sibling-before-parent.ts.lint index 3737aba..602973b 100644 --- a/test/rules/structured-imports/no-sibling-before-parent.ts.lint +++ b/test/rules/structured-imports/no-sibling-before-parent.ts.lint @@ -2,4 +2,4 @@ import Foo from './foo'; import Bar from '../bar'; ~~~~~~~~ [0] -[0]: Imports should be listed in the following order: module imports, absolute imports, ancestor imports, sibling imports. +[0]: Imports should be listed in the following order: external imports, absolute imports, ancestor imports, sibling imports.