Merge pull request #44 from bmish/max-group-size

Ensure newlines aren't inserted into large group

Author: willhoney7

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[]) {

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,21 +375,21 @@ ruleTester.run('order', rule, {
 				var fs = require('fs');
 
 				var path = require('path');
-			
+
 				var util = require('util');
 
 				var async = require('async');
 
 
 
 				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';
 		      `,