Merge pull request #50 from Tibfib/will/type-imports

willhoney7 · web-flow · commit cdd174d3c360 · 2022-10-13T21:55:31.000-06:00
Add support for a `type` group.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -18,7 +18,10 @@ module.exports = {
 			tryExtensions: ['.js', '.json', '.node', '.ts', '.d.ts'],
 		},
 	},
-	rules: {},
+	rules: {
+		'prettier/prettier': 0,
+		'eslint-plugin/prefer-message-ids': 1,
+	},
 	overrides: [
 		{
 			files: ['test/**/*.js'],
diff --git a/docs/rules/order-imports.md b/docs/rules/order-imports.md
@@ -61,13 +61,13 @@ This rule supports the following options:
 
 Groups dictates how the imports should be grouped and it what order. `groups` is an array. Each value in the array must be a valid string or an array of valid strings. The valid strings are:
 
--   `'module'` | `'absolute'` | `'parent'` | `'sibling'` | `'index'`
+-   `'module'` | `'absolute'` | `'parent'` | `'sibling'` | `'index'` | `'type'`
 -   or a regular expression like string, ex: `/^shared/`
     -   the wrapping `/` is essential
     -   in this example, it would match any import paths starting with `'shared'`
     -   note: files are first categorized as matching a regular expression group before going into another group
 
-The enforced order is the same as the order of each element in a group. Omitted types are implicitly grouped together as the last element. Example:
+The enforced order is the same as the order of each element in a group. Omitted groups are implicitly grouped together as the last element. Example:
 
 ```js
 [
@@ -88,6 +88,36 @@ You can set the options like this:
 ]
 ```
 
+#### The `type` group
+
+TypeScript has what are called type imports, e.g.,
+
+```ts
+import type { ImportantType } from './thing';
+```
+
+If you would like to treat these type imports as a completely separate group (instead of sorted according to the file it was imported from), add a `type` group to your `groups` list.
+
+With the `type` group:
+
+```ts
+/* eslint import-helpers/order-imports: ["error", {"groups": ['sibling', 'module', 'type']}] */
+import foo from './foo';
+import fs from 'fs';
+import path from 'path';
+import type { ImportantType } from './sibling';
+```
+
+Without the `type` group:
+
+```ts
+/* eslint import-helpers/order-imports: ["error", {"groups": ['sibling', 'module']}] */
+import foo from './foo';
+import type { ImportantType } from './sibling';
+import fs from 'fs';
+import path from 'path';
+```
+
 ### `newlinesBetween: [ignore|always|always-and-inside-groups|never]`:
 
 Enforces or forbids new lines between import groups:
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
 	"name": "eslint-plugin-import-helpers",
-	"version": "1.2.1",
+	"version": "1.3.0",
 	"description": "ESLint Rules to Aid with Imports",
 	"main": "lib/index.js",
 	"scripts": {
@@ -9,7 +9,7 @@
 		"lint:js": "eslint --cache .",
 		"prepublish": "npm run build",
 		"test": "npm run build && npm run test-quick",
-		"test-quick": "NODE_PATH=./lib nyc -s mocha -R dot --recursive test -t test-results"
+		"test-quick": "cross-env NODE_PATH=./lib nyc -s mocha -R dot --recursive test -t test-results"
 	},
 	"keywords": [
 		"eslint",
@@ -24,13 +24,17 @@
 	},
 	"author": "Will Honey",
 	"license": "MIT",
+	"engines": {
+		"node": "> 14"
+	},
 	"peerDependencies": {
 		"eslint": "5.x - 8.x"
 	},
 	"devDependencies": {
 		"@types/node": "^16.11.46",
 		"@typescript-eslint/eslint-plugin": "^5.31.0",
 		"@typescript-eslint/parser": "^5.31.0",
+		"cross-env": "^7.0.3",
 		"eslint": "^8.20.0",
 		"eslint-config-prettier": "^8.5.0",
 		"eslint-plugin-eslint-plugin": "^5.0.1",
diff --git a/src/rules/order-imports.ts b/src/rules/order-imports.ts
@@ -4,6 +4,7 @@ import {
 	ValidImportType,
 	KnownImportType,
 	RegExpGroups,
+	ImportKind,
 } from '../util/import-type';
 import { isStaticRequire } from '../util/static-require';
 
@@ -312,12 +313,30 @@ function getRegExpGroups(ranks: Ranks): RegExpGroups {
 
 // DETECTING
 
-function computeRank(ranks: Ranks, regExpGroups, name: string, type: ImportType): number {
-	return ranks[determineImportType(name, regExpGroups)] + (type === 'import' ? 0 : MAX_GROUP_SIZE);
+function computeRank(
+	ranks: Ranks,
+	regExpGroups,
+	name: string,
+	type: ImportType,
+	importKind: ImportKind,
+	treatTypesAsGroup: boolean
+): number {
+	return (
+		ranks[determineImportType({ name, regExpGroups, importKind, treatTypesAsGroup })] +
+		(type === 'import' ? 0 : MAX_GROUP_SIZE)
+	);
 }
 
-function registerNode(node: NodeOrToken, name: string, type: ImportType, ranks, regExpGroups, imported: Imported[]) {
-	const rank = computeRank(ranks, regExpGroups, name, type);
+function registerNode(
+	node: NodeOrToken,
+	name: string,
+	type: ImportType,
+	ranks,
+	regExpGroups,
+	imported: Imported[],
+	treatTypesAsGroup: boolean
+) {
+	const rank = computeRank(ranks, regExpGroups, name, type, node.importKind || 'value', treatTypesAsGroup);
 	if (rank !== -1) {
 		imported.push({ name, rank, node });
 	}
@@ -327,7 +346,7 @@ function isInVariableDeclarator(node: NodeOrToken): boolean {
 	return node && (node.type === 'VariableDeclarator' || isInVariableDeclarator(node.parent));
 }
 
-const knownTypes: KnownImportType[] = ['absolute', 'module', 'parent', 'sibling', 'index'];
+const knownTypes: KnownImportType[] = ['absolute', 'module', 'parent', 'sibling', 'index', 'type'];
 
 // Creates an object with type-rank pairs.
 // Example: { index: 0, sibling: 1, parent: 1, module: 2 }
@@ -519,14 +538,16 @@ export default {
 	create: function importOrderRule(context) {
 		const options: RuleOptions = context.options[0] || {};
 		const newlinesBetweenImports: NewLinesBetweenOption = options.newlinesBetween || 'ignore';
+		const groups = options.groups || defaultGroups;
+		const treatTypesAsGroup = groups.includes('type');
 
 		let alphabetize: AlphabetizeConfig;
 		let ranks: Ranks;
 		let regExpGroups: RegExpGroups;
 
 		try {
 			alphabetize = getAlphabetizeConfig(options);
-			ranks = convertGroupsToRanks(options.groups || defaultGroups);
+			ranks = convertGroupsToRanks(groups);
 			regExpGroups = getRegExpGroups(ranks);
 		} catch (error) {
 			// Malformed configuration
@@ -547,15 +568,15 @@ export default {
 				if (node.specifiers.length) {
 					// Ignoring unassigned imports
 					const name: string = node.source.value;
-					registerNode(node, name, 'import', ranks, regExpGroups, imported);
+					registerNode(node, name, 'import', ranks, regExpGroups, imported, treatTypesAsGroup);
 				}
 			},
 			CallExpression: function handleRequires(node) {
 				if (level !== 0 || !isStaticRequire(node) || !isInVariableDeclarator(node.parent)) {
 					return;
 				}
 				const name: string = node.arguments[0].value;
-				registerNode(node, name, 'require', ranks, regExpGroups, imported);
+				registerNode(node, name, 'require', ranks, regExpGroups, imported, treatTypesAsGroup);
 			},
 			'Program:exit': function reportAndReset() {
 				if (alphabetize.order !== 'ignore') {
diff --git a/src/util/import-type.ts b/src/util/import-type.ts
@@ -25,12 +25,25 @@ export function isRegularExpressionGroup(group: string): boolean {
 	return !!group && group[0] === '/' && group[group.length - 1] === '/' && group.length > 1;
 }
 
-export type KnownImportType = 'absolute' | 'module' | 'parent' | 'index' | 'sibling';
+export type KnownImportType = 'absolute' | 'module' | 'parent' | 'index' | 'sibling' | 'type';
 export type ValidImportType = KnownImportType | string; // this string should be a string surrounded with '/'
 export type EveryImportType = ValidImportType | 'unknown';
 export type RegExpGroups = [string, RegExp][]; // array of tuples of [string, RegExp]
+export type ImportKind = 'value' | 'type';
+
+export function determineImportType({
+	name,
+	regExpGroups,
+	importKind,
+	treatTypesAsGroup = false,
+}: {
+	name: string;
+	regExpGroups: RegExpGroups;
+	importKind: ImportKind;
+	treatTypesAsGroup?: boolean;
+}): EveryImportType {
+	if (treatTypesAsGroup && importKind === 'type') return 'type';
 
-export function determineImportType(name: string, regExpGroups: RegExpGroups): EveryImportType {
 	const matchingRegExpGroup = regExpGroups.find(([_groupName, regExp]) => regExp.test(name));
 	if (matchingRegExpGroup) return matchingRegExpGroup[0];
 
diff --git a/test/rules/order-imports-2.js b/test/rules/order-imports-2.js
@@ -9,8 +9,8 @@ ruleTester.run('order', rule, {
 	valid: [
 		// Default order using import
 		// absolute at top
-		// modules starting with _ or @ are sorted with modules
 		test({
+			name: 'modules starting with _ or @ are sorted with modules',
 			code: `
 				import abs from '/absolute/module';
 
@@ -89,7 +89,7 @@ ruleTester.run('order', rule, {
 				import relParent2, {foo2} from '../foo/bar';
 
 				import index from './';
-
+				
 				import sibling, {foo3} from './foo';`,
 			options: [
 				{
@@ -99,10 +99,77 @@ ruleTester.run('order', rule, {
 				},
 			],
 		}),
+		// test out "types"
+		test({
+			name: 'type at end',
+			code: `
+				import sib from './sib';
+				import type { relative } from './relative';
+		      `,
+			parser: require.resolve('@typescript-eslint/parser'),
+			options: [{ groups: ['module', 'sibling', 'type'] }],
+		}),
+		test({
+			name: 'type at beginning',
+			code: `
+				import type { relative } from './relative';
+				import sib from './sib';
+		      `,
+			parser: require.resolve('@typescript-eslint/parser'),
+			options: [{ groups: ['type', 'module', 'sibling'] }],
+		}),
+		test({
+			name: "explicit no type group means don't treat types special, both of these pass (when alphabetization is ignored) 1",
+			code: `
+				import sib from './sib';
+				import type { relative } from './relative';
+		      `,
+			parser: require.resolve('@typescript-eslint/parser'),
+			options: [{ groups: ['module', 'sibling'] }],
+		}),
+		test({
+			name: "explicit no type group means don't treat types special, both of these pass (when alphabetization is ignored) 2",
+			code: `
+				import type { relative } from './relative';
+				import sib from './sib';
+		      `,
+			parser: require.resolve('@typescript-eslint/parser'),
+			options: [{ groups: ['module', 'sibling'] }],
+		}),
+		test({
+			name: "default groups don't have a type group and so types aren't special",
+			code: `
+				import sib from './sib';
+				import type { relative } from './relative';
+		      `,
+			parser: require.resolve('@typescript-eslint/parser'),
+		}),
+		test({
+			name: "default groups don't have a type group and so types aren't special",
+			code: `
+				import type { relative } from './relative';
+				import sib from './sib';
+		      `,
+			parser: require.resolve('@typescript-eslint/parser'),
+		}),
 	],
 	invalid: [
-		// Option alphabetize: {order: 'desc', ignoreCase: true}
 		test({
+			code: `
+				import type { relative } from './relative';
+				import sib from './sib';
+		      `,
+			output: `
+				import sib from './sib';
+				import type { relative } from './relative';
+		      `,
+			parser: require.resolve('@typescript-eslint/parser'),
+
+			options: [{ groups: ['module', 'sibling', 'type'] }],
+			errors: [{ message: '`./sib` import should occur before import of `./relative`' }],
+		}),
+		test({
+			name: "Option alphabetize: {order: 'desc', ignoreCase: true}",
 			code: `
 		    import foo from 'foo';
 		    import bar from 'bar';
@@ -127,9 +194,7 @@ ruleTester.run('order', rule, {
 				},
 			],
 		}),
-
 		// // Multiple errors
-
 		// TODO FAILING TEST
 		// test({
 		// 	code: `
diff --git a/test/rules/order-imports.js b/test/rules/order-imports.js
diff --git a/test/utils.js b/test/utils.js
