From c8061d0240942de7be64c17410b403d30d9e5a1f Mon Sep 17 00:00:00 2001 From: Alexander T Date: Tue, 30 Jun 2020 19:40:30 +0300 Subject: [PATCH] fix(39332): handle quotes preference in interface method signatures --- ...sDoesntImplementInheritedAbstractMember.ts | 2 +- .../fixClassIncorrectlyImplementsInterface.ts | 2 +- src/services/codefixes/helpers.ts | 44 ++++++++++--------- ...FixClassImplementInterface1_optionQuote.ts | 21 --------- ...deFixClassImplementInterfaceAutoImports.ts | 2 +- ...sImplementInterfaceAutoImports_typeOnly.ts | 2 +- ...eFixClassImplementInterface_optionQuote.ts | 20 --------- ...ImplementInterface_quotePreferenceAuto1.ts | 32 ++++++++++++++ ...ImplementInterface_quotePreferenceAuto2.ts | 32 ++++++++++++++ ...mplementInterface_quotePreferenceDouble.ts | 33 ++++++++++++++ ...mplementInterface_quotePreferenceSingle.ts | 33 ++++++++++++++ tests/cases/fourslash/fourslash.ts | 2 +- 12 files changed, 159 insertions(+), 66 deletions(-) delete mode 100644 tests/cases/fourslash/codeFixClassImplementInterface1_optionQuote.ts delete mode 100644 tests/cases/fourslash/codeFixClassImplementInterface_optionQuote.ts create mode 100644 tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceAuto1.ts create mode 100644 tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceAuto2.ts create mode 100644 tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceDouble.ts create mode 100644 tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceSingle.ts diff --git a/src/services/codefixes/fixClassDoesntImplementInheritedAbstractMember.ts b/src/services/codefixes/fixClassDoesntImplementInheritedAbstractMember.ts index eac02f0d7135a..7fe143139454e 100644 --- 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 index 954a64ef53dcd..fb00d2477dcd4 100644 --- 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 index 405a5db924bd6..f1b557da71874 100644 --- 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 | 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 | 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 | undefined, @@ -162,7 +164,8 @@ namespace ts.codefix { const program = context.program; const checker = program.getTypeChecker(); const scriptTarget = getEmitScriptTarget(program.getCompilerOptions()); - const signatureDeclaration = 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 = 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 deleted file mode 100644 index 855d5acb76920..0000000000000 --- a/tests/cases/fourslash/codeFixClassImplementInterface1_optionQuote.ts +++ /dev/null @@ -1,21 +0,0 @@ -/// - -////interface IFoo { -//// a: 'string'; -//// c: { d: 'string'; }; -////} -////class Foo implements IFoo {} - -verify.codeFix({ - description: [ts.Diagnostics.Implement_interface_0.message, "IFoo"], - newFileContent: -`interface IFoo { - a: 'string'; - c: { d: 'string'; }; -} -class Foo implements IFoo { - a: 'string'; - c: { d: 'string'; }; -}`, - preferences: { quotePreference: "single" } -}); diff --git a/tests/cases/fourslash/codeFixClassImplementInterfaceAutoImports.ts b/tests/cases/fourslash/codeFixClassImplementInterfaceAutoImports.ts index 3a08ffa5e5a2a..473e7273ae228 100644 --- 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(p1: C): D { - 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 index 9f261a72c2aed..55103d11486e1 100644 --- 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(p1: C): D { - 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 deleted file mode 100644 index 7fa6d8d9a9c97..0000000000000 --- a/tests/cases/fourslash/codeFixClassImplementInterface_optionQuote.ts +++ /dev/null @@ -1,20 +0,0 @@ -/// - -////interface I { -//// m(): void; -////} -////class C implements I {} - -verify.codeFix({ - description: "Implement interface 'I'", - newFileContent: -`interface I { - m(): void; -} -class C implements I { - m(): void { - throw new Error('Method not implemented.'); - } -}`, - preferences: { quotePreference: "single" } -}); diff --git a/tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceAuto1.ts b/tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceAuto1.ts new file mode 100644 index 0000000000000..9c0e5e111fb63 --- /dev/null +++ b/tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceAuto1.ts @@ -0,0 +1,32 @@ +/// + +// @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 new file mode 100644 index 0000000000000..d38832e91c092 --- /dev/null +++ b/tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceAuto2.ts @@ -0,0 +1,32 @@ +/// + +// @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 new file mode 100644 index 0000000000000..0065a7712272d --- /dev/null +++ b/tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceDouble.ts @@ -0,0 +1,33 @@ +/// + +////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 new file mode 100644 index 0000000000000..8fe09629d3ca2 --- /dev/null +++ b/tests/cases/fourslash/codeFixClassImplementInterface_quotePreferenceSingle.ts @@ -0,0 +1,33 @@ +/// + +////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 index a1f91482309c6..55c12b672f76f 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -592,7 +592,7 @@ declare namespace FourSlashInterface { filesToSearch?: ReadonlyArray; } interface UserPreferences { - readonly quotePreference?: "double" | "single"; + readonly quotePreference?: "auto" | "double" | "single" readonly includeCompletionsForModuleExports?: boolean; readonly includeInsertTextCompletions?: boolean; readonly includeAutomaticOptionalChainCompletions?: boolean;