From b5056dd607b80391259af1cf6826cf9fedbc2d20 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Thu, 6 Jan 2022 15:01:32 -0500 Subject: [PATCH] fix: ensure newlines aren't inserted into large group --- src/rules/order-imports.ts | 9 +++++---- test/rules/order-imports.js | 36 ++++++++++++++++++++++++++---------- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/rules/order-imports.ts b/src/rules/order-imports.ts index c6c2d0e..0c9d427 100644 --- a/src/rules/order-imports.ts +++ b/src/rules/order-imports.ts @@ -16,6 +16,7 @@ const alphabetizeOptions: AlphabetizeOption[] = ['ignore', 'asc', 'desc']; type Groups = (ValidImportType | ValidImportType[])[]; const defaultGroups: Groups = ['absolute', 'module', 'parent', 'sibling', 'index']; +const MAX_GROUP_SIZE = 100000; // Higher than the number of imports we would ever expect to see in a single file. type RuleOptions = { groups?: Groups; @@ -71,7 +72,7 @@ function getTokensOrCommentsBefore(sourceCode, node, count): NodeOrToken[] { } function takeTokensAfterWhile(sourceCode, node, condition): NodeOrToken[] { - const tokens: NodeOrToken[] = getTokensOrCommentsAfter(sourceCode, node, 100); + const tokens: NodeOrToken[] = getTokensOrCommentsAfter(sourceCode, node, MAX_GROUP_SIZE); const result: NodeOrToken = []; for (let i = 0; i < tokens.length; i++) { if (condition(tokens[i])) { @@ -84,7 +85,7 @@ function takeTokensAfterWhile(sourceCode, node, condition): NodeOrToken[] { } function takeTokensBeforeWhile(sourceCode, node, condition): NodeOrToken[] { - const tokens: NodeOrToken[] = getTokensOrCommentsBefore(sourceCode, node, 100); + const tokens: NodeOrToken[] = getTokensOrCommentsBefore(sourceCode, node, MAX_GROUP_SIZE); const result: NodeOrToken[] = []; for (let i = tokens.length - 1; i >= 0; i--) { if (condition(tokens[i])) { @@ -292,7 +293,7 @@ function mutateRanksToAlphabetize(imported, order, ignoreCase) { // add decimal ranking to sort within the group const alphabetizedRanks = groupRanks.sort().reduce(function(acc, groupRank) { groupedByRanks[groupRank].forEach(function(importedItemName, index) { - acc[importedItemName] = +groupRank + index / 100; + acc[importedItemName] = +groupRank + index / MAX_GROUP_SIZE; }); return acc; }, {}); @@ -312,7 +313,7 @@ function getRegExpGroups(ranks: Ranks): RegExpGroups { // DETECTING function computeRank(ranks: Ranks, regExpGroups, name: string, type: ImportType): number { - return ranks[determineImportType(name, regExpGroups)] + (type === 'import' ? 0 : 100); + return ranks[determineImportType(name, regExpGroups)] + (type === 'import' ? 0 : MAX_GROUP_SIZE); } function registerNode(node: NodeOrToken, name: string, type: ImportType, ranks, regExpGroups, imported: Imported[]) { diff --git a/test/rules/order-imports.js b/test/rules/order-imports.js index 5fbd7fc..51fee73 100644 --- a/test/rules/order-imports.js +++ b/test/rules/order-imports.js @@ -9,6 +9,14 @@ function withoutAutofixOutput(test) { return Object.assign({}, test, { output: test.code }); } +function generateImports(count) { + const imports = []; + for (let i = 0; i < count; i++) { + imports.push(`import foo${i} from './foo${i}.js';`); + } + return imports.sort().join('\n'); +} + ruleTester.run('order', rule, { valid: [ // Default order using require @@ -226,7 +234,7 @@ ruleTester.run('order', rule, { test({ code: ` var fs = require('fs'); - + var async = require('async'); var index = require('./'); @@ -267,7 +275,7 @@ ruleTester.run('order', rule, { import net from 'net'; import external from 'external' - + import foo from './foo'; `, options: [{ newlinesBetween: 'always' }], @@ -367,7 +375,7 @@ ruleTester.run('order', rule, { var fs = require('fs'); var path = require('path'); - + var util = require('util'); var async = require('async'); @@ -375,13 +383,13 @@ ruleTester.run('order', rule, { var relParent1 = require('../foo'); - + var relParent2 = require('../'); var relParent3 = require('../bar'); var sibling = require('./foo'); - + var sibling2 = require('./bar'); var sibling3 = require('./foobar'); @@ -395,7 +403,7 @@ ruleTester.run('order', rule, { // Option alphabetize: {order: 'ignore'} test({ code: ` - import foo from 'foo'; + import foo from 'foo'; import bar from 'bar'; import index from './'; @@ -471,6 +479,14 @@ ruleTester.run('order', rule, { }, ], }), + // With large number of imports in the same group to ensure no newlines are inserted into group. + test({ + code: generateImports(150), + options: [{ + newlinesBetween: 'always', + alphabetize: { order: 'asc',}, + }], + }), ], invalid: [ // Option: newlinesBetween: 'always-and-inside-groups' @@ -1024,13 +1040,13 @@ comment3 */", // the spacing here is really sensitive code: ` var fs = require('fs'); /* multiline comment */ - + var index = require('./'); `, output: ` var fs = require('fs'); /* multiline comment */ - + var index = require('./'); `, options: [ @@ -1116,14 +1132,14 @@ comment3 */", // the spacing here is really sensitive code: ` import path from 'path'; import 'loud-rejection'; - + import 'something-else'; import _ from 'lodash'; `, output: ` import path from 'path'; import 'loud-rejection'; - + import 'something-else'; import _ from 'lodash'; `,