Behavior Change: All comments before firstImport are treated as top-of-file

README.md

@@ -12,12 +12,12 @@ Since then more critical features & fixes have been added, and the options have
 
 **Features not currently supported by upstream:**
 
--   Do not re-order across side-effect imports
--   Combine imports from the same source
--   Combine type and value imports
--   Type import grouping with `<TYPES>` keyword
--   Sorts node.js builtin modules to top
--   Custom import order separation
+- Do not re-order across side-effect imports
+- Combine imports from the same source
+- Combine type and value imports
+- Type import grouping with `<TYPES>` keyword
+- Sorts node.js builtin modules to top
+- Custom import order separation
 
 [We welcome contributions!](./CONTRIBUTING.md)
 
@@ -276,7 +276,7 @@ entire import statements can be ignored, line comments (`// prettier-ignore`) ar
 
 We make the following attempts at keeping comments in your imports clean:
 
-- If you have one or more comments at the top of the file, we will keep them at the top as long as there is a blank line before your first import statement.
+- If you have one or more comments at the top of the file, we will keep them at the top.
 - Comments on lines after the final import statement will not be moved. (Runtime-code between imports will be moved below all the imports).
 - In general, if you place a comment on the same line as an Import `Declaration` or `*Specifier`, we will keep it attached to that same specifier if it moves around.
 - Other comments are preserved, and are generally considered "leading" comments for the subsequent Import `Declaration` or `*Specifier`.

src/utils/__tests__/adjust-comments-on-sorted-nodes.spec.ts

