Skip to content

Commit 91c4dfd

Browse files
Copilotjakebailey
andauthored
Normalize reused string-literal property names to identifiers in declaration emit (#3965)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
1 parent 196800c commit 91c4dfd

10 files changed

Lines changed: 127 additions & 71 deletions

internal/checker/nodecopy.go

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ 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"
1011
)
1112

1213
func (b *NodeBuilderImpl) reuseNode(node *ast.Node) *ast.Node {
@@ -21,14 +22,34 @@ func (b *NodeBuilderImpl) tryJSTypeNodeToTypeNode(node *ast.Node) *ast.Node {
2122
return b.reuseNode(node)
2223
}
2324

24-
// a wrapper around `reuseNode` that handles renaming `new` to `"new"` so we don't accidentally emit constructor signatures when we don't mean to
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).
2531
func (b *NodeBuilderImpl) reuseName(node *ast.Node) *ast.Node {
2632
res := b.reuseNode(node)
27-
if res != nil && res.Kind == ast.KindIdentifier && node.AsIdentifier().Text == "new" {
33+
if res == nil {
34+
return res
35+
}
36+
if res.Kind == ast.KindIdentifier && node.AsIdentifier().Text == "new" {
2837
str := b.f.NewStringLiteral("new", ast.TokenFlagsNone)
2938
b.e.SetOriginal(str, res)
3039
return b.setTextRange(str, res)
3140
}
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+
}
52+
}
3253
return res
3354
}
3455

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//// [tests/cases/compiler/declarationEmitStringNamedPropertyConsistency.ts] ////
2+
3+
//// [declarationEmitStringNamedPropertyConsistency.ts]
4+
function createSlice<T>(o: T) {
5+
return o
6+
}
7+
export const platformSlice = createSlice({
8+
"query": {
9+
"search" : "",
10+
"user": ""
11+
},
12+
});
13+
14+
15+
16+
17+
//// [declarationEmitStringNamedPropertyConsistency.d.ts]
18+
export declare const platformSlice: {
19+
query: {
20+
search: string;
21+
user: string;
22+
};
23+
};
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
//// [tests/cases/compiler/declarationEmitStringNamedPropertyConsistency.ts] ////
2+
3+
=== declarationEmitStringNamedPropertyConsistency.ts ===
4+
function createSlice<T>(o: T) {
5+
>createSlice : Symbol(createSlice, Decl(declarationEmitStringNamedPropertyConsistency.ts, 0, 0))
6+
>T : Symbol(T, Decl(declarationEmitStringNamedPropertyConsistency.ts, 0, 21))
7+
>o : Symbol(o, Decl(declarationEmitStringNamedPropertyConsistency.ts, 0, 24))
8+
>T : Symbol(T, Decl(declarationEmitStringNamedPropertyConsistency.ts, 0, 21))
9+
10+
return o
11+
>o : Symbol(o, Decl(declarationEmitStringNamedPropertyConsistency.ts, 0, 24))
12+
}
13+
export const platformSlice = createSlice({
14+
>platformSlice : Symbol(platformSlice, Decl(declarationEmitStringNamedPropertyConsistency.ts, 3, 12))
15+
>createSlice : Symbol(createSlice, Decl(declarationEmitStringNamedPropertyConsistency.ts, 0, 0))
16+
17+
"query": {
18+
>"query" : Symbol("query", Decl(declarationEmitStringNamedPropertyConsistency.ts, 3, 42))
19+
20+
"search" : "",
21+
>"search" : Symbol("search", Decl(declarationEmitStringNamedPropertyConsistency.ts, 4, 14))
22+
23+
"user": ""
24+
>"user" : Symbol("user", Decl(declarationEmitStringNamedPropertyConsistency.ts, 5, 22))
25+
26+
},
27+
});
28+
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//// [tests/cases/compiler/declarationEmitStringNamedPropertyConsistency.ts] ////
2+
3+
=== declarationEmitStringNamedPropertyConsistency.ts ===
4+
function createSlice<T>(o: T) {
5+
>createSlice : <T>(o: T) => T
6+
>o : T
7+
8+
return o
9+
>o : T
10+
}
11+
export const platformSlice = createSlice({
12+
>platformSlice : { query: { search: string; user: string; }; }
13+
>createSlice({ "query": { "search" : "", "user": "" },}) : { query: { search: string; user: string; }; }
14+
>createSlice : <T>(o: T) => T
15+
>{ "query": { "search" : "", "user": "" },} : { query: { search: string; user: string; }; }
16+
17+
"query": {
18+
>"query" : { search: string; user: string; }
19+
>{ "search" : "", "user": "" } : { search: string; user: string; }
20+
21+
"search" : "",
22+
>"search" : string
23+
>"" : ""
24+
25+
"user": ""
26+
>"user" : string
27+
>"" : ""
28+
29+
},
30+
});
31+

