Skip to content

Commit 35b9078

Browse files
sandersnsandersn
authored
Clean up GetElementOrPropertyAccessName (#951)
1 parent b0e1b84 commit 35b9078

File tree

8 files changed

+126
-32
lines changed

8 files changed

+126
-32
lines changed

‎internal/ast/utilities.go

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1292,26 +1292,20 @@ func IsThisParameter(node *Node) bool {
12921292

12931293
// Does not handle signed numeric names like `a[+0]` - handling those would require handling prefix unary expressions
12941294
// throughout late binding handling as well, which is awkward (but ultimately probably doable if there is demand)
1295-
func GetElementOrPropertyAccessArgumentExpressionOrName(node *Node) *Node {
1295+
func GetElementOrPropertyAccessName(node *Node) *Node {
12961296
switch node.Kind {
12971297
case KindPropertyAccessExpression:
1298-
return node.Name()
1298+
if IsIdentifier(node.Name()) {
1299+
return node.Name()
1300+
}
1301+
return nil
12991302
case KindElementAccessExpression:
1300-
arg := SkipParentheses(node.AsElementAccessExpression().ArgumentExpression)
1301-
if IsStringOrNumericLiteralLike(arg) {
1303+
if arg := SkipParentheses(node.AsElementAccessExpression().ArgumentExpression); IsStringOrNumericLiteralLike(arg) {
13021304
return arg
13031305
}
1304-
return node
1305-
}
1306-
panic("Unhandled case in GetElementOrPropertyAccessArgumentExpressionOrName")
1307-
}
1308-
1309-
func GetElementOrPropertyAccessName(node *Node) string {
1310-
name := GetElementOrPropertyAccessArgumentExpressionOrName(node)
1311-
if name == nil {
1312-
return ""
1306+
return nil
13131307
}
1314-
return name.Text()
1308+
panic("Unhandled case in GetElementOrPropertyAccessName")
13151309
}
13161310

13171311
func IsExpressionWithTypeArgumentsInClassExtendsClause(node *Node) bool {
@@ -1356,7 +1350,11 @@ func GetNonAssignedNameOfDeclaration(declaration *Node) *Node {
13561350
bin := declaration.AsBinaryExpression()
13571351
kind := GetAssignmentDeclarationKind(bin)
13581352
if kind == JSDeclarationKindProperty || kind == JSDeclarationKindThisProperty {
1359-
return GetElementOrPropertyAccessArgumentExpressionOrName(bin.Left)
1353+
if name := GetElementOrPropertyAccessName(bin.Left); name != nil {
1354+
return name
1355+
} else {
1356+
return bin.Left
1357+
}
13601358
}
13611359
return nil
13621360
case KindExportAssignment, KindJSExportAssignment:
@@ -1426,24 +1424,19 @@ func GetAssignmentDeclarationKind(bin *BinaryExpression) JSDeclarationKind {
14261424
if IsModuleExportsAccessExpression(bin.Left) {
14271425
return JSDeclarationKindModuleExports
14281426
} else if (IsModuleExportsAccessExpression(bin.Left.Expression()) || IsExportsIdentifier(bin.Left.Expression())) &&
1429-
hasJSBindableName(bin.Left) {
1427+
GetElementOrPropertyAccessName(bin.Left) != nil {
14301428
return JSDeclarationKindExportsProperty
14311429
}
14321430
if bin.Left.Expression().Kind == KindThisKeyword {
14331431
return JSDeclarationKindThisProperty
14341432
}
1435-
if bin.Left.Kind == KindPropertyAccessExpression && IsIdentifier(bin.Left.Expression()) && hasJSBindableName(bin.Left) ||
1433+
if bin.Left.Kind == KindPropertyAccessExpression && IsIdentifier(bin.Left.Expression()) && IsIdentifier(bin.Left.Name()) ||
14361434
bin.Left.Kind == KindElementAccessExpression && IsIdentifier(bin.Left.Expression()) {
14371435
return JSDeclarationKindProperty
14381436
}
14391437
return JSDeclarationKindNone
14401438
}
14411439

1442-
func hasJSBindableName(node *Node) bool {
1443-
name := GetElementOrPropertyAccessArgumentExpressionOrName(node)
1444-
return IsIdentifier(name) || IsStringLiteralLike(name)
1445-
}
1446-
14471440
/**
14481441
* A declaration has a dynamic name if all of the following are true:
14491442
* 1. The declaration has a computed property name.
@@ -2679,9 +2672,12 @@ func IsVariableDeclarationInitializedToRequire(node *Node) bool {
26792672
}
26802673

26812674
func IsModuleExportsAccessExpression(node *Node) bool {
2682-
return (IsPropertyAccessExpression(node) || isLiteralLikeElementAccess(node)) &&
2683-
IsModuleIdentifier(node.Expression()) &&
2684-
GetElementOrPropertyAccessName(node) == "exports"
2675+
if IsAccessExpression(node) && IsModuleIdentifier(node.Expression()) {
2676+
if name := GetElementOrPropertyAccessName(node); name != nil {
2677+
return name.Text() == "exports"
2678+
}
2679+
}
2680+
return false
26852681
}
26862682

26872683
func isLiteralLikeElementAccess(node *Node) bool {

‎internal/checker/checker.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3575,7 +3575,7 @@ func (c *Checker) checkTestingKnownTruthyType(condExpr *ast.Node, condType *Type
35753575
if ast.IsLogicalOrCoalescingBinaryExpression(condExpr) {
35763576
location = ast.SkipParentheses(condExpr.AsBinaryExpression().Right)
35773577
}
3578-
if isModuleExportsAccessExpression(location) {
3578+
if ast.IsModuleExportsAccessExpression(location) {
35793579
return
35803580
}
35813581
if ast.IsLogicalOrCoalescingBinaryExpression(location) {
@@ -27747,7 +27747,7 @@ func (c *Checker) getContextualTypeForAssignmentDeclaration(node *ast.Node) *Typ
2774727747
return nil
2774827748
}
2774927749
// and now for one single case of object literal methods
27750-
name := ast.GetElementOrPropertyAccessArgumentExpressionOrName(left)
27750+
name := ast.GetElementOrPropertyAccessName(left)
2775127751
if name == nil {
2775227752
return nil
2775327753
} else {

‎internal/checker/utilities.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1632,10 +1632,6 @@ func minAndMax[T any](slice []T, getValue func(value T) int) (int, int) {
16321632
return minValue, maxValue
16331633
}
16341634

1635-
func isModuleExportsAccessExpression(node *ast.Node) bool {
1636-
return ast.IsAccessExpression(node) && ast.IsModuleIdentifier(node.Expression()) && ast.GetElementOrPropertyAccessName(node) == "exports"
1637-
}
1638-
16391635
func getNonModifierTokenRangeOfNode(node *ast.Node) core.TextRange {
16401636
pos := node.Pos()
16411637
if node.Modifiers() != nil {

‎internal/parser/reparser.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func (p *Parser) reparseCommonJS(node *ast.Node) {
2424
nodes[0].Flags = ast.NodeFlagsReparsed
2525
nodes[0].Loc = bin.Loc
2626
// TODO: Name can sometimes be a string literal, so downstream code needs to handle this
27-
export = p.factory.NewCommonJSExport(p.newModifierList(bin.Loc, nodes), ast.GetElementOrPropertyAccessArgumentExpressionOrName(bin.Left), bin.Right)
27+
export = p.factory.NewCommonJSExport(p.newModifierList(bin.Loc, nodes), ast.GetElementOrPropertyAccessName(bin.Left), bin.Right)
2828
}
2929
if export != nil {
3030
export.Flags = ast.NodeFlagsReparsed

‎testdata/baselines/reference/conformance/deepElementAccessExpressionInJS.errors.txt

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
elementAccessExpressionInJS.js(1,5): error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ exports: { [key: string]: any; }; }'.
2+
No index signature with a parameter of type 'string' was found on type '{ exports: { [key: string]: any; }; }'.
3+
elementAccessExpressionInJS.js(3,32): error TS7006: Parameter 'index' implicitly has an 'any' type.
4+
5+
6+
==== declarations.d.ts (0 errors) ====
7+
declare var module: {
8+
exports: {
9+
[key: string]: any;
10+
};
11+
}
12+
==== elementAccessExpressionInJS.js (2 errors) ====
13+
if (module[calculatePropertyName(1)]) {
14+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
15+
!!! error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ exports: { [key: string]: any; }; }'.
16+
!!! error TS7053: No index signature with a parameter of type 'string' was found on type '{ exports: { [key: string]: any; }; }'.
17+
}
18+
function calculatePropertyName(index) {
19+
~~~~~
20+
!!! error TS7006: Parameter 'index' implicitly has an 'any' type.
21+
// this would be some webpack index in real life
22+
return `property${index}`;
23+
}
24+

‎testdata/baselines/reference/conformance/deepElementAccessExpressionInJS.symbols

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
//// [tests/cases/conformance/salsa/deepElementAccessExpressionInJS.ts] ////
2+
3+
=== declarations.d.ts ===
4+
declare var module: {
5+
>module : Symbol(module, Decl(declarations.d.ts, 0, 11))
6+
7+
exports: {
8+
>exports : Symbol(exports, Decl(declarations.d.ts, 0, 21))
9+
10+
[key: string]: any;
11+
>key : Symbol(key, Decl(declarations.d.ts, 2, 9))
12+
13+
};
14+
}
15+
=== elementAccessExpressionInJS.js ===
16+
if (module[calculatePropertyName(1)]) {
17+
>module : Symbol(module, Decl(declarations.d.ts, 0, 11))
18+
>calculatePropertyName : Symbol(calculatePropertyName, Decl(elementAccessExpressionInJS.js, 1, 1))
19+
}
20+
function calculatePropertyName(index) {
21+
>calculatePropertyName : Symbol(calculatePropertyName, Decl(elementAccessExpressionInJS.js, 1, 1))
22+
>index : Symbol(index, Decl(elementAccessExpressionInJS.js, 2, 31))
23+
24+
// this would be some webpack index in real life
25+
return `property${index}`;
26+
>index : Symbol(index, Decl(elementAccessExpressionInJS.js, 2, 31))
27+
}
28+

‎testdata/baselines/reference/conformance/deepElementAccessExpressionInJS.types

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
//// [tests/cases/conformance/salsa/deepElementAccessExpressionInJS.ts] ////
2+
3+
=== declarations.d.ts ===
4+
declare var module: {
5+
>module : { exports: { [key: string]: any; }; }
6+
7+
exports: {
8+
>exports : { [key: string]: any; }
9+
10+
[key: string]: any;
11+
>key : string
12+
13+
};
14+
}
15+
=== elementAccessExpressionInJS.js ===
16+
if (module[calculatePropertyName(1)]) {
17+
>module[calculatePropertyName(1)] : any
18+
>module : { exports: { [key: string]: any; }; }
19+
>calculatePropertyName(1) : string
20+
>calculatePropertyName : (index: any) => string
21+
>1 : 1
22+
}
23+
function calculatePropertyName(index) {
24+
>calculatePropertyName : (index: any) => string
25+
>index : any
26+
27+
// this would be some webpack index in real life
28+
return `property${index}`;
29+
>`property${index}` : string
30+
>index : any
31+
}
32+

‎testdata/tests/cases/conformance/salsa/deepElementAccessExpressionInJS.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// @allowJs: true
2+
// @checkJs: true
3+
// @strict: true
4+
// @target: es6
5+
// @outDir: ./out
6+
// @filename: declarations.d.ts
7+
declare var module: {
8+
exports: {
9+
[key: string]: any;
10+
};
11+
}
12+
// @filename: elementAccessExpressionInJS.js
13+
if (module[calculatePropertyName(1)]) {
14+
}
15+
function calculatePropertyName(index) {
16+
// this would be some webpack index in real life
17+
return `property${index}`;
18+
}

0 commit comments

Comments
 (0)