Check @overload tag against its implementation

Also, make @overload work like other jsdoc tags: only the last jsdoc tag before a declaration is used. That means all overload tags need to be in one tag: ```js /** * @overload * @return {number} * * @overload * @return {string} */ ``` function f() { return "1" } This no longer works: ```js /** * @overload * @return {number} */ /** * @overload * @return {string} */ function f() { return "2" } ``` Fixes microsoft#51234
sandersn · Jan 28, 2023 · e796e7b · e796e7b
1 parent 55ae4a3
commit e796e7b
Show file tree

Hide file tree

Showing 13 changed files with 283 additions and 93 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -230,6 +230,7 @@ import {
     getAliasDeclarationFromName,
     getAllAccessorDeclarations,
     getAllJSDocTags,
+    getAllJSDocTagsOfKind,
     getAllowSyntheticDefaultImports,
     getAncestor,
     getAssignedExpandoInitializer,
@@ -279,6 +280,7 @@ import {
     getExternalModuleRequireArgument,
     getFirstConstructorWithBody,
     getFirstIdentifier,
+    getFirstJSDocTag,
     getFunctionFlags,
     getHostSignatureFromJSDoc,
     getIdentifierGeneratedImportReference,
@@ -745,6 +747,7 @@ import {
     JSDocMemberName,
     JSDocNullableType,
     JSDocOptionalType,
+    JSDocOverloadTag,
     JSDocParameterTag,
     JSDocPrivateTag,
     JSDocPropertyLikeTag,
@@ -14442,15 +14445,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
             }
             if (isInJSFile(decl) && decl.jsDoc) {
                 let hasJSDocOverloads = false;
-                for (const node of decl.jsDoc) {
-                    if (node.tags) {
-                        for (const tag of node.tags) {
-                            if (isJSDocOverloadTag(tag)) {
-                                result.push(getSignatureFromDeclaration(tag.typeExpression));
-                                hasJSDocOverloads = true;
-                            }
-                        }
-                    }
+                for (const tag of getAllJSDocTagsOfKind(decl, SyntaxKind.JSDocOverloadTag)) {
+                    result.push(getSignatureFromDeclaration((tag as JSDocOverloadTag).typeExpression));
+                    hasJSDocOverloads = true;
                 }
                 if (hasJSDocOverloads) {
                     continue;
@@ -38584,20 +38581,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                     }
                 }
                 if (isInJSFile(current) && isFunctionLike(current) && current.jsDoc) {
-                    // TODO: De-duplicate tag check and improve error span
-                    outer: for (const node of current.jsDoc) {
-                        if (node.tags) {
-                            for (const tag of node.tags) {
-                                if (isJSDocOverloadTag(tag)) {
-                                    hasOverloads = true;
-                                    break outer;
-                                }
-                            }
-                        }
+                    if (getFirstJSDocTag(current, isJSDocOverloadTag)) {
+                        hasOverloads = true;
                     }
                 }
             }
         }
+
         if (multipleConstructorImplementation) {
             forEach(functionDeclarations, declaration => {
                 error(declaration, Diagnostics.Multiple_constructor_implementations_are_not_allowed);
@@ -38646,8 +38636,11 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                 const bodySignature = getSignatureFromDeclaration(bodyDeclaration);
                 for (const signature of signatures) {
                     if (!isImplementationCompatibleWithOverload(bodySignature, signature)) {
+                        const errorNode = signature.declaration && isJSDocSignature(signature.declaration)
+                            ? (signature.declaration.parent as JSDocOverloadTag | JSDocCallbackTag).tagName
+                            : signature.declaration;
                         addRelatedInfo(
-                            error(signature.declaration, Diagnostics.This_overload_signature_is_not_compatible_with_its_implementation_signature),
+                            error(errorNode, Diagnostics.This_overload_signature_is_not_compatible_with_its_implementation_signature),
                             createDiagnosticForNode(bodyDeclaration, Diagnostics.The_implementation_signature_is_declared_here)
                         );
                         break;

diff --git a/src/compiler/utilitiesPublic.ts b/src/compiler/utilitiesPublic.ts
@@ -1205,8 +1205,10 @@ export function getJSDocTagsNoCache(node: Node): readonly JSDocTag[] {
     return getJSDocTagsWorker(node, /*noCache*/ true);
 }
 
-/** Get the first JSDoc tag of a specified kind, or undefined if not present. */
-function getFirstJSDocTag<T extends JSDocTag>(node: Node, predicate: (tag: JSDocTag) => tag is T, noCache?: boolean): T | undefined {
+/** @internal
+ * Get the first JSDoc tag of a specified kind, or undefined if not present.
+ */
+export function getFirstJSDocTag<T extends JSDocTag>(node: Node, predicate: (tag: JSDocTag) => tag is T, noCache?: boolean): T | undefined {
     return find(getJSDocTagsWorker(node, noCache), predicate);
 }
 

diff --git a/tests/baselines/reference/jsFileAlternativeUseOfOverloadTag.js b/tests/baselines/reference/jsFileAlternativeUseOfOverloadTag.js
@@ -105,14 +105,15 @@ var example3 = {
 
 //// [jsFileAlternativeUseOfOverloadTag.d.ts]
 declare namespace example1 {
-    function constructor(value: any, options: any): void;
+    function constructor(value: any): any;
 }
 declare namespace example2 {
-    export function constructor_1(): void;
+    export function constructor_1(value: any, secretAccessKey: any, sessionToken: any): any;
+    export function constructor_1(): any;
     export { constructor_1 as constructor };
 }
 declare namespace example3 {
-    function evaluate(options: any, callback: any): void;
+    function evaluate(): any;
 }
 /**
  * function (error, result)

diff --git a/tests/baselines/reference/jsFileAlternativeUseOfOverloadTag.types b/tests/baselines/reference/jsFileAlternativeUseOfOverloadTag.types
@@ -4,26 +4,26 @@
 // trying to make sure that our changes do not result in any crashes here.
 
 const example1 = {
->example1 : { constructor: (value: any, options: any) => void; }
->{  /**   * @overload Example1(value)   *   Creates Example1   *   @param value [String]   */  constructor: function Example1(value, options) {},} : { constructor: (value: any, options: any) => void; }
+>example1 : { constructor: (value: any) => any; }
+>{  /**   * @overload Example1(value)   *   Creates Example1   *   @param value [String]   */  constructor: function Example1(value, options) {},} : { constructor: (value: any) => any; }
 
   /**
    * @overload Example1(value)
    *   Creates Example1
    *   @param value [String]
    */
   constructor: function Example1(value, options) {},
->constructor : (value: any, options: any) => void
->function Example1(value, options) {} : (value: any, options: any) => void
->Example1 : (value: any, options: any) => void
+>constructor : (value: any) => any
+>function Example1(value, options) {} : (value: any) => any
+>Example1 : (value: any) => any
 >value : any
 >options : any
 
 };
 
 const example2 = {
->example2 : { constructor: () => void; }
->{  /**   * Example 2   *   * @overload Example2(value)   *   Creates Example2   *   @param value [String]   *   @param secretAccessKey [String]   *   @param sessionToken [String]   *   @example Creates with string value   *     const example = new Example('');   * @overload Example2(options)   *   Creates Example2   *   @option options value [String]   *   @example Creates with options object   *     const example = new Example2({   *       value: '',   *     });   */  constructor: function Example2() {},} : { constructor: () => void; }
+>example2 : { constructor: { (value: any, secretAccessKey: any, sessionToken: any): any; (): any; }; }
+>{  /**   * Example 2   *   * @overload Example2(value)   *   Creates Example2   *   @param value [String]   *   @param secretAccessKey [String]   *   @param sessionToken [String]   *   @example Creates with string value   *     const example = new Example('');   * @overload Example2(options)   *   Creates Example2   *   @option options value [String]   *   @example Creates with options object   *     const example = new Example2({   *       value: '',   *     });   */  constructor: function Example2() {},} : { constructor: { (value: any, secretAccessKey: any, sessionToken: any): any; (): any; }; }
 
   /**
    * Example 2
@@ -44,15 +44,15 @@ const example2 = {
    *     });
    */
   constructor: function Example2() {},
->constructor : () => void
->function Example2() {} : () => void
->Example2 : () => void
+>constructor : { (value: any, secretAccessKey: any, sessionToken: any): any; (): any; }
+>function Example2() {} : { (value: any, secretAccessKey: any, sessionToken: any): any; (): any; }
+>Example2 : { (value: any, secretAccessKey: any, sessionToken: any): any; (): any; }
 
 };
 
 const example3 = {
->example3 : { evaluate: (options: any, callback: any) => void; }
->{  /**   * @overload evaluate(options = {}, [callback])   *   Evaluate something   *   @note Something interesting   *   @param options [map]   *   @return [string] returns evaluation result   *   @return [null] returns nothing if callback provided   *   @callback callback function (error, result)   *     If callback is provided it will be called with evaluation result   *     @param error [Error]   *     @param result [String]   *   @see callback   */  evaluate: function evaluate(options, callback) {},} : { evaluate: (options: any, callback: any) => void; }
+>example3 : { evaluate: () => any; }
+>{  /**   * @overload evaluate(options = {}, [callback])   *   Evaluate something   *   @note Something interesting   *   @param options [map]   *   @return [string] returns evaluation result   *   @return [null] returns nothing if callback provided   *   @callback callback function (error, result)   *     If callback is provided it will be called with evaluation result   *     @param error [Error]   *     @param result [String]   *   @see callback   */  evaluate: function evaluate(options, callback) {},} : { evaluate: () => any; }
 
   /**
    * @overload evaluate(options = {}, [callback])
@@ -68,9 +68,9 @@ const example3 = {
    *   @see callback
    */
   evaluate: function evaluate(options, callback) {},
->evaluate : (options: any, callback: any) => void
->function evaluate(options, callback) {} : (options: any, callback: any) => void
->evaluate : (options: any, callback: any) => void
+>evaluate : () => any
+>function evaluate(options, callback) {} : () => any
+>evaluate : () => any
 >options : any
 >callback : any
 

diff --git a/tests/baselines/reference/jsFileFunctionOverloads.js b/tests/baselines/reference/jsFileFunctionOverloads.js
@@ -118,11 +118,46 @@ function flatMap(array, iterable) {
 
 
 //// [jsFileFunctionOverloads.d.ts]
-declare function getTypeName(x: number): 'number';
-declare function getTypeName(x: string): 'string';
-declare function getTypeName(x: boolean): 'boolean';
-declare function flatMap<T, U>(array: T[], iterable: (x: T) => U[]): U[];
-declare function flatMap<T>(array: T[][]): T[];
+/**
+ * @overload
+ * @param {number} x
+ * @returns {'number'}
+ */
+/**
+ * @overload
+ * @param {string} x
+ * @returns {'string'}
+ */
+/**
+ * @overload
+ * @param {boolean} x
+ * @returns {'boolean'}
+ */
+/**
+ * @param {unknown} x
+ * @returns {string}
+ */
+declare function getTypeName(x: unknown): string;
+/**
+ * @template T
+ * @template U
+ * @overload
+ * @param {T[]} array
+ * @param {(x: T) => U[]} iterable
+ * @returns {U[]}
+ */
+/**
+ * @template T
+ * @overload
+ * @param {T[][]} array
+ * @returns {T[]}
+ */
+/**
+ * @param {unknown[]} array
+ * @param {(x: unknown) => unknown} iterable
+ * @returns {unknown[]}
+ */
+declare function flatMap(array: unknown[], iterable?: (x: unknown) => unknown): unknown[];
 /**
  * @template T
  * @param {T} x

diff --git a/tests/baselines/reference/jsFileFunctionOverloads.types b/tests/baselines/reference/jsFileFunctionOverloads.types
@@ -19,7 +19,7 @@
  * @returns {string}
  */
  function getTypeName(x) {
->getTypeName : { (x: number): 'number'; (x: string): 'string'; (x: boolean): 'boolean'; }
+>getTypeName : (x: unknown) => string
 >x : unknown
 
   return typeof x;
@@ -58,7 +58,7 @@ const identity = x => x;
  * @returns {unknown[]}
  */
 function flatMap(array, iterable = identity) {
->flatMap : { <T, U>(array: T[], iterable: (x: T) => U[]): U[]; <T>(array: T[][]): T[]; }
+>flatMap : (array: unknown[], iterable?: (x: unknown) => unknown) => unknown[]
 >array : unknown[]
 >iterable : (x: unknown) => unknown
 >identity : <T>(x: T) => T

diff --git a/tests/baselines/reference/jsFileMethodOverloads.js b/tests/baselines/reference/jsFileMethodOverloads.js
@@ -105,8 +105,33 @@ declare class Example<T> {
      */
     constructor(value: T);
     value: T;
-    getTypeName(this: Example<number>): 'number';
-    getTypeName(this: Example<string>): 'string';
-    transform<U>(fn: (y: T) => U): U;
-    transform(): T;
+    /**
+     * @overload
+     * @param {Example<number>} this
+     * @returns {'number'}
+     */
+    /**
+     * @overload
+     * @param {Example<string>} this
+     * @returns {'string'}
+     */
+    /**
+     * @returns {string}
+     */
+    getTypeName(): string;
+    /**
+     * @template U
+     * @overload
+     * @param {(y: T) => U} fn
+     * @returns {U}
+     */
+    /**
+     * @overload
+     * @returns {T}
+     */
+    /**
+     * @param {(y: T) => unknown} [fn]
+     * @returns {unknown}
+     */
+    transform(fn?: (y: T) => unknown): unknown;
 }
diff --git a/tests/baselines/reference/jsFileMethodOverloads.types b/tests/baselines/reference/jsFileMethodOverloads.types
@@ -33,7 +33,7 @@
    * @returns {string}
    */
   getTypeName() {
->getTypeName : { (this: Example<number>): 'number'; (this: Example<string>): 'string'; }
+>getTypeName : () => string
 
     return typeof this.value;
 >typeof this.value : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
@@ -57,7 +57,7 @@
    * @returns {unknown}
    */
   transform(fn) {
->transform : { <U>(fn: (y: T) => U): U; (): T; }
+>transform : (fn?: (y: T) => unknown) => unknown
 >fn : (y: T) => unknown
 
     return fn ? fn(this.value) : this.value;

diff --git a/tests/baselines/reference/overloadTag1.errors.txt b/tests/baselines/reference/overloadTag1.errors.txt
@@ -1,24 +1,31 @@
-tests/cases/conformance/jsdoc/overloadTag1.js(27,1): error TS2769: No overload matches this call.
+tests/cases/conformance/jsdoc/overloadTag1.js(7,5): error TS2394: This overload signature is not compatible with its implementation signature.
+tests/cases/conformance/jsdoc/overloadTag1.js(25,1): error TS2769: No overload matches this call.
+  Overload 1 of 2, '(a: number, b: number): number', gave the following error.
+    Argument of type 'string' is not assignable to parameter of type 'number'.
+  Overload 2 of 2, '(a: string, b: boolean): string', gave the following error.
+    Argument of type 'string' is not assignable to parameter of type 'boolean'.
+tests/cases/conformance/jsdoc/overloadTag1.js(42,1): error TS2769: No overload matches this call.
   Overload 1 of 2, '(a: number, b: number): number', gave the following error.
     Argument of type 'string' is not assignable to parameter of type 'number'.
   Overload 2 of 2, '(a: string, b: boolean): string', gave the following error.
     Argument of type 'string' is not assignable to parameter of type 'boolean'.
 
 
-==== tests/cases/conformance/jsdoc/overloadTag1.js (1 errors) ====
+==== tests/cases/conformance/jsdoc/overloadTag1.js (3 errors) ====
     /**
      * @overload
      * @param {number} a 
      * @param {number} b
      * @returns {number} 
-     */
-    /**
+     *
      * @overload
+        ~~~~~~~~
+!!! error TS2394: This overload signature is not compatible with its implementation signature.
+!!! related TS2750 tests/cases/conformance/jsdoc/overloadTag1.js:16:17: The implementation signature is declared here.
      * @param {string} a
      * @param {boolean} b
      * @returns {string}
-     */
-    /**
+     *
      * @param {string | number} a
      * @param {string | number} b
      * @returns {string | number}
@@ -38,4 +45,28 @@ tests/cases/conformance/jsdoc/overloadTag1.js(27,1): error TS2769: No overload m
 !!! error TS2769:   Overload 1 of 2, '(a: number, b: number): number', gave the following error.
 !!! error TS2769:     Argument of type 'string' is not assignable to parameter of type 'number'.
 !!! error TS2769:   Overload 2 of 2, '(a: string, b: boolean): string', gave the following error.
-!!! error TS2769:     Argument of type 'string' is not assignable to parameter of type 'boolean'.
+!!! error TS2769:     Argument of type 'string' is not assignable to parameter of type 'boolean'.
+
+    /**
+     * @overload
+     * @param {number} a
+     * @param {number} b
+     * @returns {number}
+     *
+     * @overload
+     * @param {string} a
+     * @param {boolean} b
+     * @returns {string}
+     */
+    export function uncheckedInternally(a, b) {
+        return a + b;
+    }
+    uncheckedInternally(1,2)
+    uncheckedInternally("zero", "one")
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+!!! error TS2769: No overload matches this call.
+!!! error TS2769:   Overload 1 of 2, '(a: number, b: number): number', gave the following error.
+!!! error TS2769:     Argument of type 'string' is not assignable to parameter of type 'number'.
+!!! error TS2769:   Overload 2 of 2, '(a: string, b: boolean): string', gave the following error.
+!!! error TS2769:     Argument of type 'string' is not assignable to parameter of type 'boolean'.
+