Replace semicolon-omitting writer with formatter preference

src/compiler/types.ts

@@ -6166,7 +6166,6 @@ namespace ts {
     export interface UserPreferences {
         readonly disableSuggestions?: boolean;
         readonly quotePreference?: "auto" | "double" | "single";
-        readonly semicolonPreference?: "auto" | "semicolons" | "no semicolons";
         readonly includeCompletionsForModuleExports?: boolean;
         readonly includeCompletionsWithInsertText?: boolean;
         readonly importModuleSpecifierPreference?: "relative" | "non-relative";

src/compiler/utilities.ts

@@ -3374,11 +3374,7 @@ namespace ts {
         };
     }
 
-    export interface TrailingSemicolonDeferringWriter extends EmitTextWriter {
-        resetPendingTrailingSemicolon(): void;
-    }
-
-    export function getTrailingSemicolonDeferringWriter(writer: EmitTextWriter): TrailingSemicolonDeferringWriter {
+    export function getTrailingSemicolonDeferringWriter(writer: EmitTextWriter): EmitTextWriter {
         let pendingTrailingSemicolon = false;
 
         function commitPendingTrailingSemicolon() {
@@ -3445,20 +3441,6 @@ namespace ts {
                 commitPendingTrailingSemicolon();
                 writer.decreaseIndent();
             },
-            resetPendingTrailingSemicolon() {
-                pendingTrailingSemicolon = false;
-            }
-        };
-    }
-
-    export function getTrailingSemicolonOmittingWriter(writer: EmitTextWriter): EmitTextWriter {
-        const deferringWriter = getTrailingSemicolonDeferringWriter(writer);
-        return {
-            ...deferringWriter,
-            writeLine() {
-                deferringWriter.resetPendingTrailingSemicolon();
-                writer.writeLine();
-            },
         };
     }
 

src/services/formatting/rules.ts

@@ -313,8 +313,8 @@ namespace ts.formatting {
 
             rule("SpaceBeforeTypeAnnotation", anyToken, SyntaxKind.ColonToken, [isOptionEnabled("insertSpaceBeforeTypeAnnotation"), isNonJsxSameLineTokenContext, isTypeAnnotationContext], RuleAction.Space),
             rule("NoSpaceBeforeTypeAnnotation", anyToken, SyntaxKind.ColonToken, [isOptionDisabledOrUndefined("insertSpaceBeforeTypeAnnotation"), isNonJsxSameLineTokenContext, isTypeAnnotationContext], RuleAction.DeleteTrivia),
-            rule("NoOptionalSemicolon", SyntaxKind.SemicolonToken, anyTokenIncludingEOF, [isOptionDisabled("insertTrailingSemicolon"), isSemicolonDeletionContext], RuleAction.DeleteToken),
-            rule("OptionalSemicolon", anyToken, anyTokenIncludingEOF, [isOptionEnabled("insertTrailingSemicolon"), isSemicolonInsertionContext], RuleAction.TrailingSemicolon),
+            rule("NoOptionalSemicolon", SyntaxKind.SemicolonToken, anyTokenIncludingEOF, [optionEquals("semicolonPreference", SemicolonPreference.Remove), isSemicolonDeletionContext], RuleAction.DeleteToken),
+            rule("OptionalSemicolon", anyToken, anyTokenIncludingEOF, [optionEquals("semicolonPreference", SemicolonPreference.Insert), isSemicolonInsertionContext], RuleAction.TrailingSemicolon),
         ];
 
         // These rules are lower in priority than user-configurable. Rules earlier in this list have priority over rules later in the list.
@@ -400,6 +400,10 @@ namespace ts.formatting {
     /// Contexts
     ///
 
+    function optionEquals<K extends keyof FormatCodeSettings>(optionName: K, optionValue: FormatCodeSettings[K]): (context: FormattingContext) => boolean {
+        return (context) => context.options && context.options[optionName] === optionValue;
+    }
+
     function isOptionEnabled(optionName: keyof FormatCodeSettings): (context: FormattingContext) => boolean {
         return (context) => context.options && context.options.hasOwnProperty(optionName) && !!context.options[optionName];
     }

src/services/textChanges.ts

@@ -236,7 +236,7 @@ namespace ts.textChanges {
         private readonly deletedNodes: { readonly sourceFile: SourceFile, readonly node: Node | NodeArray<TypeParameterDeclaration> }[] = [];
 
         public static fromContext(context: TextChangesContext): ChangeTracker {
-            return new ChangeTracker(getNewLineOrDefaultFromHost(context.host, context.formatContext.options), context.formatContext, context.preferences);
+            return new ChangeTracker(getNewLineOrDefaultFromHost(context.host, context.formatContext.options), context.formatContext);
         }
 
         public static with(context: TextChangesContext, cb: (tracker: ChangeTracker) => void): FileTextChanges[] {
@@ -246,7 +246,7 @@ namespace ts.textChanges {
         }
 
         /** Public for tests only. Other callers should use `ChangeTracker.with`. */
-        constructor(private readonly newLineCharacter: string, private readonly formatContext: formatting.FormatContext, private preferences: UserPreferences) {}
+        constructor(private readonly newLineCharacter: string, private readonly formatContext: formatting.FormatContext) {}
 
         public deleteRange(sourceFile: SourceFile, range: TextRange): void {
             this.changes.push({ kind: ChangeKind.Remove, sourceFile, range });
@@ -753,9 +753,9 @@ namespace ts.textChanges {
         public getChanges(validate?: ValidateNonFormattedText): FileTextChanges[] {
             this.finishDeleteDeclarations();
             this.finishClassesWithNodesInsertedAtStart();
-            const changes = changesToText.getTextChangesFromChanges(this.changes, this.newLineCharacter, this.formatContext, this.preferences, validate);
+            const changes = changesToText.getTextChangesFromChanges(this.changes, this.newLineCharacter, this.formatContext, validate);
             for (const { oldFile, fileName, statements } of this.newFiles) {
-                changes.push(changesToText.newFileChanges(oldFile, fileName, statements, this.newLineCharacter, this.formatContext, this.preferences));
+                changes.push(changesToText.newFileChanges(oldFile, fileName, statements, this.newLineCharacter, this.formatContext));
             }
             return changes;
         }
@@ -779,12 +779,12 @@ namespace ts.textChanges {
 
     export type ValidateNonFormattedText = (node: Node, text: string) => void;
 
-    export function getNewFileText(statements: ReadonlyArray<Statement>, scriptKind: ScriptKind, newLineCharacter: string, formatContext: formatting.FormatContext, preferences: UserPreferences): string {
-        return changesToText.newFileChangesWorker(/*oldFile*/ undefined, scriptKind, statements, newLineCharacter, formatContext, preferences);
+    export function getNewFileText(statements: ReadonlyArray<Statement>, scriptKind: ScriptKind, newLineCharacter: string, formatContext: formatting.FormatContext): string {
+        return changesToText.newFileChangesWorker(/*oldFile*/ undefined, scriptKind, statements, newLineCharacter, formatContext);
     }
 
     namespace changesToText {
-        export function getTextChangesFromChanges(changes: ReadonlyArray<Change>, newLineCharacter: string, formatContext: formatting.FormatContext, preferences: UserPreferences, validate: ValidateNonFormattedText | undefined): FileTextChanges[] {
+        export function getTextChangesFromChanges(changes: ReadonlyArray<Change>, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): FileTextChanges[] {
             return group(changes, c => c.sourceFile.path).map(changesInFile => {
                 const sourceFile = changesInFile[0].sourceFile;
                 // order changes by start position
@@ -796,25 +796,25 @@ namespace ts.textChanges {
                         `${JSON.stringify(normalized[i].range)} and ${JSON.stringify(normalized[i + 1].range)}`);
                 }
                 const textChanges = normalized.map(c =>
-                    createTextChange(createTextSpanFromRange(c.range), computeNewText(c, sourceFile, newLineCharacter, formatContext, preferences, validate)));
+                    createTextChange(createTextSpanFromRange(c.range), computeNewText(c, sourceFile, newLineCharacter, formatContext, validate)));
                 return { fileName: sourceFile.fileName, textChanges };
             });
         }
 
-        export function newFileChanges(oldFile: SourceFile | undefined, fileName: string, statements: ReadonlyArray<Statement>, newLineCharacter: string, formatContext: formatting.FormatContext, preferences: UserPreferences): FileTextChanges {
-            const text = newFileChangesWorker(oldFile, getScriptKindFromFileName(fileName), statements, newLineCharacter, formatContext, preferences);
+        export function newFileChanges(oldFile: SourceFile | undefined, fileName: string, statements: ReadonlyArray<Statement>, newLineCharacter: string, formatContext: formatting.FormatContext): FileTextChanges {
+            const text = newFileChangesWorker(oldFile, getScriptKindFromFileName(fileName), statements, newLineCharacter, formatContext);
             return { fileName, textChanges: [createTextChange(createTextSpan(0, 0), text)], isNewFile: true };
         }
 
-        export function newFileChangesWorker(oldFile: SourceFile | undefined, scriptKind: ScriptKind, statements: ReadonlyArray<Statement>, newLineCharacter: string, formatContext: formatting.FormatContext, preferences: UserPreferences): string {
+        export function newFileChangesWorker(oldFile: SourceFile | undefined, scriptKind: ScriptKind, statements: ReadonlyArray<Statement>, newLineCharacter: string, formatContext: formatting.FormatContext): string {
             // TODO: this emits the file, parses it back, then formats it that -- may be a less roundabout way to do this
-            const nonFormattedText = statements.map(s => getNonformattedText(s, oldFile, newLineCharacter, preferences).text).join(newLineCharacter);
+            const nonFormattedText = statements.map(s => getNonformattedText(s, oldFile, newLineCharacter).text).join(newLineCharacter);
             const sourceFile = createSourceFile("any file name", nonFormattedText, ScriptTarget.ESNext, /*setParentNodes*/ true, scriptKind);
             const changes = formatting.formatDocument(sourceFile, formatContext);
             return applyChanges(nonFormattedText, changes) + newLineCharacter;
         }
 
-        function computeNewText(change: Change, sourceFile: SourceFile, newLineCharacter: string, formatContext: formatting.FormatContext, preferences: UserPreferences, validate: ValidateNonFormattedText | undefined): string {
+        function computeNewText(change: Change, sourceFile: SourceFile, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): string {
             if (change.kind === ChangeKind.Remove) {
                 return "";
             }
@@ -823,7 +823,7 @@ namespace ts.textChanges {
             }
 
             const { options = {}, range: { pos } } = change;
-            const format = (n: Node) => getFormattedTextOfNode(n, sourceFile, pos, options, newLineCharacter, formatContext, preferences, validate);
+            const format = (n: Node) => getFormattedTextOfNode(n, sourceFile, pos, options, newLineCharacter, formatContext, validate);
             const text = change.kind === ChangeKind.ReplaceWithMultipleNodes
                 ? change.nodes.map(n => removeSuffix(format(n), newLineCharacter)).join(change.options!.joiner || newLineCharacter) // TODO: GH#18217
                 : format(change.node);
@@ -832,30 +832,37 @@ namespace ts.textChanges {
             return (options.prefix || "") + noIndent + (options.suffix || "");
         }
 
+        function getFormatCodeSettingsForWriting(context: formatting.FormatContext, sourceFile: SourceFile): FormatCodeSettings {
+            return {
+                ...context.options,
+                // If the user has no semicolon preference defined, try to match whatever’s in the source file
+                semicolonPreference: !context.options.semicolonPreference || context.options.semicolonPreference === SemicolonPreference.Ignore
+                    ? probablyUsesSemicolons(sourceFile) ? SemicolonPreference.Ignore : SemicolonPreference.Remove
+                    : context.options.semicolonPreference,
+            };
+        }
+
         /** Note: this may mutate `nodeIn`. */
-        function getFormattedTextOfNode(nodeIn: Node, sourceFile: SourceFile, pos: number, { indentation, prefix, delta }: InsertNodeOptions, newLineCharacter: string, formatContext: formatting.FormatContext, preferences: UserPreferences, validate: ValidateNonFormattedText | undefined): string {
-            const { node, text } = getNonformattedText(nodeIn, sourceFile, newLineCharacter, preferences);
+        function getFormattedTextOfNode(nodeIn: Node, sourceFile: SourceFile, pos: number, { indentation, prefix, delta }: InsertNodeOptions, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): string {
+            const { node, text } = getNonformattedText(nodeIn, sourceFile, newLineCharacter);
             if (validate) validate(node, text);
-            const { options: formatOptions } = formatContext;
+            const formatOptions = getFormatCodeSettingsForWriting(formatContext, sourceFile);
             const initialIndentation =
                 indentation !== undefined
                     ? indentation
                     : formatting.SmartIndenter.getIndentation(pos, sourceFile, formatOptions, prefix === newLineCharacter || getLineStartPositionForPosition(pos, sourceFile) === pos);
             if (delta === undefined) {
-                delta = formatting.SmartIndenter.shouldIndentChildNode(formatContext.options, nodeIn) ? (formatOptions.indentSize || 0) : 0;
+                delta = formatting.SmartIndenter.shouldIndentChildNode(formatOptions, nodeIn) ? (formatOptions.indentSize || 0) : 0;
             }
 
             const file: SourceFileLike = { text, getLineAndCharacterOfPosition(pos) { return getLineAndCharacterOfPosition(this, pos); } };
-            const changes = formatting.formatNodeGivenIndentation(node, file, sourceFile.languageVariant, initialIndentation, delta, formatContext);
+            const changes = formatting.formatNodeGivenIndentation(node, file, sourceFile.languageVariant, initialIndentation, delta, { ...formatContext, options: formatOptions });
             return applyChanges(text, changes);
         }
 
         /** Note: output node may be mutated input node. */
-        export function getNonformattedText(node: Node, sourceFile: SourceFile | undefined, newLineCharacter: string, { semicolonPreference }: UserPreferences): { text: string, node: Node } {
-            const omitTrailingSemicolon = semicolonPreference === "no semicolons" ? true :
-                semicolonPreference === "semicolons" ? false :
-                !!sourceFile && !probablyUsesSemicolons(sourceFile);
-            const writer = createWriter(newLineCharacter, omitTrailingSemicolon);
+        export function getNonformattedText(node: Node, sourceFile: SourceFile | undefined, newLineCharacter: string): { text: string, node: Node } {
+            const writer = createWriter(newLineCharacter);
             const newLine = newLineCharacter === "\n" ? NewLineKind.LineFeed : NewLineKind.CarriageReturnLineFeed;
             createPrinter({ newLine, neverAsciiEscape: true }, writer).writeNode(EmitHint.Unspecified, node, sourceFile, writer);
             return { text: writer.getText(), node: assignPositionsToNode(node) };
@@ -897,11 +904,11 @@ namespace ts.textChanges {
 
     interface TextChangesWriter extends EmitTextWriter, PrintHandlers {}
 
-    function createWriter(newLine: string, omitTrailingSemicolon?: boolean): TextChangesWriter {
+    function createWriter(newLine: string): TextChangesWriter {
         let lastNonTriviaPosition = 0;
 
 
-        const writer = omitTrailingSemicolon ? getTrailingSemicolonOmittingWriter(createTextWriter(newLine)) : createTextWriter(newLine);
+        const writer = createTextWriter(newLine);
         const onEmitNode: PrintHandlers["onEmitNode"] = (hint, node, printCallback) => {
             if (node) {
                 setPos(node, lastNonTriviaPosition);

src/services/types.ts

@@ -676,6 +676,12 @@ namespace ts {
         Smart = 2,
     }
 
+    export enum SemicolonPreference {
+        Ignore = "ignore",
+        Insert = "insert",
+        Remove = "remove",
+    }
+
     /* @deprecated - consider using EditorSettings instead */
     export interface EditorOptions {
         BaseIndentSize?: number;
@@ -734,7 +740,7 @@ namespace ts {
         readonly placeOpenBraceOnNewLineForControlBlocks?: boolean;
         readonly insertSpaceBeforeTypeAnnotation?: boolean;
         readonly indentMultiLineObjectLiteralBeginningOnBlankLine?: boolean;
-        readonly insertTrailingSemicolon?: boolean;
+        readonly semicolonPreference?: SemicolonPreference;
     }
 
     export function getDefaultFormatCodeSettings(newLineCharacter?: string): FormatCodeSettings {
@@ -758,7 +764,7 @@ namespace ts {
             insertSpaceBeforeFunctionParenthesis: false,
             placeOpenBraceOnNewLineForFunctions: false,
             placeOpenBraceOnNewLineForControlBlocks: false,
-            insertTrailingSemicolon: true,
+            semicolonPreference: SemicolonPreference.Ignore,
         };
     }
 

src/testRunner/unittests/services/textChanges.ts

@@ -49,7 +49,7 @@ namespace ts {
             it(caption, () => {
                 const sourceFile = createSourceFile("source.ts", text, ScriptTarget.ES2015, /*setParentNodes*/ true);
                 const rulesProvider = getRuleProvider(placeOpenBraceOnNewLineForFunctions);
-                const changeTracker = new textChanges.ChangeTracker(newLineCharacter, rulesProvider, {});
+                const changeTracker = new textChanges.ChangeTracker(newLineCharacter, rulesProvider);
                 testBlock(sourceFile, changeTracker);
                 const changes = changeTracker.getChanges(validateNodes ? verifyPositions : undefined);
                 assert.equal(changes.length, 1);