fix(58166): Class parameter property with initializer before required property emits non-nullable parameter for declaration emit (microsoft#58177)

a-tarasyuk · web-flow · commit 7049af5f4f87 · 2024-08-12T15:42:09.000-07:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -8316,7 +8316,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
          * @param symbol - The symbol is used both to find an existing annotation if declaration is not provided, and to determine if `unique symbol` should be printed
          */
         function serializeTypeForDeclaration(context: NodeBuilderContext, declaration: Declaration | undefined, type: Type, symbol: Symbol) {
-            const addUndefinedForParameter = declaration && (isParameter(declaration) || isJSDocParameterTag(declaration)) && requiresAddingImplicitUndefined(declaration);
+            const addUndefinedForParameter = declaration && (isParameter(declaration) || isJSDocParameterTag(declaration)) && requiresAddingImplicitUndefined(declaration, context.enclosingDeclaration);
             const enclosingDeclaration = context.enclosingDeclaration;
             const restoreFlags = saveRestoreFlags(context);
             if (declaration && hasInferredType(declaration) && !(context.internalFlags & InternalNodeBuilderFlags.NoSyntacticPrinter)) {
@@ -49591,16 +49591,19 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         const type = getTypeFromTypeNode(typeNode);
         return containsUndefinedType(type);
     }
-    function requiresAddingImplicitUndefined(parameter: ParameterDeclaration | JSDocParameterTag) {
-        return (isRequiredInitializedParameter(parameter) || isOptionalUninitializedParameterProperty(parameter)) && !declaredParameterTypeContainsUndefined(parameter);
+
+    function requiresAddingImplicitUndefined(parameter: ParameterDeclaration | JSDocParameterTag, enclosingDeclaration: Node | undefined) {
+        return (isRequiredInitializedParameter(parameter, enclosingDeclaration) || isOptionalUninitializedParameterProperty(parameter)) && !declaredParameterTypeContainsUndefined(parameter);
     }
 
-    function isRequiredInitializedParameter(parameter: ParameterDeclaration | JSDocParameterTag): boolean {
-        return !!strictNullChecks &&
-            !isOptionalParameter(parameter) &&
-            !isJSDocParameterTag(parameter) &&
-            !!parameter.initializer &&
-            !hasSyntacticModifier(parameter, ModifierFlags.ParameterPropertyModifier);
+    function isRequiredInitializedParameter(parameter: ParameterDeclaration | JSDocParameterTag, enclosingDeclaration: Node | undefined): boolean {
+        if (!strictNullChecks || isOptionalParameter(parameter) || isJSDocParameterTag(parameter) || !parameter.initializer) {
+            return false;
+        }
+        if (hasSyntacticModifier(parameter, ModifierFlags.ParameterPropertyModifier)) {
+            return !!enclosingDeclaration && isFunctionLikeDeclaration(enclosingDeclaration);
+        }
+        return true;
     }
 
     function isOptionalUninitializedParameterProperty(parameter: ParameterDeclaration | JSDocParameterTag) {
diff --git a/src/compiler/expressionToTypeNode.ts b/src/compiler/expressionToTypeNode.ts
@@ -178,7 +178,7 @@ export function createSyntacticTypeNodeBuilder(options: CompilerOptions, resolve
             return typeFromAccessor(parent, context);
         }
         const declaredType = getEffectiveTypeAnnotationNode(node);
-        const addUndefined = resolver.requiresAddingImplicitUndefined(node);
+        const addUndefined = resolver.requiresAddingImplicitUndefined(node, context.enclosingDeclaration);
         let resultType;
         if (declaredType) {
             resultType = serializeExistingTypeAnnotation(declaredType, addUndefined);
diff --git a/src/compiler/transformers/declarations.ts b/src/compiler/transformers/declarations.ts
@@ -680,7 +680,7 @@ export function transformDeclarations(context: TransformationContext) {
             // Literal const declarations will have an initializer ensured rather than a type
             return;
         }
-        const shouldAddImplicitUndefined = node.kind === SyntaxKind.Parameter && resolver.requiresAddingImplicitUndefined(node);
+        const shouldAddImplicitUndefined = node.kind === SyntaxKind.Parameter && resolver.requiresAddingImplicitUndefined(node, enclosingDeclaration);
         if (type && !shouldAddImplicitUndefined) {
             return visitNode(type, visitDeclarationSubtree, isTypeNode);
         }
diff --git a/src/compiler/transformers/declarations/diagnostics.ts b/src/compiler/transformers/declarations/diagnostics.ts
@@ -767,7 +767,7 @@ export function createGetIsolatedDeclarationErrors(resolver: EmitResolver) {
         if (isSetAccessor(node.parent)) {
             return createAccessorTypeError(node.parent);
         }
-        const addUndefined = resolver.requiresAddingImplicitUndefined(node);
+        const addUndefined = resolver.requiresAddingImplicitUndefined(node, /*enclosingDeclaration*/ undefined);
         if (!addUndefined && node.initializer) {
             return createExpressionError(node.initializer);
         }
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -5293,7 +5293,7 @@ export interface TypeChecker {
     /** @internal */ getDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken, nodesToCheck?: Node[]): Diagnostic[];
     /** @internal */ getGlobalDiagnostics(): Diagnostic[];
     /** @internal */ getEmitResolver(sourceFile?: SourceFile, cancellationToken?: CancellationToken, forceDts?: boolean): EmitResolver;
-    /** @internal */ requiresAddingImplicitUndefined(parameter: ParameterDeclaration | JSDocParameterTag): boolean;
+    /** @internal */ requiresAddingImplicitUndefined(parameter: ParameterDeclaration | JSDocParameterTag, enclosingDeclaration: Node | undefined): boolean;
 
     /** @internal */ getNodeCount(): number;
     /** @internal */ getIdentifierCount(): number;
@@ -5814,7 +5814,7 @@ export interface EmitResolver {
     collectLinkedAliases(node: ModuleExportName, setVisibility?: boolean): Node[] | undefined;
     markLinkedReferences(node: Node): void;
     isImplementationOfOverload(node: SignatureDeclaration): boolean | undefined;
-    requiresAddingImplicitUndefined(node: ParameterDeclaration): boolean;
+    requiresAddingImplicitUndefined(node: ParameterDeclaration, enclosingDeclaration: Node | undefined): boolean;
     isExpandoFunctionDeclaration(node: FunctionDeclaration | VariableDeclaration): boolean;
     getPropertiesOfContainerFunction(node: Declaration): Symbol[];
     createTypeOfDeclaration(declaration: AccessorDeclaration | VariableLikeDeclaration | PropertyAccessExpression | ElementAccessExpression | BinaryExpression, enclosingDeclaration: Node, flags: NodeBuilderFlags, internalFlags: InternalNodeBuilderFlags, tracker: SymbolTracker): TypeNode | undefined;
@@ -10378,6 +10378,6 @@ export interface SyntacticTypeNodeBuilderResolver {
     isExpandoFunctionDeclaration(name: FunctionDeclaration | VariableDeclaration): boolean;
     getAllAccessorDeclarations(declaration: AccessorDeclaration): AllAccessorDeclarations;
     isEntityNameVisible(entityName: EntityNameOrEntityNameExpression, enclosingDeclaration: Node, shouldComputeAliasToMakeVisible?: boolean): SymbolVisibilityResult;
-    requiresAddingImplicitUndefined(parameter: ParameterDeclaration | JSDocParameterTag): boolean;
+    requiresAddingImplicitUndefined(parameter: ParameterDeclaration | JSDocParameterTag, enclosingDeclaration: Node | undefined): boolean;
     isDefinitelyReferenceToGlobalSymbolObject(node: Node): boolean;
 }
diff --git a/src/services/codefixes/fixMissingTypeAnnotationOnExports.ts b/src/services/codefixes/fixMissingTypeAnnotationOnExports.ts
@@ -914,11 +914,12 @@ function withContext<T>(
             type = widenedType;
         }
 
-        if (isParameter(node) && typeChecker.requiresAddingImplicitUndefined(node)) {
+        const enclosingDeclaration = findAncestor(node, isDeclaration) ?? sourceFile;
+        if (isParameter(node) && typeChecker.requiresAddingImplicitUndefined(node, enclosingDeclaration)) {
             type = typeChecker.getUnionType([typeChecker.getUndefinedType(), type], UnionReduction.None);
         }
         return {
-            typeNode: typeToTypeNode(type, findAncestor(node, isDeclaration) ?? sourceFile, getFlags(type)),
+            typeNode: typeToTypeNode(type, enclosingDeclaration, getFlags(type)),
             mutatedTarget: false,
         };
 
diff --git a/tests/baselines/reference/parameterPropertyInConstructor4.js b/tests/baselines/reference/parameterPropertyInConstructor4.js
@@ -0,0 +1,28 @@
+//// [tests/cases/compiler/parameterPropertyInConstructor4.ts] ////
+
+//// [parameterPropertyInConstructor4.ts]
+export class C {
+    constructor(public a: number[] = [], b: number) {
+    }
+}
+
+
+//// [parameterPropertyInConstructor4.js]
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+exports.C = void 0;
+var C = /** @class */ (function () {
+    function C(a, b) {
+        if (a === void 0) { a = []; }
+        this.a = a;
+    }
+    return C;
+}());
+exports.C = C;
+
+
+//// [parameterPropertyInConstructor4.d.ts]
+export declare class C {
+    a: number[];
+    constructor(a: number[] | undefined, b: number);
+}
diff --git a/tests/baselines/reference/parameterPropertyInConstructor4.symbols b/tests/baselines/reference/parameterPropertyInConstructor4.symbols
@@ -0,0 +1,12 @@
+//// [tests/cases/compiler/parameterPropertyInConstructor4.ts] ////
+
+=== parameterPropertyInConstructor4.ts ===
+export class C {
+>C : Symbol(C, Decl(parameterPropertyInConstructor4.ts, 0, 0))
+
+    constructor(public a: number[] = [], b: number) {
+>a : Symbol(C.a, Decl(parameterPropertyInConstructor4.ts, 1, 16))
+>b : Symbol(b, Decl(parameterPropertyInConstructor4.ts, 1, 40))
+    }
+}
+
diff --git a/tests/baselines/reference/parameterPropertyInConstructor4.types b/tests/baselines/reference/parameterPropertyInConstructor4.types
@@ -0,0 +1,17 @@
+//// [tests/cases/compiler/parameterPropertyInConstructor4.ts] ////
+
+=== parameterPropertyInConstructor4.ts ===
+export class C {
+>C : C
+>  : ^
+
+    constructor(public a: number[] = [], b: number) {
+>a : number[]
+>  : ^^^^^^^^
+>[] : never[]
+>   : ^^^^^^^
+>b : number
+>  : ^^^^^^
+    }
+}
+
diff --git a/tests/cases/compiler/parameterPropertyInConstructor4.ts b/tests/cases/compiler/parameterPropertyInConstructor4.ts
@@ -0,0 +1,7 @@
+// @declaration: true
+// @strict: true
+
+export class C {
+    constructor(public a: number[] = [], b: number) {
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -178,7 +178,7 @@ export function createSyntacticTypeNodeBuilder(options: CompilerOptions, resolve`
`178`	`178`	`return typeFromAccessor(parent, context);`
`179`	`179`	`}`
`180`	`180`	`const declaredType = getEffectiveTypeAnnotationNode(node);`
`181`		`- const addUndefined = resolver.requiresAddingImplicitUndefined(node);`
	`181`	`+ const addUndefined = resolver.requiresAddingImplicitUndefined(node, context.enclosingDeclaration);`
`182`	`182`	`let resultType;`
`183`	`183`	`if (declaredType) {`
`184`	`184`	`resultType = serializeExistingTypeAnnotation(declaredType, addUndefined);`
Original file line number	Diff line number	Diff line change
`@@ -680,7 +680,7 @@ export function transformDeclarations(context: TransformationContext) {`
`680`	`680`	`// Literal const declarations will have an initializer ensured rather than a type`
`681`	`681`	`return;`
`682`	`682`	`}`
`683`		`- const shouldAddImplicitUndefined = node.kind === SyntaxKind.Parameter && resolver.requiresAddingImplicitUndefined(node);`
	`683`	`+ const shouldAddImplicitUndefined = node.kind === SyntaxKind.Parameter && resolver.requiresAddingImplicitUndefined(node, enclosingDeclaration);`
`684`	`684`	`if (type && !shouldAddImplicitUndefined) {`
`685`	`685`	`return visitNode(type, visitDeclarationSubtree, isTypeNode);`
`686`	`686`	`}`
Original file line number	Diff line number	Diff line change
`@@ -767,7 +767,7 @@ export function createGetIsolatedDeclarationErrors(resolver: EmitResolver) {`
`767`	`767`	`if (isSetAccessor(node.parent)) {`
`768`	`768`	`return createAccessorTypeError(node.parent);`
`769`	`769`	`}`
`770`		`- const addUndefined = resolver.requiresAddingImplicitUndefined(node);`
	`770`	`+ const addUndefined = resolver.requiresAddingImplicitUndefined(node, /enclosingDeclaration/ undefined);`
`771`	`771`	`if (!addUndefined && node.initializer) {`
`772`	`772`	`return createExpressionError(node.initializer);`
`773`	`773`	`}`
Original file line number	Diff line number	Diff line change
`@@ -914,11 +914,12 @@ function withContext<T>(`
`914`	`914`	`type = widenedType;`
`915`	`915`	`}`
`916`	`916`
`917`		`- if (isParameter(node) && typeChecker.requiresAddingImplicitUndefined(node)) {`
	`917`	`+ const enclosingDeclaration = findAncestor(node, isDeclaration) ?? sourceFile;`
	`918`	`+ if (isParameter(node) && typeChecker.requiresAddingImplicitUndefined(node, enclosingDeclaration)) {`
`918`	`919`	`type = typeChecker.getUnionType([typeChecker.getUndefinedType(), type], UnionReduction.None);`
`919`	`920`	`}`
`920`	`921`	`return {`
`921`		`- typeNode: typeToTypeNode(type, findAncestor(node, isDeclaration) ?? sourceFile, getFlags(type)),`
	`922`	`+ typeNode: typeToTypeNode(type, enclosingDeclaration, getFlags(type)),`
`922`	`923`	`mutatedTarget: false,`
`923`	`924`	`};`
`924`	`925`