Skip to content

fix(39332): QuoteStyle not respected for string literal types in implement interface #39348

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 1 commit into from
Jul 7, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
44 changes: 24 additions & 20 deletions src/services/codefixes/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand All @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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");
Expand All @@ -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;
Expand All @@ -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,
Expand All @@ -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;
}
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -355,7 +359,7 @@ namespace ts.codefix {
/*typeParameters*/ undefined,
parameters,
/*returnType*/ undefined,
preferences);
quotePreference);
}

function createStubbedMethod(
Expand All @@ -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,
Expand All @@ -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);
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}
}`,
});
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}
}`,
});

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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" }
});
Original file line number Diff line number Diff line change
@@ -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" }
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/// <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"],
index: 0,
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" }
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/// <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"],
index: 0,
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" }
});
2 changes: 1 addition & 1 deletion tests/cases/fourslash/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down