Skip to content

Add reason for a disabled code action #37871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

44 changes: 44 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -5755,6 +5755,50 @@
"category": "Message",
"code": 95126
},
"Could not find a containing arrow function": {
"category": "Message",
"code": 95127
},
"Containing function is not an arrow function": {
"category": "Message",
"code": 95128
},
"Could not find export statement": {
"category": "Message",
"code": 95129
},
"This file already has a default export": {
"category": "Message",
"code": 95130
},
"Could not find import clause": {
"category": "Message",
"code": 95131
},
"Could not find namespace import or named imports": {
"category": "Message",
"code": 95132
},
"Selection is not a valid type node": {
"category": "Message",
"code": 95133
},
"No type could be extracted from this type node": {
"category": "Message",
"code": 95134
},
"Could not find property for which to generate accessor": {
"category": "Message",
"code": 95135
},
"Name is not valid": {
"category": "Message",
"code": 95136
},
"Can only convert property with modifier": {
"category": "Message",
"code": 95137
},

"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
"category": "Error",
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8024,6 +8024,7 @@ namespace ts {
readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js";
readonly allowTextChangesInNewFiles?: boolean;
readonly providePrefixAndSuffixTextForRename?: boolean;
readonly provideRefactorNotApplicableReason?: boolean;
}

/** Represents a bigint literal value without requiring bigint support */
Expand Down
54 changes: 40 additions & 14 deletions src/services/codefixes/generateAccessors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,20 @@ namespace ts.codefix {
readonly renameAccessor: boolean;
}

type InfoOrError = {
info: Info,
error?: never
} | {
info?: never,
error: string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would DiagnosticMessage be narrower and lazier than 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);
Expand Down Expand Up @@ -104,29 +112,47 @@ 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 || (!(nodeOverlapsWithStartEnd(declaration.name, file, start, end) || cursorRequest))) {
return {
error: getLocaleSpecificMessage(Diagnostics.Could_not_find_property_for_which_to_generate_accessor)
};
}

if (!isConvertibleName(declaration.name)) {
return {
error: getLocaleSpecificMessage(Diagnostics.Name_is_not_valid)
};
}

if ((getEffectiveModifierFlags(declaration) | meaning) !== meaning) {
return {
error: getLocaleSpecificMessage(Diagnostics.Can_only_convert_property_with_modifier)
};
}

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: (<AcceptedNameType>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: (<AcceptedNameType>declaration.name).text,
declaration,
fieldName,
accessorName,
renameAccessor: startWithUnderscore
}
};
}

Expand Down
99 changes: 72 additions & 27 deletions src/services/refactors/addOrRemoveBracesToArrowFunction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,33 +15,61 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction {
addBraces: boolean;
}

type InfoOrError = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this type should be generic and reused?

info: Info,
error?: never
} | {
info?: never,
error: string
};

function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] {
const { file, startPosition, triggerReason } = context;
const info = getConvertibleArrowFunctionAtPosition(file, startPosition, triggerReason === "invoked");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I'd take the hit and rename the outer info so we don't have info.info everywhere. Maybe result?

if (!info) return emptyArray;

return [{
name: refactorName,
description: refactorDescription,
actions: [
info.addBraces ?
{
name: addBracesActionName,
description: addBracesActionDescription
} : {
name: removeBracesActionName,
description: removeBracesActionDescription
}
]
}];
if (info.error === undefined) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other places, this is a non-null check on info.info.

return [{
name: refactorName,
description: refactorDescription,
actions: [
info.info.addBraces ?
{
name: addBracesActionName,
description: addBracesActionDescription
} : {
name: removeBracesActionName,
description: removeBracesActionDescription
}
]
}];
}

if (context.preferences.provideRefactorNotApplicableReason) {
return [{
name: refactorName,
description: refactorDescription,
actions: [{
name: addBracesActionName,
description: addBracesActionDescription,
notApplicableReason: info.error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without having read the code in detail, it seems intuitively unlikely to me that add-braces and remove-braces would fail for the same reason.

}, {
name: removeBracesActionName,
description: removeBracesActionDescription,
notApplicableReason: info.error
}]
}];
}

return emptyArray;
}

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;

Expand Down Expand Up @@ -70,28 +98,45 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction {
return { renameFilename: undefined, renameLocation: undefined, edits };
}

function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number, considerFunctionBodies = true): Info | undefined {
function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number, considerFunctionBodies = true): InfoOrError | undefined {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this still return undefined? When?

const node = getTokenAtPosition(file, startPosition);
const func = getContainingFunction(node);
// Only offer a refactor in the function body on explicit refactor requests.
if (!func || !isArrowFunction(func) || (!rangeContainsRange(func, node)
|| (rangeContainsRange(func.body, node) && !considerFunctionBodies))) return undefined;

if (!func) {
return {
error: getLocaleSpecificMessage(Diagnostics.Could_not_find_a_containing_arrow_function)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or any other sort of containing function?

};
}

if (!isArrowFunction(func)) {
return {
error: getLocaleSpecificMessage(Diagnostics.Containing_function_is_not_an_arrow_function)
};
}

if ((!rangeContainsRange(func, node) || rangeContainsRange(func.body, node) && !considerFunctionBodies)) {
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
}
};
}
}
Expand Down
39 changes: 30 additions & 9 deletions src/services/refactors/convertExport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,30 @@ namespace ts.refactor {
const refactorName = "Convert export";
const actionNameDefaultToNamed = "Convert default export to named export";
const actionNameNamedToDefault = "Convert named export to default 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.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, 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 }] },
];
}

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 };
},
});
Expand All @@ -27,13 +40,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: getLocaleSpecificMessage(Diagnostics.Could_not_find_export_statement) };
}

const exportingModuleSymbol = isSourceFile(exportNode.parent) ? exportNode.parent.symbol : exportNode.parent.parent.symbol;
Expand All @@ -42,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 undefined;
return { error: getLocaleSpecificMessage(Diagnostics.This_file_already_has_a_default_export) };
}

switch (exportNode.kind) {
Expand All @@ -53,7 +74,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;
Expand All @@ -64,7 +85,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;
Expand Down
Loading