Skip to content

Commit c54ff7c

Browse files
a-tarasyuka-tarasyuk
authored
fix(4048): fix declaration emit for new keyword used as property names (#4052)
1 parent 2f91510 commit c54ff7c

10 files changed

Lines changed: 248 additions & 46 deletions

File tree

‎internal/checker/nodebuilderimpl.go‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2281,16 +2281,33 @@ func (b *NodeBuilderImpl) trackComputedName(accessExpression *ast.Node, enclosin
22812281
}
22822282
}
22832283

2284+
type propertyNameNodeKind int
2285+
2286+
const (
2287+
propertyNameNodeKindIdentifier propertyNameNodeKind = iota
2288+
propertyNameNodeKindNumericLiteral
2289+
propertyNameNodeKindStringLiteral
2290+
)
2291+
2292+
func classifyPropertyName(name string, stringNamed bool, isMethod bool) propertyNameNodeKind {
2293+
if isMethod && name == "new" {
2294+
return propertyNameNodeKindStringLiteral
2295+
}
2296+
if scanner.IsIdentifierText(name, core.LanguageVariantStandard) {
2297+
return propertyNameNodeKindIdentifier
2298+
}
2299+
return core.IfElse(!stringNamed && isNumericLiteralName(name) && jsnum.FromString(name) >= 0, propertyNameNodeKindNumericLiteral, propertyNameNodeKindStringLiteral)
2300+
}
2301+
22842302
func (b *NodeBuilderImpl) createPropertyNameNodeForIdentifierOrLiteral(name string, singleQuote bool, stringNamed bool, isMethod bool, symbol *ast.Symbol) *ast.Node {
2285-
isMethodNamedNew := isMethod && name == "new"
2286-
if !isMethodNamedNew && scanner.IsIdentifierText(name, core.LanguageVariantStandard) {
2303+
switch classifyPropertyName(name, stringNamed, isMethod) {
2304+
case propertyNameNodeKindIdentifier:
22872305
return b.newIdentifier(name, symbol)
2288-
}
2289-
if !stringNamed && !isMethodNamedNew && isNumericLiteralName(name) && jsnum.FromString(name) >= 0 {
2306+
case propertyNameNodeKindNumericLiteral:
22902307
return b.f.NewNumericLiteral(name, ast.TokenFlagsNone)
2308+
default:
2309+
return b.f.NewStringLiteral(name, core.IfElse(singleQuote, ast.TokenFlagsSingleQuote, ast.TokenFlagsNone))
22912310
}
2292-
result := b.f.NewStringLiteral(name, core.IfElse(singleQuote, ast.TokenFlagsSingleQuote, ast.TokenFlagsNone))
2293-
return result
22942311
}
22952312

22962313
func (b *NodeBuilderImpl) isStringNamed(d *ast.Declaration) bool {

‎internal/checker/nodecopy.go‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"github.com/microsoft/typescript-go/internal/core"
88
"github.com/microsoft/typescript-go/internal/nodebuilder"
99
"github.com/microsoft/typescript-go/internal/printer"
10-
"github.com/microsoft/typescript-go/internal/scanner"
1110
)
1211

1312
func (b *NodeBuilderImpl) reuseNode(node *ast.Node) *ast.Node {
@@ -22,35 +21,36 @@ func (b *NodeBuilderImpl) tryJSTypeNodeToTypeNode(node *ast.Node) *ast.Node {
2221
return b.reuseNode(node)
2322
}
2423

25-
// a wrapper around `reuseNode` for property names. It handles renaming `new` to `"new"` so we don't
26-
// accidentally emit constructor signatures when we don't mean to, and normalizes string-literal
27-
// property names whose text is a valid identifier into identifiers, matching the behavior of
28-
// `createPropertyNameNodeForIdentifierOrLiteral` used when constructing fresh property name nodes
29-
// (so that reused names emit consistently regardless of whether their containing type was reused
30-
// from source or rebuilt from a type).
31-
func (b *NodeBuilderImpl) reuseName(node *ast.Node) *ast.Node {
24+
func (b *NodeBuilderImpl) reuseName(node *ast.Node, isMethod bool) *ast.Node {
3225
res := b.reuseNode(node)
3326
if res == nil {
3427
return res
3528
}
36-
if res.Kind == ast.KindIdentifier && node.AsIdentifier().Text == "new" {
37-
str := b.f.NewStringLiteral("new", ast.TokenFlagsNone)
38-
b.e.SetOriginal(str, res)
39-
return b.setTextRange(str, res)
29+
30+
text, ok := ast.TryGetTextOfPropertyName(res)
31+
if !ok {
32+
return res
4033
}
41-
if res.Kind == ast.KindStringLiteral {
42-
text := res.AsStringLiteral().Text
43-
// Skip normalization for "new" so that reused names like `"new"(): void` on a
44-
// method signature are not converted to an identifier (which would become a
45-
// construct signature). This mirrors the `isMethodNamedNew` guard in
46-
// createPropertyNameNodeForIdentifierOrLiteral.
47-
if text != "new" && scanner.IsIdentifierText(text, core.LanguageVariantStandard) {
48-
ident := b.newIdentifier(text, nil)
49-
b.e.SetOriginal(ident, res)
50-
return b.setTextRange(ident, res)
51-
}
34+
35+
kind := classifyPropertyName(text, ast.IsStringLiteral(res), isMethod)
36+
if ast.IsIdentifier(res) && kind == propertyNameNodeKindIdentifier {
37+
return res
5238
}
53-
return res
39+
if ast.IsStringLiteral(res) && kind == propertyNameNodeKindStringLiteral {
40+
return res
41+
}
42+
43+
var renamed *ast.Node
44+
switch kind {
45+
case propertyNameNodeKindIdentifier:
46+
renamed = b.newIdentifier(text, nil)
47+
case propertyNameNodeKindStringLiteral:
48+
renamed = b.f.NewStringLiteral(text, ast.TokenFlagsNone)
49+
default:
50+
return res
51+
}
52+
b.e.SetOriginal(renamed, res)
53+
return b.setTextRange(renamed, res)
5454
}
5555

5656
func (b *NodeBuilderImpl) reuseTypeNode(node *ast.Node) *ast.Node {

‎internal/checker/pseudotypenodebuilder.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ func (b *NodeBuilderImpl) pseudoTypeToNode(t *pseudochecker.PseudoType) *ast.Nod
224224
if isConst {
225225
newProp = b.f.NewPropertySignatureDeclaration(
226226
modifiers,
227-
b.reuseName(e.Name),
227+
b.reuseName(e.Name, false /*isMethod*/),
228228
nil,
229229
b.f.NewFunctionTypeNode(
230230
typeParams,
@@ -237,7 +237,7 @@ func (b *NodeBuilderImpl) pseudoTypeToNode(t *pseudochecker.PseudoType) *ast.Nod
237237
}
238238
newProp = b.f.NewMethodSignatureDeclaration(
239239
modifiers,
240-
b.reuseName(e.Name),
240+
b.reuseName(e.Name, true /*isMethod*/),
241241
nil,
242242
typeParams,
243243
b.pseudoParametersToNodeList(d.Parameters),
@@ -247,7 +247,7 @@ func (b *NodeBuilderImpl) pseudoTypeToNode(t *pseudochecker.PseudoType) *ast.Nod
247247
d := e.AsPseudoPropertyAssignment()
248248
newProp = b.f.NewPropertySignatureDeclaration(
249249
modifiers,
250-
b.reuseName(e.Name),
250+
b.reuseName(e.Name, false /*isMethod*/),
251251
nil,
252252
b.pseudoTypeToNode(d.Type),
253253
nil,
@@ -256,7 +256,7 @@ func (b *NodeBuilderImpl) pseudoTypeToNode(t *pseudochecker.PseudoType) *ast.Nod
256256
d := e.AsPseudoSetAccessor()
257257
newProp = b.f.NewSetAccessorDeclaration(
258258
nil,
259-
b.reuseName(e.Name),
259+
b.reuseName(e.Name, false /*isMethod*/),
260260
nil,
261261
b.f.NewNodeList([]*ast.Node{b.pseudoParameterToNode(d.Parameter)}),
262262
nil,
@@ -267,7 +267,7 @@ func (b *NodeBuilderImpl) pseudoTypeToNode(t *pseudochecker.PseudoType) *ast.Nod
267267
d := e.AsPseudoGetAccessor()
268268
newProp = b.f.NewGetAccessorDeclaration(
269269
nil,
270-
b.reuseName(e.Name),
270+
b.reuseName(e.Name, false /*isMethod*/),
271271
nil,
272272
nil,
273273
b.pseudoTypeToNode(d.Type),

‎testdata/baselines/reference/compiler/declarationEmitKeywordPropertyNames.js‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
//// [tests/cases/compiler/declarationEmitKeywordPropertyNames.ts] ////
2+
3+
//// [declarationEmitKeywordPropertyNames.ts]
4+
export const a = {
5+
foo: "foo",
6+
bar: "bar",
7+
buzz: "buzz",
8+
new: "new",
9+
delete: "delete",
10+
break: "break",
11+
continue: "continue",
12+
};
13+
14+
export const b = {
15+
foo: "foo",
16+
bar: "bar",
17+
buzz: "buzz",
18+
new: "new",
19+
delete: "delete",
20+
break: "break",
21+
continue: "continue",
22+
} as const;
23+
24+
25+
26+
27+
//// [declarationEmitKeywordPropertyNames.d.ts]
28+
export declare const a: {
29+
foo: string;
30+
bar: string;
31+
buzz: string;
32+
new: string;
33+
delete: string;
34+
break: string;
35+
continue: string;
36+
};
37+
export declare const b: {
38+
readonly foo: "foo";
39+
readonly bar: "bar";
40+
readonly buzz: "buzz";
41+
readonly new: "new";
42+
readonly delete: "delete";
43+
readonly break: "break";
44+
readonly continue: "continue";
45+
};

‎testdata/baselines/reference/compiler/declarationEmitKeywordPropertyNames.symbols‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
//// [tests/cases/compiler/declarationEmitKeywordPropertyNames.ts] ////
2+
3+
=== declarationEmitKeywordPropertyNames.ts ===
4+
export const a = {
5+
>a : Symbol(a, Decl(declarationEmitKeywordPropertyNames.ts, 0, 12))
6+
7+
foo: "foo",
8+
>foo : Symbol(foo, Decl(declarationEmitKeywordPropertyNames.ts, 0, 18))
9+
10+
bar: "bar",
11+
>bar : Symbol(bar, Decl(declarationEmitKeywordPropertyNames.ts, 1, 15))
12+
13+
buzz: "buzz",
14+
>buzz : Symbol(buzz, Decl(declarationEmitKeywordPropertyNames.ts, 2, 15))
15+
16+
new: "new",
17+
>new : Symbol(new, Decl(declarationEmitKeywordPropertyNames.ts, 3, 17))
18+
19+
delete: "delete",
20+
>delete : Symbol(delete, Decl(declarationEmitKeywordPropertyNames.ts, 4, 15))
21+
22+
break: "break",
23+
>break : Symbol(break, Decl(declarationEmitKeywordPropertyNames.ts, 5, 21))
24+
25+
continue: "continue",
26+
>continue : Symbol(continue, Decl(declarationEmitKeywordPropertyNames.ts, 6, 19))
27+
28+
};
29+
30+
export const b = {
31+
>b : Symbol(b, Decl(declarationEmitKeywordPropertyNames.ts, 10, 12))
32+
33+
foo: "foo",
34+
>foo : Symbol(foo, Decl(declarationEmitKeywordPropertyNames.ts, 10, 18))
35+
36+
bar: "bar",
37+
>bar : Symbol(bar, Decl(declarationEmitKeywordPropertyNames.ts, 11, 15))
38+
39+
buzz: "buzz",
40+
>buzz : Symbol(buzz, Decl(declarationEmitKeywordPropertyNames.ts, 12, 15))
41+
42+
new: "new",
43+
>new : Symbol(new, Decl(declarationEmitKeywordPropertyNames.ts, 13, 17))
44+
45+
delete: "delete",
46+
>delete : Symbol(delete, Decl(declarationEmitKeywordPropertyNames.ts, 14, 15))
47+
48+
break: "break",
49+
>break : Symbol(break, Decl(declarationEmitKeywordPropertyNames.ts, 15, 21))
50+
51+
continue: "continue",
52+
>continue : Symbol(continue, Decl(declarationEmitKeywordPropertyNames.ts, 16, 19))
53+
54+
} as const;
55+
>const : Symbol(const)
56+

‎testdata/baselines/reference/compiler/declarationEmitKeywordPropertyNames.types‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
//// [tests/cases/compiler/declarationEmitKeywordPropertyNames.ts] ////
2+
3+
=== declarationEmitKeywordPropertyNames.ts ===
4+
export const a = {
5+
>a : { foo: string; bar: string; buzz: string; new: string; delete: string; break: string; continue: string; }
6+
>{ foo: "foo", bar: "bar", buzz: "buzz", new: "new", delete: "delete", break: "break", continue: "continue",} : { foo: string; bar: string; buzz: string; new: string; delete: string; break: string; continue: string; }
7+
8+
foo: "foo",
9+
>foo : string
10+
>"foo" : "foo"
11+
12+
bar: "bar",
13+
>bar : string
14+
>"bar" : "bar"
15+
16+
buzz: "buzz",
17+
>buzz : string
18+
>"buzz" : "buzz"
19+
20+
new: "new",
21+
>new : string
22+
>"new" : "new"
23+
24+
delete: "delete",
25+
>delete : string
26+
>"delete" : "delete"
27+
28+
break: "break",
29+
>break : string
30+
>"break" : "break"
31+
32+
continue: "continue",
33+
>continue : string
34+
>"continue" : "continue"
35+
36+
};
37+
38+
export const b = {
39+
>b : { readonly foo: "foo"; readonly bar: "bar"; readonly buzz: "buzz"; readonly new: "new"; readonly delete: "delete"; readonly break: "break"; readonly continue: "continue"; }
40+
>{ foo: "foo", bar: "bar", buzz: "buzz", new: "new", delete: "delete", break: "break", continue: "continue",} as const : { readonly foo: "foo"; readonly bar: "bar"; readonly buzz: "buzz"; readonly new: "new"; readonly delete: "delete"; readonly break: "break"; readonly continue: "continue"; }
41+
>{ foo: "foo", bar: "bar", buzz: "buzz", new: "new", delete: "delete", break: "break", continue: "continue",} : { readonly foo: "foo"; readonly bar: "bar"; readonly buzz: "buzz"; readonly new: "new"; readonly delete: "delete"; readonly break: "break"; readonly continue: "continue"; }
42+
43+
foo: "foo",
44+
>foo : "foo"
45+
>"foo" : "foo"
46+
47+
bar: "bar",
48+
>bar : "bar"
49+
>"bar" : "bar"
50+
51+
buzz: "buzz",
52+
>buzz : "buzz"
53+
>"buzz" : "buzz"
54+
55+
new: "new",
56+
>new : "new"
57+
>"new" : "new"
58+
59+
delete: "delete",
60+
>delete : "delete"
61+
>"delete" : "delete"
62+
63+
break: "break",
64+
>break : "break"
65+
>"break" : "break"
66+
67+
continue: "continue",
68+
>continue : "continue"
69+
>"continue" : "continue"
70+
71+
} as const;
72+

‎testdata/baselines/reference/submodule/compiler/emitMethodCalledNew.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,5 @@ export declare const b: {
3535
"new"(x: number): number;
3636
};
3737
export declare const c: {
38-
["new"](x: number): number;
38+
"new"(x: number): number;
3939
};

‎testdata/baselines/reference/submoduleAccepted/compiler/emitMethodCalledNew.js.diff‎

Lines changed: 0 additions & 9 deletions
This file was deleted.

‎testdata/submoduleAccepted.txt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,6 @@ compiler/declarationEmitDefaultExportWithStaticAssignment.symbols.diff
374374
compiler/declarationEmitDefaultExportWithStaticAssignment.types.diff
375375
compiler/decoratorMetadataTypeOnlyExport.symbols.diff
376376
compiler/decoratorMetadataTypeOnlyImport.symbols.diff
377-
compiler/emitMethodCalledNew.js.diff
378377
compiler/es6ImportWithJsDocTags.symbols.diff
379378
compiler/es2018ObjectAssign.types.diff
380379
compiler/expandoFunctionSymbolPropertyJs.js.diff

‎testdata/tests/cases/compiler/declarationEmitKeywordPropertyNames.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// @declaration: true
2+
// @emitDeclarationOnly: true
3+
4+
export const a = {
5+
foo: "foo",
6+
bar: "bar",
7+
buzz: "buzz",
8+
new: "new",
9+
delete: "delete",
10+
break: "break",
11+
continue: "continue",
12+
};
13+
14+
export const b = {
15+
foo: "foo",
16+
bar: "bar",
17+
buzz: "buzz",
18+
new: "new",
19+
delete: "delete",
20+
break: "break",
21+
continue: "continue",
22+
} as const;

0 commit comments

Comments
 (0)