🐛 Fix smart selection of propery signatures having JSDoc comments (#49804)

babakks · web-flow · commit 8e6e87fea646 · 2022-07-12T09:04:21.000-07:00
* 🐛 Avoid grouping JSDoc nodes of propery signatures with others in smart selection Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com> * ⚗️ Add test case for JSDoc smart selection (#39618) Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com> * ⚗️ Add test baseline for JSDoc smart selection (#39618) Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com> * 🐛 Fix skipping SyntaxList first child's JSDoc in smart selection Signed-off-by: GitHub <noreply@github.com> * ⚗️ Add tests to ensure not skipping first SyntaxList child's JSDoc Signed-off-by: GitHub <noreply@github.com> * 🔨 Exclude JSDoc token from tokens pivoting property signature Signed-off-by: GitHub <noreply@github.com> * ⚗️ Update test case to also include modifier Signed-off-by: GitHub <noreply@github.com> * ⚗️ Update test case reference baseline Signed-off-by: GitHub <noreply@github.com>
diff --git a/src/services/smartSelection.ts b/src/services/smartSelection.ts
@@ -52,13 +52,24 @@ namespace ts.SmartSelectionRange {
                     // selected from open to close, including whitespace but not including the braces/etc. themselves.
                     const isBetweenMultiLineBookends = isSyntaxList(node) && isListOpener(prevNode) && isListCloser(nextNode)
                         && !positionsAreOnSameLine(prevNode.getStart(), nextNode.getStart(), sourceFile);
-                    const start = isBetweenMultiLineBookends ? prevNode.getEnd() : node.getStart();
+                    let start = isBetweenMultiLineBookends ? prevNode.getEnd() : node.getStart();
                     const end = isBetweenMultiLineBookends ? nextNode.getStart() : getEndPos(sourceFile, node);
 
                     if (hasJSDocNodes(node) && node.jsDoc?.length) {
                         pushSelectionRange(first(node.jsDoc).getStart(), end);
                     }
 
+                    // (#39618 & #49807)
+                    // When the node is a SyntaxList and its first child has a JSDoc comment, then the node's
+                    // `start` (which usually is the result of calling `node.getStart()`) points to the first
+                    // token after the JSDoc comment. So, we have to make sure we'd pushed the selection
+                    // covering the JSDoc comment before diving further.
+                    if (isSyntaxList(node)) {
+                        const firstChild = node.getChildren()[0];
+                        if (firstChild && hasJSDocNodes(firstChild) && firstChild.jsDoc?.length && firstChild.getStart() !== node.pos) {
+                            start = Math.min(start, first(firstChild.jsDoc).getStart());
+                        }
+                    }
                     pushSelectionRange(start, end);
 
                     // String literals should have a stop both inside and outside their quotes.
@@ -184,7 +195,10 @@ namespace ts.SmartSelectionRange {
         if (isPropertySignature(node)) {
             const children = groupChildren(node.getChildren(), child =>
                 child === node.name || contains(node.modifiers, child));
-            return splitChildren(children, ({ kind }) => kind === SyntaxKind.ColonToken);
+            const firstJSDocChild = children[0]?.kind === SyntaxKind.JSDoc ? children[0] : undefined;
+            const withJSDocSeparated = firstJSDocChild? children.slice(1) : children;
+            const splittedChildren = splitChildren(withJSDocSeparated, ({ kind }) => kind === SyntaxKind.ColonToken);
+            return firstJSDocChild? [firstJSDocChild, createSyntaxList(splittedChildren)] : splittedChildren;
         }
 
         // Group the parameter name with its `...`, then that group with its `?`, then pivot on `=`.
diff --git a/tests/baselines/reference/smartSelection_JSDocTags11.baseline b/tests/baselines/reference/smartSelection_JSDocTags11.baseline
@@ -0,0 +1,83 @@
+const x = 1;
+type Foo = {
+  /** comment */
+  /**/readonly status: number;
+};
+
+
+  readonly                
+
+
+  readonly status         
+
+
+  readonly status: number;
+
+
+  /** comment */
+  readonly status: number;
+
+
+            ↲
+••/** comment */
+  readonly status: number;↲
+
+
+           {
+  /** comment */
+  readonly status: number;
+} 
+
+type Foo = {
+  /** comment */
+  readonly status: number;
+};
+
+const x = 1;
+type Foo = {
+  /** comment */
+  readonly status: number;
+};
+
+================================================================================
+
+const x = 1;
+type Foo = {
+  /** comment */
+  readonly /**/status: number;
+};
+
+
+           status         
+
+
+  readonly status         
+
+
+  readonly status: number;
+
+
+  /** comment */
+  readonly status: number;
+
+
+            ↲
+••/** comment */
+  readonly status: number;↲
+
+
+           {
+  /** comment */
+  readonly status: number;
+} 
+
+type Foo = {
+  /** comment */
+  readonly status: number;
+};
+
+const x = 1;
+type Foo = {
+  /** comment */
+  readonly status: number;
+};
diff --git a/tests/baselines/reference/smartSelection_JSDocTags12.baseline b/tests/baselines/reference/smartSelection_JSDocTags12.baseline
@@ -0,0 +1,117 @@
+type B = {};
+type A = {
+    a(/** Comment */ /**/p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;
+};
+
+
+                     p0                                                                        
+
+
+                     p0: number                                                                
+
+
+      /** Comment */ p0: number                                                                
+
+
+      /** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number          
+
+
+    a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;
+
+
+          ↲
+••••a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;↲
+
+
+         {
+    a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;
+} 
+
+type A = {
+    a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;
+};
+
+type B = {};
+type A = {
+    a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;
+};
+
+================================================================================
+
+type B = {};
+type A = {
+    a(/** Comment */ p0: number, /** Comment */ /**/p1: number, /** Comment */ p2: number): string;
+};
+
+
+                                                p1                                             
+
+
+                                                p1: number                                     
+
+
+                                 /** Comment */ p1: number                                     
+
+
+      /** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number          
+
+
+    a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;
+
+
+          ↲
+••••a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;↲
+
+
+         {
+    a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;
+} 
+
+type A = {
+    a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;
+};
+
+type B = {};
+type A = {
+    a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;
+};
+
+================================================================================
+
+type B = {};
+type A = {
+    a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ /**/p2: number): string;
+};
+
+
+                                                                           p2                  
+
+
+                                                                           p2: number          
+
+
+                                                            /** Comment */ p2: number          
+
+
+      /** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number          
+
+
+    a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;
+
+
+          ↲
+••••a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;↲
+
+
+         {
+    a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;
+} 
+
+type A = {
+    a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;
+};
+
+type B = {};
+type A = {
+    a(/** Comment */ p0: number, /** Comment */ p1: number, /** Comment */ p2: number): string;
+};
diff --git a/tests/baselines/reference/smartSelection_JSDocTags13.baseline b/tests/baselines/reference/smartSelection_JSDocTags13.baseline
@@ -0,0 +1,141 @@
+let a;
+let b: {
+    /** Comment */ /**/p0: number
+    /** Comment */ p1: number
+    /** Comment */ p2: number
+};
+let c;
+
+
+                   p0        
+
+
+                   p0: number
+
+
+    /** Comment */ p0: number
+
+
+        ↲
+••••/** Comment */ p0: number
+    /** Comment */ p1: number
+    /** Comment */ p2: number↲
+
+
+       {
+    /** Comment */ p0: number
+    /** Comment */ p1: number
+    /** Comment */ p2: number
+} 
+
+
+let b: {
+    /** Comment */ p0: number
+    /** Comment */ p1: number
+    /** Comment */ p2: number
+};
+
+
+let a;
+let b: {
+    /** Comment */ p0: number
+    /** Comment */ p1: number
+    /** Comment */ p2: number
+};
+let c;
+
+================================================================================
+
+let a;
+let b: {
+    /** Comment */ p0: number
+    /** Comment */ /**/p1: number
+    /** Comment */ p2: number
+};
+let c;
+
+
+                   p1        
+
+
+                   p1: number
+
+
+    /** Comment */ p1: number
+
+
+        ↲
+••••/** Comment */ p0: number
+    /** Comment */ p1: number
+    /** Comment */ p2: number↲
+
+
+       {
+    /** Comment */ p0: number
+    /** Comment */ p1: number
+    /** Comment */ p2: number
+} 
+
+
+let b: {
+    /** Comment */ p0: number
+    /** Comment */ p1: number
+    /** Comment */ p2: number
+};
+
+
+let a;
+let b: {
+    /** Comment */ p0: number
+    /** Comment */ p1: number
+    /** Comment */ p2: number
+};
+let c;
+
+================================================================================
+
+let a;
+let b: {
+    /** Comment */ p0: number
+    /** Comment */ p1: number
+    /** Comment */ /**/p2: number
+};
+let c;
+
+
+                   p2        
+
+
+                   p2: number
+
+
+    /** Comment */ p2: number
+
+
+        ↲
+••••/** Comment */ p0: number
+    /** Comment */ p1: number
+    /** Comment */ p2: number↲
+
+
+       {
+    /** Comment */ p0: number
+    /** Comment */ p1: number
+    /** Comment */ p2: number
+} 
+
+
+let b: {
+    /** Comment */ p0: number
+    /** Comment */ p1: number
+    /** Comment */ p2: number
+};
+
+
+let a;
+let b: {
+    /** Comment */ p0: number
+    /** Comment */ p1: number
+    /** Comment */ p2: number
+};
+let c;
diff --git a/tests/cases/fourslash/smartSelection_JSDocTags11.ts b/tests/cases/fourslash/smartSelection_JSDocTags11.ts
@@ -0,0 +1,10 @@
+/// <reference path="fourslash.ts" />
+
+////const x = 1;
+////type Foo = {
+////  /** comment */
+////  /*2*/readonly /*1*/status: number;
+////};
+
+// https://github.com/microsoft/TypeScript/issues/39618
+verify.baselineSmartSelection();
diff --git a/tests/cases/fourslash/smartSelection_JSDocTags12.ts b/tests/cases/fourslash/smartSelection_JSDocTags12.ts
diff --git a/tests/cases/fourslash/smartSelection_JSDocTags13.ts b/tests/cases/fourslash/smartSelection_JSDocTags13.ts
