fix(39332): handle quotes preference in interface method signatures

a-tarasyuk · a-tarasyuk · commit c8061d024094 · 2020-07-03T12:27:53.000+03:00
diff --git a/src/services/codefixes/fixClassDoesntImplementInheritedAbstractMember.ts b/src/services/codefixes/fixClassDoesntImplementInheritedAbstractMember.ts
@@ -42,7 +42,7 @@ namespace ts.codefix {
         const abstractAndNonPrivateExtendsSymbols = checker.getPropertiesOfType(instantiatedExtendsType).filter(symbolPointsToNonPrivateAndAbstractMember);
 
         const importAdder = createImportAdder(sourceFile, context.program, preferences, context.host);
-        createMissingMemberNodes(classDeclaration, abstractAndNonPrivateExtendsSymbols, context, preferences, importAdder, member => changeTracker.insertNodeAtClassStart(sourceFile, classDeclaration, member));
+        createMissingMemberNodes(classDeclaration, abstractAndNonPrivateExtendsSymbols, sourceFile, context, preferences, importAdder, member => changeTracker.insertNodeAtClassStart(sourceFile, classDeclaration, member));
         importAdder.writeFixes(changeTracker);
     }
 
diff --git a/src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts b/src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts
@@ -64,7 +64,7 @@ namespace ts.codefix {
         }
 
         const importAdder = createImportAdder(sourceFile, context.program, preferences, context.host);