@@ -23,18 +23,14 @@ test('it preserves the single leading comment for each import declaration', () =
     expect(importNodes).toHaveLength(3);
     const finalNodes = [importNodes[2], importNodes[1], importNodes[0]];
     const adjustedNodes = adjustCommentsOnSortedNodes(importNodes, finalNodes);
-    expect(adjustedNodes).toHaveLength(4);
-    // First node is a dummy EmptyStatement
-    expect(adjustedNodes[0].type).toEqual('EmptyStatement');
-    expect(leadingComments(adjustedNodes[0])).toEqual([]);
+    expect(adjustedNodes).toHaveLength(3);
+    expect(leadingComments(adjustedNodes[0])).toEqual([' comment a']);
     expect(trailingComments(adjustedNodes[0])).toEqual([]);
-    expect(leadingComments(adjustedNodes[1])).toEqual([' comment a']);
+    expect(leadingComments(adjustedNodes[1])).toEqual([' comment b']);
     expect(trailingComments(adjustedNodes[1])).toEqual([]);
-    expect(leadingComments(adjustedNodes[2])).toEqual([' comment b']);
-    expect(trailingComments(adjustedNodes[2])).toEqual([]);
     // Import from "c" has no leading comment, and the trailing was kept with "b"
-    expect(leadingComments(adjustedNodes[3])).toEqual([]);
-    expect(trailingComments(adjustedNodes[3])).toEqual([]);
+    expect(leadingComments(adjustedNodes[2])).toEqual([]);
+    expect(trailingComments(adjustedNodes[2])).toEqual([]);
 });
 
 test('it preserves multiple leading comments for each import declaration', () => {
@@ -52,21 +48,21 @@ test('it preserves multiple leading comments for each import declaration', () =>
     expect(importNodes).toHaveLength(3);
     const finalNodes = [importNodes[2], importNodes[1], importNodes[0]];
     const adjustedNodes = adjustCommentsOnSortedNodes(importNodes, finalNodes);
-    expect(adjustedNodes).toHaveLength(4);
-    expect(leadingComments(adjustedNodes[1])).toEqual([
+    expect(adjustedNodes).toHaveLength(3);
+    expect(leadingComments(adjustedNodes[0])).toEqual([
         ' comment a1',
         ' comment a2',
         ' comment a3',
     ]);
-    expect(trailingComments(adjustedNodes[1])).toEqual([]);
-    expect(leadingComments(adjustedNodes[2])).toEqual([
+    expect(trailingComments(adjustedNodes[0])).toEqual([]);
+    expect(leadingComments(adjustedNodes[1])).toEqual([
         ' comment b1',
         ' comment b2',
         ' comment b3',
     ]);
+    expect(trailingComments(adjustedNodes[1])).toEqual([]);
+    expect(leadingComments(adjustedNodes[2])).toEqual([]);
     expect(trailingComments(adjustedNodes[2])).toEqual([]);
-    expect(leadingComments(adjustedNodes[3])).toEqual([]);
-    expect(trailingComments(adjustedNodes[3])).toEqual([]);
 });
 
 test('it does not move comments more than one line before all import declarations', () => {
@@ -81,15 +77,16 @@ test('it does not move comments more than one line before all import declaration
     const finalNodes = [importNodes[2], importNodes[1], importNodes[0]];
     const adjustedNodes = adjustCommentsOnSortedNodes(importNodes, finalNodes);
     expect(adjustedNodes).toHaveLength(4);
-    // Comment c1 is more than one line above the first import, so it stays with the top-of-file
-    expect(leadingComments(adjustedNodes[0])).toEqual([' comment c1']);
+    // Comment c1 is above the first import, so it stays with the top-of-file attached to a dummy statement
+    expect(adjustedNodes[0].type).toEqual('EmptyStatement');
+    expect(trailingComments(adjustedNodes[0])).toEqual([
+        ' comment c1',
+        ' comment c2',
+    ]);
 
     expect(leadingComments(adjustedNodes[2])).toEqual([]);
     expect(trailingComments(adjustedNodes[2])).toEqual([]);
     expect(trailingComments(adjustedNodes[3])).toEqual([]);
-
-    // Comment c2 is attached to import from "c"
-    expect(leadingComments(adjustedNodes[3])).toEqual([' comment c2']);
 });
 
 test('it does not affect comments after all import declarations', () => {
@@ -103,11 +100,11 @@ test('it does not affect comments after all import declarations', () => {
     expect(importNodes).toHaveLength(3);
     const finalNodes = [importNodes[2], importNodes[1], importNodes[0]];
     const adjustedNodes = adjustCommentsOnSortedNodes(importNodes, finalNodes);
-    expect(adjustedNodes).toHaveLength(4);
+    expect(adjustedNodes).toHaveLength(3);
+    expect(leadingComments(adjustedNodes[0])).toEqual([]);
+    expect(trailingComments(adjustedNodes[0])).toEqual([]);
     expect(leadingComments(adjustedNodes[1])).toEqual([]);
     expect(trailingComments(adjustedNodes[1])).toEqual([]);
     expect(leadingComments(adjustedNodes[2])).toEqual([]);
     expect(trailingComments(adjustedNodes[2])).toEqual([]);
-    expect(leadingComments(adjustedNodes[3])).toEqual([]);
-    expect(trailingComments(adjustedNodes[3])).toEqual([]);
 });

src/utils/__tests__/get-sorted-import-specifiers.spec.ts

@@ -31,7 +31,7 @@ test('should return correct sorted nodes with default import', () => {
     ]);
 });
 
-test('should group type imports after value imports', () => {
+test('should group type imports after value imports - typescript', () => {
     const code = `import Component, { type TypeB, filter, type TypeA, reduce, eventHandler } from '@server/z';`;
     const [importNode] = getImportNodes(code, {
         plugins: ['typescript'],
@@ -50,3 +50,23 @@ test('should group type imports after value imports', () => {
         'TypeB',
     ]);
 });
+
+test.only('should group type imports after value imports - flow', () => {
+    const code = `import Component, { type TypeB, filter, type TypeA, reduce, eventHandler } from '@server/z';`;
+    const [importNode] = getImportNodes(code, {
+        plugins: ['flow'],
+    });
+    const sortedImportSpecifiers = getSortedImportSpecifiers(importNode);
+    const specifiersList = getSortedNodesModulesNames(
+        sortedImportSpecifiers.specifiers,
+    );
+
+    expect(specifiersList).toEqual([
+        'Component',
+        'eventHandler',
+        'filter',
+        'reduce',
+        'TypeA',
+        'TypeB',
+    ]);
+});

src/utils/__tests__/merge-nodes-with-matching-flavors.spec.ts

@@ -61,7 +61,6 @@ import { Junk } from "junk-group-1";
 import "./side-effects1";
 
 // C, E and D will be separated from A, B because side-effects in-between
-
 import { D, type C, type E } from "a";
 
 // prettier-ignore

src/utils/adjust-comments-on-sorted-nodes.ts

@@ -26,9 +26,11 @@ export const adjustCommentsOnSortedNodes = (
         return finalNodes;
     }
 
+    const firstImport = originalDeclarationNodes[0];
+
     const registry = getCommentRegistryFromImportDeclarations({
         outputNodes,
-        firstImport: originalDeclarationNodes[0],
+        firstImport,
     });
 
     // Make a copy of the nodes for easier debugging & remove the existing comments to reattach them
@@ -44,7 +46,7 @@ export const adjustCommentsOnSortedNodes = (
         return noDirectCommentsNode;
     });
 
-    attachCommentsToOutputNodes(registry, finalNodesClone);
+    attachCommentsToOutputNodes(registry, finalNodesClone, firstImport);
 
     return finalNodesClone;
 };

src/utils/get-comment-registry.ts

@@ -174,12 +174,10 @@ const attachCommentsToRegistryMap = ({
             const currentOwnerIsFirstImport =
                 nodeId(owner) === nodeId(firstImport);
 
-            // endsMoreThanOneLineAboveOwner is used with firstImport to protect top-of-file comments,
-            // and pick the right ImportSpecifier when Specifiers are re-sorted
-            const endsMoreThanOneLineAboveOwner =
-                (comment.loc?.end.line || 0) < (owner.loc?.start.line || 0) - 1;
+            const endsBeforeOwner =
+                (comment.loc?.end.line || 0) <= (owner.loc?.start.line || 0);
 
-            if (currentOwnerIsFirstImport && endsMoreThanOneLineAboveOwner) {
+            if (currentOwnerIsFirstImport && endsBeforeOwner) {
                 debugLog?.('Found a disconnected leading comment', {
                     comment,
                     owner,
@@ -367,16 +365,19 @@ export const getCommentRegistryFromImportDeclarations = ({
 export function attachCommentsToOutputNodes(
     commentEntriesFromRegistry: CommentEntry[],
     outputNodes: ImportOrLine[],
+    /** Original declaration, not the re-sorted output-node! */
+    firstImport: ImportDeclaration,
 ) {
     if (outputNodes.length === 0) {
         // attachCommentsToOutputNodes implies that there's at least one output node so this shouldn't happen
         throw new Error(
             "Fatal Internal Error: Can't attach comments to empty output",
         );
     }
-    if (outputNodes[0].type !== 'EmptyStatement') {
-        // Put in a dummy empty statement to attach top-of-file-comments to if one was not provided
-        outputNodes.unshift(emptyStatement());
+    if (firstImport == null) {
+        throw new Error(
+            "Fatal Internal Error: Can't attach comments if there was no firstImport",
+        );
     }
 
     /** Store a mapping of Specifier to ImportDeclaration */
@@ -394,6 +395,8 @@ export function attachCommentsToOutputNodes(
         }
     }
 
+    const newFirstImport = outputNodes[0];
+
     for (const commentEntry of commentEntriesFromRegistry) {
         const {
             owner,
@@ -403,6 +406,46 @@ export function attachCommentsToOutputNodes(
             needsLastSpecifierOwner,
         } = commentEntry;
 
+        if (needsTopOfFileOwner && outputNodes[0].type !== 'EmptyStatement') {
+            // Put in a dummy empty statement to attach top-of-file-comments to if one was not provided
+            const dummy = emptyStatement();
+            dummy.loc = {
+                start: { line: 0, column: 0 },
+                end: { line: 0, column: 0 },
+            };
+            outputNodes.unshift(dummy);
+
+            // Put the first import in the right spot, where the original first import started
+            // Otherwise, comments at the top of the file will not be formatted correctly.
+            // This is a little tricky, because the new first import might have leading comments,
+            // and we have to move the node and all comments the same distance
+            const commentHeight = getHeightOfLeadingComments(newFirstImport);
+            const originalLoc = newFirstImport.loc;
+            if (firstImport.loc && originalLoc) {
+                newFirstImport.loc = {
+                    start: {
+                        ...firstImport.loc?.start,
+                        line: firstImport.loc?.start.line + commentHeight,
+                    },
+                    end: {
+                        ...firstImport.loc?.end,
+                        line: firstImport.loc?.end.line + commentHeight,
+                    },
+                };
+                const moveDist =
+                    originalLoc.start.line - newFirstImport.loc.start.line;
+
+                for (const commentType of orderedCommentKeysToRegister) {
+                    newFirstImport[commentType]?.forEach((c) => {
+                        if (c.loc) {
+                            c.loc.start.line -= moveDist;
+                            c.loc.end.line -= moveDist;
+                        }
+                    });
+                }
+            }
+        }
+
         let ownerNode = needsTopOfFileOwner
             ? outputNodes[0]
             : outputRegistry.get(nodeId(owner));
@@ -441,23 +484,37 @@ export function attachCommentsToOutputNodes(
             throw new Error("Fatal Internal Error: Couldn't find owner node");
         }
 
+        // // Since we mucked with the loc of the newFirstImport, we need to be careful to
+        // // keep its comments in the right place, so adjust their loc too
+        if (
+            ownerNode === newFirstImport &&
+            association !== CommentAssociation.leading &&
+            comment.loc &&
+            ownerNode.loc &&
+            !needsLastSpecifierOwner
+        ) {
+            comment.loc.start.line = ownerNode.loc.start.line;
+        }
+
         // addComments(ownerNode, association, [comment]);
         // using Babel's addComments will reverse the comments if you iteratively attach them, so push them directly
-        const commentCollection = (ownerNode[
-            CommentAssociationByValue[association]
-        ] = ownerNode[CommentAssociationByValue[association]] || []);
+        const commentType = needsTopOfFileOwner
+            ? 'trailingComments' // use trailing comments to preserve trailing blank line if present
+            : CommentAssociationByValue[association];
+        const commentCollection = (ownerNode[commentType] =
+            ownerNode[commentType] || []);
         (commentCollection as Comment[]).push(comment);
     }
+}
 
-    if (Array.isArray(outputNodes[0].leadingComments)) {
-        if (outputNodes[0].leadingComments.length > 0) {
-            // Convert this to a newline node!
-            outputNodes[0] = {
-                ...newLineNode, // Inject a newline after top-of-file comments
-                leadingComments: outputNodes[0].leadingComments,
-            };
-        } else {
-            outputNodes.shift(); // Remove the empty statement
-        }
+function getHeightOfLeadingComments(node: ImportOrLine) {
+    if (
+        Array.isArray(node.leadingComments) &&
+        node.leadingComments.length &&
+        node.leadingComments[0].loc &&
+        node.loc
+    ) {
+        return node.loc.start.line - node.leadingComments[0].loc.start.line;
     }
+    return 0;
 }

src/utils/get-sorted-import-specifiers.ts

@@ -22,7 +22,8 @@ export const getSortedImportSpecifiers = (node: ImportDeclaration) => {
             b.type === 'ImportSpecifier' &&
             a.importKind !== b.importKind
         ) {
-            return a.importKind === 'value' ? -1 : 1;
+            // flow uses null for value import specifiers
+            return a.importKind === 'value' || a.importKind == null ? -1 : 1;
         }
 
         return naturalSort(a.local.name, b.local.name);

src/utils/get-sorted-nodes-by-import-order.ts

@@ -27,7 +27,7 @@ export const getSortedNodesByImportOrder: GetSortedNodes = (
         importOrder = [THIRD_PARTY_MODULES_SPECIAL_WORD, ...importOrder];
     }
 
-    // IDEA: We could make built-ins a special word, if people do not want them up top
+    // Opinionated decision: builtin modules should always be first
     importOrder = [BUILTIN_MODULES, ...importOrder];
 
     const importOrderGroups = importOrder.reduce<ImportGroups>(
@@ -42,6 +42,7 @@ export const getSortedNodesByImportOrder: GetSortedNodes = (
         {},
     );
 
+    // Select just the SPECIAL WORDS and the matchers
     const sanitizedImportOrder = importOrder.filter(
         (group) =>
             !isCustomGroupSeparator(group) &&