feat(45549): track string literal references within call expressions #45791
Conversation
* Enables string literals in call expression to refer to property names.
* Enables property names in call expression to refer to string literals.
* Changes string literal rename/reference behavior when string literals are in a call expression.
Previously, all string references with matching text would reference one another globally and
allow renaming even if such an operation would result in a compiler error.
Fixes microsoft#45549
paranoiacblack
force-pushed
the
stringLiteralTracking
branch
from
0cd60ae to
1152369
Compare
| // might all refer to each other, but if the string literal being considered is in a | ||
| // call expression, any reference should also be from the same call expression. | ||
| const refHasSameScope = !callExpr || callExpr === findAncestor(ref, isCallExpression); | ||
| const refHasSameText = isStringLiteralLike(ref) || isPropertyNameLiteral(ref) && getTextOfIdentifierOrLiteral(ref) === node.text; |
There was a problem hiding this comment.
Currently all string literal refs are true without checking whether their text is === node.text. I think it should instead be:
| const refHasSameText = isStringLiteralLike(ref) || isPropertyNameLiteral(ref) && getTextOfIdentifierOrLiteral(ref) === node.text; | |
| const refHasSameText = (isStringLiteralLike(ref) || isPropertyNameLiteral(ref)) && getTextOfIdentifierOrLiteral(ref) === node.text; |
| } | ||
| else { | ||
| // Special case: Every string literal refers at least to itself. | ||
| if (node === ref) return nodeEntry(ref, EntryKind.StringLiteral); |
There was a problem hiding this comment.
Does this happen? I'd expect getPossibleSymbolReferenceNodes should filter node out of the refs list.
It's fine if it doesn't, but maybe we should consider making it do that.
|
|
||
| if (type && isStringLiteralLike(ref) && refHasSameText) { | ||
| const refType = getContextualTypeFromParentOrAncestorTypeNode(ref, checker); | ||
| if ((refHasSameScope || !type.isStringLiteral() || type.isUnionOrIntersection()) && type === refType) { |
There was a problem hiding this comment.
!type.isStringLiteral() is different from the old check type !== checker.getStringType(). Does that invert the set of refs that will be marked as references?
| return nodeEntry(ref, EntryKind.StringLiteral); | ||
| } | ||
| } | ||
| if (callExpr) { | ||
| if (refHasSameScope && isPropertyNameLiteral(ref) && refHasSameText) { |
There was a problem hiding this comment.
A nested a could refer to something else besides the string literal node "a" even though they're both used inside the same call expression.
eg
f(a => { a: a }, "a")| const params = sig.getParameters(); | ||
| for (let i = 0; i < params.length; i++) { | ||
| const paramType = checker.getParameterType(sig, i); | ||
| if (isLiteralTypeNode(ref.parent) && paramType.isStringLiteral() && refHasSameText) { |
There was a problem hiding this comment.
why is it isLiteralTypeNode(ref.parent) and not isLiteralTypeNode(ref)?
|
@paranoiacblack Do you want to keep working on this? Otherwise I'd like to close it to reduce the number of open PRs. |
|
To help with PR housekeeping, I'm going to close this PR since it's pretty old now. |
Fixes #45549