Skip to content

Commit 1702238

Browse files
Add refactor convertToOptionalChainExpression (#39135)
* add convertOptionalChain * cover more cases * expose containsMatchingReference * clean up performing edits * bound start position * add tests * refactor and handle edge cases * update tests * consider explicit requests for empty spans * update fourslash to use trigger reason * add tests cases for trigger reason * fix errors * remove type assertion * fix non ampersand chains * clean up some logic * add ternary case * add diagnostic message * add nullish check for ternary expressions * Update src/services/refactors/convertToOptionalChainExpression.ts Co-authored-by: Nathan Shively-Sanders <[email protected]> * Update src/services/refactors/convertToOptionalChainExpression.ts Co-authored-by: Nathan Shively-Sanders <[email protected]> * Update tests/cases/fourslash/refactorConvertToOptionalChainExpressionForTriggerReason3.ts Co-authored-by: Nathan Shively-Sanders <[email protected]> * Update tests/cases/fourslash/refactorConvertToOptionalChainExpressionForTriggerReason1.ts Co-authored-by: Nathan Shively-Sanders <[email protected]> * reformat and remove unused checks * allow any for ternary refactor * add tests * add tests * check return and variable statements * use isMatchingReference instead of containsMatchingReference * allow partial selections * fine tune selection ranges * recurse for call expressions * fix spellings * add recursive cases * remove isOrContainsMatchingReference * cleanup * more refactoring * cleanup * rename tests * address PR comments * check match syntactically * handle another call expression case * some renames * inline some checks * add test * address comments * add refactorNotAvailableReason Co-authored-by: Nathan Shively-Sanders <[email protected]>
1 parent 6279f98 commit 1702238

File tree

52 files changed

+1069
-4
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+1069
-4
lines changed

src/compiler/diagnosticMessages.json

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5827,6 +5827,22 @@
58275827
"category": "Message",
58285828
"code": 95138
58295829
},
5830+
"Convert to optional chain expression": {
5831+
"category": "Message",
5832+
"code": 95139
5833+
},
5834+
"Could not find convertible access expression": {
5835+
"category": "Message",
5836+
"code": 95140
5837+
},
5838+
"Could not find matching access expressions": {
5839+
"category": "Message",
5840+
"code": 95141
5841+
},
5842+
"Can only convert logical AND access chains": {
5843+
"category": "Message",
5844+
"code": 95142
5845+
},
58305846

58315847
"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
58325848
"category": "Error",

src/compiler/utilities.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2452,7 +2452,7 @@ namespace ts {
24522452
}
24532453
}
24542454

2455-
function getSingleVariableOfVariableStatement(node: Node): VariableDeclaration | undefined {
2455+
export function getSingleVariableOfVariableStatement(node: Node): VariableDeclaration | undefined {
24562456
return isVariableStatement(node) ? firstOrUndefined(node.declarationList.declarations) : undefined;
24572457
}
24582458

src/harness/fourslashImpl.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3289,9 +3289,9 @@ namespace FourSlash {
32893289
}
32903290
}
32913291

