Don’t enforce export/declare overload modifier consistency across mod…

…ule augmentations (#59416)
microsoft · Jul 29, 2024 · 9405f21 · 9405f21
1 parent 9757109
commit 9405f21
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 22 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -41906,7 +41906,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
     }
 
     function checkFunctionOrConstructorSymbolWorker(symbol: Symbol): void {
-        function getCanonicalOverload(overloads: Declaration[], implementation: FunctionLikeDeclaration | undefined): Declaration {
+        function getCanonicalOverload(overloads: readonly Declaration[], implementation: FunctionLikeDeclaration | undefined): Declaration {
             // Consider the canonical set of flags to be the flags of the bodyDeclaration or the first declaration
             // Error on all deviations from this canonical set of flags
             // The caveat is that if some overloads are defined in lib.d.ts, we don't want to
@@ -41922,19 +41922,35 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
             const someButNotAllOverloadFlags = someOverloadFlags ^ allOverloadFlags;
             if (someButNotAllOverloadFlags !== 0) {
                 const canonicalFlags = getEffectiveDeclarationFlags(getCanonicalOverload(overloads, implementation), flagsToCheck);
-                forEach(overloads, o => {
-                    const deviation = getEffectiveDeclarationFlags(o, flagsToCheck) ^ canonicalFlags;
-                    if (deviation & ModifierFlags.Export) {
-                        error(getNameOfDeclaration(o), Diagnostics.Overload_signatures_must_all_be_exported_or_non_exported);
-                    }
-                    else if (deviation & ModifierFlags.Ambient) {
-                        error(getNameOfDeclaration(o), Diagnostics.Overload_signatures_must_all_be_ambient_or_non_ambient);
-                    }
-                    else if (deviation & (ModifierFlags.Private | ModifierFlags.Protected)) {
-                        error(getNameOfDeclaration(o) || o, Diagnostics.Overload_signatures_must_all_be_public_private_or_protected);
-                    }
-                    else if (deviation & ModifierFlags.Abstract) {
-                        error(getNameOfDeclaration(o), Diagnostics.Overload_signatures_must_all_be_abstract_or_non_abstract);
+                group(overloads, o => getSourceFileOfNode(o).fileName).forEach(overloadsInFile => {
+                    const canonicalFlagsForFile = getEffectiveDeclarationFlags(getCanonicalOverload(overloadsInFile, implementation), flagsToCheck);
+                    for (const o of overloadsInFile) {
+                        const deviation = getEffectiveDeclarationFlags(o, flagsToCheck) ^ canonicalFlags;
+                        const deviationInFile = getEffectiveDeclarationFlags(o, flagsToCheck) ^ canonicalFlagsForFile;
+                        if (deviationInFile & ModifierFlags.Export) {
+                            // Overloads in different files need not all have export modifiers. This is ok:
+                            //   // lib.d.ts
+                            //   declare function foo(s: number): string;
+                            //   declare function foo(s: string): number;
+                            //   export { foo };
+                            //
+                            //   // app.ts
+                            //   declare module "lib" {
+                            //     export function foo(s: boolean): boolean;
+                            //   }
+                            error(getNameOfDeclaration(o), Diagnostics.Overload_signatures_must_all_be_exported_or_non_exported);
+                        }
+                        else if (deviationInFile & ModifierFlags.Ambient) {
+                            // Though rare, a module augmentation (necessarily ambient) is allowed to add overloads
+                            // to a non-ambient function in an implementation file.
+                            error(getNameOfDeclaration(o), Diagnostics.Overload_signatures_must_all_be_ambient_or_non_ambient);
+                        }
+                        else if (deviation & (ModifierFlags.Private | ModifierFlags.Protected)) {
+                            error(getNameOfDeclaration(o) || o, Diagnostics.Overload_signatures_must_all_be_public_private_or_protected);
+                        }
+                        else if (deviation & ModifierFlags.Abstract) {
+                            error(getNameOfDeclaration(o), Diagnostics.Overload_signatures_must_all_be_abstract_or_non_abstract);
+                        }
                     }
                 });
             }

diff --git a/tests/baselines/reference/missingFunctionImplementation2.errors.txt b/tests/baselines/reference/missingFunctionImplementation2.errors.txt
@@ -1,13 +1,10 @@
-missingFunctionImplementation2_a.ts(3,19): error TS2384: Overload signatures must all be ambient or non-ambient.
 missingFunctionImplementation2_b.ts(1,17): error TS2391: Function implementation is missing or not immediately following the declaration.
 
 
-==== missingFunctionImplementation2_a.ts (1 errors) ====
+==== missingFunctionImplementation2_a.ts (0 errors) ====
     export {};
     declare module "./missingFunctionImplementation2_b" {
       export function f(a, b): void;
-                      ~
-!!! error TS2384: Overload signatures must all be ambient or non-ambient.
     }
 
 ==== missingFunctionImplementation2_b.ts (1 errors) ====

diff --git a/tests/baselines/reference/parserStrictMode8.errors.txt b/tests/baselines/reference/parserStrictMode8.errors.txt
@@ -1,12 +1,9 @@
 parserStrictMode8.ts(2,10): error TS1100: Invalid use of 'eval' in strict mode.
-parserStrictMode8.ts(2,10): error TS2384: Overload signatures must all be ambient or non-ambient.
 
 
-==== parserStrictMode8.ts (2 errors) ====
+==== parserStrictMode8.ts (1 errors) ====
     "use strict";
     function eval() {
              ~~~~
 !!! error TS1100: Invalid use of 'eval' in strict mode.
-             ~~~~
-!!! error TS2384: Overload signatures must all be ambient or non-ambient.
     }
diff --git a/tests/cases/compiler/crossFileOverloadModifierConsistency.ts b/tests/cases/compiler/crossFileOverloadModifierConsistency.ts
@@ -0,0 +1,23 @@
+// @strict: true
+// @module: preserve
+// @noEmit: true
+// @noTypesAndSymbols: true
+
+// @Filename: node_modules/library/index.d.ts
+declare function get(): string;
+
+export { get };
+
+// @Filename: node_modules/non-ambient/index.ts
+export function real(arg?: string): any {}
+
+// @Filename: augmentations.d.ts
+export {};
+
+declare module "library" {
+  export function get(): string | null;
+}
+
+declare module "non-ambient" {
+  export function real(arg: string): string;
+}