-        createMissingMemberNodes(classDeclaration, nonPrivateAndNotExistedInHeritageClauseMembers, context, preferences, importAdder, member => insertInterfaceMemberNode(sourceFile, classDeclaration, member));
+        createMissingMemberNodes(classDeclaration, nonPrivateAndNotExistedInHeritageClauseMembers, sourceFile, context, preferences, importAdder, member => insertInterfaceMemberNode(sourceFile, classDeclaration, member));
         importAdder.writeFixes(changeTracker);
 
         function createMissingIndexSignatureDeclaration(type: InterfaceType, kind: IndexKind): void {
diff --git a/src/services/codefixes/helpers.ts b/src/services/codefixes/helpers.ts
@@ -7,11 +7,11 @@ namespace ts.codefix {
      * @param importAdder If provided, type annotations will use identifier type references instead of ImportTypeNodes, and the missing imports will be added to the importAdder.
      * @returns Empty string iff there are no member insertions.
      */
-    export function createMissingMemberNodes(classDeclaration: ClassLikeDeclaration, possiblyMissingSymbols: readonly Symbol[], context: TypeConstructionContext, preferences: UserPreferences, importAdder: ImportAdder | undefined, addClassElement: (node: ClassElement) => void): void {
+    export function createMissingMemberNodes(classDeclaration: ClassLikeDeclaration, possiblyMissingSymbols: readonly Symbol[], sourceFile: SourceFile, context: TypeConstructionContext, preferences: UserPreferences, importAdder: ImportAdder | undefined, addClassElement: (node: ClassElement) => void): void {
         const classMembers = classDeclaration.symbol.members!;
         for (const symbol of possiblyMissingSymbols) {
             if (!classMembers.has(symbol.escapedName)) {
-                addNewNodeForMemberSymbol(symbol, classDeclaration, context, preferences, importAdder, addClassElement);
+                addNewNodeForMemberSymbol(symbol, classDeclaration, sourceFile, context, preferences, importAdder, addClassElement);
             }
         }
     }
@@ -31,7 +31,7 @@ namespace ts.codefix {
     /**
      * @returns Empty string iff there we can't figure out a representation for `symbol` in `enclosingDeclaration`.
      */
-    function addNewNodeForMemberSymbol(symbol: Symbol, enclosingDeclaration: ClassLikeDeclaration, context: TypeConstructionContext, preferences: UserPreferences, importAdder: ImportAdder | undefined, addClassElement: (node: Node) => void): void {
+    function addNewNodeForMemberSymbol(symbol: Symbol, enclosingDeclaration: ClassLikeDeclaration, sourceFile: SourceFile, context: TypeConstructionContext, preferences: UserPreferences, importAdder: ImportAdder | undefined, addClassElement: (node: Node) => void): void {
         const declarations = symbol.getDeclarations();
         if (!(declarations && declarations.length)) {
             return undefined;
@@ -45,11 +45,12 @@ namespace ts.codefix {
         const type = checker.getWidenedType(checker.getTypeOfSymbolAtLocation(symbol, enclosingDeclaration));
         const optional = !!(symbol.flags & SymbolFlags.Optional);
         const ambient = !!(enclosingDeclaration.flags & NodeFlags.Ambient);
+        const quotePreference = getQuotePreference(sourceFile, preferences);
 
         switch (declaration.kind) {
             case SyntaxKind.PropertySignature:
             case SyntaxKind.PropertyDeclaration:
-                const flags = preferences.quotePreference === "single" ? NodeBuilderFlags.UseSingleQuotesForStringLiteralType : undefined;
+                const flags = quotePreference === QuotePreference.Single ? NodeBuilderFlags.UseSingleQuotesForStringLiteralType : undefined;
                 let typeNode = checker.typeToTypeNode(type, enclosingDeclaration, flags, getNoopSymbolTrackerWithResolver(context));
                 if (importAdder) {
                     const importableReference = tryGetAutoImportableReferenceFromImportTypeNode(typeNode, type, scriptTarget);
@@ -88,7 +89,7 @@ namespace ts.codefix {
                             name,
                             emptyArray,
                             typeNode,
-                            ambient ? undefined : createStubbedMethodBody(preferences)));
+                            ambient ? undefined : createStubbedMethodBody(quotePreference)));
                     }
                     else {
                         Debug.assertNode(accessor, isSetAccessorDeclaration, "The counterpart to a getter should be a setter");
@@ -99,7 +100,7 @@ namespace ts.codefix {
                             modifiers,
                             name,
                             createDummyParameters(1, [parameterName], [typeNode], 1, /*inJs*/ false),
-                            ambient ? undefined : createStubbedMethodBody(preferences)));
+                            ambient ? undefined : createStubbedMethodBody(quotePreference)));
                     }
                 }
                 break;
@@ -121,36 +122,37 @@ namespace ts.codefix {
                 if (declarations.length === 1) {
                     Debug.assert(signatures.length === 1, "One declaration implies one signature");
                     const signature = signatures[0];
-                    outputMethod(signature, modifiers, name, ambient ? undefined : createStubbedMethodBody(preferences));
+                    outputMethod(quotePreference, signature, modifiers, name, ambient ? undefined : createStubbedMethodBody(quotePreference));
                     break;
                 }
 
                 for (const signature of signatures) {
                     // Need to ensure nodes are fresh each time so they can have different positions.
-                    outputMethod(signature, getSynthesizedDeepClones(modifiers, /*includeTrivia*/ false), getSynthesizedDeepClone(name, /*includeTrivia*/ false));
+                    outputMethod(quotePreference, signature, getSynthesizedDeepClones(modifiers, /*includeTrivia*/ false), getSynthesizedDeepClone(name, /*includeTrivia*/ false));
                 }
 
                 if (!ambient) {
                     if (declarations.length > signatures.length) {
                         const signature = checker.getSignatureFromDeclaration(declarations[declarations.length - 1] as SignatureDeclaration)!;
-                        outputMethod(signature, modifiers, name, createStubbedMethodBody(preferences));
+                        outputMethod(quotePreference, signature, modifiers, name, createStubbedMethodBody(quotePreference));
                     }
                     else {
                         Debug.assert(declarations.length === signatures.length, "Declarations and signatures should match count");
-                        addClassElement(createMethodImplementingSignatures(signatures, name, optional, modifiers, preferences));
+                        addClassElement(createMethodImplementingSignatures(signatures, name, optional, modifiers, quotePreference));
                     }
                 }
                 break;
         }
 
-        function outputMethod(signature: Signature, modifiers: NodeArray<Modifier> | undefined, name: PropertyName, body?: Block): void {
-            const method = signatureToMethodDeclaration(context, signature, enclosingDeclaration, modifiers, name, optional, body, importAdder);
+        function outputMethod(quotePreference: QuotePreference, signature: Signature, modifiers: NodeArray<Modifier> | undefined, name: PropertyName, body?: Block): void {
+            const method = signatureToMethodDeclaration(context, quotePreference, signature, enclosingDeclaration, modifiers, name, optional, body, importAdder);
             if (method) addClassElement(method);
         }
     }
 
     function signatureToMethodDeclaration(
         context: TypeConstructionContext,
+        quotePreference: QuotePreference,
         signature: Signature,
         enclosingDeclaration: ClassLikeDeclaration,
         modifiers: NodeArray<Modifier> | undefined,
@@ -162,7 +164,8 @@ namespace ts.codefix {
         const program = context.program;
         const checker = program.getTypeChecker();
         const scriptTarget = getEmitScriptTarget(program.getCompilerOptions());
-        const signatureDeclaration = <MethodDeclaration>checker.signatureToSignatureDeclaration(signature, SyntaxKind.MethodDeclaration, enclosingDeclaration, NodeBuilderFlags.NoTruncation | NodeBuilderFlags.SuppressAnyReturnType, getNoopSymbolTrackerWithResolver(context));
+        const flags = NodeBuilderFlags.NoTruncation | NodeBuilderFlags.SuppressAnyReturnType | (quotePreference === QuotePreference.Single ? NodeBuilderFlags.UseSingleQuotesForStringLiteralType : 0);
+        const signatureDeclaration = <MethodDeclaration>checker.signatureToSignatureDeclaration(signature, SyntaxKind.MethodDeclaration, enclosingDeclaration, flags, getNoopSymbolTrackerWithResolver(context));
         if (!signatureDeclaration) {
             return undefined;
         }
@@ -266,6 +269,7 @@ namespace ts.codefix {
             isIdentifier(arg) ? arg.text : isPropertyAccessExpression(arg) && isIdentifier(arg.name) ? arg.name.text : undefined);
         const contextualType = checker.getContextualType(call);
         const returnType = (inJs || !contextualType) ? undefined : checker.typeToTypeNode(contextualType, contextNode, /*flags*/ undefined, tracker);
+        const quotePreference = getQuotePreference(context.sourceFile, context.preferences);
         return factory.createMethodDeclaration(
             /*decorators*/ undefined,
             /*modifiers*/ modifierFlags ? factory.createNodeArray(factory.createModifiersFromModifierFlags(modifierFlags)) : undefined,
@@ -276,7 +280,7 @@ namespace ts.codefix {
                 factory.createTypeParameterDeclaration(CharacterCodes.T + typeArguments!.length - 1 <= CharacterCodes.Z ? String.fromCharCode(CharacterCodes.T + i) : `T${i}`)),
             /*parameters*/ createDummyParameters(args.length, names, types, /*minArgumentCount*/ undefined, inJs),
             /*type*/ returnType,
-            body ? createStubbedMethodBody(context.preferences) : undefined);
+            body ? createStubbedMethodBody(quotePreference) : undefined);
     }
 
     export function typeToAutoImportableTypeNode(checker: TypeChecker, importAdder: ImportAdder, type: Type, contextNode: Node, scriptTarget: ScriptTarget, flags?: NodeBuilderFlags, tracker?: SymbolTracker): TypeNode | undefined {
@@ -312,7 +316,7 @@ namespace ts.codefix {
         name: PropertyName,
         optional: boolean,
         modifiers: readonly Modifier[] | undefined,
-        preferences: UserPreferences,
+        quotePreference: QuotePreference,
     ): MethodDeclaration {
         /** This is *a* signature with the maximal number of arguments,
          * such that if there is a "maximal" signature without rest arguments,
@@ -355,7 +359,7 @@ namespace ts.codefix {
             /*typeParameters*/ undefined,
             parameters,
             /*returnType*/ undefined,
-            preferences);
+            quotePreference);
     }
 
     function createStubbedMethod(
@@ -365,7 +369,7 @@ namespace ts.codefix {
         typeParameters: readonly TypeParameterDeclaration[] | undefined,
         parameters: readonly ParameterDeclaration[],
         returnType: TypeNode | undefined,
-        preferences: UserPreferences
+        quotePreference: QuotePreference
     ): MethodDeclaration {
         return factory.createMethodDeclaration(
             /*decorators*/ undefined,
@@ -376,17 +380,17 @@ namespace ts.codefix {
             typeParameters,
             parameters,
             returnType,
-            createStubbedMethodBody(preferences));
+            createStubbedMethodBody(quotePreference));
     }
 
-    function createStubbedMethodBody(preferences: UserPreferences): Block {
+    function createStubbedMethodBody(quotePreference: QuotePreference): Block {
         return factory.createBlock(
             [factory.createThrowStatement(
                 factory.createNewExpression(
                     factory.createIdentifier("Error"),
                     /*typeArguments*/ undefined,
                     // TODO Handle auto quote preference.
-                    [factory.createStringLiteral("Method not implemented.", /*isSingleQuote*/ preferences.quotePreference === "single")]))],
+                    [factory.createStringLiteral("Method not implemented.", /*isSingleQuote*/ quotePreference === QuotePreference.Single)]))],
             /*multiline*/ true);
     }
 
