Skip to content

Commit bef82cb

Browse files
Address code review feedback - revert declarations.ts changes and clean up test
Co-authored-by: RyanCavanaugh <[email protected]>
1 parent 64358a2 commit bef82cb

File tree

6 files changed

+25
-141
lines changed

6 files changed

+25
-141
lines changed

src/compiler/transformers/declarations.ts

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,7 @@ import {
103103
isExpandoPropertyDeclaration,
104104
isExportAssignment,
105105
isExportDeclaration,
106-
isExpressionWithTypeArguments,
107-
isExpression,
106+
isExpressionWithTypeArguments,
108107
isExternalModule,
109108
isExternalModuleAugmentation,
110109
isExternalModuleIndicator,
@@ -129,9 +128,8 @@ import {
129128
isObjectLiteralExpression,
130129
isOmittedExpression,
131130
isParameter,
132-
isPrimitiveLiteralValue,
133-
isPrivateIdentifier,
134-
isPropertyAccessExpression,
131+
isPrimitiveLiteralValue,
132+
isPrivateIdentifier,
135133
isSemicolonClassElement,
136134
isSetAccessorDeclaration,
137135
isSourceFile,
@@ -202,8 +200,8 @@ import {
202200
TransformationContext,
203201
Transformer,
204202
transformNodes,
205-
tryCast,
206-
TypeAliasDeclaration,
203+
tryCast,
204+
TypeAliasDeclaration,
207205
TypeNode,
208206
TypeParameterDeclaration,
209207
TypeReferenceNode,
@@ -656,21 +654,21 @@ export function transformDeclarations(context: TransformationContext): Transform
656654
return newParam;
657655
}
658656

659-
function shouldPrintWithInitializer(node: Node): node is CanHaveLiteralInitializer & { initializer: Expression; } {
660-
return canHaveLiteralInitializer(node)
661-
&& !!node.initializer
662-
&& resolver.isLiteralConstDeclaration(getParseTreeNode(node) as CanHaveLiteralInitializer); // TODO: Make safea
657+
function shouldPrintWithInitializer(node: Node): node is CanHaveLiteralInitializer & { initializer: Expression; } {
658+
return canHaveLiteralInitializer(node)
659+
&& !!node.initializer
660+
&& resolver.isLiteralConstDeclaration(getParseTreeNode(node) as CanHaveLiteralInitializer); // TODO: Make safea
663661
}
664662

665-
function ensureNoInitializer(node: CanHaveLiteralInitializer) {
666-
if (shouldPrintWithInitializer(node)) {
667-
const unwrappedInitializer = unwrapParenthesizedExpression(node.initializer);
668-
if (!isPrimitiveLiteralValue(unwrappedInitializer)) {
669-
reportInferenceFallback(node);
670-
}
671-
return resolver.createLiteralConstValue(getParseTreeNode(node, canHaveLiteralInitializer)!, symbolTracker);
672-
}
673-
return undefined;
663+
function ensureNoInitializer(node: CanHaveLiteralInitializer) {
664+
if (shouldPrintWithInitializer(node)) {
665+
const unwrappedInitializer = unwrapParenthesizedExpression(node.initializer);
666+
if (!isPrimitiveLiteralValue(unwrappedInitializer)) {
667+
reportInferenceFallback(node);
668+
}
669+
return resolver.createLiteralConstValue(getParseTreeNode(node, canHaveLiteralInitializer)!, symbolTracker);
670+
}
671+
return undefined;
674672
}
675673
function ensureType(node: VariableDeclaration | ParameterDeclaration | BindingElement | PropertyDeclaration | PropertySignature | ExportAssignment | SignatureDeclaration, ignorePrivate?: boolean): TypeNode | undefined {
676674
if (!ignorePrivate && hasEffectiveModifier(node, ModifierFlags.Private)) {
@@ -1051,12 +1049,12 @@ export function transformDeclarations(context: TransformationContext): Transform
10511049
const oldWithinObjectLiteralType = suppressNewDiagnosticContexts;
10521050
let shouldEnterSuppressNewDiagnosticsContextContext = (input.kind === SyntaxKind.TypeLiteral || input.kind === SyntaxKind.MappedType) && input.parent.kind !== SyntaxKind.TypeAliasDeclaration;
10531051

1054-
// Emit methods which are private as properties with no type information
1055-
if (isMethodDeclaration(input) || isMethodSignature(input)) {
1056-
if (hasEffectiveModifier(input, ModifierFlags.Private)) {
1057-
if (input.symbol && input.symbol.declarations && input.symbol.declarations[0] !== input) return; // Elide all but the first overload
1058-
return cleanup(factory.createPropertyDeclaration(ensureModifiers(input), input.name, /*questionOrExclamationToken*/ undefined, /*type*/ undefined, /*initializer*/ undefined));
1059-
}
1052+
// Emit methods which are private as properties with no type information
1053+
if (isMethodDeclaration(input) || isMethodSignature(input)) {
1054+
if (hasEffectiveModifier(input, ModifierFlags.Private)) {
1055+
if (input.symbol && input.symbol.declarations && input.symbol.declarations[0] !== input) return; // Elide all but the first overload
1056+
return cleanup(factory.createPropertyDeclaration(ensureModifiers(input), input.name, /*questionOrExclamationToken*/ undefined, /*type*/ undefined, /*initializer*/ undefined));
1057+
}
10601058
}
10611059

10621060
if (canProduceDiagnostic && !suppressNewDiagnosticContexts) {

tests/baselines/reference/enumNamespaceConstantsDeclaration.js

Lines changed: 1 addition & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,7 @@ namespace MyEnum {
1919
export const value2 = MyEnum.Second;
2020
}
2121

22-
// String enum
23-
enum StringEnum {
24-
Option1 = "option1",
25-
Option2 = "option2"
26-
}
27-
namespace StringEnum {
28-
export const selected = StringEnum.Option1;
29-
}
22+
3023

3124
//// [enumNamespaceConstantsDeclaration.js]
3225
// Test for constant declarations inside namespace merged with enum
@@ -47,15 +40,6 @@ var MyEnum;
4740
MyEnum.value1 = MyEnum.First;
4841
MyEnum.value2 = MyEnum.Second;
4942
})(MyEnum || (MyEnum = {}));
50-
// String enum
51-
var StringEnum;
52-
(function (StringEnum) {
53-
StringEnum["Option1"] = "option1";
54-
StringEnum["Option2"] = "option2";
55-
})(StringEnum || (StringEnum = {}));
56-
(function (StringEnum) {
57-
StringEnum.selected = StringEnum.Option1;
58-
})(StringEnum || (StringEnum = {}));
5943

6044

6145
//// [enumNamespaceConstantsDeclaration.d.ts]
@@ -73,25 +57,3 @@ declare namespace MyEnum {
7357
const value1 = MyEnum.First;
7458
const value2 = MyEnum.Second;
7559
}
76-
declare enum StringEnum {
77-
Option1 = "option1",
78-
Option2 = "option2"
79-
}
80-
declare namespace StringEnum {
81-
const selected: any;
82-
}
83-
84-
85-
!!!! File enumNamespaceConstantsDeclaration.d.ts differs from original emit in noCheck emit
86-
//// [enumNamespaceConstantsDeclaration.d.ts]
87-
===================================================================
88-
--- Expected The full check baseline
89-
+++ Actual with noCheck set
90-
@@ -16,6 +16,6 @@
91-
Option1 = "option1",
92-
Option2 = "option2"
93-
}
94-
declare namespace StringEnum {
95-
- const selected: any;
96-
+ const selected = StringEnum.Option1;
97-
}

tests/baselines/reference/enumNamespaceConstantsDeclaration.symbols

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,22 +44,4 @@ namespace MyEnum {
4444
>Second : Symbol(Second, Decl(enumNamespaceConstantsDeclaration.ts, 10, 14))
4545
}
4646

47-
// String enum
48-
enum StringEnum {
49-
>StringEnum : Symbol(StringEnum, Decl(enumNamespaceConstantsDeclaration.ts, 16, 1), Decl(enumNamespaceConstantsDeclaration.ts, 22, 1))
5047

51-
Option1 = "option1",
52-
>Option1 : Symbol(StringEnum.Option1, Decl(enumNamespaceConstantsDeclaration.ts, 19, 17))
53-
54-
Option2 = "option2"
55-
>Option2 : Symbol(StringEnum.Option2, Decl(enumNamespaceConstantsDeclaration.ts, 20, 24))
56-
}
57-
namespace StringEnum {
58-
>StringEnum : Symbol(StringEnum, Decl(enumNamespaceConstantsDeclaration.ts, 16, 1), Decl(enumNamespaceConstantsDeclaration.ts, 22, 1))
59-
60-
export const selected = StringEnum.Option1;
61-
>selected : Symbol(selected, Decl(enumNamespaceConstantsDeclaration.ts, 24, 16))
62-
>StringEnum.Option1 : Symbol(Option1, Decl(enumNamespaceConstantsDeclaration.ts, 19, 17))
63-
>StringEnum : Symbol(StringEnum, Decl(enumNamespaceConstantsDeclaration.ts, 16, 1), Decl(enumNamespaceConstantsDeclaration.ts, 22, 1))
64-
>Option1 : Symbol(Option1, Decl(enumNamespaceConstantsDeclaration.ts, 19, 17))
65-
}

tests/baselines/reference/enumNamespaceConstantsDeclaration.types

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -67,33 +67,4 @@ namespace MyEnum {
6767
> : ^^^^^^^^^^^^^
6868
}
6969

70-
// String enum
71-
enum StringEnum {
72-
>StringEnum : StringEnum
73-
> : ^^^^^^^^^^
7470

75-
Option1 = "option1",
76-
>Option1 : StringEnum.Option1
77-
> : ^^^^^^^^^^^^^^^^^^
78-
>"option1" : "option1"
79-
> : ^^^^^^^^^
80-
81-
Option2 = "option2"
82-
>Option2 : StringEnum.Option2
83-
> : ^^^^^^^^^^^^^^^^^^
84-
>"option2" : "option2"
85-
> : ^^^^^^^^^
86-
}
87-
namespace StringEnum {
88-
>StringEnum : typeof StringEnum
89-
> : ^^^^^^^^^^^^^^^^^
90-
91-
export const selected = StringEnum.Option1;
92-
>selected : any
93-
>StringEnum.Option1 : StringEnum.Option1
94-
> : ^^^^^^^^^^^^^^^^^^
95-
>StringEnum : typeof StringEnum
96-
> : ^^^^^^^^^^^^^^^^^
97-
>Option1 : StringEnum.Option1
98-
> : ^^^^^^^^^^^^^^^^^^
99-
}

tests/cases/compiler/enumNamespaceConstantsDeclaration.d.ts

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

tests/cases/compiler/enumNamespaceConstantsDeclaration.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,3 @@ namespace MyEnum {
1818
export const value2 = MyEnum.Second;
1919
}
2020

21-
// String enum
22-
enum StringEnum {
23-
Option1 = "option1",
24-
Option2 = "option2"
25-
}
26-
namespace StringEnum {
27-
export const selected = StringEnum.Option1;
28-
}

0 commit comments

Comments
 (0)