🐛 Fix not emitting comments between sibling fields of object literals (#50097)

Co-authored-by: Jake Bailey <[email protected]>

Author: babakks

src/compiler/emitter.ts

@@ -5121,6 +5121,11 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri
                         shouldDecreaseIndentAfterEmit = true;
                     }
 
+                    if (shouldEmitInterveningComments && format & ListFormat.DelimitersMask && !positionIsSynthesized(child.pos)) {
+                        const commentRange = getCommentRange(child);
+                        emitTrailingCommentsOfPosition(commentRange.pos, /*prefixSpace*/ !!(format & ListFormat.SpaceBetweenSiblings), /*forceNoNewline*/ true);
+                    }
+
                     writeLine(separatingLineTerminatorCount);
                     shouldEmitInterveningComments = false;
                 }

tests/baselines/reference/arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.js

@@ -20,7 +20,7 @@ const test = () => ({
 //// [arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.js]
 var test = function () { return ({
     // "Identifier expected." error on "!" and two "Duplicate identifier '(Missing)'." errors on space.
-    prop: !value,
+    prop: !value, // remove ! to see that errors will be gone
     run: function () {
         // comment next line or remove "()" to see that errors will be gone
         if (!a.b()) {

tests/baselines/reference/assignmentCompatBug3.js

@@ -30,8 +30,8 @@ foo(x + y);
 //// [assignmentCompatBug3.js]
 function makePoint(x, y) {
     return {
-        get x() { return x; },
-        get y() { return y; },
+        get x() { return x; }, // shouldn't be "void"
+        get y() { return y; }, // shouldn't be "void"
         //x: "yo",
         //y: "boo",
         dist: function () {

tests/baselines/reference/bindingPatternCannotBeOnlyInferenceSource.js

@@ -45,7 +45,7 @@ var _d = useReduxDispatch1(function (d, f) { return ({
             p[_i] = arguments[_i];
         }
         return d(f.funcA.apply(f, p));
-    },
+    }, // p should be inferrable
     funcB: function () {
         var p = [];
         for (var _i = 0; _i < arguments.length; _i++) {

tests/baselines/reference/callSignaturesWithParameterInitializers2.js

@@ -49,7 +49,7 @@ c.foo(1);
 var b = {
     foo: function (x) {
         if (x === void 0) { x = 1; }
-    },
+    }, // error
     foo: function (x) {
         if (x === void 0) { x = 1; }
     },

tests/baselines/reference/circularResolvedSignature.js

@@ -24,7 +24,7 @@ Object.defineProperty(exports, "__esModule", { value: true });
 exports.Component = void 0;
 function Component() {
     var _a = useState(function () { return ({
-        value: "string",
+        value: "string", // this should be a number
         foo: function (arg) { return setState(arg); },
         bar: function (arg) { return setState(arg); },
     }); }), state = _a[0], setState = _a[1];

tests/baselines/reference/commentsOnObjectLiteral5.js

@@ -0,0 +1,18 @@
+//// [tests/cases/compiler/commentsOnObjectLiteral5.ts] ////
+
+//// [commentsOnObjectLiteral5.ts]
+const a = {
+    p0: 0, // Comment 0
+    p1: 0, /* Comment 1
+    A multiline comment. */
+    p2: 0, // Comment 2
+};
+
+
+//// [commentsOnObjectLiteral5.js]
+var a = {
+    p0: 0, // Comment 0
+    p1: 0, /* Comment 1
+    A multiline comment. */
+    p2: 0, // Comment 2
+};

tests/baselines/reference/commentsOnObjectLiteral5.symbols

@@ -0,0 +1,18 @@
+//// [tests/cases/compiler/commentsOnObjectLiteral5.ts] ////
+
+=== commentsOnObjectLiteral5.ts ===
+const a = {
+>a : Symbol(a, Decl(commentsOnObjectLiteral5.ts, 0, 5))
+
+    p0: 0, // Comment 0
+>p0 : Symbol(p0, Decl(commentsOnObjectLiteral5.ts, 0, 11))
+
+    p1: 0, /* Comment 1
+>p1 : Symbol(p1, Decl(commentsOnObjectLiteral5.ts, 1, 10))
+
+    A multiline comment. */
+    p2: 0, // Comment 2
+>p2 : Symbol(p2, Decl(commentsOnObjectLiteral5.ts, 2, 10))
+
+};
+

tests/baselines/reference/commentsOnObjectLiteral5.types

@@ -0,0 +1,22 @@
+//// [tests/cases/compiler/commentsOnObjectLiteral5.ts] ////
+
+=== commentsOnObjectLiteral5.ts ===
+const a = {
+>a : { p0: number; p1: number; p2: number; }
+>{    p0: 0, // Comment 0    p1: 0, /* Comment 1    A multiline comment. */    p2: 0, // Comment 2} : { p0: number; p1: number; p2: number; }
+
+    p0: 0, // Comment 0
+>p0 : number
+>0 : 0
+
+    p1: 0, /* Comment 1
+>p1 : number
+>0 : 0
+
+    A multiline comment. */
+    p2: 0, // Comment 2
+>p2 : number
+>0 : 0
+
+};
+

tests/baselines/reference/divergentAccessorsTypes6.js

@@ -49,9 +49,9 @@ var o1 = {
 };
 // A setter annotation still implies the getter return type.
 var o2 = {
-    get p1() { return 0; },
+    get p1() { return 0; }, // error - no annotation means type is implied from the setter annotation
     set p1(value) { },
-    get p2() { return 0; },
+    get p2() { return 0; }, // ok - explicit annotation
     set p2(value) { },
 };
 

tests/baselines/reference/duplicateObjectLiteralProperty.js

@@ -23,9 +23,9 @@ var y = {
 //// [duplicateObjectLiteralProperty.js]
 var x = {
     a: 1,
-    b: true,
-    a: 56,
-    \u0061: "ss",
+    b: true, // OK
+    a: 56, // Duplicate
+    \u0061: "ss", // Duplicate
     a: {
         c: 1,
         "c": 56, // Duplicate

tests/baselines/reference/excessPropertyCheckWithMultipleDiscriminants.js

@@ -149,7 +149,7 @@ Object.defineProperty(exports, "__esModule", { value: true });
 var foo = {
     type: "number",
     value: 10,
-    multipleOf: 5,
+    multipleOf: 5, // excess property
     format: "what?"
 };
 // This has excess error because variant three is the only applicable case.

tests/baselines/reference/excessPropertyCheckWithNestedArrayIntersection.js

@@ -29,7 +29,7 @@ const repro: BugRepro = {
 var repro = {
     dataType: {
         fields: [{
-                key: 'bla',
+                key: 'bla', // should be OK: Not excess
                 value: null,
             }],
     }

tests/baselines/reference/indexSignatures1.js

@@ -356,7 +356,7 @@ const y2 = dom.data123;
 dom = { data123: 'hello' };
 dom = { date123: 'hello' }; // Error
 const funcs = {
-    sfoo: x => x.length,
+    sfoo: x => x.length, // x: string
     nfoo: x => x * 2, // n: number
 };
 i1[s0]; // Error

tests/baselines/reference/namespaceImportTypeQuery2.js

@@ -38,6 +38,6 @@ exports.B = B;
 "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
 var t = {
-    A: undefined,
+    A: undefined, // ok
     B: undefined,
 };

tests/baselines/reference/namespaceImportTypeQuery3.js

@@ -31,6 +31,6 @@ exports.B = B;
 "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
 var t = {
-    A: undefined,
+    A: undefined, // ok
     B: undefined,
 };

tests/baselines/reference/namespaceImportTypeQuery4.js

@@ -29,6 +29,6 @@ exports.B = B;
 "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
 var t = {
-    A: undefined,
+    A: undefined, // error
     B: undefined,
 };

tests/baselines/reference/objectLitArrayDeclNoNew.js

@@ -43,7 +43,7 @@ var Test;
     function bug() {
         var state = null;
         return {
-            tokens: Gar[],
+            tokens: Gar[], //IToken[],  // Missing new. Correct syntax is: tokens: new IToken[]
             endState: state
         };
     }

tests/baselines/reference/objectLiteralShorthandPropertiesErrorFromNoneExistingIdentifier.js

@@ -9,6 +9,6 @@ var x = {
 
 //// [objectLiteralShorthandPropertiesErrorFromNoneExistingIdentifier.js]
 var x = {
-    x: x,
+    x: x, // OK
     undefinedVariable: undefinedVariable // Error
 };

tests/baselines/reference/privateNameBadDeclaration.js

@@ -22,8 +22,8 @@ class C {
 //// [privateNameBadDeclaration.js]
 function A() { }
 A.prototype = {
-    : 1,
-    : function () { },
+    : 1, // Error
+    : function () { }, // Error
     get () { return ""; } // Error
 };
 var B = /** @class */ (function () {
@@ -32,8 +32,8 @@ var B = /** @class */ (function () {
     return B;
 }());
 B.prototype = {
-    : 2,
-    : function () { },
+    : 2, // Error
+    : function () { }, // Error
     get () { return ""; } // Error
 };
 var C = /** @class */ (function () {

tests/baselines/reference/thisTypeInFunctions.js

@@ -254,7 +254,7 @@ function implicitThis(n) {
 }
 var impl = {
     a: 12,
-    explicitVoid2: function () { return _this.a; },
+    explicitVoid2: function () { return _this.a; }, // ok, this: any because it refers to some outer object (window?)
     explicitVoid1: function () { return 12; },
     explicitStructural: function () {
         return this.a;

tests/baselines/reference/thisTypeInFunctionsNegative.js

@@ -227,7 +227,7 @@ let impl = {
     explicitVoid1() {
         return this.a; // error, no 'a' in 'void'
     },
-    explicitVoid2: () => this.a,
+    explicitVoid2: () => this.a, // ok, `this:any` because it refers to an outer object
     explicitStructural: () => 12,
     explicitInterface: () => 12,
     explicitThis() {

tests/baselines/reference/typeSatisfaction_contextualTyping2.js

@@ -13,7 +13,7 @@ let obj: { f(s: string): void } & Record<string, unknown> = {
 //// [typeSatisfaction_contextualTyping2.js]
 "use strict";
 var obj = {
-    f: function (s) { },
+    f: function (s) { }, // "incorrect" implicit any on 's'
     g: function (s) { }
 };
 // This needs to not crash (outer node is not expression)

tests/baselines/reference/typeSatisfaction_propertyValueConformance3.js

@@ -18,6 +18,6 @@ exports.Palette = void 0;
 // All of these should be Colors, but I only use some of them here.
 exports.Palette = {
     white: { r: 255, g: 255, b: 255 },
-    black: { r: 0, g: 0, d: 0 },
+    black: { r: 0, g: 0, d: 0 }, // <- oops! 'd' in place of 'b'
     blue: { r: 0, g: 0, b: 255 },
 };

tests/baselines/reference/variableDeclaratorResolvedDuringContextualTyping.js

@@ -153,7 +153,7 @@ var FileService = /** @class */ (function () {
             data: "someData"
         }).then(function (response) {
             var result = {
-                stat: _this.jsonToStat(newFilePath, "someString"),
+                stat: _this.jsonToStat(newFilePath, "someString"), // _this needs to be emitted to the js file
                 isNew: response.status === 201
             };
             return WinJS.TPromise.as(result);

tests/cases/compiler/commentsOnObjectLiteral5.ts

@@ -0,0 +1,9 @@
+// @removeComments: false
+// @target: ES5
+
+const a = {
+    p0: 0, // Comment 0
+    p1: 0, /* Comment 1
+    A multiline comment. */
+    p2: 0, // Comment 2
+};