Skip to content

Commit b15d8aa

Browse files
JsonFreemanJsonFreeman
committed
Address PR feedback
1 parent a27fbff commit b15d8aa

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+547
-418
lines changed

‎src/compiler/checker.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10911,9 +10911,11 @@ module ts {
1091110911
getSymbolDisplayBuilder().buildTypeDisplay(getReturnTypeOfSignature(signature), writer, enclosingDeclaration, flags);
1091210912
}
1091310913

10914-
function isUnknownIdentifier(location: Node, name: string, sourceFile: SourceFile): boolean {
10914+
function isUnknownIdentifier(location: Node, name: string): boolean {
10915+
// Do not call resolveName on a synthesized node!
10916+
Debug.assert(!nodeIsSynthesized(location), "isUnknownIdentifier called with a synthesized location");
1091510917
return !resolveName(location, name, SymbolFlags.Value, /*nodeNotFoundMessage*/ undefined, /*nameArg*/ undefined) &&
10916-
!hasProperty(getGeneratedNamesForSourceFile(sourceFile), name);
10918+
!hasProperty(getGeneratedNamesForSourceFile(getSourceFile(location)), name);
1091710919
}
1091810920

1091910921
function getBlockScopedVariableId(n: Identifier): number {

‎src/compiler/emitter.ts

Lines changed: 47 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1641,14 +1641,13 @@ module ts {
16411641
}
16421642

16431643
if (root) {
1644-
currentSourceFile = root;
1645-
emit(root);
1644+
// Do not call emit directly. It does not set the currentSourceFile.
1645+
emitSourceFile(root);
16461646
}
16471647
else {
16481648
forEach(host.getSourceFiles(), sourceFile => {
1649-
currentSourceFile = sourceFile;
16501649
if (!isExternalModuleOrDeclarationFile(sourceFile)) {
1651-
emit(sourceFile);
1650+
emitSourceFile(sourceFile);
16521651
}
16531652
});
16541653
}
@@ -1657,6 +1656,11 @@ module ts {
16571656
writeEmittedFiles(writer.getText(), /*writeByteOrderMark*/ compilerOptions.emitBOM);
16581657
return;
16591658

1659+
function emitSourceFile(sourceFile: SourceFile): void {
1660+
currentSourceFile = sourceFile;
1661+
emit(sourceFile);
1662+
}
1663+
16601664
// enters the new lexical environment
16611665
// return value should be passed to matching call to exitNameScope.
16621666
function enterNameScope(): boolean {
@@ -1689,10 +1693,10 @@ module ts {
16891693
name = generateUniqueName(baseName, n => isExistingName(location, n));
16901694
}
16911695

1692-
return putNameInCurrentScopeNames(name);
1696+
return recordNameInCurrentScope(name);
16931697
}
16941698

1695-
function putNameInCurrentScopeNames(name: string): string {
1699+
function recordNameInCurrentScope(name: string): string {
16961700
if (!currentScopeNames) {
16971701
currentScopeNames = {};
16981702
}
@@ -1702,7 +1706,7 @@ module ts {
17021706

17031707
function isExistingName(location: Node, name: string) {
17041708
// check if resolver is aware of this name (if name was seen during the typecheck)
1705-
if (!resolver.isUnknownIdentifier(location, name, currentSourceFile)) {
1709+
if (!resolver.isUnknownIdentifier(location, name)) {
17061710
return true;
17071711
}
17081712

@@ -2107,13 +2111,14 @@ module ts {
21072111
break;
21082112
}
21092113
// _a .. _h, _j ... _z, _0, _1, ...
2114+
// Note that _i is skipped
21102115
name = "_" + (tempCount < 25 ? String.fromCharCode(tempCount + (tempCount < 8 ? 0 : 1) + CharacterCodes.a) : tempCount - 25);
21112116
tempCount++;
21122117
}
21132118

21142119
// This is necessary so that a name generated via renameNonTopLevelLetAndConst will see the name
21152120
// we just generated.
2116-
putNameInCurrentScopeNames(name);
2121+
recordNameInCurrentScope(name);
21172122

21182123
var result = <Identifier>createSynthesizedNode(SyntaxKind.Identifier);
21192124
result.text = name;
@@ -3315,7 +3320,7 @@ module ts {
33153320
function emitBinaryExpression(node: BinaryExpression) {
33163321
if (languageVersion < ScriptTarget.ES6 && node.operatorToken.kind === SyntaxKind.EqualsToken &&
33173322
(node.left.kind === SyntaxKind.ObjectLiteralExpression || node.left.kind === SyntaxKind.ArrayLiteralExpression)) {
3318-
emitDestructuring(node);
3323+
emitDestructuring(node, node.parent.kind === SyntaxKind.ExpressionStatement);
33193324
}
33203325
else {
33213326
emit(node.left);
@@ -3595,7 +3600,7 @@ module ts {
35953600

35963601
// Do not emit the LHS var declaration yet, because it might contain destructuring.
35973602

3598-
// Do not call create recordTempDeclaration because we are declaring the temps
3603+
// Do not call recordTempDeclaration because we are declaring the temps
35993604
// right here. Recording means they will be declared later.
36003605
// In the case where the user wrote an identifier as the RHS, like this:
36013606
//
@@ -3655,12 +3660,12 @@ module ts {
36553660
if (node.initializer.kind === SyntaxKind.VariableDeclarationList) {
36563661
write("var ");
36573662
var variableDeclarationList = <VariableDeclarationList>node.initializer;
3658-
if (variableDeclarationList.declarations.length >= 1) {
3663+
if (variableDeclarationList.declarations.length > 0) {
36593664
var declaration = variableDeclarationList.declarations[0];
36603665
if (isBindingPattern(declaration.name)) {
36613666
// This works whether the declaration is a var, let, or const.
36623667
// It will use rhsIterationValue _a[_i] as the initializer.
3663-
emitDestructuring(declaration, rhsIterationValue);
3668+
emitDestructuring(declaration, /*isAssignmentExpressionStatement*/ false, rhsIterationValue);
36643669
}
36653670
else {
36663671
// The following call does not include the initializer, so we have
@@ -3673,8 +3678,7 @@ module ts {
36733678
else {
36743679
// It's an empty declaration list. This can only happen in an error case, if the user wrote
36753680
// for (var of []) {}
3676-
var emptyDeclarationListTemp = createTempVariable(node, /*forLoopVariable*/ false);
3677-
emitNode(emptyDeclarationListTemp);
3681+
emitNode(createTempVariable(node, /*forLoopVariable*/ false));
36783682
write(" = ");
36793683
emitNode(rhsIterationValue);
36803684
}
@@ -3683,11 +3687,10 @@ module ts {
36833687
// Initializer is an expression. Emit the expression in the body, so that it's
36843688
// evaluated on every iteration.
36853689
var assignmentExpression = createBinaryExpression(<Expression>node.initializer, SyntaxKind.EqualsToken, rhsIterationValue, /*startsOnNewLine*/ false);
3686-
var assignmentExpressionStatement = createExpressionStatement(assignmentExpression);
36873690
if (node.initializer.kind === SyntaxKind.ArrayLiteralExpression || node.initializer.kind === SyntaxKind.ObjectLiteralExpression) {
36883691
// This is a destructuring pattern, so call emitDestructuring instead of emit. Calling emit will not work, because it will cause
36893692
// the BinaryExpression to be passed in instead of the expression statement, which will cause emitDestructuring to crash.
3690-
emitDestructuring(assignmentExpressionStatement);
3693+
emitDestructuring(assignmentExpression, /*isAssignmentExpressionStatement*/ true, /*value*/ undefined, /*locationForCheckingExistingName*/ node);
36913694
}
36923695
else {
36933696
emitNode(assignmentExpression);
@@ -3864,16 +3867,25 @@ module ts {
38643867
}
38653868
}
38663869

3867-
// Note that a destructuring assignment can be either an ExpressionStatement or a BinaryExpression.
3868-
function emitDestructuring(root: ExpressionStatement | BinaryExpression | VariableDeclaration | ParameterDeclaration, value?: Expression) {
3870+
/**
3871+
* If the root has a chance of being a synthesized node, callers should also pass a value for
3872+
* lowestNonSynthesizedAncestor. This should be an ancestor of root, it should not be synthesized,
3873+
* and there should not be a lower ancestor that introduces a scope. This node will be used as the
3874+
* location for ensuring that temporary names are unique.
3875+
*/
3876+
function emitDestructuring(root: BinaryExpression | VariableDeclaration | ParameterDeclaration,
3877+
isAssignmentExpressionStatement: boolean,
3878+
value?: Expression,
3879+
lowestNonSynthesizedAncestor?: Node) {
38693880
var emitCount = 0;
38703881
// An exported declaration is actually emitted as an assignment (to a property on the module object), so
38713882
// temporary variables in an exported declaration need to have real declarations elsewhere
38723883
var isDeclaration = (root.kind === SyntaxKind.VariableDeclaration && !(getCombinedNodeFlags(root) & NodeFlags.Export)) || root.kind === SyntaxKind.Parameter;
3873-
if (root.kind === SyntaxKind.ExpressionStatement || root.kind === SyntaxKind.BinaryExpression) {
3874-
emitAssignmentExpression(<ExpressionStatement | BinaryExpression>root);
3884+
if (root.kind === SyntaxKind.BinaryExpression) {
3885+
emitAssignmentExpression(<BinaryExpression>root);
38753886
}
38763887
else {
3888+
Debug.assert(!isAssignmentExpressionStatement);
38773889
emitBindingElement(<BindingElement>root, value);
38783890
}
38793891

@@ -3895,7 +3907,10 @@ module ts {
38953907

38963908
function ensureIdentifier(expr: Expression): Expression {
38973909
if (expr.kind !== SyntaxKind.Identifier) {
3898-
var identifier = createTempVariable(root);
3910+
// In case the root is a synthesized node, we need to pass lowestNonSynthesizedAncestor
3911+
// as the location for determining uniqueness of the variable we are about to
3912+
// generate.
3913+
var identifier = createTempVariable(lowestNonSynthesizedAncestor || root);
38993914
if (!isDeclaration) {
39003915
recordTempDeclaration(identifier);
39013916
}
@@ -4013,14 +4028,13 @@ module ts {
40134028
}
40144029
}
40154030

4016-
function emitAssignmentExpression(root: ExpressionStatement | BinaryExpression) {
4017-
// Synthesized nodes will not have parents, so the ExpressionStatements will have to be passed
4018-
// in directly. Otherwise, it will crash when we access the parent of a synthesized binary expression.
4019-
var emitParenthesized = root.kind !== SyntaxKind.ExpressionStatement && root.parent.kind !== SyntaxKind.ExpressionStatement;
4020-
var expression = <BinaryExpression>(root.kind === SyntaxKind.ExpressionStatement ? (<ExpressionStatement>root).expression : <BinaryExpression>root);
4021-
var target = expression.left;
4022-
var value = expression.right;
4023-
if (emitParenthesized) {
4031+
function emitAssignmentExpression(root: BinaryExpression) {
4032+
var target = root.left;
4033+
var value = root.right;
4034+
if (isAssignmentExpressionStatement) {
4035+
emitDestructuringAssignment(target, value);
4036+
}
4037+
else {
40244038
if (root.parent.kind !== SyntaxKind.ParenthesizedExpression) {
40254039
write("(");
40264040
}
@@ -4032,9 +4046,6 @@ module ts {
40324046
write(")");
40334047
}
40344048
}
4035-
else {
4036-
emitDestructuringAssignment(target, value);
4037-
}
40384049
}
40394050

40404051
function emitBindingElement(target: BindingElement, value: Expression) {
@@ -4085,7 +4096,7 @@ module ts {
40854096
function emitVariableDeclaration(node: VariableDeclaration) {
40864097
if (isBindingPattern(node.name)) {
40874098
if (languageVersion < ScriptTarget.ES6) {
4088-
emitDestructuring(node);
4099+
emitDestructuring(node, /*isAssignmentExpressionStatement*/ false);
40894100
}
40904101
else {
40914102
emit(node.name);
@@ -4248,7 +4259,7 @@ module ts {
42484259
if (isBindingPattern(p.name)) {
42494260
writeLine();
42504261
write("var ");
4251-
emitDestructuring(p, tempParameters[tempIndex]);
4262+
emitDestructuring(p, /*isAssignmentExpressionStatement*/ false, tempParameters[tempIndex]);
42524263
write(";");
42534264
tempIndex++;
42544265
}
@@ -5310,7 +5321,7 @@ module ts {
53105321
return statements.length;
53115322
}
53125323

5313-
function emitSourceFile(node: SourceFile) {
5324+
function emitSourceFileNode(node: SourceFile) {
53145325
// Start new file on new line
53155326
writeLine();
53165327
emitDetachedComments(node);
@@ -5553,7 +5564,7 @@ module ts {
55535564
case SyntaxKind.ExportDeclaration:
55545565
return emitExportDeclaration(<ExportDeclaration>node);
55555566
case SyntaxKind.SourceFile:
5556-
return emitSourceFile(<SourceFile>node);
5567+
return emitSourceFileNode(<SourceFile>node);
55575568
}
55585569
}
55595570

‎src/compiler/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1205,7 +1205,7 @@ module ts {
12051205
isEntityNameVisible(entityName: EntityName, enclosingDeclaration: Node): SymbolVisibilityResult;
12061206
// Returns the constant value this property access resolves to, or 'undefined' for a non-constant
12071207
getConstantValue(node: EnumMember | PropertyAccessExpression | ElementAccessExpression): number;
1208-
isUnknownIdentifier(location: Node, name: string, sourceFile: SourceFile): boolean;
1208+
isUnknownIdentifier(location: Node, name: string): boolean;
12091209
getBlockScopedVariableId(node: Identifier): number;
12101210
}
12111211

‎tests/baselines/reference/APISample_compile.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -942,7 +942,7 @@ declare module "typescript" {
942942
isSymbolAccessible(symbol: Symbol, enclosingDeclaration: Node, meaning: SymbolFlags): SymbolAccessiblityResult;
943943
isEntityNameVisible(entityName: EntityName, enclosingDeclaration: Node): SymbolVisibilityResult;
944944
getConstantValue(node: EnumMember | PropertyAccessExpression | ElementAccessExpression): number;
945-
isUnknownIdentifier(location: Node, name: string, sourceFile: SourceFile): boolean;
945+
isUnknownIdentifier(location: Node, name: string): boolean;
946946
getBlockScopedVariableId(node: Identifier): number;
947947
}
948948
const enum SymbolFlags {

‎tests/baselines/reference/APISample_compile.types

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3063,13 +3063,11 @@ declare module "typescript" {
30633063
>PropertyAccessExpression : PropertyAccessExpression
30643064
>ElementAccessExpression : ElementAccessExpression
30653065

3066-
isUnknownIdentifier(location: Node, name: string, sourceFile: SourceFile): boolean;
3067-
>isUnknownIdentifier : (location: Node, name: string, sourceFile: SourceFile) => boolean
3066+
isUnknownIdentifier(location: Node, name: string): boolean;
3067+
>isUnknownIdentifier : (location: Node, name: string) => boolean
30683068
>location : Node
30693069
>Node : Node
30703070
>name : string
3071-
>sourceFile : SourceFile
3072-
>SourceFile : SourceFile
30733071

30743072
getBlockScopedVariableId(node: Identifier): number;
30753073
>getBlockScopedVariableId : (node: Identifier) => number

‎tests/baselines/reference/APISample_linter.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -973,7 +973,7 @@ declare module "typescript" {
973973
isSymbolAccessible(symbol: Symbol, enclosingDeclaration: Node, meaning: SymbolFlags): SymbolAccessiblityResult;
974974
isEntityNameVisible(entityName: EntityName, enclosingDeclaration: Node): SymbolVisibilityResult;
975975
getConstantValue(node: EnumMember | PropertyAccessExpression | ElementAccessExpression): number;
976-
isUnknownIdentifier(location: Node, name: string, sourceFile: SourceFile): boolean;
976+
isUnknownIdentifier(location: Node, name: string): boolean;
977977
getBlockScopedVariableId(node: Identifier): number;
978978
}
979979
const enum SymbolFlags {

‎tests/baselines/reference/APISample_linter.types

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3209,13 +3209,11 @@ declare module "typescript" {
32093209
>PropertyAccessExpression : PropertyAccessExpression
32103210
>ElementAccessExpression : ElementAccessExpression
32113211

3212-
isUnknownIdentifier(location: Node, name: string, sourceFile: SourceFile): boolean;
3213-
>isUnknownIdentifier : (location: Node, name: string, sourceFile: SourceFile) => boolean
3212+
isUnknownIdentifier(location: Node, name: string): boolean;
3213+
>isUnknownIdentifier : (location: Node, name: string) => boolean
32143214
>location : Node
32153215
>Node : Node
32163216
>name : string
3217-
>sourceFile : SourceFile
3218-
>SourceFile : SourceFile
32193217

32203218
getBlockScopedVariableId(node: Identifier): number;
32213219
>getBlockScopedVariableId : (node: Identifier) => number

‎tests/baselines/reference/APISample_transform.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -974,7 +974,7 @@ declare module "typescript" {
974974
isSymbolAccessible(symbol: Symbol, enclosingDeclaration: Node, meaning: SymbolFlags): SymbolAccessiblityResult;
975975
isEntityNameVisible(entityName: EntityName, enclosingDeclaration: Node): SymbolVisibilityResult;
976976
getConstantValue(node: EnumMember | PropertyAccessExpression | ElementAccessExpression): number;
977-
isUnknownIdentifier(location: Node, name: string, sourceFile: SourceFile): boolean;
977+
isUnknownIdentifier(location: Node, name: string): boolean;
978978
getBlockScopedVariableId(node: Identifier): number;
979979
}
980980
const enum SymbolFlags {

‎tests/baselines/reference/APISample_transform.types

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3159,13 +3159,11 @@ declare module "typescript" {
31593159
>PropertyAccessExpression : PropertyAccessExpression
31603160
>ElementAccessExpression : ElementAccessExpression
31613161

3162-
isUnknownIdentifier(location: Node, name: string, sourceFile: SourceFile): boolean;
3163-
>isUnknownIdentifier : (location: Node, name: string, sourceFile: SourceFile) => boolean
3162+
isUnknownIdentifier(location: Node, name: string): boolean;
3163+
>isUnknownIdentifier : (location: Node, name: string) => boolean
31643164
>location : Node
31653165
>Node : Node
31663166
>name : string
3167-
>sourceFile : SourceFile
3168-
>SourceFile : SourceFile
31693167

31703168
getBlockScopedVariableId(node: Identifier): number;
31713169
>getBlockScopedVariableId : (node: Identifier) => number

‎tests/baselines/reference/APISample_watcher.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1011,7 +1011,7 @@ declare module "typescript" {
10111011
isSymbolAccessible(symbol: Symbol, enclosingDeclaration: Node, meaning: SymbolFlags): SymbolAccessiblityResult;
10121012
isEntityNameVisible(entityName: EntityName, enclosingDeclaration: Node): SymbolVisibilityResult;
10131013
getConstantValue(node: EnumMember | PropertyAccessExpression | ElementAccessExpression): number;
1014-
isUnknownIdentifier(location: Node, name: string, sourceFile: SourceFile): boolean;
1014+
isUnknownIdentifier(location: Node, name: string): boolean;
10151015
getBlockScopedVariableId(node: Identifier): number;
10161016
}
10171017
const enum SymbolFlags {

‎tests/baselines/reference/APISample_watcher.types

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3332,13 +3332,11 @@ declare module "typescript" {
33323332
>PropertyAccessExpression : PropertyAccessExpression
33333333
>ElementAccessExpression : ElementAccessExpression
33343334

3335-
isUnknownIdentifier(location: Node, name: string, sourceFile: SourceFile): boolean;
3336-
>isUnknownIdentifier : (location: Node, name: string, sourceFile: SourceFile) => boolean
3335+
isUnknownIdentifier(location: Node, name: string): boolean;
3336+
>isUnknownIdentifier : (location: Node, name: string) => boolean
33373337
>location : Node
33383338
>Node : Node
33393339
>name : string
3340-
>sourceFile : SourceFile
3341-
>SourceFile : SourceFile
33423340

33433341
getBlockScopedVariableId(node: Identifier): number;
33443342
>getBlockScopedVariableId : (node: Identifier) => number

0 commit comments

Comments
 (0)