From bd9f3cf3caebe8e0d962f9234bb50b0e95be5745 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Thu, 9 Apr 2020 11:25:40 -0700 Subject: [PATCH 01/10] add reason for a disabled code action --- src/services/refactors/extractSymbol.ts | 36 ++++++++++++++++++++----- src/services/types.ts | 2 ++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index e7f57a57351c4..c6bd96722cd70 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -23,18 +23,20 @@ namespace ts.refactor.extractSymbol { const functionActions: RefactorActionInfo[] = []; const usedFunctionNames: Map = createMap(); + let innermostErrorFunctionAction: ts.RefactorActionInfo | undefined; const constantActions: RefactorActionInfo[] = []; const usedConstantNames: Map = createMap(); + let innermostErrorConstantAction: ts.RefactorActionInfo | undefined; + let i = 0; for (const {functionExtraction, constantExtraction} of extractions) { - // Skip these since we don't have a way to report errors yet + const description = functionExtraction.description; if (functionExtraction.errors.length === 0) { // Don't issue refactorings with duplicated names. // Scopes come back in "innermost first" order, so extractions will // preferentially go into nearer scopes - const description = functionExtraction.description; if (!usedFunctionNames.has(description)) { usedFunctionNames.set(description, true); functionActions.push({ @@ -42,6 +44,17 @@ namespace ts.refactor.extractSymbol { name: `function_scope_${i}` }); } + } else if (!innermostErrorFunctionAction) { + let error = functionExtraction.errors[0].messageText; + if (typeof error !== 'string') { + error = error.messageText; + } + + innermostErrorFunctionAction = { + description, + name: `function_scope_${i}`, + error: error + } } // Skip these since we don't have a way to report errors yet @@ -57,6 +70,17 @@ namespace ts.refactor.extractSymbol { name: `constant_scope_${i}` }); } + } else if (!innermostErrorConstantAction) { + let error = constantExtraction.errors[0].messageText; + if (typeof error !== 'string') { + error = error.messageText; + } + + innermostErrorConstantAction = { + description, + name: `constant_scope_${i}`, + error: error + } } // *do* increment i anyway because we'll look for the i-th scope @@ -66,19 +90,19 @@ namespace ts.refactor.extractSymbol { const infos: ApplicableRefactorInfo[] = []; - if (functionActions.length) { + if (functionActions.length || innermostErrorFunctionAction) { infos.push({ name: refactorName, description: getLocaleSpecificMessage(Diagnostics.Extract_function), - actions: functionActions + actions: functionActions.length ? functionActions : [ innermostErrorFunctionAction! ] }); } - if (constantActions.length) { + if (constantActions.length || innermostErrorConstantAction) { infos.push({ name: refactorName, description: getLocaleSpecificMessage(Diagnostics.Extract_constant), - actions: constantActions + actions: constantActions.length ? constantActions : [ innermostErrorConstantAction! ] }); } diff --git a/src/services/types.ts b/src/services/types.ts index 21a92e169d861..490598584692c 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -623,6 +623,8 @@ namespace ts { * so this description should make sense by itself if the parent is inlineable=true */ description: string; + + error?: string; } /** From ecb957d306985cd2b9128071806786922737253e Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Fri, 1 May 2020 12:20:52 -0700 Subject: [PATCH 02/10] add addOrRemoveBracesToArrowFunction --- .../addOrRemoveBracesToArrowFunction.ts | 64 +++++++++++++++---- src/services/refactors/extractSymbol.ts | 46 +++++++++---- 2 files changed, 84 insertions(+), 26 deletions(-) diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index 89a03c3c23989..cfa24240b34ae 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -4,6 +4,7 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { const refactorDescription = Diagnostics.Add_or_remove_braces_in_an_arrow_function.message; const addBracesActionName = "Add braces to arrow function"; const removeBracesActionName = "Remove braces from arrow function"; + const errorBracesActionName = "Error adding or removing braces from arrow function"; const addBracesActionDescription = Diagnostics.Add_braces_to_arrow_function.message; const removeBracesActionDescription = Diagnostics.Remove_braces_from_arrow_function.message; registerRefactor(refactorName, { getEditsForAction, getAvailableActions }); @@ -15,16 +16,36 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { addBraces: boolean; } + type InfoOrError = { + info: Info, + error?: never + } | { + info?: never, + error: string + } + function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { const { file, startPosition } = context; const info = getConvertibleArrowFunctionAtPosition(file, startPosition); if (!info) return emptyArray; + if (info.error !== undefined) { + return [{ + name: refactorName, + description: refactorDescription, + actions: [{ + name: errorBracesActionName, + description: "Error adding or removing braces from arrow function", + error: info.error + }] + }]; + } + return [{ name: refactorName, description: refactorDescription, actions: [ - info.addBraces ? + info.info.addBraces ? { name: addBracesActionName, description: addBracesActionDescription @@ -39,9 +60,9 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { const { file, startPosition } = context; const info = getConvertibleArrowFunctionAtPosition(file, startPosition); - if (!info) return undefined; + if (!info || !info.info) return undefined; - const { expression, returnStatement, func } = info; + const { expression, returnStatement, func } = info.info; let body: ConciseBody; if (actionName === addBracesActionName) { @@ -68,26 +89,45 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { return isBinaryExpression(expression) && expression.operatorToken.kind === SyntaxKind.CommaToken || isObjectLiteralExpression(expression); } - function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number): Info | undefined { + function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number): InfoOrError | undefined { const node = getTokenAtPosition(file, startPosition); const func = getContainingFunction(node); - if (!func || !isArrowFunction(func) || (!rangeContainsRange(func, node) || rangeContainsRange(func.body, node))) return undefined; + + if (!func) { + return { + error: 'Could not find a containing arrow function.' + } + } + + if (!isArrowFunction(func)) { + return { + error: 'Containing function is not an arrow function.' + } + } + + if ((!rangeContainsRange(func, node) || rangeContainsRange(func.body, node))) { + return undefined; + } if (isExpression(func.body)) { return { - func, - addBraces: true, - expression: func.body + info: { + func, + addBraces: true, + expression: func.body + } }; } else if (func.body.statements.length === 1) { const firstStatement = first(func.body.statements); if (isReturnStatement(firstStatement)) { return { - func, - addBraces: false, - expression: firstStatement.expression, - returnStatement: firstStatement + info: { + func, + addBraces: false, + expression: firstStatement.expression, + returnStatement: firstStatement + } }; } } diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index c6bd96722cd70..94b547e635a73 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -12,7 +12,28 @@ namespace ts.refactor.extractSymbol { const targetRange = rangeToExtract.targetRange; if (targetRange === undefined) { - return emptyArray; + if (!rangeToExtract.errors || rangeToExtract.errors.length == 0) { + return emptyArray; + } + + return [{ + name: refactorName, + description: getLocaleSpecificMessage(Diagnostics.Extract_function), + actions: [{ + description: getLocaleSpecificMessage(Diagnostics.Extract_function), + name: 'extract_function_error', + error: getStringError(rangeToExtract.errors) + }] + }, + { + name: refactorName, + description: getLocaleSpecificMessage(Diagnostics.Extract_constant), + actions: [{ + description: getLocaleSpecificMessage(Diagnostics.Extract_constant), + name: 'extract_constant_error', + error: getStringError(rangeToExtract.errors) + }] + }]; } const extractions = getPossibleExtractions(targetRange, context); @@ -29,7 +50,6 @@ namespace ts.refactor.extractSymbol { const usedConstantNames: Map = createMap(); let innermostErrorConstantAction: ts.RefactorActionInfo | undefined; - let i = 0; for (const {functionExtraction, constantExtraction} of extractions) { const description = functionExtraction.description; @@ -45,15 +65,10 @@ namespace ts.refactor.extractSymbol { }); } } else if (!innermostErrorFunctionAction) { - let error = functionExtraction.errors[0].messageText; - if (typeof error !== 'string') { - error = error.messageText; - } - innermostErrorFunctionAction = { description, name: `function_scope_${i}`, - error: error + error: getStringError(functionExtraction.errors) } } @@ -71,15 +86,10 @@ namespace ts.refactor.extractSymbol { }); } } else if (!innermostErrorConstantAction) { - let error = constantExtraction.errors[0].messageText; - if (typeof error !== 'string') { - error = error.messageText; - } - innermostErrorConstantAction = { description, name: `constant_scope_${i}`, - error: error + error: getStringError(constantExtraction.errors) } } @@ -107,6 +117,14 @@ namespace ts.refactor.extractSymbol { } return infos.length ? infos : emptyArray; + + function getStringError(errors: readonly Diagnostic[]) { + let error = errors[0].messageText; + if (typeof error !== 'string') { + error = error.messageText; + } + return error; + } } /* Exported for tests */ From dd177411c4bd1e268ca27cf7eb75cde1b018a3d1 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Thu, 18 Jun 2020 07:30:41 -0700 Subject: [PATCH 03/10] use user preferences --- src/compiler/types.ts | 1 + .../addOrRemoveBracesToArrowFunction.ts | 35 +++++++++++-------- src/services/refactors/extractSymbol.ts | 20 ++++++++--- 3 files changed, 37 insertions(+), 19 deletions(-) diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 03b849b36c79f..eb1d02bac53f2 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -6447,6 +6447,7 @@ namespace ts { readonly importModuleSpecifierEnding?: "minimal" | "index" | "js"; readonly allowTextChangesInNewFiles?: boolean; readonly providePrefixAndSuffixTextForRename?: boolean; + readonly provideRefactorErrorReason?: boolean; } /** Represents a bigint literal value without requiring bigint support */ diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index cfa24240b34ae..4998421062c11 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -29,7 +29,25 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { const info = getConvertibleArrowFunctionAtPosition(file, startPosition); if (!info) return emptyArray; - if (info.error !== undefined) { + if (info.error === undefined) { + return [{ + name: refactorName, + description: refactorDescription, + actions: [ + info.info.addBraces ? + { + name: addBracesActionName, + description: addBracesActionDescription + } : { + name: removeBracesActionName, + description: removeBracesActionDescription + } + ] + }]; + } + + if (context.preferences.provideRefactorErrorReason) + { return [{ name: refactorName, description: refactorDescription, @@ -41,20 +59,7 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { }]; } - return [{ - name: refactorName, - description: refactorDescription, - actions: [ - info.info.addBraces ? - { - name: addBracesActionName, - description: addBracesActionDescription - } : { - name: removeBracesActionName, - description: removeBracesActionDescription - } - ] - }]; + return emptyArray; } function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 94b547e635a73..6fb7ea31d1376 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -12,7 +12,7 @@ namespace ts.refactor.extractSymbol { const targetRange = rangeToExtract.targetRange; if (targetRange === undefined) { - if (!rangeToExtract.errors || rangeToExtract.errors.length == 0) { + if (!rangeToExtract.errors || rangeToExtract.errors.length == 0 || !context.preferences.provideRefactorErrorReason) { return emptyArray; } @@ -100,11 +100,17 @@ namespace ts.refactor.extractSymbol { const infos: ApplicableRefactorInfo[] = []; - if (functionActions.length || innermostErrorFunctionAction) { + if (functionActions.length) { infos.push({ name: refactorName, description: getLocaleSpecificMessage(Diagnostics.Extract_function), - actions: functionActions.length ? functionActions : [ innermostErrorFunctionAction! ] + actions: functionActions + }); + } else if (context.preferences.provideRefactorErrorReason && innermostErrorFunctionAction) { + infos.push({ + name: refactorName, + description: getLocaleSpecificMessage(Diagnostics.Extract_function), + actions: [ innermostErrorFunctionAction ] }); } @@ -112,7 +118,13 @@ namespace ts.refactor.extractSymbol { infos.push({ name: refactorName, description: getLocaleSpecificMessage(Diagnostics.Extract_constant), - actions: constantActions.length ? constantActions : [ innermostErrorConstantAction! ] + actions: constantActions + }); + } else if (context.preferences.provideRefactorErrorReason && innermostErrorConstantAction) { + infos.push({ + name: refactorName, + description: getLocaleSpecificMessage(Diagnostics.Extract_constant), + actions: [ innermostErrorConstantAction ] }); } From 93723fb9a3ecdd5ac6744f894439b7e558b9da65 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Thu, 18 Jun 2020 09:41:15 -0700 Subject: [PATCH 04/10] add error reason --- src/services/codefixes/generateAccessors.ts | 58 ++++++++++++++----- src/services/refactors/convertExport.ts | 37 +++++++++--- src/services/refactors/convertImport.ts | 43 +++++++++++--- src/services/refactors/extractSymbol.ts | 10 ++-- src/services/refactors/extractType.ts | 56 +++++++++++++----- .../generateGetAccessorAndSetAccessor.ts | 46 ++++++++++----- 6 files changed, 185 insertions(+), 65 deletions(-) diff --git a/src/services/codefixes/generateAccessors.ts b/src/services/codefixes/generateAccessors.ts index 75e3c9b9d24b2..ddad4d6132f8e 100644 --- a/src/services/codefixes/generateAccessors.ts +++ b/src/services/codefixes/generateAccessors.ts @@ -16,12 +16,20 @@ namespace ts.codefix { readonly renameAccessor: boolean; } + type InfoOrError = { + info: Info, + error?: never + } | { + info?: never, + error: string + } + export function generateAccessorFromProperty(file: SourceFile, start: number, end: number, context: textChanges.TextChangesContext, _actionName: string): FileTextChanges[] | undefined { const fieldInfo = getAccessorConvertiblePropertyAtPosition(file, start, end); - if (!fieldInfo) return undefined; + if (!fieldInfo || !fieldInfo.info) return undefined; const changeTracker = textChanges.ChangeTracker.fromContext(context); - const { isStatic, isReadonly, fieldName, accessorName, originalName, type, container, declaration } = fieldInfo; + const { isStatic, isReadonly, fieldName, accessorName, originalName, type, container, declaration } = fieldInfo.info; suppressLeadingAndTrailingTrivia(fieldName); suppressLeadingAndTrailingTrivia(accessorName); @@ -104,29 +112,51 @@ namespace ts.codefix { return modifierFlags; } - export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, start: number, end: number, considerEmptySpans = true): Info | undefined { + export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, start: number, end: number, considerEmptySpans = true): InfoOrError | undefined { const node = getTokenAtPosition(file, start); const cursorRequest = start === end && considerEmptySpans; const declaration = findAncestor(node.parent, isAcceptedDeclaration); // make sure declaration have AccessibilityModifier or Static Modifier or Readonly Modifier const meaning = ModifierFlags.AccessibilityModifier | ModifierFlags.Static | ModifierFlags.Readonly; - if (!declaration || !(nodeOverlapsWithStartEnd(declaration.name, file, start, end) || cursorRequest) - || !isConvertibleName(declaration.name) || (getEffectiveModifierFlags(declaration) | meaning) !== meaning) return undefined; + + if (!declaration) { + return { + error: 'Could not find property for which to generate accessor.' + } + } + + if (!(nodeOverlapsWithStartEnd(declaration.name, file, start, end) || cursorRequest)) { + return undefined; + } + + if (!isConvertibleName(declaration.name)) { + return { + error: 'Name is not valid.' + } + } + + if ((getEffectiveModifierFlags(declaration) | meaning) !== meaning) { + return { + error: 'Property has invalid accessibility.' + } + } const name = declaration.name.text; const startWithUnderscore = startsWithUnderscore(name); const fieldName = createPropertyName(startWithUnderscore ? name : getUniqueName(`_${name}`, file), declaration.name); const accessorName = createPropertyName(startWithUnderscore ? getUniqueName(name.substring(1), file) : name, declaration.name); return { - isStatic: hasStaticModifier(declaration), - isReadonly: hasEffectiveReadonlyModifier(declaration), - type: getTypeAnnotationNode(declaration), - container: declaration.kind === SyntaxKind.Parameter ? declaration.parent.parent : declaration.parent, - originalName: (declaration.name).text, - declaration, - fieldName, - accessorName, - renameAccessor: startWithUnderscore + info: { + isStatic: hasStaticModifier(declaration), + isReadonly: hasEffectiveReadonlyModifier(declaration), + type: getTypeAnnotationNode(declaration), + container: declaration.kind === SyntaxKind.Parameter ? declaration.parent.parent : declaration.parent, + originalName: (declaration.name).text, + declaration, + fieldName, + accessorName, + renameAccessor: startWithUnderscore + } }; } diff --git a/src/services/refactors/convertExport.ts b/src/services/refactors/convertExport.ts index 9bfcfb2f8263a..ca5885f1e577c 100644 --- a/src/services/refactors/convertExport.ts +++ b/src/services/refactors/convertExport.ts @@ -3,17 +3,28 @@ namespace ts.refactor { const refactorName = "Convert export"; const actionNameDefaultToNamed = "Convert default export to named export"; const actionNameNamedToDefault = "Convert named export to default export"; + const errorConvertingExport = "Error converting export"; + registerRefactor(refactorName, { getAvailableActions(context): readonly ApplicableRefactorInfo[] { const info = getInfo(context, context.triggerReason === "invoked"); if (!info) return emptyArray; - const description = info.wasDefault ? Diagnostics.Convert_default_export_to_named_export.message : Diagnostics.Convert_named_export_to_default_export.message; - const actionName = info.wasDefault ? actionNameDefaultToNamed : actionNameNamedToDefault; - return [{ name: refactorName, description, actions: [{ name: actionName, description }] }]; + + if (info.error == undefined) { + const description = info.info.wasDefault ? Diagnostics.Convert_default_export_to_named_export.message : Diagnostics.Convert_named_export_to_default_export.message; + const actionName = info.info.wasDefault ? actionNameDefaultToNamed : actionNameNamedToDefault; + return [{ name: refactorName, description, actions: [{ name: actionName, description }] }]; + } + + if (context.preferences.provideRefactorErrorReason) { + return [{ name: refactorName, description: errorConvertingExport, actions: [{ name: errorConvertingExport, description: errorConvertingExport }] }]; + } + + return emptyArray; }, getEditsForAction(context, actionName): RefactorEditInfo { Debug.assert(actionName === actionNameDefaultToNamed || actionName === actionNameNamedToDefault, "Unexpected action name"); - const edits = textChanges.ChangeTracker.with(context, t => doChange(context.file, context.program, Debug.checkDefined(getInfo(context), "context must have info"), t, context.cancellationToken)); + const edits = textChanges.ChangeTracker.with(context, t => doChange(context.file, context.program, Debug.checkDefined(getInfo(context)?.info, "context must have info"), t, context.cancellationToken)); return { edits, renameFilename: undefined, renameLocation: undefined }; }, }); @@ -27,13 +38,21 @@ namespace ts.refactor { readonly exportingModuleSymbol: Symbol; } - function getInfo(context: RefactorContext, considerPartialSpans = true): Info | undefined { + type InfoOrError = { + info: Info, + error?: never + } | { + info?: never, + error: string + } + + function getInfo(context: RefactorContext, considerPartialSpans = true): InfoOrError | undefined { const { file } = context; const span = getRefactorContextSpan(context); const token = getTokenAtPosition(file, span.start); const exportNode = !!(token.parent && getSyntacticModifierFlags(token.parent) & ModifierFlags.Export) && considerPartialSpans ? token.parent : getParentNodeInSpan(token, file, span); if (!exportNode || (!isSourceFile(exportNode.parent) && !(isModuleBlock(exportNode.parent) && isAmbientModule(exportNode.parent.parent)))) { - return undefined; + return { error: 'Could not find export statement.' }; } const exportingModuleSymbol = isSourceFile(exportNode.parent) ? exportNode.parent.symbol : exportNode.parent.parent.symbol; @@ -42,7 +61,7 @@ namespace ts.refactor { const wasDefault = !!(flags & ModifierFlags.Default); // If source file already has a default export, don't offer refactor. if (!(flags & ModifierFlags.Export) || !wasDefault && exportingModuleSymbol.exports!.has(InternalSymbolName.Default)) { - return undefined; + return { error: 'This file already has a default export.' }; } switch (exportNode.kind) { @@ -53,7 +72,7 @@ namespace ts.refactor { case SyntaxKind.TypeAliasDeclaration: case SyntaxKind.ModuleDeclaration: { const node = exportNode as FunctionDeclaration | ClassDeclaration | InterfaceDeclaration | EnumDeclaration | TypeAliasDeclaration | NamespaceDeclaration; - return node.name && isIdentifier(node.name) ? { exportNode: node, exportName: node.name, wasDefault, exportingModuleSymbol } : undefined; + return node.name && isIdentifier(node.name) ? { info: { exportNode: node, exportName: node.name, wasDefault, exportingModuleSymbol } } : undefined; } case SyntaxKind.VariableStatement: { const vs = exportNode as VariableStatement; @@ -64,7 +83,7 @@ namespace ts.refactor { const decl = first(vs.declarationList.declarations); if (!decl.initializer) return undefined; Debug.assert(!wasDefault, "Can't have a default flag here"); - return isIdentifier(decl.name) ? { exportNode: vs, exportName: decl.name, wasDefault, exportingModuleSymbol } : undefined; + return isIdentifier(decl.name) ? { info: { exportNode: vs, exportName: decl.name, wasDefault, exportingModuleSymbol } } : undefined; } default: return undefined; diff --git a/src/services/refactors/convertImport.ts b/src/services/refactors/convertImport.ts index 9d33f086aa844..0d5f5091d4910 100644 --- a/src/services/refactors/convertImport.ts +++ b/src/services/refactors/convertImport.ts @@ -3,30 +3,59 @@ namespace ts.refactor { const refactorName = "Convert import"; const actionNameNamespaceToNamed = "Convert namespace import to named imports"; const actionNameNamedToNamespace = "Convert named imports to namespace import"; + const errorConvertingImport = "Error converting import"; + + type NamedImportBindingsOrError = { + info: NamedImportBindings, + error?: never + } | { + info?: never, + error: string + } + registerRefactor(refactorName, { getAvailableActions(context): readonly ApplicableRefactorInfo[] { const i = getImportToConvert(context, context.triggerReason === "invoked"); if (!i) return emptyArray; - const description = i.kind === SyntaxKind.NamespaceImport ? Diagnostics.Convert_namespace_import_to_named_imports.message : Diagnostics.Convert_named_imports_to_namespace_import.message; - const actionName = i.kind === SyntaxKind.NamespaceImport ? actionNameNamespaceToNamed : actionNameNamedToNamespace; - return [{ name: refactorName, description, actions: [{ name: actionName, description }] }]; + + if (i.error == undefined) { + const description = i.info.kind === SyntaxKind.NamespaceImport ? Diagnostics.Convert_namespace_import_to_named_imports.message : Diagnostics.Convert_named_imports_to_namespace_import.message; + const actionName = i.info.kind === SyntaxKind.NamespaceImport ? actionNameNamespaceToNamed : actionNameNamedToNamespace; + return [{ name: refactorName, description, actions: [{ name: actionName, description }] }]; + } + + if (context.preferences.provideRefactorErrorReason) { + return [{ name: refactorName, description: errorConvertingImport, actions: [{ name: errorConvertingImport, description: errorConvertingImport }] }]; + } + + return emptyArray; }, getEditsForAction(context, actionName): RefactorEditInfo { Debug.assert(actionName === actionNameNamespaceToNamed || actionName === actionNameNamedToNamespace, "Unexpected action name"); - const edits = textChanges.ChangeTracker.with(context, t => doChange(context.file, context.program, t, Debug.checkDefined(getImportToConvert(context), "Context must provide an import to convert"))); + const edits = textChanges.ChangeTracker.with(context, t => doChange(context.file, context.program, t, Debug.checkDefined(getImportToConvert(context)?.info, "Context must provide an import to convert"))); return { edits, renameFilename: undefined, renameLocation: undefined }; } }); // Can convert imports of the form `import * as m from "m";` or `import d, { x, y } from "m";`. - function getImportToConvert(context: RefactorContext, considerPartialSpans = true): NamedImportBindings | undefined { + function getImportToConvert(context: RefactorContext, considerPartialSpans = true): NamedImportBindingsOrError | undefined { const { file } = context; const span = getRefactorContextSpan(context); const token = getTokenAtPosition(file, span.start); const importDecl = considerPartialSpans ? findAncestor(token, isImportDeclaration) : getParentNodeInSpan(token, file, span); - if (!importDecl || !isImportDeclaration(importDecl) || (importDecl.getEnd() < span.start + span.length)) return undefined; + if (!importDecl || !isImportDeclaration(importDecl)) return { error: 'Selection is not an import declaration.' } + if (importDecl.getEnd() < span.start + span.length) return undefined; + const { importClause } = importDecl; - return importClause && importClause.namedBindings; + if (!importClause) { + return { error: 'Could not find import clause.' }; + } + + if (!importClause.namedBindings) { + return { error: 'Could not find namespace import or named imports.' } + } + + return { info: importClause.namedBindings }; } function doChange(sourceFile: SourceFile, program: Program, changes: textChanges.ChangeTracker, toConvert: NamedImportBindings): void { diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 7752a4611ae45..13398e35eb9fb 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -100,11 +100,11 @@ namespace ts.refactor.extractSymbol { const infos: ApplicableRefactorInfo[] = []; - if (constantActions.length) { + if (functionActions.length) { infos.push({ name: refactorName, - description: getLocaleSpecificMessage(Diagnostics.Extract_constant), - actions: constantActions + description: getLocaleSpecificMessage(Diagnostics.Extract_function), + actions: functionActions }); } else if (context.preferences.provideRefactorErrorReason && innermostErrorFunctionAction) { infos.push({ @@ -117,8 +117,8 @@ namespace ts.refactor.extractSymbol { if (constantActions.length) { infos.push({ name: refactorName, - description: getLocaleSpecificMessage(Diagnostics.Extract_function), - actions: functionActions + description: getLocaleSpecificMessage(Diagnostics.Extract_constant), + actions: constantActions }); } else if (context.preferences.provideRefactorErrorReason && innermostErrorConstantAction) { infos.push({ diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index 4824958fe3395..d5c1b36893270 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -4,26 +4,43 @@ namespace ts.refactor { const extractToTypeAlias = "Extract to type alias"; const extractToInterface = "Extract to interface"; const extractToTypeDef = "Extract to typedef"; + const errorExtractingType = "Error extracting type"; registerRefactor(refactorName, { getAvailableActions(context): readonly ApplicableRefactorInfo[] { const info = getRangeToExtract(context, context.triggerReason === "invoked"); if (!info) return emptyArray; - return [{ - name: refactorName, - description: getLocaleSpecificMessage(Diagnostics.Extract_type), - actions: info.isJS ? [{ - name: extractToTypeDef, description: getLocaleSpecificMessage(Diagnostics.Extract_to_typedef) - }] : append([{ - name: extractToTypeAlias, description: getLocaleSpecificMessage(Diagnostics.Extract_to_type_alias) - }], info.typeElements && { - name: extractToInterface, description: getLocaleSpecificMessage(Diagnostics.Extract_to_interface) - }) - }]; + if (info.error === undefined) { + return [{ + name: refactorName, + description: getLocaleSpecificMessage(Diagnostics.Extract_type), + actions: info.info.isJS ? [{ + name: extractToTypeDef, description: getLocaleSpecificMessage(Diagnostics.Extract_to_typedef) + }] : append([{ + name: extractToTypeAlias, description: getLocaleSpecificMessage(Diagnostics.Extract_to_type_alias) + }], info.info.typeElements && { + name: extractToInterface, description: getLocaleSpecificMessage(Diagnostics.Extract_to_interface) + }) + }]; + } + + if (context.preferences.provideRefactorErrorReason) { + return [{ + name: refactorName, + description: getLocaleSpecificMessage(Diagnostics.Extract_type), + actions: [{ + name: errorExtractingType, + description: "Error extracting type", + error: info.error + }] + }]; + } + + return emptyArray; }, getEditsForAction(context, actionName): RefactorEditInfo { const { file, } = context; - const info = Debug.checkDefined(getRangeToExtract(context), "Expected to find a range to extract"); + const info = Debug.checkDefined(getRangeToExtract(context)?.info, "Expected to find a range to extract"); const name = getUniqueName("NewType", file); const edits = textChanges.ChangeTracker.with(context, changes => { @@ -57,8 +74,15 @@ namespace ts.refactor { } type Info = TypeAliasInfo | InterfaceInfo; + type InfoOrError = { + info: Info, + error?: never + } | { + info?: never, + error: string + } - function getRangeToExtract(context: RefactorContext, considerEmptySpans = true): Info | undefined { + function getRangeToExtract(context: RefactorContext, considerEmptySpans = true): InfoOrError | undefined { const { file, startPosition } = context; const isJS = isSourceFileJS(file); const current = getTokenAtPosition(file, startPosition); @@ -67,15 +91,15 @@ namespace ts.refactor { const selection = findAncestor(current, (node => node.parent && isTypeNode(node) && !rangeContainsSkipTrivia(range, node.parent, file) && (cursorRequest || nodeOverlapsWithStartEnd(current, file, range.pos, range.end)))); - if (!selection || !isTypeNode(selection)) return undefined; + if (!selection || !isTypeNode(selection)) return { error: "Selection is not a valid type node." }; const checker = context.program.getTypeChecker(); const firstStatement = Debug.checkDefined(findAncestor(selection, isStatement), "Should find a statement"); const typeParameters = collectTypeParameters(checker, selection, firstStatement, file); - if (!typeParameters) return undefined; + if (!typeParameters) return { error: "No type could be extracted from this type node." }; const typeElements = flattenTypeLiteralNodeReference(checker, selection); - return { isJS, selection, firstStatement, typeParameters, typeElements }; + return { info: { isJS, selection, firstStatement, typeParameters, typeElements } }; } function flattenTypeLiteralNodeReference(checker: TypeChecker, node: TypeNode | undefined): readonly TypeElement[] | undefined { diff --git a/src/services/refactors/generateGetAccessorAndSetAccessor.ts b/src/services/refactors/generateGetAccessorAndSetAccessor.ts index 53bdf728fbe94..b1340db20e8cc 100644 --- a/src/services/refactors/generateGetAccessorAndSetAccessor.ts +++ b/src/services/refactors/generateGetAccessorAndSetAccessor.ts @@ -1,36 +1,54 @@ /* @internal */ namespace ts.refactor.generateGetAccessorAndSetAccessor { const actionName = "Generate 'get' and 'set' accessors"; + const errorActionName = "Error generating 'get' and 'set' accessors"; const actionDescription = Diagnostics.Generate_get_and_set_accessors.message; registerRefactor(actionName, { getEditsForAction(context, actionName) { if (!context.endPosition) return undefined; const info = codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.startPosition, context.endPosition); - if (!info) return undefined; + if (!info || !info.info) return undefined; const edits = codefix.generateAccessorFromProperty(context.file, context.startPosition, context.endPosition, context, actionName); if (!edits) return undefined; const renameFilename = context.file.fileName; - const nameNeedRename = info.renameAccessor ? info.accessorName : info.fieldName; + const nameNeedRename = info.info.renameAccessor ? info.info.accessorName : info.info.fieldName; const renameLocationOffset = isIdentifier(nameNeedRename) ? 0 : -1; - const renameLocation = renameLocationOffset + getRenameLocation(edits, renameFilename, nameNeedRename.text, /*preferLastLocation*/ isParameter(info.declaration)); + const renameLocation = renameLocationOffset + getRenameLocation(edits, renameFilename, nameNeedRename.text, /*preferLastLocation*/ isParameter(info.info.declaration)); return { renameFilename, renameLocation, edits }; }, getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { if (!context.endPosition) return emptyArray; - if (!codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.startPosition, context.endPosition, context.triggerReason === "invoked")) return emptyArray; + const info = codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.startPosition, context.endPosition, context.triggerReason === "invoked"); + if (!info) return emptyArray; - return [{ - name: actionName, - description: actionDescription, - actions: [ - { - name: actionName, - description: actionDescription - } - ] - }]; + if (!info.error) { + return [{ + name: actionName, + description: actionDescription, + actions: [ + { + name: actionName, + description: actionDescription + } + ] + }]; + } + + if (context.preferences.provideRefactorErrorReason) { + return [{ + name: actionName, + description: actionDescription, + actions: [{ + name: errorActionName, + description: "Error generating 'get' and 'set' accessors", + error: info.error + }] + }]; + } + + return emptyArray; } }); } From a57100e2b8a3b3228b18c2704e407b62ad644b83 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Thu, 18 Jun 2020 13:58:04 -0700 Subject: [PATCH 05/10] accept baseline changes --- src/services/types.ts | 4 ++++ tests/baselines/reference/api/tsserverlibrary.d.ts | 6 ++++++ tests/baselines/reference/api/typescript.d.ts | 6 ++++++ 3 files changed, 16 insertions(+) diff --git a/src/services/types.ts b/src/services/types.ts index 54349aa5654be..814f1221fe2f7 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -730,6 +730,10 @@ namespace ts { */ description: string; + /** + * An error message to show to the user if the refactoring cannot be applied in + * the current context. + */ error?: string; } diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index f03ad8d3f6849..e8e0df8ad7114 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -3779,6 +3779,7 @@ declare namespace ts { readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js"; readonly allowTextChangesInNewFiles?: boolean; readonly providePrefixAndSuffixTextForRename?: boolean; + readonly provideRefactorErrorReason?: boolean; } /** Represents a bigint literal value without requiring bigint support */ export interface PseudoBigInt { @@ -5646,6 +5647,11 @@ declare namespace ts { * so this description should make sense by itself if the parent is inlineable=true */ description: string; + /** + * An error message to show to the user if the refactoring cannot be applied in + * the current context. + */ + error?: string; } /** * A set of edits to make in response to a refactor action, plus an optional diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 989a845468197..33d21050b9c5c 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -3779,6 +3779,7 @@ declare namespace ts { readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js"; readonly allowTextChangesInNewFiles?: boolean; readonly providePrefixAndSuffixTextForRename?: boolean; + readonly provideRefactorErrorReason?: boolean; } /** Represents a bigint literal value without requiring bigint support */ export interface PseudoBigInt { @@ -5646,6 +5647,11 @@ declare namespace ts { * so this description should make sense by itself if the parent is inlineable=true */ description: string; + /** + * An error message to show to the user if the refactoring cannot be applied in + * the current context. + */ + error?: string; } /** * A set of edits to make in response to a refactor action, plus an optional From 569c6f8809b85324d28b2c07abcc842773d9c007 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Thu, 18 Jun 2020 14:48:12 -0700 Subject: [PATCH 06/10] fix lint rules --- src/services/codefixes/generateAccessors.ts | 14 ++++----- .../addOrRemoveBracesToArrowFunction.ts | 15 +++++----- src/services/refactors/convertExport.ts | 8 ++--- src/services/refactors/convertImport.ts | 10 +++---- src/services/refactors/extractSymbol.ts | 30 +++++++++++-------- src/services/refactors/extractType.ts | 4 +-- 6 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/services/codefixes/generateAccessors.ts b/src/services/codefixes/generateAccessors.ts index ddad4d6132f8e..a853f5c295d1d 100644 --- a/src/services/codefixes/generateAccessors.ts +++ b/src/services/codefixes/generateAccessors.ts @@ -22,7 +22,7 @@ namespace ts.codefix { } | { info?: never, error: string - } + }; export function generateAccessorFromProperty(file: SourceFile, start: number, end: number, context: textChanges.TextChangesContext, _actionName: string): FileTextChanges[] | undefined { const fieldInfo = getAccessorConvertiblePropertyAtPosition(file, start, end); @@ -121,8 +121,8 @@ namespace ts.codefix { if (!declaration) { return { - error: 'Could not find property for which to generate accessor.' - } + error: "Could not find property for which to generate accessor." + }; } if (!(nodeOverlapsWithStartEnd(declaration.name, file, start, end) || cursorRequest)) { @@ -131,14 +131,14 @@ namespace ts.codefix { if (!isConvertibleName(declaration.name)) { return { - error: 'Name is not valid.' - } + error: "Name is not valid." + }; } if ((getEffectiveModifierFlags(declaration) | meaning) !== meaning) { return { - error: 'Property has invalid accessibility.' - } + error: "Property has invalid accessibility." + }; } const name = declaration.name.text; diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index ae9182100bbad..9cecdfd080016 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -22,7 +22,7 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { } | { info?: never, error: string - } + }; function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { const { file, startPosition, triggerReason } = context; @@ -46,8 +46,7 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { }]; } - if (context.preferences.provideRefactorErrorReason) - { + if (context.preferences.provideRefactorErrorReason) { return [{ name: refactorName, description: refactorDescription, @@ -102,19 +101,19 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { if (!func) { return { - error: 'Could not find a containing arrow function.' - } + error: "Could not find a containing arrow function." + }; } if (!isArrowFunction(func)) { return { - error: 'Containing function is not an arrow function.' - } + error: "Containing function is not an arrow function." + }; } if ((!rangeContainsRange(func, node) || rangeContainsRange(func.body, node) && !considerFunctionBodies)) { return undefined; - } + } if (isExpression(func.body)) { return { diff --git a/src/services/refactors/convertExport.ts b/src/services/refactors/convertExport.ts index ca5885f1e577c..f217df2704442 100644 --- a/src/services/refactors/convertExport.ts +++ b/src/services/refactors/convertExport.ts @@ -10,7 +10,7 @@ namespace ts.refactor { const info = getInfo(context, context.triggerReason === "invoked"); if (!info) return emptyArray; - if (info.error == undefined) { + if (info.error === undefined) { const description = info.info.wasDefault ? Diagnostics.Convert_default_export_to_named_export.message : Diagnostics.Convert_named_export_to_default_export.message; const actionName = info.info.wasDefault ? actionNameDefaultToNamed : actionNameNamedToDefault; return [{ name: refactorName, description, actions: [{ name: actionName, description }] }]; @@ -44,7 +44,7 @@ namespace ts.refactor { } | { info?: never, error: string - } + }; function getInfo(context: RefactorContext, considerPartialSpans = true): InfoOrError | undefined { const { file } = context; @@ -52,7 +52,7 @@ namespace ts.refactor { const token = getTokenAtPosition(file, span.start); const exportNode = !!(token.parent && getSyntacticModifierFlags(token.parent) & ModifierFlags.Export) && considerPartialSpans ? token.parent : getParentNodeInSpan(token, file, span); if (!exportNode || (!isSourceFile(exportNode.parent) && !(isModuleBlock(exportNode.parent) && isAmbientModule(exportNode.parent.parent)))) { - return { error: 'Could not find export statement.' }; + return { error: "Could not find export statement." }; } const exportingModuleSymbol = isSourceFile(exportNode.parent) ? exportNode.parent.symbol : exportNode.parent.parent.symbol; @@ -61,7 +61,7 @@ namespace ts.refactor { const wasDefault = !!(flags & ModifierFlags.Default); // If source file already has a default export, don't offer refactor. if (!(flags & ModifierFlags.Export) || !wasDefault && exportingModuleSymbol.exports!.has(InternalSymbolName.Default)) { - return { error: 'This file already has a default export.' }; + return { error: "This file already has a default export." }; } switch (exportNode.kind) { diff --git a/src/services/refactors/convertImport.ts b/src/services/refactors/convertImport.ts index 0d5f5091d4910..55a9bd3ab8739 100644 --- a/src/services/refactors/convertImport.ts +++ b/src/services/refactors/convertImport.ts @@ -11,14 +11,14 @@ namespace ts.refactor { } | { info?: never, error: string - } + }; registerRefactor(refactorName, { getAvailableActions(context): readonly ApplicableRefactorInfo[] { const i = getImportToConvert(context, context.triggerReason === "invoked"); if (!i) return emptyArray; - if (i.error == undefined) { + if (i.error === undefined) { const description = i.info.kind === SyntaxKind.NamespaceImport ? Diagnostics.Convert_namespace_import_to_named_imports.message : Diagnostics.Convert_named_imports_to_namespace_import.message; const actionName = i.info.kind === SyntaxKind.NamespaceImport ? actionNameNamespaceToNamed : actionNameNamedToNamespace; return [{ name: refactorName, description, actions: [{ name: actionName, description }] }]; @@ -43,16 +43,16 @@ namespace ts.refactor { const span = getRefactorContextSpan(context); const token = getTokenAtPosition(file, span.start); const importDecl = considerPartialSpans ? findAncestor(token, isImportDeclaration) : getParentNodeInSpan(token, file, span); - if (!importDecl || !isImportDeclaration(importDecl)) return { error: 'Selection is not an import declaration.' } + if (!importDecl || !isImportDeclaration(importDecl)) return { error: "Selection is not an import declaration." }; if (importDecl.getEnd() < span.start + span.length) return undefined; const { importClause } = importDecl; if (!importClause) { - return { error: 'Could not find import clause.' }; + return { error: "Could not find import clause." }; } if (!importClause.namedBindings) { - return { error: 'Could not find namespace import or named imports.' } + return { error: "Could not find namespace import or named imports." }; } return { info: importClause.namedBindings }; diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 13398e35eb9fb..95184a40f3643 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -12,7 +12,7 @@ namespace ts.refactor.extractSymbol { const targetRange = rangeToExtract.targetRange; if (targetRange === undefined) { - if (!rangeToExtract.errors || rangeToExtract.errors.length == 0 || !context.preferences.provideRefactorErrorReason) { + if (!rangeToExtract.errors || rangeToExtract.errors.length === 0 || !context.preferences.provideRefactorErrorReason) { return emptyArray; } @@ -21,7 +21,7 @@ namespace ts.refactor.extractSymbol { description: getLocaleSpecificMessage(Diagnostics.Extract_function), actions: [{ description: getLocaleSpecificMessage(Diagnostics.Extract_function), - name: 'extract_function_error', + name: "extract_function_error", error: getStringError(rangeToExtract.errors) }] }, @@ -30,7 +30,7 @@ namespace ts.refactor.extractSymbol { description: getLocaleSpecificMessage(Diagnostics.Extract_constant), actions: [{ description: getLocaleSpecificMessage(Diagnostics.Extract_constant), - name: 'extract_constant_error', + name: "extract_constant_error", error: getStringError(rangeToExtract.errors) }] }]; @@ -44,12 +44,12 @@ namespace ts.refactor.extractSymbol { const functionActions: RefactorActionInfo[] = []; const usedFunctionNames: Map = createMap(); - let innermostErrorFunctionAction: ts.RefactorActionInfo | undefined; + let innermostErrorFunctionAction: RefactorActionInfo | undefined; const constantActions: RefactorActionInfo[] = []; const usedConstantNames: Map = createMap(); - let innermostErrorConstantAction: ts.RefactorActionInfo | undefined; - + let innermostErrorConstantAction: RefactorActionInfo | undefined; + let i = 0; for (const { functionExtraction, constantExtraction } of extractions) { const description = functionExtraction.description; @@ -64,12 +64,13 @@ namespace ts.refactor.extractSymbol { name: `function_scope_${i}` }); } - } else if (!innermostErrorFunctionAction) { + } + else if (!innermostErrorFunctionAction) { innermostErrorFunctionAction = { description, name: `function_scope_${i}`, error: getStringError(functionExtraction.errors) - } + }; } // Skip these since we don't have a way to report errors yet @@ -85,12 +86,13 @@ namespace ts.refactor.extractSymbol { name: `constant_scope_${i}` }); } - } else if (!innermostErrorConstantAction) { + } + else if (!innermostErrorConstantAction) { innermostErrorConstantAction = { description, name: `constant_scope_${i}`, error: getStringError(constantExtraction.errors) - } + }; } // *do* increment i anyway because we'll look for the i-th scope @@ -106,7 +108,8 @@ namespace ts.refactor.extractSymbol { description: getLocaleSpecificMessage(Diagnostics.Extract_function), actions: functionActions }); - } else if (context.preferences.provideRefactorErrorReason && innermostErrorFunctionAction) { + } + else if (context.preferences.provideRefactorErrorReason && innermostErrorFunctionAction) { infos.push({ name: refactorName, description: getLocaleSpecificMessage(Diagnostics.Extract_function), @@ -120,7 +123,8 @@ namespace ts.refactor.extractSymbol { description: getLocaleSpecificMessage(Diagnostics.Extract_constant), actions: constantActions }); - } else if (context.preferences.provideRefactorErrorReason && innermostErrorConstantAction) { + } + else if (context.preferences.provideRefactorErrorReason && innermostErrorConstantAction) { infos.push({ name: refactorName, description: getLocaleSpecificMessage(Diagnostics.Extract_constant), @@ -132,7 +136,7 @@ namespace ts.refactor.extractSymbol { function getStringError(errors: readonly Diagnostic[]) { let error = errors[0].messageText; - if (typeof error !== 'string') { + if (typeof error !== "string") { error = error.messageText; } return error; diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index d5c1b36893270..ea97f71cb98a8 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -33,7 +33,7 @@ namespace ts.refactor { description: "Error extracting type", error: info.error }] - }]; + }]; } return emptyArray; @@ -80,7 +80,7 @@ namespace ts.refactor { } | { info?: never, error: string - } + }; function getRangeToExtract(context: RefactorContext, considerEmptySpans = true): InfoOrError | undefined { const { file, startPosition } = context; From 6b6d9705528d255b5d68c7f2edf60ccad19b5d05 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Fri, 19 Jun 2020 12:33:27 -0700 Subject: [PATCH 07/10] move messages to diagnosticMessages.json --- src/compiler/diagnosticMessages.json | 44 +++++++++++++++++++ src/services/codefixes/generateAccessors.ts | 6 +-- .../addOrRemoveBracesToArrowFunction.ts | 13 +++--- src/services/refactors/convertExport.ts | 10 +++-- src/services/refactors/convertImport.ts | 10 +++-- src/services/refactors/extractType.ts | 15 +++---- .../generateGetAccessorAndSetAccessor.ts | 5 +-- 7 files changed, 76 insertions(+), 27 deletions(-) diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 16dd1663fb5d5..cb313e9fbf6f3 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -5737,6 +5737,50 @@ "category": "Message", "code": 95125 }, + "Could_not_find_a_containing_arrow_function": { + "category": "Message", + "code": 95126 + }, + "Containing_function_is_not_an_arrow_function": { + "category": "Message", + "code": 95127 + }, + "Could_not_find_export_statement": { + "category": "Message", + "code": 95128 + }, + "This_file_already_has_a_default_export": { + "category": "Message", + "code": 95129 + }, + "Could_not_find_import_clause": { + "category": "Message", + "code": 95130 + }, + "Could_not_find_namespace_import_or_named_imports": { + "category": "Message", + "code": 95131 + }, + "Selection_is_not_a_valid_type_node": { + "category": "Message", + "code": 95132 + }, + "No_type_could_be_extracted_from_this_type_node": { + "category": "Message", + "code": 95133 + }, + "Could_not_find_property_for_which_to_generate_accessor": { + "category": "Message", + "code": 95134 + }, + "Name_is_not_valid": { + "category": "Message", + "code": 95135 + }, + "Property_has_invalid_accessibility": { + "category": "Message", + "code": 95136 + }, "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": { "category": "Error", diff --git a/src/services/codefixes/generateAccessors.ts b/src/services/codefixes/generateAccessors.ts index a853f5c295d1d..1e6ab74ee9cd7 100644 --- a/src/services/codefixes/generateAccessors.ts +++ b/src/services/codefixes/generateAccessors.ts @@ -121,7 +121,7 @@ namespace ts.codefix { if (!declaration) { return { - error: "Could not find property for which to generate accessor." + error: Diagnostics.Could_not_find_property_for_which_to_generate_accessor.message }; } @@ -131,13 +131,13 @@ namespace ts.codefix { if (!isConvertibleName(declaration.name)) { return { - error: "Name is not valid." + error: Diagnostics.Name_is_not_valid.message }; } if ((getEffectiveModifierFlags(declaration) | meaning) !== meaning) { return { - error: "Property has invalid accessibility." + error: Diagnostics.Property_has_invalid_accessibility.message }; } diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index 9cecdfd080016..ca782d9ea2d2f 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -4,7 +4,6 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { const refactorDescription = Diagnostics.Add_or_remove_braces_in_an_arrow_function.message; const addBracesActionName = "Add braces to arrow function"; const removeBracesActionName = "Remove braces from arrow function"; - const errorBracesActionName = "Error adding or removing braces from arrow function"; const addBracesActionDescription = Diagnostics.Add_braces_to_arrow_function.message; const removeBracesActionDescription = Diagnostics.Remove_braces_from_arrow_function.message; registerRefactor(refactorName, { getEditsForAction, getAvailableActions }); @@ -51,8 +50,12 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { name: refactorName, description: refactorDescription, actions: [{ - name: errorBracesActionName, - description: "Error adding or removing braces from arrow function", + name: addBracesActionName, + description: addBracesActionDescription, + error: info.error + }, { + name: removeBracesActionName, + description: removeBracesActionDescription, error: info.error }] }]; @@ -101,13 +104,13 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { if (!func) { return { - error: "Could not find a containing arrow function." + error: Diagnostics.Could_not_find_a_containing_arrow_function.message }; } if (!isArrowFunction(func)) { return { - error: "Containing function is not an arrow function." + error: Diagnostics.Containing_function_is_not_an_arrow_function.message }; } diff --git a/src/services/refactors/convertExport.ts b/src/services/refactors/convertExport.ts index f217df2704442..27898187503e7 100644 --- a/src/services/refactors/convertExport.ts +++ b/src/services/refactors/convertExport.ts @@ -3,7 +3,6 @@ namespace ts.refactor { const refactorName = "Convert export"; const actionNameDefaultToNamed = "Convert default export to named export"; const actionNameNamedToDefault = "Convert named export to default export"; - const errorConvertingExport = "Error converting export"; registerRefactor(refactorName, { getAvailableActions(context): readonly ApplicableRefactorInfo[] { @@ -17,7 +16,10 @@ namespace ts.refactor { } if (context.preferences.provideRefactorErrorReason) { - return [{ name: refactorName, description: errorConvertingExport, actions: [{ name: errorConvertingExport, description: errorConvertingExport }] }]; + return [ + { name: refactorName, description: Diagnostics.Convert_default_export_to_named_export.message, actions: [{ name: actionNameDefaultToNamed, description: Diagnostics.Convert_default_export_to_named_export.message, error: info.error }] }, + { name: refactorName, description: Diagnostics.Convert_named_export_to_default_export.message, actions: [{ name: actionNameNamedToDefault, description: Diagnostics.Convert_named_export_to_default_export.message, error: info.error }] }, + ]; } return emptyArray; @@ -52,7 +54,7 @@ namespace ts.refactor { const token = getTokenAtPosition(file, span.start); const exportNode = !!(token.parent && getSyntacticModifierFlags(token.parent) & ModifierFlags.Export) && considerPartialSpans ? token.parent : getParentNodeInSpan(token, file, span); if (!exportNode || (!isSourceFile(exportNode.parent) && !(isModuleBlock(exportNode.parent) && isAmbientModule(exportNode.parent.parent)))) { - return { error: "Could not find export statement." }; + return { error: Diagnostics.Could_not_find_export_statement.message }; } const exportingModuleSymbol = isSourceFile(exportNode.parent) ? exportNode.parent.symbol : exportNode.parent.parent.symbol; @@ -61,7 +63,7 @@ namespace ts.refactor { const wasDefault = !!(flags & ModifierFlags.Default); // If source file already has a default export, don't offer refactor. if (!(flags & ModifierFlags.Export) || !wasDefault && exportingModuleSymbol.exports!.has(InternalSymbolName.Default)) { - return { error: "This file already has a default export." }; + return { error: Diagnostics.This_file_already_has_a_default_export.message }; } switch (exportNode.kind) { diff --git a/src/services/refactors/convertImport.ts b/src/services/refactors/convertImport.ts index 55a9bd3ab8739..e882cdb71bb4f 100644 --- a/src/services/refactors/convertImport.ts +++ b/src/services/refactors/convertImport.ts @@ -3,7 +3,6 @@ namespace ts.refactor { const refactorName = "Convert import"; const actionNameNamespaceToNamed = "Convert namespace import to named imports"; const actionNameNamedToNamespace = "Convert named imports to namespace import"; - const errorConvertingImport = "Error converting import"; type NamedImportBindingsOrError = { info: NamedImportBindings, @@ -25,7 +24,10 @@ namespace ts.refactor { } if (context.preferences.provideRefactorErrorReason) { - return [{ name: refactorName, description: errorConvertingImport, actions: [{ name: errorConvertingImport, description: errorConvertingImport }] }]; + return [ + { name: refactorName, description: Diagnostics.Convert_namespace_import_to_named_imports.message, actions: [{ name: actionNameNamespaceToNamed, description: Diagnostics.Convert_namespace_import_to_named_imports.message, error: i.error }] }, + { name: refactorName, description: Diagnostics.Convert_named_imports_to_namespace_import.message, actions: [{ name: actionNameNamedToNamespace, description: Diagnostics.Convert_named_imports_to_namespace_import.message, error: i.error }] } + ]; } return emptyArray; @@ -48,11 +50,11 @@ namespace ts.refactor { const { importClause } = importDecl; if (!importClause) { - return { error: "Could not find import clause." }; + return { error: Diagnostics.Could_not_find_import_clause.message }; } if (!importClause.namedBindings) { - return { error: "Could not find namespace import or named imports." }; + return { error: Diagnostics.Could_not_find_namespace_import_or_named_imports.message }; } return { info: importClause.namedBindings }; diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index ea97f71cb98a8..5895402fec309 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -4,7 +4,6 @@ namespace ts.refactor { const extractToTypeAlias = "Extract to type alias"; const extractToInterface = "Extract to interface"; const extractToTypeDef = "Extract to typedef"; - const errorExtractingType = "Error extracting type"; registerRefactor(refactorName, { getAvailableActions(context): readonly ApplicableRefactorInfo[] { const info = getRangeToExtract(context, context.triggerReason === "invoked"); @@ -28,11 +27,11 @@ namespace ts.refactor { return [{ name: refactorName, description: getLocaleSpecificMessage(Diagnostics.Extract_type), - actions: [{ - name: errorExtractingType, - description: "Error extracting type", - error: info.error - }] + actions: [ + { name: extractToTypeDef, description: getLocaleSpecificMessage(Diagnostics.Extract_to_typedef), error: info.error }, + { name: extractToTypeAlias, description: getLocaleSpecificMessage(Diagnostics.Extract_to_type_alias), error: info.error }, + { name: extractToInterface, description: getLocaleSpecificMessage(Diagnostics.Extract_to_interface), error: info.error }, + ] }]; } @@ -91,12 +90,12 @@ namespace ts.refactor { const selection = findAncestor(current, (node => node.parent && isTypeNode(node) && !rangeContainsSkipTrivia(range, node.parent, file) && (cursorRequest || nodeOverlapsWithStartEnd(current, file, range.pos, range.end)))); - if (!selection || !isTypeNode(selection)) return { error: "Selection is not a valid type node." }; + if (!selection || !isTypeNode(selection)) return { error: Diagnostics.Selection_is_not_a_valid_type_node.message }; const checker = context.program.getTypeChecker(); const firstStatement = Debug.checkDefined(findAncestor(selection, isStatement), "Should find a statement"); const typeParameters = collectTypeParameters(checker, selection, firstStatement, file); - if (!typeParameters) return { error: "No type could be extracted from this type node." }; + if (!typeParameters) return { error: Diagnostics.No_type_could_be_extracted_from_this_type_node.message }; const typeElements = flattenTypeLiteralNodeReference(checker, selection); return { info: { isJS, selection, firstStatement, typeParameters, typeElements } }; diff --git a/src/services/refactors/generateGetAccessorAndSetAccessor.ts b/src/services/refactors/generateGetAccessorAndSetAccessor.ts index b1340db20e8cc..f0e078659c99f 100644 --- a/src/services/refactors/generateGetAccessorAndSetAccessor.ts +++ b/src/services/refactors/generateGetAccessorAndSetAccessor.ts @@ -1,7 +1,6 @@ /* @internal */ namespace ts.refactor.generateGetAccessorAndSetAccessor { const actionName = "Generate 'get' and 'set' accessors"; - const errorActionName = "Error generating 'get' and 'set' accessors"; const actionDescription = Diagnostics.Generate_get_and_set_accessors.message; registerRefactor(actionName, { getEditsForAction(context, actionName) { @@ -41,8 +40,8 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { name: actionName, description: actionDescription, actions: [{ - name: errorActionName, - description: "Error generating 'get' and 'set' accessors", + name: actionName, + description: actionDescription, error: info.error }] }]; From 8ae83043ceb365084ebe0115a76ed0ba662ae399 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Fri, 19 Jun 2020 12:41:56 -0700 Subject: [PATCH 08/10] rename 'error' to 'notApplicableReason' --- src/compiler/types.ts | 2 +- .../refactors/addOrRemoveBracesToArrowFunction.ts | 6 +++--- src/services/refactors/convertExport.ts | 6 +++--- src/services/refactors/convertImport.ts | 6 +++--- src/services/refactors/extractSymbol.ts | 14 +++++++------- src/services/refactors/extractType.ts | 8 ++++---- .../refactors/generateGetAccessorAndSetAccessor.ts | 4 ++-- src/services/types.ts | 4 ++-- 8 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 5dc1bfca209cf..559f5a514503a 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -8009,7 +8009,7 @@ namespace ts { readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js"; readonly allowTextChangesInNewFiles?: boolean; readonly providePrefixAndSuffixTextForRename?: boolean; - readonly provideRefactorErrorReason?: boolean; + readonly provideRefactorNotApplicableReason?: boolean; } /** Represents a bigint literal value without requiring bigint support */ diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index ca782d9ea2d2f..87060e4bcb281 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -45,18 +45,18 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { }]; } - if (context.preferences.provideRefactorErrorReason) { + if (context.preferences.provideRefactorNotApplicableReason) { return [{ name: refactorName, description: refactorDescription, actions: [{ name: addBracesActionName, description: addBracesActionDescription, - error: info.error + notApplicableReason: info.error }, { name: removeBracesActionName, description: removeBracesActionDescription, - error: info.error + notApplicableReason: info.error }] }]; } diff --git a/src/services/refactors/convertExport.ts b/src/services/refactors/convertExport.ts index 27898187503e7..07a5b2d764118 100644 --- a/src/services/refactors/convertExport.ts +++ b/src/services/refactors/convertExport.ts @@ -15,10 +15,10 @@ namespace ts.refactor { return [{ name: refactorName, description, actions: [{ name: actionName, description }] }]; } - if (context.preferences.provideRefactorErrorReason) { + if (context.preferences.provideRefactorNotApplicableReason) { return [ - { name: refactorName, description: Diagnostics.Convert_default_export_to_named_export.message, actions: [{ name: actionNameDefaultToNamed, description: Diagnostics.Convert_default_export_to_named_export.message, error: info.error }] }, - { name: refactorName, description: Diagnostics.Convert_named_export_to_default_export.message, actions: [{ name: actionNameNamedToDefault, description: Diagnostics.Convert_named_export_to_default_export.message, error: info.error }] }, + { name: refactorName, description: Diagnostics.Convert_default_export_to_named_export.message, actions: [{ name: actionNameDefaultToNamed, description: Diagnostics.Convert_default_export_to_named_export.message, notApplicableReason: info.error }] }, + { name: refactorName, description: Diagnostics.Convert_named_export_to_default_export.message, actions: [{ name: actionNameNamedToDefault, description: Diagnostics.Convert_named_export_to_default_export.message, notApplicableReason: info.error }] }, ]; } diff --git a/src/services/refactors/convertImport.ts b/src/services/refactors/convertImport.ts index e882cdb71bb4f..90766629839b9 100644 --- a/src/services/refactors/convertImport.ts +++ b/src/services/refactors/convertImport.ts @@ -23,10 +23,10 @@ namespace ts.refactor { return [{ name: refactorName, description, actions: [{ name: actionName, description }] }]; } - if (context.preferences.provideRefactorErrorReason) { + if (context.preferences.provideRefactorNotApplicableReason) { return [ - { name: refactorName, description: Diagnostics.Convert_namespace_import_to_named_imports.message, actions: [{ name: actionNameNamespaceToNamed, description: Diagnostics.Convert_namespace_import_to_named_imports.message, error: i.error }] }, - { name: refactorName, description: Diagnostics.Convert_named_imports_to_namespace_import.message, actions: [{ name: actionNameNamedToNamespace, description: Diagnostics.Convert_named_imports_to_namespace_import.message, error: i.error }] } + { name: refactorName, description: Diagnostics.Convert_namespace_import_to_named_imports.message, actions: [{ name: actionNameNamespaceToNamed, description: Diagnostics.Convert_namespace_import_to_named_imports.message, notApplicableReason: i.error }] }, + { name: refactorName, description: Diagnostics.Convert_named_imports_to_namespace_import.message, actions: [{ name: actionNameNamedToNamespace, description: Diagnostics.Convert_named_imports_to_namespace_import.message, notApplicableReason: i.error }] } ]; } diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 95184a40f3643..f931952ef11cd 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -12,7 +12,7 @@ namespace ts.refactor.extractSymbol { const targetRange = rangeToExtract.targetRange; if (targetRange === undefined) { - if (!rangeToExtract.errors || rangeToExtract.errors.length === 0 || !context.preferences.provideRefactorErrorReason) { + if (!rangeToExtract.errors || rangeToExtract.errors.length === 0 || !context.preferences.provideRefactorNotApplicableReason) { return emptyArray; } @@ -22,7 +22,7 @@ namespace ts.refactor.extractSymbol { actions: [{ description: getLocaleSpecificMessage(Diagnostics.Extract_function), name: "extract_function_error", - error: getStringError(rangeToExtract.errors) + notApplicableReason: getStringError(rangeToExtract.errors) }] }, { @@ -31,7 +31,7 @@ namespace ts.refactor.extractSymbol { actions: [{ description: getLocaleSpecificMessage(Diagnostics.Extract_constant), name: "extract_constant_error", - error: getStringError(rangeToExtract.errors) + notApplicableReason: getStringError(rangeToExtract.errors) }] }]; } @@ -69,7 +69,7 @@ namespace ts.refactor.extractSymbol { innermostErrorFunctionAction = { description, name: `function_scope_${i}`, - error: getStringError(functionExtraction.errors) + notApplicableReason: getStringError(functionExtraction.errors) }; } @@ -91,7 +91,7 @@ namespace ts.refactor.extractSymbol { innermostErrorConstantAction = { description, name: `constant_scope_${i}`, - error: getStringError(constantExtraction.errors) + notApplicableReason: getStringError(constantExtraction.errors) }; } @@ -109,7 +109,7 @@ namespace ts.refactor.extractSymbol { actions: functionActions }); } - else if (context.preferences.provideRefactorErrorReason && innermostErrorFunctionAction) { + else if (context.preferences.provideRefactorNotApplicableReason && innermostErrorFunctionAction) { infos.push({ name: refactorName, description: getLocaleSpecificMessage(Diagnostics.Extract_function), @@ -124,7 +124,7 @@ namespace ts.refactor.extractSymbol { actions: constantActions }); } - else if (context.preferences.provideRefactorErrorReason && innermostErrorConstantAction) { + else if (context.preferences.provideRefactorNotApplicableReason && innermostErrorConstantAction) { infos.push({ name: refactorName, description: getLocaleSpecificMessage(Diagnostics.Extract_constant), diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index 5895402fec309..c8f429056f5a6 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -23,14 +23,14 @@ namespace ts.refactor { }]; } - if (context.preferences.provideRefactorErrorReason) { + if (context.preferences.provideRefactorNotApplicableReason) { return [{ name: refactorName, description: getLocaleSpecificMessage(Diagnostics.Extract_type), actions: [ - { name: extractToTypeDef, description: getLocaleSpecificMessage(Diagnostics.Extract_to_typedef), error: info.error }, - { name: extractToTypeAlias, description: getLocaleSpecificMessage(Diagnostics.Extract_to_type_alias), error: info.error }, - { name: extractToInterface, description: getLocaleSpecificMessage(Diagnostics.Extract_to_interface), error: info.error }, + { name: extractToTypeDef, description: getLocaleSpecificMessage(Diagnostics.Extract_to_typedef), notApplicableReason: info.error }, + { name: extractToTypeAlias, description: getLocaleSpecificMessage(Diagnostics.Extract_to_type_alias), notApplicableReason: info.error }, + { name: extractToInterface, description: getLocaleSpecificMessage(Diagnostics.Extract_to_interface), notApplicableReason: info.error }, ] }]; } diff --git a/src/services/refactors/generateGetAccessorAndSetAccessor.ts b/src/services/refactors/generateGetAccessorAndSetAccessor.ts index f0e078659c99f..7ac211ed76515 100644 --- a/src/services/refactors/generateGetAccessorAndSetAccessor.ts +++ b/src/services/refactors/generateGetAccessorAndSetAccessor.ts @@ -35,14 +35,14 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { }]; } - if (context.preferences.provideRefactorErrorReason) { + if (context.preferences.provideRefactorNotApplicableReason) { return [{ name: actionName, description: actionDescription, actions: [{ name: actionName, description: actionDescription, - error: info.error + notApplicableReason: info.error }] }]; } diff --git a/src/services/types.ts b/src/services/types.ts index 814f1221fe2f7..7c294dcbed9a5 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -731,10 +731,10 @@ namespace ts { description: string; /** - * An error message to show to the user if the refactoring cannot be applied in + * A message to show to the user if the refactoring cannot be applied in * the current context. */ - error?: string; + notApplicableReason?: string; } /** From 7734694af58d5bb9a5dec35964891bd76cd7acf5 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Fri, 19 Jun 2020 13:39:53 -0700 Subject: [PATCH 09/10] accept baseline change --- tests/baselines/reference/api/tsserverlibrary.d.ts | 6 +++--- tests/baselines/reference/api/typescript.d.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 082f46be159f5..c528d3738bc8f 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -3789,7 +3789,7 @@ declare namespace ts { readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js"; readonly allowTextChangesInNewFiles?: boolean; readonly providePrefixAndSuffixTextForRename?: boolean; - readonly provideRefactorErrorReason?: boolean; + readonly provideRefactorNotApplicableReason?: boolean; } /** Represents a bigint literal value without requiring bigint support */ export interface PseudoBigInt { @@ -5662,10 +5662,10 @@ declare namespace ts { */ description: string; /** - * An error message to show to the user if the refactoring cannot be applied in + * A message to show to the user if the refactoring cannot be applied in * the current context. */ - error?: string; + notApplicableReason?: string; } /** * A set of edits to make in response to a refactor action, plus an optional diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 679269d8e5f47..f5c952615866a 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -3789,7 +3789,7 @@ declare namespace ts { readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js"; readonly allowTextChangesInNewFiles?: boolean; readonly providePrefixAndSuffixTextForRename?: boolean; - readonly provideRefactorErrorReason?: boolean; + readonly provideRefactorNotApplicableReason?: boolean; } /** Represents a bigint literal value without requiring bigint support */ export interface PseudoBigInt { @@ -5662,10 +5662,10 @@ declare namespace ts { */ description: string; /** - * An error message to show to the user if the refactoring cannot be applied in + * A message to show to the user if the refactoring cannot be applied in * the current context. */ - error?: string; + notApplicableReason?: string; } /** * A set of edits to make in response to a refactor action, plus an optional From cf992adf4bc5ee923a93b51bcaf230ec713f2086 Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Mon, 22 Jun 2020 07:47:37 -0700 Subject: [PATCH 10/10] address comments --- src/compiler/diagnosticMessages.json | 22 +++++++++---------- src/services/codefixes/generateAccessors.ts | 12 ++++------ .../addOrRemoveBracesToArrowFunction.ts | 4 ++-- src/services/refactors/convertExport.ts | 4 ++-- src/services/refactors/convertImport.ts | 4 ++-- src/services/refactors/extractSymbol.ts | 4 ++-- src/services/refactors/extractType.ts | 4 ++-- 7 files changed, 25 insertions(+), 29 deletions(-) diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index f0827d824e647..5f29748d4ef82 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -5755,47 +5755,47 @@ "category": "Message", "code": 95126 }, - "Could_not_find_a_containing_arrow_function": { + "Could not find a containing arrow function": { "category": "Message", "code": 95127 }, - "Containing_function_is_not_an_arrow_function": { + "Containing function is not an arrow function": { "category": "Message", "code": 95128 }, - "Could_not_find_export_statement": { + "Could not find export statement": { "category": "Message", "code": 95129 }, - "This_file_already_has_a_default_export": { + "This file already has a default export": { "category": "Message", "code": 95130 }, - "Could_not_find_import_clause": { + "Could not find import clause": { "category": "Message", "code": 95131 }, - "Could_not_find_namespace_import_or_named_imports": { + "Could not find namespace import or named imports": { "category": "Message", "code": 95132 }, - "Selection_is_not_a_valid_type_node": { + "Selection is not a valid type node": { "category": "Message", "code": 95133 }, - "No_type_could_be_extracted_from_this_type_node": { + "No type could be extracted from this type node": { "category": "Message", "code": 95134 }, - "Could_not_find_property_for_which_to_generate_accessor": { + "Could not find property for which to generate accessor": { "category": "Message", "code": 95135 }, - "Name_is_not_valid": { + "Name is not valid": { "category": "Message", "code": 95136 }, - "Property_has_invalid_accessibility": { + "Can only convert property with modifier": { "category": "Message", "code": 95137 }, diff --git a/src/services/codefixes/generateAccessors.ts b/src/services/codefixes/generateAccessors.ts index 1e6ab74ee9cd7..518278d041c12 100644 --- a/src/services/codefixes/generateAccessors.ts +++ b/src/services/codefixes/generateAccessors.ts @@ -119,25 +119,21 @@ namespace ts.codefix { // make sure declaration have AccessibilityModifier or Static Modifier or Readonly Modifier const meaning = ModifierFlags.AccessibilityModifier | ModifierFlags.Static | ModifierFlags.Readonly; - if (!declaration) { + if (!declaration || (!(nodeOverlapsWithStartEnd(declaration.name, file, start, end) || cursorRequest))) { return { - error: Diagnostics.Could_not_find_property_for_which_to_generate_accessor.message + error: getLocaleSpecificMessage(Diagnostics.Could_not_find_property_for_which_to_generate_accessor) }; } - if (!(nodeOverlapsWithStartEnd(declaration.name, file, start, end) || cursorRequest)) { - return undefined; - } - if (!isConvertibleName(declaration.name)) { return { - error: Diagnostics.Name_is_not_valid.message + error: getLocaleSpecificMessage(Diagnostics.Name_is_not_valid) }; } if ((getEffectiveModifierFlags(declaration) | meaning) !== meaning) { return { - error: Diagnostics.Property_has_invalid_accessibility.message + error: getLocaleSpecificMessage(Diagnostics.Can_only_convert_property_with_modifier) }; } diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index 87060e4bcb281..a2a14152da738 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -104,13 +104,13 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { if (!func) { return { - error: Diagnostics.Could_not_find_a_containing_arrow_function.message + error: getLocaleSpecificMessage(Diagnostics.Could_not_find_a_containing_arrow_function) }; } if (!isArrowFunction(func)) { return { - error: Diagnostics.Containing_function_is_not_an_arrow_function.message + error: getLocaleSpecificMessage(Diagnostics.Containing_function_is_not_an_arrow_function) }; } diff --git a/src/services/refactors/convertExport.ts b/src/services/refactors/convertExport.ts index 07a5b2d764118..1ab52fadc9bd4 100644 --- a/src/services/refactors/convertExport.ts +++ b/src/services/refactors/convertExport.ts @@ -54,7 +54,7 @@ namespace ts.refactor { const token = getTokenAtPosition(file, span.start); const exportNode = !!(token.parent && getSyntacticModifierFlags(token.parent) & ModifierFlags.Export) && considerPartialSpans ? token.parent : getParentNodeInSpan(token, file, span); if (!exportNode || (!isSourceFile(exportNode.parent) && !(isModuleBlock(exportNode.parent) && isAmbientModule(exportNode.parent.parent)))) { - return { error: Diagnostics.Could_not_find_export_statement.message }; + return { error: getLocaleSpecificMessage(Diagnostics.Could_not_find_export_statement) }; } const exportingModuleSymbol = isSourceFile(exportNode.parent) ? exportNode.parent.symbol : exportNode.parent.parent.symbol; @@ -63,7 +63,7 @@ namespace ts.refactor { const wasDefault = !!(flags & ModifierFlags.Default); // If source file already has a default export, don't offer refactor. if (!(flags & ModifierFlags.Export) || !wasDefault && exportingModuleSymbol.exports!.has(InternalSymbolName.Default)) { - return { error: Diagnostics.This_file_already_has_a_default_export.message }; + return { error: getLocaleSpecificMessage(Diagnostics.This_file_already_has_a_default_export) }; } switch (exportNode.kind) { diff --git a/src/services/refactors/convertImport.ts b/src/services/refactors/convertImport.ts index 90766629839b9..5ab2b316d5cc8 100644 --- a/src/services/refactors/convertImport.ts +++ b/src/services/refactors/convertImport.ts @@ -50,11 +50,11 @@ namespace ts.refactor { const { importClause } = importDecl; if (!importClause) { - return { error: Diagnostics.Could_not_find_import_clause.message }; + return { error: getLocaleSpecificMessage(Diagnostics.Could_not_find_import_clause) }; } if (!importClause.namedBindings) { - return { error: Diagnostics.Could_not_find_namespace_import_or_named_imports.message }; + return { error: getLocaleSpecificMessage(Diagnostics.Could_not_find_namespace_import_or_named_imports) }; } return { info: importClause.namedBindings }; diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index f931952ef11cd..2fce499024a0e 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -21,7 +21,7 @@ namespace ts.refactor.extractSymbol { description: getLocaleSpecificMessage(Diagnostics.Extract_function), actions: [{ description: getLocaleSpecificMessage(Diagnostics.Extract_function), - name: "extract_function_error", + name: "function_extract_error", notApplicableReason: getStringError(rangeToExtract.errors) }] }, @@ -30,7 +30,7 @@ namespace ts.refactor.extractSymbol { description: getLocaleSpecificMessage(Diagnostics.Extract_constant), actions: [{ description: getLocaleSpecificMessage(Diagnostics.Extract_constant), - name: "extract_constant_error", + name: "constant_extract_error", notApplicableReason: getStringError(rangeToExtract.errors) }] }]; diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index c8f429056f5a6..ef922bce633b8 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -90,12 +90,12 @@ namespace ts.refactor { const selection = findAncestor(current, (node => node.parent && isTypeNode(node) && !rangeContainsSkipTrivia(range, node.parent, file) && (cursorRequest || nodeOverlapsWithStartEnd(current, file, range.pos, range.end)))); - if (!selection || !isTypeNode(selection)) return { error: Diagnostics.Selection_is_not_a_valid_type_node.message }; + if (!selection || !isTypeNode(selection)) return { error: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_type_node) }; const checker = context.program.getTypeChecker(); const firstStatement = Debug.checkDefined(findAncestor(selection, isStatement), "Should find a statement"); const typeParameters = collectTypeParameters(checker, selection, firstStatement, file); - if (!typeParameters) return { error: Diagnostics.No_type_could_be_extracted_from_this_type_node.message }; + if (!typeParameters) return { error: getLocaleSpecificMessage(Diagnostics.No_type_could_be_extracted_from_this_type_node) }; const typeElements = flattenTypeLiteralNodeReference(checker, selection); return { info: { isJS, selection, firstStatement, typeParameters, typeElements } };