From bb3a7aec11a20afb30c5098a75e001d1a4eb08f9 Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Sat, 27 Aug 2022 00:33:23 +0300 Subject: [PATCH] fix(50415): Language server debug failure - Did not expect GetAccessor to have an Identifier in its trivia (#50470) * fix(50415): clone props for get/set accessors * add additional tests * create helpers to create name, body, modifiers, typeName * cleanup --- src/compiler/factory/nodeFactory.ts | 2 + src/services/codefixes/helpers.ts | 48 ++-- .../completionsOverridingMethod15.baseline | 211 ++++++++++++++++++ .../completionsOverridingMethod16.baseline | 211 ++++++++++++++++++ .../completionsOverridingMethod15.ts | 17 ++ .../completionsOverridingMethod16.ts | 17 ++ 6 files changed, 490 insertions(+), 16 deletions(-) create mode 100644 tests/baselines/reference/completionsOverridingMethod15.baseline create mode 100644 tests/baselines/reference/completionsOverridingMethod16.baseline create mode 100644 tests/cases/fourslash/completionsOverridingMethod15.ts create mode 100644 tests/cases/fourslash/completionsOverridingMethod16.ts diff --git a/src/compiler/factory/nodeFactory.ts b/src/compiler/factory/nodeFactory.ts index f072083e15401..02e72593f07f7 100644 --- a/src/compiler/factory/nodeFactory.ts +++ b/src/compiler/factory/nodeFactory.ts @@ -6823,6 +6823,7 @@ namespace ts { constantValue, helpers, startsOnNewLine, + snippetElement, } = sourceEmitNode; if (!destEmitNode) destEmitNode = {} as EmitNode; // We are using `.slice()` here in case `destEmitNode.leadingComments` is pushed to later. @@ -6839,6 +6840,7 @@ namespace ts { } } if (startsOnNewLine !== undefined) destEmitNode.startsOnNewLine = startsOnNewLine; + if (snippetElement !== undefined) destEmitNode.snippetElement = snippetElement; return destEmitNode; } diff --git a/src/services/codefixes/helpers.ts b/src/services/codefixes/helpers.ts index 2f6e4f067968c..ba8bd0453dd09 100644 --- a/src/services/codefixes/helpers.ts +++ b/src/services/codefixes/helpers.ts @@ -79,9 +79,8 @@ namespace ts.codefix { * In such cases, we assume the declaration to be a `PropertySignature`. */ const kind = declaration?.kind ?? SyntaxKind.PropertySignature; - const name = getSynthesizedDeepClone(getNameOfDeclaration(declaration), /*includeTrivia*/ false) as PropertyName; + const declarationName = getNameOfDeclaration(declaration) as PropertyName; const visibilityModifier = createVisibilityModifier(declaration ? getEffectiveModifierFlags(declaration) : ModifierFlags.None); - const modifiers = visibilityModifier ? factory.createNodeArray([visibilityModifier]) : undefined; const type = checker.getWidenedType(checker.getTypeOfSymbolAtLocation(symbol, enclosingDeclaration)); const optional = !!(symbol.flags & SymbolFlags.Optional); const ambient = !!(enclosingDeclaration.flags & NodeFlags.Ambient) || isAmbient; @@ -100,8 +99,8 @@ namespace ts.codefix { } } addClassElement(factory.createPropertyDeclaration( - modifiers, - declaration ? name : symbol.getName(), + createModifiers(visibilityModifier), + declaration ? createName(declarationName) : symbol.getName(), optional && (preserveOptional & PreserveOptionalFlags.Property) ? factory.createToken(SyntaxKind.QuestionToken) : undefined, typeNode, /*initializer*/ undefined)); @@ -124,21 +123,21 @@ namespace ts.codefix { for (const accessor of orderedAccessors) { if (isGetAccessorDeclaration(accessor)) { addClassElement(factory.createGetAccessorDeclaration( - modifiers, - name, + createModifiers(visibilityModifier), + createName(declarationName), emptyArray, - typeNode, - ambient ? undefined : body || createStubbedMethodBody(quotePreference))); + createTypeNode(typeNode), + createBody(body, quotePreference, ambient))); } else { Debug.assertNode(accessor, isSetAccessorDeclaration, "The counterpart to a getter should be a setter"); const parameter = getSetAccessorValueParameter(accessor); const parameterName = parameter && isIdentifier(parameter.name) ? idText(parameter.name) : undefined; addClassElement(factory.createSetAccessorDeclaration( - modifiers, - name, - createDummyParameters(1, [parameterName], [typeNode], 1, /*inJs*/ false), - ambient ? undefined : body || createStubbedMethodBody(quotePreference))); + createModifiers(visibilityModifier), + createName(declarationName), + createDummyParameters(1, [parameterName], [createTypeNode(typeNode)], 1, /*inJs*/ false), + createBody(body, quotePreference, ambient))); } } break; @@ -161,23 +160,23 @@ namespace ts.codefix { if (declarations.length === 1) { Debug.assert(signatures.length === 1, "One declaration implies one signature"); const signature = signatures[0]; - outputMethod(quotePreference, signature, modifiers, name, ambient ? undefined : body || createStubbedMethodBody(quotePreference)); + outputMethod(quotePreference, signature, createModifiers(visibilityModifier), createName(declarationName), createBody(body, quotePreference, ambient)); break; } for (const signature of signatures) { // Ensure nodes are fresh so they can have different positions when going through formatting. - outputMethod(quotePreference, signature, getSynthesizedDeepClones(modifiers, /*includeTrivia*/ false), getSynthesizedDeepClone(name, /*includeTrivia*/ false)); + outputMethod(quotePreference, signature, createModifiers(visibilityModifier), createName(declarationName)); } if (!ambient) { if (declarations.length > signatures.length) { const signature = checker.getSignatureFromDeclaration(declarations[declarations.length - 1] as SignatureDeclaration)!; - outputMethod(quotePreference, signature, modifiers, name, body || createStubbedMethodBody(quotePreference)); + outputMethod(quotePreference, signature, createModifiers(visibilityModifier), createName(declarationName), createBody(body, quotePreference)); } else { Debug.assert(declarations.length === signatures.length, "Declarations and signatures should match count"); - addClassElement(createMethodImplementingSignatures(checker, context, enclosingDeclaration, signatures, name, optional && !!(preserveOptional & PreserveOptionalFlags.Method), modifiers, quotePreference, body)); + addClassElement(createMethodImplementingSignatures(checker, context, enclosingDeclaration, signatures, createName(declarationName), optional && !!(preserveOptional & PreserveOptionalFlags.Method), createModifiers(visibilityModifier), quotePreference, body)); } } break; @@ -187,6 +186,23 @@ namespace ts.codefix { const method = createSignatureDeclarationFromSignature(SyntaxKind.MethodDeclaration, context, quotePreference, signature, body, name, modifiers, optional && !!(preserveOptional & PreserveOptionalFlags.Method), enclosingDeclaration, importAdder) as MethodDeclaration; if (method) addClassElement(method); } + + function createName(node: PropertyName) { + return getSynthesizedDeepClone(node, /*includeTrivia*/ false); + } + + function createModifiers(modifier: Modifier | undefined) { + return modifier ? factory.createNodeArray([modifier]) : undefined; + } + + function createBody(block: Block | undefined, quotePreference: QuotePreference, ambient?: boolean) { + return ambient ? undefined : + getSynthesizedDeepClone(block, /*includeTrivia*/ false) || createStubbedMethodBody(quotePreference); + } + + function createTypeNode(typeNode: TypeNode | undefined) { + return getSynthesizedDeepClone(typeNode, /*includeTrivia*/ false); + } } export function createSignatureDeclarationFromSignature( diff --git a/tests/baselines/reference/completionsOverridingMethod15.baseline b/tests/baselines/reference/completionsOverridingMethod15.baseline new file mode 100644 index 0000000000000..16f78ebae8115 --- /dev/null +++ b/tests/baselines/reference/completionsOverridingMethod15.baseline @@ -0,0 +1,211 @@ +[ + { + "marker": { + "fileName": "/tests/cases/fourslash/completionsOverridingMethod15.ts", + "position": 89, + "name": "" + }, + "completionList": { + "flags": 0, + "isGlobalCompletion": false, + "isMemberCompletion": true, + "isNewIdentifierLocation": true, + "entries": [ + { + "name": "abstract", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "abstract", + "kind": "keyword" + } + ] + }, + { + "name": "async", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "async", + "kind": "keyword" + } + ] + }, + { + "name": "constructor", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "constructor", + "kind": "keyword" + } + ] + }, + { + "name": "declare", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "declare", + "kind": "keyword" + } + ] + }, + { + "name": "get", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "get", + "kind": "keyword" + } + ] + }, + { + "name": "override", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "override", + "kind": "keyword" + } + ] + }, + { + "name": "private", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "private", + "kind": "keyword" + } + ] + }, + { + "name": "protected", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "protected", + "kind": "keyword" + } + ] + }, + { + "name": "public", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "public", + "kind": "keyword" + } + ] + }, + { + "name": "readonly", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "readonly", + "kind": "keyword" + } + ] + }, + { + "name": "set", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "set", + "kind": "keyword" + } + ] + }, + { + "name": "static", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "static", + "kind": "keyword" + } + ] + }, + { + "name": "foo", + "kind": "property", + "kindModifiers": "declare", + "sortText": "17", + "insertText": "get foo(): any {\n}\nset foo(value: any) {\n}", + "displayParts": [ + { + "text": "(", + "kind": "punctuation" + }, + { + "text": "property", + "kind": "text" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "B", + "kind": "className" + }, + { + "text": ".", + "kind": "punctuation" + }, + { + "text": "foo", + "kind": "propertyName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "any", + "kind": "keyword" + } + ], + "documentation": [] + } + ] + } + } +] \ No newline at end of file diff --git a/tests/baselines/reference/completionsOverridingMethod16.baseline b/tests/baselines/reference/completionsOverridingMethod16.baseline new file mode 100644 index 0000000000000..ec29be4e1004b --- /dev/null +++ b/tests/baselines/reference/completionsOverridingMethod16.baseline @@ -0,0 +1,211 @@ +[ + { + "marker": { + "fileName": "/tests/cases/fourslash/completionsOverridingMethod16.ts", + "position": 89, + "name": "" + }, + "completionList": { + "flags": 0, + "isGlobalCompletion": false, + "isMemberCompletion": true, + "isNewIdentifierLocation": true, + "entries": [ + { + "name": "abstract", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "abstract", + "kind": "keyword" + } + ] + }, + { + "name": "async", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "async", + "kind": "keyword" + } + ] + }, + { + "name": "constructor", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "constructor", + "kind": "keyword" + } + ] + }, + { + "name": "declare", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "declare", + "kind": "keyword" + } + ] + }, + { + "name": "get", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "get", + "kind": "keyword" + } + ] + }, + { + "name": "override", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "override", + "kind": "keyword" + } + ] + }, + { + "name": "private", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "private", + "kind": "keyword" + } + ] + }, + { + "name": "protected", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "protected", + "kind": "keyword" + } + ] + }, + { + "name": "public", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "public", + "kind": "keyword" + } + ] + }, + { + "name": "readonly", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "readonly", + "kind": "keyword" + } + ] + }, + { + "name": "set", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "set", + "kind": "keyword" + } + ] + }, + { + "name": "static", + "kind": "keyword", + "kindModifiers": "", + "sortText": "15", + "displayParts": [ + { + "text": "static", + "kind": "keyword" + } + ] + }, + { + "name": "foo", + "kind": "property", + "kindModifiers": "declare", + "sortText": "17", + "insertText": "set foo(value: any) {\n}\nget foo(): any {\n}", + "displayParts": [ + { + "text": "(", + "kind": "punctuation" + }, + { + "text": "property", + "kind": "text" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "B", + "kind": "className" + }, + { + "text": ".", + "kind": "punctuation" + }, + { + "text": "foo", + "kind": "propertyName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "any", + "kind": "keyword" + } + ], + "documentation": [] + } + ] + } + } +] \ No newline at end of file diff --git a/tests/cases/fourslash/completionsOverridingMethod15.ts b/tests/cases/fourslash/completionsOverridingMethod15.ts new file mode 100644 index 0000000000000..1fdc3616a8b07 --- /dev/null +++ b/tests/cases/fourslash/completionsOverridingMethod15.ts @@ -0,0 +1,17 @@ +/// + +// @newline: LF +////declare class B { +//// get foo(): any; +//// set foo(value: any); +////} +////class A extends B { +//// /**/ +////} + +goTo.marker(""); +verify.baselineCompletions({ + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: false, + includeCompletionsWithClassMemberSnippets: true, +}); diff --git a/tests/cases/fourslash/completionsOverridingMethod16.ts b/tests/cases/fourslash/completionsOverridingMethod16.ts new file mode 100644 index 0000000000000..5d71a70119945 --- /dev/null +++ b/tests/cases/fourslash/completionsOverridingMethod16.ts @@ -0,0 +1,17 @@ +/// + +// @newline: LF +////declare class B { +//// set foo(value: any); +//// get foo(): any; +////} +////class A extends B { +//// /**/ +////} + +goTo.marker(""); +verify.baselineCompletions({ + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: false, + includeCompletionsWithClassMemberSnippets: true, +});