Skip to content
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

Fix writing multi-line text without semicolons #32903

Merged
merged 2 commits into from
Aug 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3506,7 +3506,7 @@ namespace ts {
const sig = nodeBuilder.signatureToSignatureDeclaration(signature, sigOutput, enclosingDeclaration, toNodeBuilderFlags(flags) | NodeBuilderFlags.IgnoreErrors | NodeBuilderFlags.WriteTypeParametersInQualifiedName);
const printer = createPrinter({ removeComments: true, omitTrailingSemicolon: true });
const sourceFile = enclosingDeclaration && getSourceFileOfNode(enclosingDeclaration);
printer.writeNode(EmitHint.Unspecified, sig!, /*sourceFile*/ sourceFile, getTrailingSemicolonOmittingWriter(writer)); // TODO: GH#18217
printer.writeNode(EmitHint.Unspecified, sig!, /*sourceFile*/ sourceFile, getTrailingSemicolonDeferringWriter(writer)); // TODO: GH#18217
return writer;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1056,7 +1056,7 @@ namespace ts {

function setWriter(_writer: EmitTextWriter | undefined, _sourceMapGenerator: SourceMapGenerator | undefined) {
if (_writer && printerOptions.omitTrailingSemicolon) {
_writer = getTrailingSemicolonOmittingWriter(_writer);
_writer = getTrailingSemicolonDeferringWriter(_writer);
}

writer = _writer!; // TODO: GH#18217
Expand Down Expand Up @@ -2514,7 +2514,7 @@ namespace ts {
}

emitWhileClause(node, node.statement.end);
writePunctuation(";");
writeTrailingSemicolon();
}

function emitWhileStatement(node: WhileStatement) {
Expand Down
20 changes: 19 additions & 1 deletion src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3369,7 +3369,11 @@ namespace ts {
};
}

export function getTrailingSemicolonOmittingWriter(writer: EmitTextWriter): EmitTextWriter {
export interface TrailingSemicolonDeferringWriter extends EmitTextWriter {
resetPendingTrailingSemicolon(): void;
}

export function getTrailingSemicolonDeferringWriter(writer: EmitTextWriter): TrailingSemicolonDeferringWriter {
let pendingTrailingSemicolon = false;

function commitPendingTrailingSemicolon() {
Expand Down Expand Up @@ -3435,10 +3439,24 @@ namespace ts {
decreaseIndent() {
commitPendingTrailingSemicolon();
writer.decreaseIndent();
},
resetPendingTrailingSemicolon() {
pendingTrailingSemicolon = false;
}
};
}

export function getTrailingSemicolonOmittingWriter(writer: EmitTextWriter): EmitTextWriter {
const deferringWriter = getTrailingSemicolonDeferringWriter(writer);
return {
...deferringWriter,
writeLine() {
deferringWriter.resetPendingTrailingSemicolon();
writer.writeLine();
},
};
}

export function getResolvedExternalModuleName(host: EmitHost, file: SourceFile, referenceFile?: SourceFile): string {
return file.moduleName || getExternalModuleNameFromPath(host, file.fileName, referenceFile && referenceFile.fileName);
}
Expand Down
2 changes: 1 addition & 1 deletion src/services/textChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,7 @@ namespace ts.textChanges {
const omitTrailingSemicolon = !!sourceFile && !probablyUsesSemicolons(sourceFile);
const writer = createWriter(newLineCharacter, omitTrailingSemicolon);
const newLine = newLineCharacter === "\n" ? NewLineKind.LineFeed : NewLineKind.CarriageReturnLineFeed;
createPrinter({ newLine, neverAsciiEscape: true, omitTrailingSemicolon }, writer).writeNode(EmitHint.Unspecified, node, sourceFile, writer);
createPrinter({ newLine, neverAsciiEscape: true }, writer).writeNode(EmitHint.Unspecified, node, sourceFile, writer);
return { text: writer.getText(), node: assignPositionsToNode(node) };
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ verify.codeFix({
constructor() {
}
foo() {
({ bar: () => { } });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know much about our emit with respect to formatting. Did we detect that the input didn't have semicolons, so the output shouldn't either?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. That change was in #31801, but this was a case that didn’t work until now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this only applies to edits made with TextChanges—JS emit to file always uses semicolons.

({ bar: () => { } })
}
}
`,
Expand Down
2 changes: 1 addition & 1 deletion tests/cases/fourslash/extract-method25.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ edit.applyRefactor({
q[0]++

function newFunction() {
return [0];
return [0]
}
}`
});