Skip to content

Commit 78830f3

Browse files
authored
fix(40510): add element access expressions support in convertToOptionalChainExpression (#40524)
1 parent 9eb6424 commit 78830f3

File tree

3 files changed

+77
-19
lines changed

3 files changed

+77
-19
lines changed

src/services/refactors/convertToOptionalChainExpression.ts

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,11 @@ namespace ts.refactor.convertToOptionalChainExpression {
5151
error: string;
5252
};
5353

54+
type Occurrence = PropertyAccessExpression | ElementAccessExpression | Identifier;
55+
5456
interface Info {
55-
finalExpression: PropertyAccessExpression | CallExpression,
56-
occurrences: (PropertyAccessExpression | Identifier)[],
57+
finalExpression: PropertyAccessExpression | ElementAccessExpression | CallExpression,
58+
occurrences: Occurrence[],
5759
expression: ValidExpression,
5860
};
5961

@@ -107,7 +109,7 @@ namespace ts.refactor.convertToOptionalChainExpression {
107109

108110
if (!finalExpression || checker.isNullableType(checker.getTypeAtLocation(finalExpression))) {
109111
return { error: getLocaleSpecificMessage(Diagnostics.Could_not_find_convertible_access_expression) };
110-
};
112+
}
111113

112114
if ((isPropertyAccessExpression(condition) || isIdentifier(condition))
113115
&& getMatchingStart(condition, finalExpression.expression)) {
@@ -136,8 +138,8 @@ namespace ts.refactor.convertToOptionalChainExpression {
136138
/**
137139
* Gets a list of property accesses that appear in matchTo and occur in sequence in expression.
138140
*/
139-
function getOccurrencesInExpression(matchTo: Expression, expression: Expression): (PropertyAccessExpression | Identifier)[] | undefined {
140-
const occurrences: (PropertyAccessExpression | Identifier)[] = [];
141+
function getOccurrencesInExpression(matchTo: Expression, expression: Expression): Occurrence[] | undefined {
142+
const occurrences: Occurrence[] = [];
141143
while (isBinaryExpression(expression) && expression.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken) {
142144
const match = getMatchingStart(skipParentheses(matchTo), skipParentheses(expression.right));
143145
if (!match) {
@@ -157,31 +159,46 @@ namespace ts.refactor.convertToOptionalChainExpression {
157159
/**
158160
* Returns subchain if chain begins with subchain syntactically.
159161
*/
160-
function getMatchingStart(chain: Expression, subchain: Expression): PropertyAccessExpression | Identifier | undefined {
161-
return (isIdentifier(subchain) || isPropertyAccessExpression(subchain)) &&
162-
chainStartsWith(chain, subchain) ? subchain : undefined;
162+
function getMatchingStart(chain: Expression, subchain: Expression): PropertyAccessExpression | ElementAccessExpression | Identifier | undefined {
163+
if (!isIdentifier(subchain) && !isPropertyAccessExpression(subchain) && !isElementAccessExpression(subchain)) {
164+
return undefined;
165+
}
166+
return chainStartsWith(chain, subchain) ? subchain : undefined;
163167
}
164168

165169
/**
166170
* Returns true if chain begins with subchain syntactically.
167171
*/
168172
function chainStartsWith(chain: Node, subchain: Node): boolean {
169173
// 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;
174+
while (isCallExpression(chain) || isPropertyAccessExpression(chain) || isElementAccessExpression(chain)) {
175+
if (getTextOfChainNode(chain) === getTextOfChainNode(subchain)) break;
173176
chain = chain.expression;
174177
}
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+
// check that the chains match at each access. Call chains in subchain are not valid.
179+
while ((isPropertyAccessExpression(chain) && isPropertyAccessExpression(subchain)) ||
180+
(isElementAccessExpression(chain) && isElementAccessExpression(subchain))) {
181+
if (getTextOfChainNode(chain) !== getTextOfChainNode(subchain)) return false;
178182
chain = chain.expression;
179183
subchain = subchain.expression;
180184
}
181185
// check if we have reached a final identifier.
182186
return isIdentifier(chain) && isIdentifier(subchain) && chain.getText() === subchain.getText();
183187
}
184188

189+
function getTextOfChainNode(node: Node): string | undefined {
190+
if (isIdentifier(node) || isStringOrNumericLiteralLike(node)) {
191+
return node.getText();
192+
}
193+
if (isPropertyAccessExpression(node)) {
194+
return getTextOfChainNode(node.name);
195+
}
196+
if (isElementAccessExpression(node)) {
197+
return getTextOfChainNode(node.argumentExpression);
198+
}
199+
return undefined;
200+
}
201+
185202
/**
186203
* Find the least ancestor of the input node that is a valid type for extraction and contains the input span.
187204
*/
@@ -229,15 +246,15 @@ namespace ts.refactor.convertToOptionalChainExpression {
229246
* it is followed by a different binary operator.
230247
* @param node the right child of a binary expression or a call expression.
231248
*/
232-
function getFinalExpressionInChain(node: Expression): CallExpression | PropertyAccessExpression | undefined {
249+
function getFinalExpressionInChain(node: Expression): CallExpression | PropertyAccessExpression | ElementAccessExpression | undefined {
233250
// foo && |foo.bar === 1|; - here the right child of the && binary expression is another binary expression.
234251
// the rightmost member of the && chain should be the leftmost child of that expression.
235252
node = skipParentheses(node);
236253
if (isBinaryExpression(node)) {
237254
return getFinalExpressionInChain(node.left);
238255
}
239256
// foo && |foo.bar()()| - nested calls are treated like further accesses.
240-
else if ((isPropertyAccessExpression(node) || isCallExpression(node)) && !isOptionalChain(node)) {
257+
else if ((isPropertyAccessExpression(node) || isElementAccessExpression(node) || isCallExpression(node)) && !isOptionalChain(node)) {
241258
return node;
242259
}
243260
return undefined;
@@ -246,8 +263,8 @@ namespace ts.refactor.convertToOptionalChainExpression {
246263
/**
247264
* Creates an access chain from toConvert with '?.' accesses at expressions appearing in occurrences.
248265
*/
249-
function convertOccurrences(checker: TypeChecker, toConvert: Expression, occurrences: (PropertyAccessExpression | Identifier)[]): Expression {
250-
if (isPropertyAccessExpression(toConvert) || isCallExpression(toConvert)) {
266+
function convertOccurrences(checker: TypeChecker, toConvert: Expression, occurrences: Occurrence[]): Expression {
267+
if (isPropertyAccessExpression(toConvert) || isElementAccessExpression(toConvert) || isCallExpression(toConvert)) {
251268
const chain = convertOccurrences(checker, toConvert.expression, occurrences);
252269
const lastOccurrence = occurrences.length > 0 ? occurrences[occurrences.length - 1] : undefined;
253270
const isOccurrence = lastOccurrence?.getText() === toConvert.expression.getText();
@@ -262,6 +279,11 @@ namespace ts.refactor.convertToOptionalChainExpression {
262279
factory.createPropertyAccessChain(chain, factory.createToken(SyntaxKind.QuestionDotToken), toConvert.name) :
263280
factory.createPropertyAccessChain(chain, toConvert.questionDotToken, toConvert.name);
264281
}
282+
else if (isElementAccessExpression(toConvert)) {
283+
return isOccurrence ?
284+
factory.createElementAccessChain(chain, factory.createToken(SyntaxKind.QuestionDotToken), toConvert.argumentExpression) :
285+
factory.createElementAccessChain(chain, toConvert.questionDotToken, toConvert.argumentExpression);
286+
}
265287
}
266288
return toConvert;
267289
}
@@ -270,7 +292,7 @@ namespace ts.refactor.convertToOptionalChainExpression {
270292
const { finalExpression, occurrences, expression } = info;
271293
const firstOccurrence = occurrences[occurrences.length - 1];
272294
const convertedChain = convertOccurrences(checker, finalExpression, occurrences);
273-
if (convertedChain && (isPropertyAccessExpression(convertedChain) || isCallExpression(convertedChain))) {
295+
if (convertedChain && (isPropertyAccessExpression(convertedChain) || isElementAccessExpression(convertedChain) || isCallExpression(convertedChain))) {
274296
if (isBinaryExpression(expression)) {
275297
changes.replaceNodeRange(sourceFile, firstOccurrence, finalExpression, convertedChain);
276298
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////const a = {
4+
//// b: { c: 1 }
5+
////}
6+
/////*a*/a && a['b'] && a['b']['c']/*b*/
7+
8+
goTo.select("a", "b");
9+
edit.applyRefactor({
10+
refactorName: "Convert to optional chain expression",
11+
actionName: "Convert to optional chain expression",
12+
actionDescription: "Convert to optional chain expression",
13+
newContent:
14+
`const a = {
15+
b: { c: 1 }
16+
}
17+
a?.['b']?.['c']`
18+
});
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////const a = {
4+
//// b: { c: 1 }
5+
////}
6+
/////*a*/a && a.b && a.b['c']/*b*/
7+
8+
goTo.select("a", "b");
9+
edit.applyRefactor({
10+
refactorName: "Convert to optional chain expression",
11+
actionName: "Convert to optional chain expression",
12+
actionDescription: "Convert to optional chain expression",
13+
newContent:
14+
`const a = {
15+
b: { c: 1 }
16+
}
17+
a?.b?.['c']`
18+
});

0 commit comments

Comments
 (0)