Skip to content

Commit

Permalink
fix(39332): handle quotes preference in interface method signatures (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
a-tarasyuk authored Jul 7, 2020
1 parent 4533652 commit 2a92a6e
Show file tree
Hide file tree
Showing 12 changed files with 161 additions and 66 deletions.
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

0 comments on commit 2a92a6e

Please sign in to comment.