fix(3560): fix nested export variable transformation in if/labeled statements (#3732)

Author: a-tarasyuk

internal/ast/visitor.go

@@ -76,11 +76,11 @@ func (v *NodeVisitor) VisitEmbeddedStatement(node *Statement) *Statement {
 		return node
 	}
 
-	if v.Visit != nil {
-		return v.liftToBlock(v.Visit(node))
+	visited := v.Visit(node)
+	if visited == nil {
+		return nil
 	}
-
-	return node
+	return v.liftToBlock(visited)
 }
 
 // Visits a NodeList, possibly returning a new NodeList in its place.

internal/transformers/moduletransforms/commonjsmodule.go

@@ -94,7 +94,7 @@ func (tx *CommonJSModuleTransformer) visitTopLevelNested(node *ast.Node) *ast.No
 func (tx *CommonJSModuleTransformer) visitTopLevelNestedNoStack(node *ast.Node) *ast.Node {
 	switch node.Kind {
 	case ast.KindVariableStatement:
-		node = tx.visitTopLevelNestedVariableStatement(node.AsVariableStatement())
+		node = tx.visitTopLevelVariableStatement(node.AsVariableStatement())
 	case ast.KindForStatement:
 		node = tx.visitTopLevelNestedForStatement(node.AsForStatement())
 	case ast.KindForInStatement, ast.KindForOfStatement:
@@ -1243,11 +1243,11 @@ func (tx *CommonJSModuleTransformer) visitTopLevelNestedWhileStatement(node *ast
 // Visits a top-level nested labeled statement as it may contain `var` declarations that are hoisted and may still be
 // exported with `export {}`.
 func (tx *CommonJSModuleTransformer) visitTopLevelNestedLabeledStatement(node *ast.LabeledStatement) *ast.Node {
-	return tx.Factory().UpdateLabeledStatement(
-		node,
-		node.Label,
-		tx.topLevelNestedVisitor.VisitEmbeddedStatement(node.Statement),
-	)
+	statement := tx.topLevelNestedVisitor.VisitEmbeddedStatement(node.Statement)
+	if statement == nil {
+		statement = tx.Factory().NewEmptyStatement()
+	}
+	return tx.Factory().UpdateLabeledStatement(node, node.Label, statement)
 }
 
 // Visits a top-level nested `with` statement as it may contain `var` declarations that are hoisted and may still be
@@ -1263,12 +1263,13 @@ func (tx *CommonJSModuleTransformer) visitTopLevelNestedWithStatement(node *ast.
 // Visits a top-level nested `if` statement as it may contain `var` declarations that are hoisted and may still be
 // exported with `export {}`.
 func (tx *CommonJSModuleTransformer) visitTopLevelNestedIfStatement(node *ast.IfStatement) *ast.Node {
-	return tx.Factory().UpdateIfStatement(
-		node,
-		tx.Visitor().VisitNode(node.Expression),
-		tx.topLevelNestedVisitor.VisitEmbeddedStatement(node.ThenStatement),
-		tx.topLevelNestedVisitor.VisitEmbeddedStatement(node.ElseStatement),
-	)
+	expression := tx.Visitor().VisitNode(node.Expression)
+	thenStatement := tx.topLevelNestedVisitor.VisitEmbeddedStatement(node.ThenStatement)
+	if thenStatement == nil {
+		thenStatement = tx.Factory().NewBlock(tx.Factory().NewNodeList(nil), false /*multiLine*/)
+	}
+	elseStatement := tx.topLevelNestedVisitor.VisitEmbeddedStatement(node.ElseStatement)
+	return tx.Factory().UpdateIfStatement(node, expression, thenStatement, elseStatement)
 }
 
 // Visits a top-level nested `switch` statement as it may contain `var` declarations that are hoisted and may still be

internal/transformers/utilities.go

@@ -220,6 +220,9 @@ func ConvertVariableDeclarationToAssignmentExpression(emitContext *printer.EmitC
 }
 
 func SingleOrMany(nodes []*ast.Node, factory *printer.NodeFactory) *ast.Node {
+	if nodes == nil {
+		return nil
+	}
 	if len(nodes) == 1 {
 		return nodes[0]
 	}

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

@@ -12,7 +12,7 @@ export type MyClass = typeof myClass;
 "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
 if (false) {
-    export var myClass = 0;
+    exports.myClass = 0;
 }
 
 

testdata/baselines/reference/submodule/conformance/exportNonInitializedVariablesInIfThenStatementNoCrash1(module=commonjs).js

@@ -12,6 +12,5 @@ export default cssExports;
 "use strict";
 // https://github.com/microsoft/TypeScript/issues/59373
 Object.defineProperty(exports, "__esModule", { value: true });
-if (true)
-    export const cssExports;
+if (true) { }
 exports.default = exports.cssExports;

testdata/baselines/reference/submodule/conformance/labeledStatementExportDeclarationNoCrash1(module=commonjs).js

@@ -13,4 +13,4 @@ export const title: string
 // https://github.com/microsoft/TypeScript/issues/59372
 Object.defineProperty(exports, "__esModule", { value: true });
 exports.box = void 0;
-subTitle: export const title;
+subTitle: ;

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

@@ -1,11 +1,8 @@
 --- old.declarationEmitInvalidExport.js
 +++ new.declarationEmitInvalidExport.js
-@@= skipped -11, +11 lines =@@
- "use strict";
- Object.defineProperty(exports, "__esModule", { value: true });
+@@= skipped -13, +13 lines =@@
  if (false) {
--    exports.myClass = 0;
-+    export var myClass = 0;
+     exports.myClass = 0;
  }
 +
 +

testdata/baselines/reference/submoduleTriaged/conformance/exportNonInitializedVariablesInIfThenStatementNoCrash1(module=commonjs).js.diff

@@ -196,21 +196,13 @@ conformance/callbackTagVariadicType.js.diff
 ## https://github.com/microsoft/typescript-go/issues/3557
 conformance/checkJsdocSatisfiesTag15.js.diff
 
-# Labeled statement followed by export declaration loses original text
-## https://github.com/microsoft/typescript-go/issues/3558
-conformance/labeledStatementExportDeclarationNoCrash1(module=commonjs).js.diff
-
 # CJS exports pattern changes in declaration emit
 ## https://github.com/microsoft/typescript-go/issues/3559
 compiler/jsExportAssignmentNonMutableLocation.js.diff
 conformance/commonJSImportClassTypeReference.js.diff
 conformance/commonJSImportNestedClassTypeReference.js.diff
 conformance/jsDeclarationsExportDoubleAssignmentInClosure.js.diff
 
-# exportNonInitializedVariables crash in declaration emit
-## https://github.com/microsoft/typescript-go/issues/3560
-conformance/exportNonInitializedVariablesInIfThenStatementNoCrash1(module=commonjs).js.diff
-
 # noImplicitThis functions returning this now fully expand the object type
 ## https://github.com/microsoft/typescript-go/issues/3561
 compiler/noImplicitThisBigThis.js.diff