3292-
public applyRefactor({ refactorName, actionName, actionDescription, newContent: newContentWithRenameMarker }: FourSlashInterface.ApplyRefactorOptions) {
3292+
public applyRefactor({ refactorName, actionName, actionDescription, newContent: newContentWithRenameMarker, triggerReason }: FourSlashInterface.ApplyRefactorOptions) {
32933293
const range = this.getSelection();
3294-
const refactors = this.getApplicableRefactorsAtSelection();
3294+
const refactors = this.getApplicableRefactorsAtSelection(triggerReason);
32953295
const refactorsWithName = refactors.filter(r => r.name === refactorName);
32963296
if (refactorsWithName.length === 0) {
32973297
this.raiseError(`The expected refactor: ${refactorName} is not available at the marker location.\nAvailable refactors: ${refactors.map(r => r.name)}`);

src/harness/fourslashInterfaceImpl.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1503,6 +1503,7 @@ namespace FourSlashInterface {
15031503
actionName: string;
15041504
actionDescription: string;
15051505
newContent: NewFileContent;
1506+
triggerReason?: ts.RefactorTriggerReason;
15061507
}
15071508

15081509
export type ExpectedCompletionEntry = string | ExpectedCompletionEntryObject;
Lines changed: 284 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,284 @@
1+
/* @internal */
2+
namespace ts.refactor.convertToOptionalChainExpression {
3+
const refactorName = "Convert to optional chain expression";
4+
const convertToOptionalChainExpressionMessage = getLocaleSpecificMessage(Diagnostics.Convert_to_optional_chain_expression);
5+
6+
registerRefactor(refactorName, { getAvailableActions, getEditsForAction });
7+
8+
function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] {
9+
const info = getInfo(context, context.triggerReason === "invoked");
10+
if (!info) return emptyArray;
11+
12+
if (!info.error) {
13+
return [{
14+
name: refactorName,
15+
description: convertToOptionalChainExpressionMessage,
16+
actions: [{
17+
name: refactorName,
18+
description: convertToOptionalChainExpressionMessage
19+
}]
20+
}];
21+
}
22+
23+
if (context.preferences.provideRefactorNotApplicableReason) {
24+
return [{
25+
name: refactorName,
26+
description: convertToOptionalChainExpressionMessage,
27+
actions: [{
28+
name: refactorName,
29+
description: convertToOptionalChainExpressionMessage,
30+
notApplicableReason: info.error
31+
}]
32+
}];
33+
}
34+
return emptyArray;
35+
}
36+
37+
function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined {
38+
const info = getInfo(context);
39+
if (!info || !info.info) return undefined;
40+
const edits = textChanges.ChangeTracker.with(context, t =>
41+
doChange(context.file, context.program.getTypeChecker(), t, Debug.checkDefined(info.info, "context must have info"), actionName)
42+
);
43+
return { edits, renameFilename: undefined, renameLocation: undefined };
44+
}
45+
46+
type InfoOrError = {
47+
info: Info,
48+
error?: never;
49+
} | {
50+
info?: never,
51+
error: string;
52+
};
53+
54+
interface Info {
55+
finalExpression: PropertyAccessExpression | CallExpression,
56+
occurrences: (PropertyAccessExpression | Identifier)[],
57+
expression: ValidExpression,
58+
};
59+
60+
type ValidExpressionOrStatement = ValidExpression | ValidStatement;
61+
62+
/**
63+
* Types for which a "Convert to optional chain refactor" are offered.
64+
*/
65+
type ValidExpression = BinaryExpression | ConditionalExpression;
66+
67+
/**
68+
* Types of statements which are likely to include a valid expression for extraction.
69+
*/
70+
type ValidStatement = ExpressionStatement | ReturnStatement | VariableStatement;
71+
72+
function isValidExpression(node: Node): node is ValidExpression {
73+
return isBinaryExpression(node) || isConditionalExpression(node);
74+
}
75+
76+
function isValidStatement(node: Node): node is ValidStatement {
77+
return isExpressionStatement(node) || isReturnStatement(node) || isVariableStatement(node);
78+
}
79+
80+
function isValidExpressionOrStatement(node: Node): node is ValidExpressionOrStatement {
81+
return isValidExpression(node) || isValidStatement(node);
82+
}
83+
84+
function getInfo(context: RefactorContext, considerEmptySpans = true): InfoOrError | undefined {
85+
const { file, program } = context;
86+
const span = getRefactorContextSpan(context);
87+
88+
const forEmptySpan = span.length === 0;
89+
if (forEmptySpan && !considerEmptySpans) return undefined;
90+
91+
// selecting fo[|o && foo.ba|]r should be valid, so adjust span to fit start and end tokens
92+
const startToken = getTokenAtPosition(file, span.start);
93+
const endToken = findTokenOnLeftOfPosition(file, span.start + span.length);
94+
const adjustedSpan = createTextSpanFromBounds(startToken.pos, endToken && endToken.end >= startToken.pos ? endToken.getEnd() : startToken.getEnd());
95+
96+
const parent = forEmptySpan ? getValidParentNodeOfEmptySpan(startToken) : getValidParentNodeContainingSpan(startToken, adjustedSpan);
97+
const expression = parent && isValidExpressionOrStatement(parent) ? getExpression(parent) : undefined;
98+
if (!expression) return { error: getLocaleSpecificMessage(Diagnostics.Could_not_find_convertible_access_expression) };
99+
100+
const checker = program.getTypeChecker();
101+
return isConditionalExpression(expression) ? getConditionalInfo(expression, checker) : getBinaryInfo(expression);
102+
}
103+
104+
function getConditionalInfo(expression: ConditionalExpression, checker: TypeChecker): InfoOrError | undefined {
105+
const condition = expression.condition;
106+
const finalExpression = getFinalExpressionInChain(expression.whenTrue);
107+
108+
if (!finalExpression || checker.isNullableType(checker.getTypeAtLocation(finalExpression))) {
109+
return { error: getLocaleSpecificMessage(Diagnostics.Could_not_find_convertible_access_expression) };
110+
};
111+
112+
if ((isPropertyAccessExpression(condition) || isIdentifier(condition))
113+
&& getMatchingStart(condition, finalExpression.expression)) {
114+
return { info: { finalExpression, occurrences: [condition], expression } };
115+
}
116+
else if (isBinaryExpression(condition)) {
117+
const occurrences = getOccurrencesInExpression(finalExpression.expression, condition);
118+
return occurrences ? { info: { finalExpression, occurrences, expression } } :
119+
{ error: getLocaleSpecificMessage(Diagnostics.Could_not_find_matching_access_expressions) };
120+
}
121+
}
122+
123+
function getBinaryInfo(expression: BinaryExpression): InfoOrError | undefined {
124+
if (expression.operatorToken.kind !== SyntaxKind.AmpersandAmpersandToken) {
125+
return { error: getLocaleSpecificMessage(Diagnostics.Can_only_convert_logical_AND_access_chains) };
126+
};
127+
const finalExpression = getFinalExpressionInChain(expression.right);
128+
129+
if (!finalExpression) return { error: getLocaleSpecificMessage(Diagnostics.Could_not_find_convertible_access_expression) };
130+
131+
const occurrences = getOccurrencesInExpression(finalExpression.expression, expression.left);
132+
return occurrences ? { info: { finalExpression, occurrences, expression } } :
133+
{ error: getLocaleSpecificMessage(Diagnostics.Could_not_find_matching_access_expressions) };
134+
}
135+
136+
/**
137+
* Gets a list of property accesses that appear in matchTo and occur in sequence in expression.
138+
*/
139+
function getOccurrencesInExpression(matchTo: Expression, expression: Expression): (PropertyAccessExpression | Identifier)[] | undefined {
140+
const occurrences: (PropertyAccessExpression | Identifier)[] = [];
141+
while (isBinaryExpression(expression) && expression.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken) {
142+
const match = getMatchingStart(skipParentheses(matchTo), skipParentheses(expression.right));
143+
if (!match) {
144+
break;
145+
}
146+
occurrences.push(match);
147+
matchTo = match;
148+
expression = expression.left;
149+
}
150+
const finalMatch = getMatchingStart(matchTo, expression);
151+
if (finalMatch) {
152+
occurrences.push(finalMatch);
153+
}
154+
return occurrences.length > 0 ? occurrences: undefined;
155+
}
156+
157+
/**
158+
* Returns subchain if chain begins with subchain syntactically.
159+
*/
160+
function getMatchingStart(chain: Expression, subchain: Expression): PropertyAccessExpression | Identifier | undefined {
161+
return (isIdentifier(subchain) || isPropertyAccessExpression(subchain)) &&
162+
chainStartsWith(chain, subchain) ? subchain : undefined;
163+
}
164+
165+
/**
166+
* Returns true if chain begins with subchain syntactically.
167+
*/
168+
function chainStartsWith(chain: Node, subchain: Node): boolean {
169+
// skip until we find a matching identifier.
170+
while (isCallExpression(chain) || isPropertyAccessExpression(chain)) {
171+
const subchainName = isPropertyAccessExpression(subchain) ? subchain.name.getText() : subchain.getText();
172+
if (isPropertyAccessExpression(chain) && chain.name.getText() === subchainName) break;
173+
chain = chain.expression;
174+
}
175+
// check that the chains match at each access. Call chains in subchain are not valid.
176+
while (isPropertyAccessExpression(chain) && isPropertyAccessExpression(subchain)) {
177+
if (chain.name.getText() !== subchain.name.getText()) return false;
178+
chain = chain.expression;
179+
subchain = subchain.expression;
180+
}
181+
// check if we have reached a final identifier.
182+
return isIdentifier(chain) && isIdentifier(subchain) && chain.getText() === subchain.getText();
183+
}
184+
185+
/**
186+
* Find the least ancestor of the input node that is a valid type for extraction and contains the input span.
187+
*/
188+
function getValidParentNodeContainingSpan(node: Node, span: TextSpan): ValidExpressionOrStatement | undefined {
189+
while (node.parent) {
190+
if (isValidExpressionOrStatement(node) && span.length !== 0 && node.end >= span.start + span.length) {
191+
return node;
192+
}
193+
node = node.parent;
194+
}
195+
return undefined;
196+
}
197+
198+
/**
199+
* Finds an ancestor of the input node that is a valid type for extraction, skipping subexpressions.
200+
*/
201+
function getValidParentNodeOfEmptySpan(node: Node): ValidExpressionOrStatement | undefined {
202+
while (node.parent) {
203+
if (isValidExpressionOrStatement(node) && !isValidExpressionOrStatement(node.parent)) {
204+
return node;
205+
}
206+
node = node.parent;
207+
}
208+
return undefined;
209+
}
210+
211+
/**
212+
* Gets an expression of valid extraction type from a valid statement or expression.
213+
*/
214+
function getExpression(node: ValidExpressionOrStatement): ValidExpression | undefined {
215+
if (isValidExpression(node)) {
216+
return node;
217+
}
218+
if (isVariableStatement(node)) {
219+
const variable = getSingleVariableOfVariableStatement(node);
220+
const initializer = variable?.initializer;
221+
return initializer && isValidExpression(initializer) ? initializer : undefined;
222+
}
223+
return node.expression && isValidExpression(node.expression) ? node.expression : undefined;
224+
}
225+
226+
/**
227+
* Gets a property access expression which may be nested inside of a binary expression. The final
228+
* expression in an && chain will occur as the right child of the parent binary expression, unless
229+
* it is followed by a different binary operator.
230+
* @param node the right child of a binary expression or a call expression.
231+
*/
232+
function getFinalExpressionInChain(node: Expression): CallExpression | PropertyAccessExpression | undefined {
233+
// foo && |foo.bar === 1|; - here the right child of the && binary expression is another binary expression.
234+
// the rightmost member of the && chain should be the leftmost child of that expression.
235+
node = skipParentheses(node);
236+
if (isBinaryExpression(node)) {
237+
return getFinalExpressionInChain(node.left);
238+
}
239+
// foo && |foo.bar()()| - nested calls are treated like further accesses.
240+
else if ((isPropertyAccessExpression(node) || isCallExpression(node)) && !isOptionalChain(node)) {
241+
return node;
242+
}
243+
return undefined;
244+
}
245+
246+
/**
247+
* Creates an access chain from toConvert with '?.' accesses at expressions appearing in occurrences.
248+
*/
249+
function convertOccurrences(checker: TypeChecker, toConvert: Expression, occurrences: (PropertyAccessExpression | Identifier)[]): Expression {
250+
if (isPropertyAccessExpression(toConvert) || isCallExpression(toConvert)) {
251+
const chain = convertOccurrences(checker, toConvert.expression, occurrences);
252+
const lastOccurrence = occurrences.length > 0 ? occurrences[occurrences.length - 1] : undefined;
253+
const isOccurrence = lastOccurrence?.getText() === toConvert.expression.getText();
254+
if (isOccurrence) occurrences.pop();
255+
if (isCallExpression(toConvert)) {
256+
return isOccurrence ?
257+
factory.createCallChain(chain, factory.createToken(SyntaxKind.QuestionDotToken), toConvert.typeArguments, toConvert.arguments) :
258+
factory.createCallChain(chain, toConvert.questionDotToken, toConvert.typeArguments, toConvert.arguments);
259+
}
260+
else if (isPropertyAccessExpression(toConvert)) {
261+
return isOccurrence ?
262+
factory.createPropertyAccessChain(chain, factory.createToken(SyntaxKind.QuestionDotToken), toConvert.name) :
263+
factory.createPropertyAccessChain(chain, toConvert.questionDotToken, toConvert.name);
264+
}
265+
}
266+
return toConvert;
267+
}
268+
269+
function doChange(sourceFile: SourceFile, checker: TypeChecker, changes: textChanges.ChangeTracker, info: Info, _actionName: string): void {
270+
const { finalExpression, occurrences, expression } = info;
271+
const firstOccurrence = occurrences[occurrences.length - 1];
272+
const convertedChain = convertOccurrences(checker, finalExpression, occurrences);
273+
if (convertedChain && (isPropertyAccessExpression(convertedChain) || isCallExpression(convertedChain))) {
274+
if (isBinaryExpression(expression)) {
275+
changes.replaceNodeRange(sourceFile, firstOccurrence, finalExpression, convertedChain);
276+
}
277+
else if (isConditionalExpression(expression)) {
278+
changes.replaceNode(sourceFile, expression,
279+
factory.createBinaryExpression(convertedChain, factory.createToken(SyntaxKind.QuestionQuestionToken), expression.whenFalse)
280+
);
281+
}
282+
}
283+
}
284+
}

src/services/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@
107107
"codefixes/fixExpectedComma.ts",
108108
"refactors/convertExport.ts",
109109
"refactors/convertImport.ts",
110+
"refactors/convertToOptionalChainExpression.ts",
110111
"refactors/convertOverloadListToSingleSignature.ts",
111112
"refactors/extractSymbol.ts",
112113
"refactors/extractType.ts",

tests/cases/fourslash/fourslash.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ declare namespace FourSlashInterface {
426426
enableFormatting(): void;
427427
disableFormatting(): void;
428428

429-
applyRefactor(options: { refactorName: string, actionName: string, actionDescription: string, newContent: NewFileContent }): void;
429+
applyRefactor(options: { refactorName: string, actionName: string, actionDescription: string, newContent: NewFileContent, triggerReason?: RefactorTriggerReason }): void;
430430
}
431431
class debug {
432432
printCurrentParameterHelp(): void;
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////let a = { b: () => { return () => { c: 0 } } }
4+
/////*a*/a && a.b && a.b()().c/*b*/;
5+
6+
goTo.select("a", "b");
7+
edit.applyRefactor({
8+
refactorName: "Convert to optional chain expression",
9+
actionName: "Convert to optional chain expression",
10+
actionDescription: "Convert to optional chain expression",
11+
newContent:
12+
`let a = { b: () => { return () => { c: 0 } } }
13+
a?.b?.()().c;`
14+
});
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////let a = { b: () => { return { c: 0 } } }
4+
/////*a*/a && a.b && a.b().c/*b*/;
5+
6+
goTo.select("a", "b");
7+
edit.applyRefactor({
8+
refactorName: "Convert to optional chain expression",
9+
actionName: "Convert to optional chain expression",
10+
actionDescription: "Convert to optional chain expression",
11+
newContent:
12+
`let a = { b: () => { return { c: 0 } } }
13+
a?.b?.().c;`
14+
});
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
/////*a*/a && a.b && a.b()/*b*/;
4+
5+
goTo.select("a", "b");
6+
edit.applyRefactor({
7+
refactorName: "Convert to optional chain expression",
8+
actionName: "Convert to optional chain expression",
9+
actionDescription: "Convert to optional chain expression",
10+
newContent:
11+
`a?.b?.();`
12+
});

0 commit comments

Comments
 (0)