Improve generation of comments without location (#15037)

babel · Oct 17, 2022 · 9a0c496 · 9a0c496
1 parent ddaee90
commit 9a0c496
Show file tree

Hide file tree

Showing 7 changed files with 269 additions and 15 deletions.
diff --git a/packages/babel-generator/src/printer.ts b/packages/babel-generator/src/printer.ts
@@ -2,6 +2,12 @@ import Buffer from "./buffer";
 import type { Loc } from "./buffer";
 import * as n from "./node";
 import type * as t from "@babel/types";
+import {
+  isFunction,
+  isStatement,
+  isClassBody,
+  isTSInterfaceBody,
+} from "@babel/types";
 import type {
   RecordAndTuplePluginOptions,
   PipelineOperatorPluginOptions,
@@ -626,18 +632,18 @@ class Printer {
 
     this._lastCommentLine = 0;
 
-    this._printLeadingComments(node);
+    this._printLeadingComments(node, parent);
 
     const loc = nodeType === "Program" || nodeType === "File" ? null : node.loc;
 
     this.exactSource(loc, printMethod.bind(this, node, parent));
 
     if (noLineTerminator && !this._noLineTerminator) {
       this._noLineTerminator = true;
-      this._printTrailingComments(node);
+      this._printTrailingComments(node, parent);
       this._noLineTerminator = false;
     } else {
-      this._printTrailingComments(node, trailingCommentsLineOffset);
+      this._printTrailingComments(node, parent, trailingCommentsLineOffset);
     }
 
     if (shouldPrintParens) this.token(")");
@@ -768,16 +774,22 @@ class Printer {
     this.print(node, parent);
   }
 
-  _printTrailingComments(node: t.Node, lineOffset?: number) {
+  _printTrailingComments(node: t.Node, parent?: t.Node, lineOffset?: number) {
     const comments = this._getComments(false, node);
     if (!comments?.length) return;
-    this._printComments(COMMENT_TYPE.TRAILING, comments, node, lineOffset);
+    this._printComments(
+      COMMENT_TYPE.TRAILING,
+      comments,
+      node,
+      parent,
+      lineOffset,
+    );
   }
 
-  _printLeadingComments(node: t.Node) {
+  _printLeadingComments(node: t.Node, parent: t.Node) {
     const comments = this._getComments(true, node);
     if (!comments?.length) return;
-    this._printComments(COMMENT_TYPE.LEADING, comments, node);
+    this._printComments(COMMENT_TYPE.LEADING, comments, node, parent);
   }
 
   printInnerComments(node: t.Node, indent = true) {
@@ -936,6 +948,7 @@ class Printer {
     type: COMMENT_TYPE,
     comments: readonly t.Comment[],
     node: t.Node,
+    parent?: t.Node,
     lineOffset: number = 0,
   ) {
     {
@@ -1004,11 +1017,34 @@ class Printer {
           hasLoc = false;
 
           if (len === 1) {
-            this._printComment(comment, COMMENT_SKIP_NEWLINE.SKIP_ALL);
+            const singleLine = comment.loc
+              ? comment.loc.start.line === comment.loc.end.line
+              : !comment.value.includes("\n");
+
+            const shouldSkipNewline =
+              singleLine &&
+              !isStatement(node) &&
+              !isClassBody(parent) &&
+              !isTSInterfaceBody(parent);
+
+            if (type === COMMENT_TYPE.LEADING) {
+              this._printComment(
+                comment,
+                (shouldSkipNewline && node.type !== "ObjectExpression") ||
+                  (singleLine && isFunction(parent, { body: node }))
+                  ? COMMENT_SKIP_NEWLINE.SKIP_ALL
+                  : COMMENT_SKIP_NEWLINE.DEFAULT,
+              );
+            } else if (shouldSkipNewline && type === COMMENT_TYPE.TRAILING) {
+              this._printComment(comment, COMMENT_SKIP_NEWLINE.SKIP_ALL);
+            } else {
+              this._printComment(comment, COMMENT_SKIP_NEWLINE.DEFAULT);
+            }
           } else if (
             type === COMMENT_TYPE.INNER &&
             !(node.type === "ObjectExpression" && node.properties.length > 1) &&
-            node.type !== "ClassBody"
+            node.type !== "ClassBody" &&
+            node.type !== "TSInterfaceBody"
           ) {
             // class X {
             //   /*:: a: number*/

diff --git a/packages/babel-generator/test/index.js b/packages/babel-generator/test/index.js
@@ -483,6 +483,218 @@ describe("generation", function () {
 
     expect(generate(ast).code).toBe("/*#__PURE__*/\n/*#__PURE__*/");
   });
+
+  it("comments without loc", () => {
+    const ast = parse(
+      `
+        import {
+            Attribute,
+            AttributeSDKType
+        }
+        from "../../base/v1beta1/attribute";
+        import {
+            Rpc
+        }
+        from "../../../helpers";
+        import * as _m0 from "protobufjs/minimal";
+        import {
+            MsgSignProviderAttributes,
+            MsgSignProviderAttributesSDKType,
+            MsgSignProviderAttributesResponse,
+            MsgSignProviderAttributesResponseSDKType,
+            MsgDeleteProviderAttributes,
+            MsgDeleteProviderAttributesSDKType,
+            MsgDeleteProviderAttributesResponse,
+            MsgDeleteProviderAttributesResponseSDKType
+        }
+        from "./audit";
+        /** Msg defines the provider Msg service */
+        export interface Msg {
+            /** SignProviderAttributes defines a method that signs provider attributes */
+            signProviderAttributes(request: MsgSignProviderAttributes): Promise < MsgSignProviderAttributesResponse > ;
+            /** DeleteProviderAttributes defines a method that deletes provider attributes */
+            deleteProviderAttributes(request: MsgDeleteProviderAttributes): Promise < MsgDeleteProviderAttributesResponse > ;
+        }
+        export class MsgClientImpl implements Msg {
+            private readonly rpc: Rpc;
+            constructor(rpc: Rpc) {
+                this.rpc = rpc;
+            }
+            /* SignProviderAttributes defines a method that signs provider attributes */
+            signProviderAttributes = async(request: MsgSignProviderAttributes): Promise < MsgSignProviderAttributesResponse >  => {
+                const data = MsgSignProviderAttributes.encode(request).finish();
+                const promise = this.rpc.request("akash.audit.v1beta1.Msg", "SignProviderAttributes", data);
+                return promise.then(data => MsgSignProviderAttributesResponse.decode(new _m0.Reader(data)));
+            };
+            /* DeleteProviderAttributes defines a method that deletes provider attributes */
+            deleteProviderAttributes = async(request: MsgDeleteProviderAttributes): Promise < MsgDeleteProviderAttributesResponse >  => {
+                const data = MsgDeleteProviderAttributes.encode(request).finish();
+                const promise = this.rpc.request("akash.audit.v1beta1.Msg", "DeleteProviderAttributes", data);
+                return promise.then(data => MsgDeleteProviderAttributesResponse.decode(new _m0.Reader(data)));
+            };
+        }
+    `,
+      { sourceType: "module", plugins: ["typescript"] },
+    );
+
+    for (const comment of ast.comments) {
+      comment.loc = undefined;
+    }
+
+    expect(generate(ast).code).toMatchInlineSnapshot(`
+      "import { Attribute, AttributeSDKType } from \\"../../base/v1beta1/attribute\\";
+      import { Rpc } from \\"../../../helpers\\";
+      import * as _m0 from \\"protobufjs/minimal\\";
+      import { MsgSignProviderAttributes, MsgSignProviderAttributesSDKType, MsgSignProviderAttributesResponse, MsgSignProviderAttributesResponseSDKType, MsgDeleteProviderAttributes, MsgDeleteProviderAttributesSDKType, MsgDeleteProviderAttributesResponse, MsgDeleteProviderAttributesResponseSDKType } from \\"./audit\\";
+      /** Msg defines the provider Msg service */
+      export interface Msg {
+        /** SignProviderAttributes defines a method that signs provider attributes */
+        signProviderAttributes(request: MsgSignProviderAttributes): Promise<MsgSignProviderAttributesResponse>;
+        /** DeleteProviderAttributes defines a method that deletes provider attributes */
+        deleteProviderAttributes(request: MsgDeleteProviderAttributes): Promise<MsgDeleteProviderAttributesResponse>;
+      }
+      export class MsgClientImpl implements Msg {
+        private readonly rpc: Rpc;
+        constructor(rpc: Rpc) {
+          this.rpc = rpc;
+        }
+        /* SignProviderAttributes defines a method that signs provider attributes */
+        signProviderAttributes = async (request: MsgSignProviderAttributes): Promise<MsgSignProviderAttributesResponse> => {
+          const data = MsgSignProviderAttributes.encode(request).finish();
+          const promise = this.rpc.request(\\"akash.audit.v1beta1.Msg\\", \\"SignProviderAttributes\\", data);
+          return promise.then(data => MsgSignProviderAttributesResponse.decode(new _m0.Reader(data)));
+        };
+        /* DeleteProviderAttributes defines a method that deletes provider attributes */
+        deleteProviderAttributes = async (request: MsgDeleteProviderAttributes): Promise<MsgDeleteProviderAttributesResponse> => {
+          const data = MsgDeleteProviderAttributes.encode(request).finish();
+          const promise = this.rpc.request(\\"akash.audit.v1beta1.Msg\\", \\"DeleteProviderAttributes\\", data);
+          return promise.then(data => MsgDeleteProviderAttributesResponse.decode(new _m0.Reader(data)));
+        };
+      }"
+    `);
+  });
+
+  it("comments without loc2", () => {
+    const ast = parse(
+      `
+        (function (_templateFactory) {
+            "use strict";
+
+            const template = (0, _templateFactory.createTemplateFactory)(
+            /*{{somevalue}}*/
+            {
+                "id": null,
+                "block": "[[[1,[34,0]]],[],false,[\\"somevalue\\"]]",
+                "moduleName": "(unknown template module)",
+                "isStrictMode": false
+            });
+        });
+
+        const template = (0, _templateFactory.createTemplateFactory)(
+        /*
+          {{somevalue}}
+        */
+        {
+            "id": null,
+            "block": "[[[1,[34,0]]],[],false,[\\"somevalue\\"]]",
+            "moduleName": "(unknown template module)",
+            "isStrictMode": false
+        });
+      `,
+      { sourceType: "module" },
+    );
+
+    for (const comment of ast.comments) {
+      comment.loc = undefined;
+    }
+
+    expect(generate(ast).code).toMatchInlineSnapshot(`
+      "(function (_templateFactory) {
+        \\"use strict\\";
+
+        const template = (0, _templateFactory.createTemplateFactory)(
+        /*{{somevalue}}*/
+        {
+          \\"id\\": null,
+          \\"block\\": \\"[[[1,[34,0]]],[],false,[\\\\\\"somevalue\\\\\\"]]\\",
+          \\"moduleName\\": \\"(unknown template module)\\",
+          \\"isStrictMode\\": false
+        });
+      });
+      const template = (0, _templateFactory.createTemplateFactory)(
+      /*
+                {{somevalue}}
+              */
+      {
+        \\"id\\": null,
+        \\"block\\": \\"[[[1,[34,0]]],[],false,[\\\\\\"somevalue\\\\\\"]]\\",
+        \\"moduleName\\": \\"(unknown template module)\\",
+        \\"isStrictMode\\": false
+      });"
+    `);
+  });
+
+  it("comments without node.loc", () => {
+    const ast = parse(
+      `
+        (function (_templateFactory) {
+            "use strict";
+
+            const template = (0, _templateFactory.createTemplateFactory)(
+            /*{{somevalue}}*/
+            {
+                "id": null,
+                "block": "[[[1,[34,0]]],[],false,[\\"somevalue\\"]]",
+                "moduleName": "(unknown template module)",
+                "isStrictMode": false
+            });
+        });
+
+        const template = (0, _templateFactory.createTemplateFactory)(
+        /*
+          {{somevalue}}
+        */
+        {
+            "id": null,
+            "block": "[[[1,[34,0]]],[],false,[\\"somevalue\\"]]",
+            "moduleName": "(unknown template module)",
+            "isStrictMode": false
+        });
+      `,
+      { sourceType: "module" },
+    );
+
+    const ast2 = t.cloneNode(ast, true, true);
+
+    for (let i = 0; i < ast.comments.length; i++) {
+      ast2.comments[i].loc = ast.comments[i].loc;
+    }
+
+    expect(generate(ast2).code).toMatchInlineSnapshot(`
+      "(function (_templateFactory) {
+        \\"use strict\\";
+
+        const template = (0, _templateFactory.createTemplateFactory)(
+        /*{{somevalue}}*/
+        {
+          \\"id\\": null,
+          \\"block\\": \\"[[[1,[34,0]]],[],false,[\\\\\\"somevalue\\\\\\"]]\\",
+          \\"moduleName\\": \\"(unknown template module)\\",
+          \\"isStrictMode\\": false
+        });
+      });
+      const template = (0, _templateFactory.createTemplateFactory)(
+      /*
+        {{somevalue}}
+      */
+      {
+        \\"id\\": null,
+        \\"block\\": \\"[[[1,[34,0]]],[],false,[\\\\\\"somevalue\\\\\\"]]\\",
+        \\"moduleName\\": \\"(unknown template module)\\",
+        \\"isStrictMode\\": false
+      });"
+    `);
+  });
 });
 
 describe("programmatic generation", function () {

diff --git a/...el-plugin-transform-flow-comments/test/fixtures/flow-comments/class-prop-values/output.js b/...el-plugin-transform-flow-comments/test/fixtures/flow-comments/class-prop-values/output.js
@@ -1,4 +1,5 @@
 class X {
   foo = 2;
-  bar /*: number*/ = 3; /*:: baz: ?string*/
+  bar /*: number*/ = 3;
+  /*:: baz: ?string*/
 }
diff --git a/...el-plugin-transform-flow-comments/test/fixtures/flow-comments/def-site-variance/output.js b/...el-plugin-transform-flow-comments/test/fixtures/flow-comments/def-site-variance/output.js
@@ -1,2 +1,3 @@
 class C /*:: <+T, -U>*/{}
-function f /*:: <+T, -U>*/() {} /*:: type T<+T, -U> = {};*/
+function f /*:: <+T, -U>*/() {}
+/*:: type T<+T, -U> = {};*/
diff --git a/...babel-plugin-transform-flow-comments/test/fixtures/flow-comments/import-export/output.mjs b/...babel-plugin-transform-flow-comments/test/fixtures/flow-comments/import-export/output.mjs
@@ -1,2 +1,4 @@
-/*:: export type { A } from './types';*/import lib from 'library';
-export { foo } from 'foo'; /*:: export type { B, C } from './types';*/
+/*:: export type { A } from './types';*/
+import lib from 'library';
+export { foo } from 'foo';
+/*:: export type { B, C } from './types';*/
diff --git a/...plugin-transform-flow-comments/test/fixtures/flow-comments/import-type-alias-2/output.mjs b/...plugin-transform-flow-comments/test/fixtures/flow-comments/import-type-alias-2/output.mjs
@@ -1,4 +1,5 @@
-import A, { B, E } from './types'; /*:: import { type C, type D } from './types';*/
+import A, { B, E } from './types';
+/*:: import { type C, type D } from './types';*/
 import F, { H } from './types';
 /*:: import { typeof G, type I } from './types';*/
 /*:: import { type J } from './types';*/

diff --git a/...l-plugin-transform-flow-comments/test/fixtures/flow-comments/import-type-alias/output.mjs b/...l-plugin-transform-flow-comments/test/fixtures/flow-comments/import-type-alias/output.mjs
@@ -1,3 +1,4 @@
-/*:: import type A, { B, C } from './types';*/import lib from 'library';
+/*:: import type A, { B, C } from './types';*/
+import lib from 'library';
 /*:: import type D from './types';*/
 /*:: import typeof E, { F, G } from './types';*/