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-comment-registry.spec.ts

@@ -0,0 +1,79 @@
+import {
+    emptyStatement,
+    stringLiteral,
+    type CommentBlock,
+    type ImportDeclaration,
+} from '@babel/types';
+import { describe, expect, test } from 'vitest';
+
+import {
+    attachCommentsToOutputNodes,
+    CommentAssociation,
+    getCommentRegistryFromImportDeclarations,
+    testingOnlyExports,
+} from '../get-comment-registry';
+
+describe('getCommentRegistryFromImportDeclarations', () => {
+    test('is empty if provided no comments or no first-import', () => {
+        expect(
+            getCommentRegistryFromImportDeclarations({
+                firstImport: emptyStatement() as any,
+                outputNodes: [],
+            }),
+        ).toEqual([]);
+    });
+});
+
+describe('attachCommentsToOutputNodes', () => {
+    test('throws when missing inputs', () => {
+        expect(() =>
+            attachCommentsToOutputNodes([], [], emptyStatement() as any),
+        ).toThrow(
+            new Error(
+                "Fatal Internal Error: Can't attach comments to empty output",
+            ),
+        );
+    });
+    test('does not inject an EmptyStatement if there are no top-of-file comments', () => {
+        const firstImport = {
+            type: 'ImportDeclaration',
+            specifiers: [],
+            source: stringLiteral('foo'),
+        } as ImportDeclaration;
+        const outputNodes = [firstImport];
+
+        attachCommentsToOutputNodes([], outputNodes, firstImport);
+
+        expect(outputNodes[0].type).not.toEqual('EmptyStatement');
+    });
+    test("injects an EmptyStatement if there's a top-of-file comment", () => {
+        const firstImport = {
+            type: 'ImportDeclaration',
+            specifiers: [],
+            source: stringLiteral('foo'),
+        } as ImportDeclaration;
+        const comment = {
+            type: 'CommentBlock',
+            value: '@prettier',
+        } as CommentBlock;
+        const outputNodes = [firstImport];
+
+        attachCommentsToOutputNodes(
+            [
+                {
+                    needsTopOfFileOwner: true,
+                    comment,
+                    ownerIsSpecifier: false,
+                    commentId: testingOnlyExports.nodeId(comment),
+                    owner: firstImport,
+                    association: CommentAssociation.trailing,
+                    processingPriority: 0,
+                },
+            ],
+            outputNodes,
+            firstImport,
+        );
+
+        expect(outputNodes[0].type).toEqual('EmptyStatement');
+    });
+});

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('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

@@ -15,20 +15,22 @@ import {
  * @returns A copied and adjusted set of nodes, containing comments
  */
 export const adjustCommentsOnSortedNodes = (
-    originalDeclarationNodes: ImportDeclaration[],
-    finalNodes: ImportOrLine[],
+    originalDeclarationNodes: readonly ImportDeclaration[],
+    finalNodes: readonly ImportOrLine[],
 ) => {
     const outputNodes: ImportDeclaration[] = finalNodes.filter(
         (n) => n.type === 'ImportDeclaration',
     ) as ImportDeclaration[];
     if (originalDeclarationNodes.length === 0 || outputNodes.length === 0) {
         // Nothing to do, because there are no ImportDeclarations!
-        return finalNodes;
+        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-all-comments-from-nodes.ts

@@ -8,7 +8,7 @@ import type {
 import { SomeSpecifier } from '../types';
 
 export const getAllCommentsFromNodes = (
-    nodes: (Directive | Statement | SomeSpecifier)[],
+    nodes: readonly (Directive | Statement | SomeSpecifier)[],
 ) =>
     nodes.reduce((acc, node) => {
         if (