From d218cab0f7588a9c29abcddeaf077737ccbcb0d7 Mon Sep 17 00:00:00 2001 From: Ian VanSchooten Date: Wed, 6 Nov 2024 15:03:28 -0500 Subject: [PATCH] Throw an error when no matching sortOrder group (#192) Closes #190 This adds a more understandable error message when we encounter an import that does not fit into any group within `sortOrder`. I think the only time that should happen is when the user adds groups with `^.....` but no fallback `` bucket. We could try to shove one in somewhere automatically like we do with ``, but that could be pretty confusing and I don't know where it should go in that case. This also includes a hint about how to fix the error. --- src/utils/get-sorted-nodes-by-import-order.ts | 11 ++++- test-setup/run_spec.ts | 41 +++++++++++++++++++ .../ImportOrderMissing/importOrderMissing.ts | 4 ++ tests/ImportOrderMissing/ppsi.spec.ts | 11 +++++ 4 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 tests/ImportOrderMissing/importOrderMissing.ts create mode 100644 tests/ImportOrderMissing/ppsi.spec.ts diff --git a/src/utils/get-sorted-nodes-by-import-order.ts b/src/utils/get-sorted-nodes-by-import-order.ts index dc0f8e3..9c04215 100644 --- a/src/utils/get-sorted-nodes-by-import-order.ts +++ b/src/utils/get-sorted-nodes-by-import-order.ts @@ -56,11 +56,18 @@ export const getSortedNodesByImportOrder: GetSortedNodesByImportOrder = ( // Assign import nodes into import order groups for (const node of originalNodes) { - const matchedGroup = getImportNodesMatchedGroup( + const matchedGroupName = getImportNodesMatchedGroup( node, sanitizedImportOrder, ); - importOrderGroups[matchedGroup].push(node); + const matchedGroup = importOrderGroups[matchedGroupName]; + if (matchedGroup) { + matchedGroup.push(node); + } else { + throw new Error( + `Could not find a matching group in importOrder for: "${node.source.value}" on line ${node.source.loc?.start.line}.${node.importKind === 'type' ? ' Did you forget to include ""?' : ''}`, + ); + } } for (const group of importOrder) { diff --git a/test-setup/run_spec.ts b/test-setup/run_spec.ts index 48a0e85..7a277d8 100644 --- a/test-setup/run_spec.ts +++ b/test-setup/run_spec.ts @@ -64,6 +64,47 @@ export async function run_spec(dirname, parsers, options) { } } +export async function expectError( + dirname: string, + parser: string, + expectedError: string | Error | RegExp, + options, +) { + options = Object.assign( + { + plugins: options.plugins ?? [plugin], + tabWidth: 4, + }, + options, + ); + + /* instabul ignore if */ + if (!parser) { + throw new Error(`No parser was specified for ${dirname}`); + } + + for (const filename of fs.readdirSync(dirname)) { + const path = dirname + '/' + filename; + if ( + extname(filename) !== '.snap' && + fs.lstatSync(path).isFile() && + filename[0] !== '.' && + filename !== 'ppsi.spec.ts' + ) { + const source = read(path).replace(/\r\n/g, '\n'); + + const mergedOptions = Object.assign({}, options, { + parser, + }); + test(`${filename} - verify-error`, async () => { + expect(() => + prettyprint(source, path, mergedOptions), + ).rejects.toThrowError(expectedError); + }); + } + } +} + async function prettyprint(src, filename, options) { return await format( src, diff --git a/tests/ImportOrderMissing/importOrderMissing.ts b/tests/ImportOrderMissing/importOrderMissing.ts new file mode 100644 index 0000000..8afc9b4 --- /dev/null +++ b/tests/ImportOrderMissing/importOrderMissing.ts @@ -0,0 +1,4 @@ +import Foo from "./bar"; +import type {Justify} from "#utils" +import type {React} from "React"; +import type {Internal} from "./types.ts"; diff --git a/tests/ImportOrderMissing/ppsi.spec.ts b/tests/ImportOrderMissing/ppsi.spec.ts new file mode 100644 index 0000000..6e7058d --- /dev/null +++ b/tests/ImportOrderMissing/ppsi.spec.ts @@ -0,0 +1,11 @@ +import {expectError} from '../../test-setup/run_spec'; + +expectError( + __dirname, + "typescript", + /Could not find a matching group in importOrder for: \"React\" on line 3. Did you forget to include \"\"\?$/, + { + importOrder: ['^[./]', '^#utils$', '[.]'], + importOrderParserPlugins: ['typescript'], + } +);