Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Sep 15, 2023
5 changes: 5 additions & 0 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/assignmentCompatBug3.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
},
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/circularResolvedSignature.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
18 changes: 18 additions & 0 deletions tests/baselines/reference/commentsOnObjectLiteral5.js
Original file line number Diff line number Diff line change
@@ -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
};
18 changes: 18 additions & 0 deletions tests/baselines/reference/commentsOnObjectLiteral5.symbols
Original file line number Diff line number Diff line change
@@ -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))

};

22 changes: 22 additions & 0 deletions tests/baselines/reference/commentsOnObjectLiteral5.types
Original file line number Diff line number Diff line change
@@ -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

};

4 changes: 2 additions & 2 deletions tests/baselines/reference/divergentAccessorsTypes6.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) { },
};

Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/duplicateObjectLiteralProperty.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const repro: BugRepro = {
var repro = {
dataType: {
fields: [{
key: 'bla',
key: 'bla', // should be OK: Not excess
value: null,
}],
}
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/indexSignatures1.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/namespaceImportTypeQuery2.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@ exports.B = B;
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var t = {
A: undefined,
A: undefined, // ok
B: undefined,
};
2 changes: 1 addition & 1 deletion tests/baselines/reference/namespaceImportTypeQuery3.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ exports.B = B;
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var t = {
A: undefined,
A: undefined, // ok
B: undefined,
};
2 changes: 1 addition & 1 deletion tests/baselines/reference/namespaceImportTypeQuery4.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ exports.B = B;
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var t = {
A: undefined,
A: undefined, // error
B: undefined,
};
2 changes: 1 addition & 1 deletion tests/baselines/reference/objectLitArrayDeclNoNew.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ var x = {

//// [objectLiteralShorthandPropertiesErrorFromNoneExistingIdentifier.js]
var x = {
x: x,
x: x, // OK
undefinedVariable: undefinedVariable // Error
};
8 changes: 4 additions & 4 deletions tests/baselines/reference/privateNameBadDeclaration.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -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 () {
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/thisTypeInFunctions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/thisTypeInFunctionsNegative.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
};
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 9 additions & 0 deletions tests/cases/compiler/commentsOnObjectLiteral5.ts
Original file line number Diff line number Diff line change
@@ -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
};