PR Feedback: only-same-line-comments should be treated as trailing

Fixes:
- trailing attachments should be used much more rarely / leading comment on code shouldn't treated as trailing comment on an import
- various unexpected newline injections
- unnecessary test3, is duplicate of test2
- some test cases now behave correctly, but had comments added implying things about gaps being unexpectedly deleted (when they're definitely present)

README.md

@@ -277,8 +277,8 @@ We make the following attempts at keeping comments in your imports clean, but th
 
 Specific cases we handle:
 
-- If you leave a gap after a comment at the top of your file, we will avoid moving it around if the imports below it shift.
-- If you have comments that come after after your last import (with a gap); that comment and following code will stay below the imports.
+- If you have one or more comments that end above the line immediately preceding your first import, we will treat them as top-of-file comment(s) and avoid moving them when the first-import moves down.
+- If you have comments that come after after your last import; those comments and following code will stay below the imports. (Runtime-code between imports will be moved below all the imports)
 - In general, if you place a single-line comment on the same line as an Import `Declaration` or `*Specifier`, we will keep it attached to that same specifier if that line moves around (due to mergers, or sorting changes due to newly inserted imports).
 - Other comments are preserved, and are generally considered `leadingComments` for the subsequent Import `Declaration` or `*Specifier`
 

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

@@ -100,12 +100,7 @@ test('it does not affect comments after all import declarations', () => {
     const adjustedNodes = adjustCommentsOnSortedNodes(importNodes, finalNodes);
     expect(adjustedNodes).toHaveLength(4);
     expect(leadingComments(adjustedNodes[1])).toEqual([]);
-    // "final 1" is attached as a trailing-comment for import from "a"
-    // but "final 2" is detached so it stays with the bottom-of-imports
-    const expectedNode1TrailingComments = [' comment final 1'];
-    expect(trailingComments(adjustedNodes[1])).toEqual(
-        expectedNode1TrailingComments,
-    );
+    expect(trailingComments(adjustedNodes[1])).toEqual([]);
     expect(leadingComments(adjustedNodes[2])).toEqual([]);
     expect(trailingComments(adjustedNodes[2])).toEqual([]);
     expect(leadingComments(adjustedNodes[3])).toEqual([]);

src/utils/get-comment-registry.ts

@@ -147,13 +147,13 @@ const attachCommentsToRegistryMap = <
             commentIsSingleLineType && // Prettier doesn't allow block comments to stay on same line as expressions
             owner.loc?.start.line === comment.loc?.start.line;
 
-        // LeadingGap is used with firstImport to protect top-of-file comments, and pick the right ImportSpecifier when Specifiers are re-sorted
-        const hasLeadingGap =
+        // 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;
 
-        // TrailingGap is used with lastImport to protect bottom-of-imports comments, and pick the right ImportSpecifier when Specifiers are re-sorted
-        const hasTrailingGap =
-            (comment.loc?.start.line || 0) > (owner.loc?.end.line || 0) + 1;
+        // startsBelowOwner is used with lastImport to protect bottom-of-imports comments, and pick the right ImportSpecifier when Specifiers are re-sorted
+        const startsBelowOwner =
+            (comment.loc?.start.line || 0) > (owner.loc?.end.line || 0);
 
         if (attachmentKey === 'trailingComments') {
             // Trailing comments might be on the same line "attached"
@@ -162,25 +162,19 @@ const attachCommentsToRegistryMap = <
 
             debugLog?.({
                 isSameLineAsCurrentOwner,
-                hasLeadingGap,
-                hasTrailingGap,
+                endsMoreThanOneLineAboveOwner,
+                startsBelowOwner,
                 owner,
                 comment,
             });
 
             if (isSameLineAsCurrentOwner) {
                 commentRegistry.set(commentId, commentEntry);
-            } else if (hasTrailingGap) {
+            } else if (startsBelowOwner) {
                 // This comment is either a leading comment on the next node, or it's an unrelated comment following the imports
                 // Trailing comment, not on the same line, so either it will get attached correctly, or it will be dropped below imports
-                if (currentOwnerIsLastImport) {
-                    // [Intentional empty block] will automatically be attached from other nodes, or will fall to bottom
-                } else {
-                    // This is a specifier or a non-last import.
-                    commentEntry.processingPriority +=
-                        DeferredCommentClaimPriorityAdjustment.trailingNonSameLine;
-                    deferredCommentClaims.push(commentEntry);
-                }
+                // -- will automatically be attached from other nodes, or will fall to bottom of imports
+                // [Intentional empty block]
             } else {
                 // This comment should be kept close to the ImportDeclaration it follows
                 commentEntry.processingPriority +=
@@ -189,7 +183,7 @@ const attachCommentsToRegistryMap = <
             }
             continue; // Unnecessary, but explicit
         } else if (attachmentKey === 'leadingComments') {
-            if (currentOwnerIsFirstImport && hasLeadingGap) {
+            if (currentOwnerIsFirstImport && endsMoreThanOneLineAboveOwner) {
                 debugLog?.('Found a disconnected leading comment', {
                     comment,
                     owner,

tests/ImportCommentsPreserved/__snapshots__/ppsi.spec.ts.snap

@@ -8,8 +8,8 @@ import B from "b";
 
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 import B from "b";
-// Trailing comment (not supposed to be on same line)
 
+// Trailing comment (not supposed to be on same line)
 // Bottom of imports single-line comment
 
 `;
@@ -32,34 +32,15 @@ import b from "b";
 exports[`import-comments-document-top-test2.ts - typescript-verify > import-comments-document-top-test2.ts 1`] = `
 /**
  * This is part of multiple-lines at document-top
- * Second line of text, comment is followed by a gap (gap gets eaten)
+ * Second line of text, comment is followed by a gap
  */
 
 import b from "b";
 import a from "a";
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 /**
  * This is part of multiple-lines at document-top
- * Second line of text, comment is followed by a gap (gap gets eaten)
- */
-
-import a from "a";
-import b from "b";
-
-`;
-
-exports[`import-comments-document-top-test3.ts - typescript-verify > import-comments-document-top-test3.ts 1`] = `
-/**
- * This comment is before the whole block,
- * it is not attached to the following import.
- */
-
-import b from "b";
-import a from "a";
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-/**
- * This comment is before the whole block,
- * it is not attached to the following import.
+ * Second line of text, comment is followed by a gap
  */
 
 import a from "a";
@@ -71,26 +52,26 @@ exports[`import-comments-outer-comments-block-comments.ts - typescript-verify >
 // Loose leading comment before imports (should not be dragged down with B)
 
 /* Leading comment before B */
-import B from "b"; /* Trailing comment on same-line as B (babel doesn't like CommentBlocks on same-line as anything else, so it will be shifted to a clean line) */
-/* Trailing comment on first line after B (This is treated as a leading comment for next import A!) */
+import B from "b"; /* Trailing comment on same-line as B */
+/* Trailing comment on first line after B (this is treated as a leading comment for next import A!) */
 
 
 /* Leading comment before A */
-import A from "a"; /* Trailing comment on same-line as A (babel doesn't like CommentBlocks on same-line as anything else, so it will be shifted to a clean line) */
-/* Trailing comment on first line after A (will initially, because this is "close" to last-import 'a' */
+import A from "a"; /* Trailing comment on same-line as A */
+/* Trailing comment on first line after A (this is treated as a "bottom-of-imports comment) */
 
 // Loose trailing comment after imports (should not be dragged up with A)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 // Loose leading comment before imports (should not be dragged down with B)
 
-/* Trailing comment on same-line as B (babel doesn't like CommentBlocks on same-line as anything else, so it will be shifted to a clean line) */
-/* Trailing comment on first line after B (This is treated as a leading comment for next import A!) */
+/* Trailing comment on first line after B (this is treated as a leading comment for next import A!) */
 
 /* Leading comment before A */
-import A from "a"; /* Trailing comment on same-line as A (babel doesn't like CommentBlocks on same-line as anything else, so it will be shifted to a clean line) */
-/* Trailing comment on first line after A (will initially, because this is "close" to last-import 'a' */
+import A from "a"; /* Trailing comment on same-line as A */
 /* Leading comment before B */
-import B from "b";
+import B from "b"; /* Trailing comment on same-line as B */
+
+/* Trailing comment on first line after A (this is treated as a "bottom-of-imports comment) */
 
 // Loose trailing comment after imports (should not be dragged up with A)
 
@@ -101,24 +82,25 @@ exports[`import-comments-outer-comments-single-line.ts - typescript-verify > imp
 
 // Leading comment before B
 import B from "b"; // Trailing comment on same-line as B
-// Trailing comment on first line after B (This is treated as a leading comment for next import A!)
+// Trailing comment on first line after B (this is treated as a leading comment for next import A!)
 
 // Leading comment before A
 import A from "a"; // Trailing comment on same-line as A
-// Trailing comment on first line after A (will follow A initially, because this is "close" to last-import 'a')
+// Trailing comment on first line after A (this is treated as a "bottom-of-imports comment)
 
 // Loose trailing comment after imports (should not be dragged up with A)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 // Loose leading comment before imports (should not be dragged down with B)
 
-// Trailing comment on first line after B (This is treated as a leading comment for next import A!)
+// Trailing comment on first line after B (this is treated as a leading comment for next import A!)
 
 // Leading comment before A
 import A from "a"; // Trailing comment on same-line as A
-// Trailing comment on first line after A (will follow A initially, because this is "close" to last-import 'a')
 // Leading comment before B
 import B from "b"; // Trailing comment on same-line as B
 
+// Trailing comment on first line after A (this is treated as a "bottom-of-imports comment)
+
 // Loose trailing comment after imports (should not be dragged up with A)
 
 `;

tests/ImportCommentsPreserved/import-comments-document-top-test2.ts

@@ -1,6 +1,6 @@
 /**
  * This is part of multiple-lines at document-top
- * Second line of text, comment is followed by a gap (gap gets eaten)
+ * Second line of text, comment is followed by a gap
  */
 
 import b from "b";

tests/ImportCommentsPreserved/import-comments-outer-comments-block-comments.ts

@@ -1,12 +1,12 @@
 // Loose leading comment before imports (should not be dragged down with B)
 
 /* Leading comment before B */
-import B from "b"; /* Trailing comment on same-line as B (babel doesn't like CommentBlocks on same-line as anything else, so it will be shifted to a clean line) */
-/* Trailing comment on first line after B (This is treated as a leading comment for next import A!) */
+import B from "b"; /* Trailing comment on same-line as B */
+/* Trailing comment on first line after B (this is treated as a leading comment for next import A!) */
 
 
 /* Leading comment before A */
-import A from "a"; /* Trailing comment on same-line as A (babel doesn't like CommentBlocks on same-line as anything else, so it will be shifted to a clean line) */
-/* Trailing comment on first line after A (will initially, because this is "close" to last-import 'a' */
+import A from "a"; /* Trailing comment on same-line as A */
+/* Trailing comment on first line after A (this is treated as a "bottom-of-imports comment) */
 
 // Loose trailing comment after imports (should not be dragged up with A)

tests/ImportCommentsPreserved/import-comments-outer-comments-single-line.ts

@@ -2,10 +2,10 @@
 
 // Leading comment before B
 import B from "b"; // Trailing comment on same-line as B
-// Trailing comment on first line after B (This is treated as a leading comment for next import A!)
+// Trailing comment on first line after B (this is treated as a leading comment for next import A!)
 
 // Leading comment before A
 import A from "a"; // Trailing comment on same-line as A
-// Trailing comment on first line after A (will follow A initially, because this is "close" to last-import 'a')
+// Trailing comment on first line after A (this is treated as a "bottom-of-imports comment)
 
 // Loose trailing comment after imports (should not be dragged up with A)

tests/ImportsNotSeparated/__snapshots__/ppsi.spec.ts.snap

@@ -102,8 +102,8 @@ function add(a:number,b:number) {
 
 import "./commands";
 import React from "react";
-// Comment
 
+// Comment
 // Comment
 
 function add(a: number, b: number) {
@@ -170,7 +170,7 @@ import thirdParty from "third-party";
 import {
     random // inner comment
 } from './random';
-// trailing comment for import-random (export statement is code that will be sorted to below imports, and so doesn't have precedence over the import-comment-attachment rules!)
+// leading comment for export from random
 export {
     random // inner comment
 } from './random';
@@ -212,10 +212,10 @@ import oneLevelRelativePath from "../oneLevelRelativePath";
 import {
     random, // inner comment
 } from "./random";
-// trailing comment for import-random (export statement is code that will be sorted to below imports, and so doesn't have precedence over the import-comment-attachment rules!)
 // I am stick to sameLevelRelativePath
 import sameLevelRelativePath from "./sameLevelRelativePath";
 
+// leading comment for export from random
 export {
     random, // inner comment
 } from "./random";

tests/ImportsNotSeparated/imports-with-file-level-comments.ts

@@ -10,7 +10,7 @@ import thirdParty from "third-party";
 import {
     random // inner comment
 } from './random';
-// trailing comment for import-random (export statement is code that will be sorted to below imports, and so doesn't have precedence over the import-comment-attachment rules!)
+// leading comment for export from random
 export {
     random // inner comment
 } from './random';

tests/ImportsSeparatedByEmptyRegex/__snapshots__/ppsi.spec.ts.snap

@@ -157,8 +157,8 @@ function add(a:number,b:number) {
 import "./commands";
 
 import React from "react";
-// Comment
 
+// Comment
 // Comment
 
 function add(a: number, b: number) {
@@ -227,7 +227,7 @@ import thirdParty from "third-party";
 import {
     random // inner comment
 } from './random';
-// trailing comment for import-random (export statement is code that will be sorted to below imports, and so doesn't have precedence over the import-comment-attachment rules!)
+// leading comment for export from random
 export {
     random // inner comment
 } from './random';
@@ -271,10 +271,10 @@ import oneLevelRelativePath from "../oneLevelRelativePath";
 import {
     random, // inner comment
 } from "./random";
-// trailing comment for import-random (export statement is code that will be sorted to below imports, and so doesn't have precedence over the import-comment-attachment rules!)
 // I am stick to sameLevelRelativePath
 import sameLevelRelativePath from "./sameLevelRelativePath";
 
+// leading comment for export from random
 export {
     random, // inner comment
 } from "./random";

tests/ImportsSeparatedByEmptyRegex/imports-with-file-level-comments.ts

@@ -10,7 +10,7 @@ import thirdParty from "third-party";
 import {
     random // inner comment
 } from './random';
-// trailing comment for import-random (export statement is code that will be sorted to below imports, and so doesn't have precedence over the import-comment-attachment rules!)
+// leading comment for export from random
 export {
     random // inner comment
 } from './random';

tests/ImportsThirdParty/__snapshots__/ppsi.spec.ts.snap

@@ -102,8 +102,8 @@ function add(a:number,b:number) {
 
 import "./commands";
 import React from "react";
-// Comment
 
+// Comment
 // Comment
 
 function add(a: number, b: number) {
@@ -170,7 +170,7 @@ import thirdParty from "third-party";
 import {
     random // inner comment
 } from './random';
-// trailing comment for import-random (export statement is code that will be sorted to below imports, and so doesn't have precedence over the import-comment-attachment rules!)
+// leading comment for export from random
 export {
     random // inner comment
 } from './random';
@@ -212,10 +212,10 @@ import oneLevelRelativePath from "../oneLevelRelativePath";
 import {
     random, // inner comment
 } from "./random";
-// trailing comment for import-random (export statement is code that will be sorted to below imports, and so doesn't have precedence over the import-comment-attachment rules!)
 // I am stick to sameLevelRelativePath
 import sameLevelRelativePath from "./sameLevelRelativePath";
 
+// leading comment for export from random
 export {
     random, // inner comment
 } from "./random";

tests/ImportsThirdParty/imports-with-file-level-comments.ts

@@ -10,7 +10,7 @@ import thirdParty from "third-party";
 import {
     random // inner comment
 } from './random';
-// trailing comment for import-random (export statement is code that will be sorted to below imports, and so doesn't have precedence over the import-comment-attachment rules!)
+// leading comment for export from random
 export {
     random // inner comment
 } from './random';