diff --git a/tests/cases/fourslash/codeFixClassImplementInterface1_optionQuote.ts b/tests/cases/fourslash/codeFixClassImplementInterface1_optionQuote.ts
diff --git a/tests/cases/fourslash/codeFixClassImplementInterfaceAutoImports.ts b/tests/cases/fourslash/codeFixClassImplementInterfaceAutoImports.ts
@@ -34,7 +34,7 @@ import { B, C, D } from './types2';
 export class C implements Base {
     a: A;
     b<T extends B = B>(p1: C): D<C> {
-        throw new Error("Method not implemented.");
+        throw new Error('Method not implemented.');
     }
 }`,
 });
diff --git a/tests/cases/fourslash/codeFixClassImplementInterfaceAutoImports_typeOnly.ts b/tests/cases/fourslash/codeFixClassImplementInterfaceAutoImports_typeOnly.ts
@@ -36,7 +36,7 @@ import type { B, C, D } from './types2';
 export class C implements Base {
     a: A;
     b<T extends B = B>(p1: C): D<C> {
-        throw new Error("Method not implemented.");
+        throw new Error('Method not implemented.');
     }
 }`,
 });
diff --git a/tests/cases/fourslash/codeFixClassImplementInterface_optionQuote.ts b/tests/cases/fourslash/codeFixClassImplementInterface_optionQuote.ts
diff --git a/tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceAuto1.ts b/tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceAuto1.ts
@@ -0,0 +1,32 @@
+/// <reference path='fourslash.ts' />
+
+// @filename: a.ts
+////export interface I {
+////    a(): void;
+////    b(x: "x", y: "a" | "b"): "b";
+////
+////    c: "c";
+////    d: { e: "e"; };
+////}
+// @filename: b.ts
+////import { I } from "./a";
+////class Foo implements I {}
+
+goTo.file("b.ts")
+verify.codeFix({
+    description: [ts.Diagnostics.Implement_interface_0.message, "I"],
+    index: 0,
+    newFileContent:
+`import { I } from "./a";
+class Foo implements I {
+    a(): void {
+        throw new Error("Method not implemented.");
+    }
+    b(x: "x", y: "a" | "b"): "b" {
+        throw new Error("Method not implemented.");
+    }
+    c: "c";
+    d: { e: "e"; };
+}`,
+    preferences: { quotePreference: "auto" }
+});
diff --git a/tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceAuto2.ts b/tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceAuto2.ts
@@ -0,0 +1,32 @@
+/// <reference path='fourslash.ts' />
+
+// @filename: a.ts
+////export interface I {
+////    a(): void;
+////    b(x: 'x', y: 'a' | 'b'): 'b';
+////
+////    c: 'c';
+////    d: { e: 'e'; };
+////}
+// @filename: b.ts
+////import { I } from './a';
+////class Foo implements I {}
+
+goTo.file("b.ts")
+verify.codeFix({
+    description: [ts.Diagnostics.Implement_interface_0.message, "I"],
+    index: 0,
+    newFileContent:
+`import { I } from './a';
+class Foo implements I {
+    a(): void {
+        throw new Error('Method not implemented.');
+    }
+    b(x: 'x', y: 'a' | 'b'): 'b' {
+        throw new Error('Method not implemented.');
+    }
+    c: 'c';
+    d: { e: 'e'; };
+}`,
+    preferences: { quotePreference: "auto" }
+});
diff --git a/tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceDouble.ts b/tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceDouble.ts
@@ -0,0 +1,33 @@
+/// <reference path='fourslash.ts' />
+
+////interface I {
+////    a(): void;
+////    b(x: "x", y: "a" | "b"): "b";
+////
+////    c: "c";
+////    d: { e: "e"; };
+////}
+////class Foo implements I {}
+
+verify.codeFix({
+    description: [ts.Diagnostics.Implement_interface_0.message, "I"],
+    newFileContent:
+`interface I {
+    a(): void;
+    b(x: "x", y: "a" | "b"): "b";
+
+    c: "c";
+    d: { e: "e"; };
+}
+class Foo implements I {
+    a(): void {
+        throw new Error("Method not implemented.");
+    }
+    b(x: "x", y: "a" | "b"): "b" {
+        throw new Error("Method not implemented.");
+    }
+    c: "c";
+    d: { e: "e"; };
+}`,
+    preferences: { quotePreference: "double" }
+});
diff --git a/tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceSingle.ts b/tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceSingle.ts
@@ -0,0 +1,33 @@
+/// <reference path='fourslash.ts' />
+
+////interface I {
+////    a(): void;
+////    b(x: 'x', y: 'a' | 'b'): 'b';
+////
+////    c: 'c';
+////    d: { e: 'e'; };
+////}
+////class Foo implements I {}
+
+verify.codeFix({
+    description: [ts.Diagnostics.Implement_interface_0.message, "I"],
+    newFileContent:
+`interface I {
+    a(): void;
+    b(x: 'x', y: 'a' | 'b'): 'b';
+
+    c: 'c';
+    d: { e: 'e'; };
+}
+class Foo implements I {
+    a(): void {
+        throw new Error('Method not implemented.');
+    }
+    b(x: 'x', y: 'a' | 'b'): 'b' {
+        throw new Error('Method not implemented.');
+    }
+    c: 'c';
+    d: { e: 'e'; };
+}`,
+    preferences: { quotePreference: "single" }
+});
diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts
@@ -592,7 +592,7 @@ declare namespace FourSlashInterface {
         filesToSearch?: ReadonlyArray<string>;
     }
     interface UserPreferences {
-        readonly quotePreference?: "double" | "single";
+        readonly quotePreference?: "auto" | "double" | "single"
         readonly includeCompletionsForModuleExports?: boolean;
         readonly includeInsertTextCompletions?: boolean;
         readonly includeAutomaticOptionalChainCompletions?: boolean;

Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ namespace ts.codefix {`
`42`	`42`	`const abstractAndNonPrivateExtendsSymbols = checker.getPropertiesOfType(instantiatedExtendsType).filter(symbolPointsToNonPrivateAndAbstractMember);`
`43`	`43`
`44`	`44`	`const importAdder = createImportAdder(sourceFile, context.program, preferences, context.host);`
`45`		`- createMissingMemberNodes(classDeclaration, abstractAndNonPrivateExtendsSymbols, context, preferences, importAdder, member => changeTracker.insertNodeAtClassStart(sourceFile, classDeclaration, member));`
	`45`	`+ createMissingMemberNodes(classDeclaration, abstractAndNonPrivateExtendsSymbols, sourceFile, context, preferences, importAdder, member => changeTracker.insertNodeAtClassStart(sourceFile, classDeclaration, member));`
`46`	`46`	`importAdder.writeFixes(changeTracker);`
`47`	`47`	`}`
`48`	`48`
Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ namespace ts.codefix {`
`64`	`64`	`}`
`65`	`65`
`66`	`66`	`const importAdder = createImportAdder(sourceFile, context.program, preferences, context.host);`
`67`		`- createMissingMemberNodes(classDeclaration, nonPrivateAndNotExistedInHeritageClauseMembers, context, preferences, importAdder, member => insertInterfaceMemberNode(sourceFile, classDeclaration, member));`
	`67`	`+ createMissingMemberNodes(classDeclaration, nonPrivateAndNotExistedInHeritageClauseMembers, sourceFile, context, preferences, importAdder, member => insertInterfaceMemberNode(sourceFile, classDeclaration, member));`
`68`	`68`	`importAdder.writeFixes(changeTracker);`
`69`	`69`
`70`	`70`	`function createMissingIndexSignatureDeclaration(type: InterfaceType, kind: IndexKind): void {`
Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,7 @@ import { B, C, D } from './types2';`
`34`	`34`	`export class C implements Base {`
`35`	`35`	`a: A;`
`36`	`36`	`b<T extends B = B>(p1: C): D<C> {`
`37`		`- throw new Error("Method not implemented.");`
	`37`	`+ throw new Error('Method not implemented.');`
`38`	`38`	`}`
`39`	`39`	}`,
`40`	`40`	`});`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ import type { B, C, D } from './types2';`
`36`	`36`	`export class C implements Base {`
`37`	`37`	`a: A;`
`38`	`38`	`b<T extends B = B>(p1: C): D<C> {`
`39`		`- throw new Error("Method not implemented.");`
	`39`	`+ throw new Error('Method not implemented.');`
`40`	`40`	`}`
`41`	`41`	}`,
`42`	`42`	`});`
Original file line number	Diff line number	Diff line change
`@@ -592,7 +592,7 @@ declare namespace FourSlashInterface {`
`592`	`592`	`filesToSearch?: ReadonlyArray<string>;`
`593`	`593`	`}`
`594`	`594`	`interface UserPreferences {`
`595`		`- readonly quotePreference?: "double" \| "single";`
	`595`	`+ readonly quotePreference?: "auto" \| "double" \| "single"`
`596`	`596`	`readonly includeCompletionsForModuleExports?: boolean;`
`597`	`597`	`readonly includeInsertTextCompletions?: boolean;`
`598`	`598`	`readonly includeAutomaticOptionalChainCompletions?: boolean;`