Skip to content
This repository has been archived by the owner on Apr 27, 2021. It is now read-only.

Created a custom rule for import-type-order #48

Merged
merged 17 commits into from
Mar 15, 2017
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions docs/rules/import-type-order.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Enforces order of import statement types.

## Rationale
- Improves readability and organization by grouping naturally related items together.

## Rule Details
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
import Foo from './foo'; // Should come after external and parent imports.
import Baz = '../baz'; // Should come after external imports
import BarModule from 'BarModule';
```

The following import order is valid:

```js
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.
1 change: 1 addition & 0 deletions src/customRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@ module.exports = {
'rulesDirectory': ['./customRules'],
'rules': {
'trailing-comma-interface': true,
'import-type-order': true,
},
};
68 changes: 68 additions & 0 deletions src/customRules/importTypeOrderRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
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 = {
'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.',
Copy link
Member

Choose a reason for hiding this comment

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

Improves readability and organization by grouping related imports together. Imports should be listed in order of: external modules, absolute paths, relative paths, relative siblings.

Copy link
Member

Choose a reason for hiding this comment

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

This should've been fixed.

'hasFix': false,
'optionsDescription': 'Not configurable.',
'options': null,
'optionExamples': null,
'type': 'style',
'typescriptOnly': true,
};
/* tslint:enable:object-literal-sort-keys */

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);
}
}

enum ImportType {
External = 0, // 'external'
Absolute = 1, // '/some/folder/file'
Ancestor = 2, // '../parentFolder'
Copy link
Member

Choose a reason for hiding this comment

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

I'm still on the fence about Ancestor, but I can't think of anything better. Internal? Local? No. Bah.

Ohwell, they're not used outside this file, so we can easily change when inspiration strikes 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's annoying because parents and ancestors are both valid imports for this type. I think Ancestor is the best name, given all our options.

Sibling = 3, // './siblingFolder'
};

const importStuctureOrder = [ImportType.External, ImportType.Absolute, ImportType.Ancestor, ImportType.Sibling];

function walker(context: Lint.WalkContext<void>): void {
Copy link
Member

Choose a reason for hiding this comment

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

👌

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; }

let lastValidType = getImportType(importNodes.shift());
while (importNodes.length) {
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 {
lastValidType = currentType;
}
}
}

function getImportType({moduleSpecifier}: ts.ImportDeclaration): ImportType {
const path = moduleSpecifier.getText();
if (path.substr(1, 2) === './') {
return ImportType.Sibling;
} else if (path.substr(1, 3) === '../') {
return ImportType.Ancestor;
} else if (path[1] === '/') {
return ImportType.Absolute;
}
return ImportType.External;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import './Bar';
import '../foo';
~~~~~~~~ [0]
import '/Baz';
~~~~~~ [0]
import 'FooBarBaz';
~~~~~~~~~~~ [0]

[0]: Imports should be listed in the following order: external imports, absolute imports, ancestor imports, sibling imports.
10 changes: 10 additions & 0 deletions test/rules/structured-imports/invalid-order.ts.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

import BarTwo from './Bar';
import FooTwo from '../foo';
~~~~~~~~ [0]
import BazTwo from '/Baz';
~~~~~~ [0]
import {Foo, Bar, Baz} from 'FooBarBaz';
~~~~~~~~~~~ [0]

[0]: Imports should be listed in the following order: external imports, absolute imports, ancestor imports, sibling imports.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import Foo from '/foo';
import {Bar, Baz} from 'BarBaz';
~~~~~~~~ [0]

[0]: Imports should be listed in the following order: external imports, absolute imports, ancestor imports, sibling imports.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import Foo from '../foo';
import BarModule from '/bar';
~~~~~~ [0]

[0]: Imports should be listed in the following order: external imports, absolute imports, ancestor imports, sibling imports.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import Foo from './foo';
import Bar from '../bar';
~~~~~~~~ [0]

[0]: Imports should be listed in the following order: external imports, absolute imports, ancestor imports, sibling imports.
5 changes: 5 additions & 0 deletions test/rules/structured-imports/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"import-type-order": true
}
}
5 changes: 5 additions & 0 deletions test/rules/structured-imports/valid-order.ts.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import {Foo, Bar, Baz} from 'FooBarBaz';
import BazTwo from '/Baz';
import FooTwo from '../foo';
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a side-effecty import somewhere in here, please?

import './side-effect'
import BarTwo from './Bar';