testdata/baselines/reference/submodule/compiler/escapedReservedCompilerNamedIdentifier.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ var b5 = o5["_proto__"];
6969
//// [escapedReservedCompilerNamedIdentifier.d.ts]
7070
declare var __proto__: number;
7171
declare var o: {
72-
"__proto__": number;
72+
__proto__: number;
7373
};
7474
declare var b: number;
7575
declare var o1: {
@@ -78,7 +78,7 @@ declare var o1: {
7878
declare var b1: number;
7979
declare var ___proto__: number;
8080
declare var o2: {
81-
"___proto__": number;
81+
___proto__: number;
8282
};
8383
declare var b2: number;
8484
declare var o3: {
@@ -87,7 +87,7 @@ declare var o3: {
8787
declare var b3: number;
8888
declare var _proto__: number;
8989
declare var o4: {
90-
"_proto__": number;
90+
_proto__: number;
9191
};
9292
declare var b4: number;
9393
declare var o5: {

testdata/baselines/reference/submodule/conformance/jsDeclarationsPackageJson(target=es2015).types

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,17 @@
22

33
=== index.js ===
44
const j = require("./package.json");
5-
>j : { name: string; version: string; description: string; main: string; bin: { "cli": string; }; engines: { "node": string; }; scripts: { "scriptname": string; }; devDependencies: { "@ns/dep": string; }; dependencies: { "dep": string; }; repository: string; keywords: string[]; author: string; license: string; homepage: string; config: { "o": string[]; }; }
6-
>require("./package.json") : { name: string; version: string; description: string; main: string; bin: { "cli": string; }; engines: { "node": string; }; scripts: { "scriptname": string; }; devDependencies: { "@ns/dep": string; }; dependencies: { "dep": string; }; repository: string; keywords: string[]; author: string; license: string; homepage: string; config: { "o": string[]; }; }
5+
>j : { name: string; version: string; description: string; main: string; bin: { cli: string; }; engines: { node: string; }; scripts: { scriptname: string; }; devDependencies: { "@ns/dep": string; }; dependencies: { dep: string; }; repository: string; keywords: string[]; author: string; license: string; homepage: string; config: { o: string[]; }; }
6+
>require("./package.json") : { name: string; version: string; description: string; main: string; bin: { cli: string; }; engines: { node: string; }; scripts: { scriptname: string; }; devDependencies: { "@ns/dep": string; }; dependencies: { dep: string; }; repository: string; keywords: string[]; author: string; license: string; homepage: string; config: { o: string[]; }; }
77
>require : any
88
>"./package.json" : "./package.json"
99

1010
module.exports = j;
11-
>module.exports = j : { name: string; version: string; description: string; main: string; bin: { "cli": string; }; engines: { "node": string; }; scripts: { "scriptname": string; }; devDependencies: { "@ns/dep": string; }; dependencies: { "dep": string; }; repository: string; keywords: string[]; author: string; license: string; homepage: string; config: { "o": string[]; }; }
12-
>module.exports : { name: string; version: string; description: string; main: string; bin: { "cli": string; }; engines: { "node": string; }; scripts: { "scriptname": string; }; devDependencies: { "@ns/dep": string; }; dependencies: { "dep": string; }; repository: string; keywords: string[]; author: string; license: string; homepage: string; config: { "o": string[]; }; }
13-
>module : { exports: { name: string; version: string; description: string; main: string; bin: { "cli": string; }; engines: { "node": string; }; scripts: { "scriptname": string; }; devDependencies: { "@ns/dep": string; }; dependencies: { "dep": string; }; repository: string; keywords: string[]; author: string; license: string; homepage: string; config: { "o": string[]; }; }; }
14-
>exports : { name: string; version: string; description: string; main: string; bin: { "cli": string; }; engines: { "node": string; }; scripts: { "scriptname": string; }; devDependencies: { "@ns/dep": string; }; dependencies: { "dep": string; }; repository: string; keywords: string[]; author: string; license: string; homepage: string; config: { "o": string[]; }; }
15-
>j : { name: string; version: string; description: string; main: string; bin: { "cli": string; }; engines: { "node": string; }; scripts: { "scriptname": string; }; devDependencies: { "@ns/dep": string; }; dependencies: { "dep": string; }; repository: string; keywords: string[]; author: string; license: string; homepage: string; config: { "o": string[]; }; }
11+
>module.exports = j : { name: string; version: string; description: string; main: string; bin: { cli: string; }; engines: { node: string; }; scripts: { scriptname: string; }; devDependencies: { "@ns/dep": string; }; dependencies: { dep: string; }; repository: string; keywords: string[]; author: string; license: string; homepage: string; config: { o: string[]; }; }
12+
>module.exports : { name: string; version: string; description: string; main: string; bin: { cli: string; }; engines: { node: string; }; scripts: { scriptname: string; }; devDependencies: { "@ns/dep": string; }; dependencies: { dep: string; }; repository: string; keywords: string[]; author: string; license: string; homepage: string; config: { o: string[]; }; }
13+
>module : { exports: { name: string; version: string; description: string; main: string; bin: { cli: string; }; engines: { node: string; }; scripts: { scriptname: string; }; devDependencies: { "@ns/dep": string; }; dependencies: { dep: string; }; repository: string; keywords: string[]; author: string; license: string; homepage: string; config: { o: string[]; }; }; }
14+
>exports : { name: string; version: string; description: string; main: string; bin: { cli: string; }; engines: { node: string; }; scripts: { scriptname: string; }; devDependencies: { "@ns/dep": string; }; dependencies: { dep: string; }; repository: string; keywords: string[]; author: string; license: string; homepage: string; config: { o: string[]; }; }
15+
>j : { name: string; version: string; description: string; main: string; bin: { cli: string; }; engines: { node: string; }; scripts: { scriptname: string; }; devDependencies: { "@ns/dep": string; }; dependencies: { dep: string; }; repository: string; keywords: string[]; author: string; license: string; homepage: string; config: { o: string[]; }; }
1616

1717
=== package.json ===
1818
{

testdata/baselines/reference/submodule/conformance/jsDeclarationsPackageJson(target=es2015).types.diff

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

testdata/baselines/reference/submoduleAccepted/compiler/escapedReservedCompilerNamedIdentifier.js.diff

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

testdata/submoduleAccepted.txt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,9 +1247,6 @@ compiler/defaultDeclarationEmitShadowedNamedCorrectly.js.diff
12471247
compiler/es5ExportEqualsDts(target=es2015).js.diff
12481248
compiler/declarationEmitNameConflicts.js.diff
12491249

1250-
## Properties with __proto__-like names now quoted in declaration emit (it's quoted in the originating sourcefile)
1251-
compiler/escapedReservedCompilerNamedIdentifier.js.diff
1252-
12531250
## Nested /*elided*/ type annotation comments removed at deeper nesting levels (`/*elided*/` comments are internal artifacts with no semantic meaning)
12541251
## Declaration emit no longer preserves elided comments in nested class expressions
12551252
compiler/emitClassExpressionInDeclarationFile.js.diff
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// @declaration: true
2+
// @emitDeclarationOnly: true
3+
4+
function createSlice<T>(o: T) {
5+
return o
6+
}
7+
export const platformSlice = createSlice({
8+
"query": {
9+
"search" : "",
10+
"user": ""
11+
},
12+
});

0 commit comments

Comments
 (0)