From 520b47132af8e21868df5dc4dfdf5e003a38d158 Mon Sep 17 00:00:00 2001 From: Russell Bicknell Date: Mon, 6 Mar 2023 10:36:25 -0800 Subject: [PATCH] [labs/analyzer] Analyze overloaded functions and methods. (#3702) * Add tests for overloaded function exports and methods. * Ignore non-implementation signatures of overloaded functions. * Make the string concatenation in the overloaded example function more obvious. * Add different JSDoc comment combinations for overloaded functions. * Add tests for overloaded function JSDoc comments. * Read JSDoc tags from another declaration if there are none on a function's implementation node. * Extract `getDocNodeForFunctionLikeDeclaration`. * Add missing JSDoc tags to JS properties test fixture. * Add support for retrieving JSDoc descriptions from method overload declarations. * Add a test for JSDoc descriptions on multiple overload signatures. * Add a changeset. * Add `FunctionOverloadDeclaration`. * Don't use documentation from one signature for another. * Collect function overloads. * Fix tests for overloaded non-method functions. * Fix overloaded methods. * Add a comment about checking for `.body` on method declarations. * Update the changeset. --- .changeset/early-berries-live.md | 8 ++ .../analyzer/src/lib/javascript/classes.ts | 9 ++- .../analyzer/src/lib/javascript/functions.ts | 35 +++++++- .../analyzer/src/lib/javascript/modules.ts | 4 +- packages/labs/analyzer/src/lib/model.ts | 14 ++++ .../src/test/javascript/functions_test.ts | 79 +++++++++++++++++++ .../src/test/lit-element/properties_test.ts | 77 ++++++++++++++++++ .../test-files/js/functions/functions.js | 13 +++ .../test-files/js/properties/element-a.js | 13 +++ .../test-files/ts/functions/src/functions.ts | 25 ++++++ .../test-files/ts/properties/src/element-a.ts | 25 ++++++ 11 files changed, 295 insertions(+), 7 deletions(-) create mode 100644 .changeset/early-berries-live.md diff --git a/.changeset/early-berries-live.md b/.changeset/early-berries-live.md new file mode 100644 index 0000000000..1a29288911 --- /dev/null +++ b/.changeset/early-berries-live.md @@ -0,0 +1,8 @@ +--- +'@lit-labs/analyzer': minor +--- + +Adds support for overloaded functions. Methods of model objects that accept a +string key will now specifically return the `FunctionDeclaration` of the +implementation signature of an overloaded function, which has a new `overloads` +field containing a `FunctionOverloadDeclaration` for each overload signature. diff --git a/packages/labs/analyzer/src/lib/javascript/classes.ts b/packages/labs/analyzer/src/lib/javascript/classes.ts index deb4dac9ee..a01d92a380 100644 --- a/packages/labs/analyzer/src/lib/javascript/classes.ts +++ b/packages/labs/analyzer/src/lib/javascript/classes.ts @@ -81,13 +81,16 @@ export const getClassMembers = ( const methodMap = new Map(); const staticMethodMap = new Map(); declaration.members.forEach((node) => { - if (ts.isMethodDeclaration(node)) { + // Ignore non-implementation signatures of overloaded methods by checking + // for `node.body`. + if (ts.isMethodDeclaration(node) && node.body) { const info = getMemberInfo(node); + const name = node.name.getText(); (info.static ? staticMethodMap : methodMap).set( - node.name.getText(), + name, new ClassMethod({ ...info, - ...getFunctionLikeInfo(node, analyzer), + ...getFunctionLikeInfo(node, name, analyzer), ...parseNodeJSDocInfo(node), }) ); diff --git a/packages/labs/analyzer/src/lib/javascript/functions.ts b/packages/labs/analyzer/src/lib/javascript/functions.ts index c70d19ae1c..5638439255 100644 --- a/packages/labs/analyzer/src/lib/javascript/functions.ts +++ b/packages/labs/analyzer/src/lib/javascript/functions.ts @@ -16,6 +16,8 @@ import { AnalyzerInterface, DeclarationInfo, FunctionDeclaration, + FunctionLikeInit, + FunctionOverloadDeclaration, Parameter, Return, } from '../model.js'; @@ -68,9 +70,8 @@ export const getFunctionDeclaration = ( docNode?: ts.Node ): FunctionDeclaration => { return new FunctionDeclaration({ - name, ...parseNodeJSDocInfo(docNode ?? declaration), - ...getFunctionLikeInfo(declaration, analyzer), + ...getFunctionLikeInfo(declaration, name, analyzer), }); }; @@ -79,11 +80,39 @@ export const getFunctionDeclaration = ( */ export const getFunctionLikeInfo = ( node: ts.FunctionLikeDeclaration, + name: string, analyzer: AnalyzerInterface -) => { +): FunctionLikeInit => { + let overloads = undefined; + if (node.body) { + // Overloaded functions have multiple declaration nodes. + const type = analyzer.program.getTypeChecker().getTypeAtLocation(node); + const overloadDeclarations = type + .getSymbol() + ?.getDeclarations() + ?.filter((x) => x !== node) as Array; + + overloads = overloadDeclarations?.map((overload) => { + const info = getFunctionLikeInfo(overload, name, analyzer); + return new FunctionOverloadDeclaration({ + // `docNode ?? overload` isn't needed here because TS doesn't allow + // const function assignments to be overloaded as of now. + ...parseNodeJSDocInfo(overload), + + // `info` can't be spread because `FunctionLikeInit` has an `overloads` + // property, even though it's always `undefined` in this case. + name: info.name, + parameters: info.parameters, + return: info.return, + }); + }); + } + return { + name, parameters: node.parameters.map((p) => getParameter(p, analyzer)), return: getReturn(node, analyzer), + overloads, }; }; diff --git a/packages/labs/analyzer/src/lib/javascript/modules.ts b/packages/labs/analyzer/src/lib/javascript/modules.ts index c8b56fc410..57e18cea86 100644 --- a/packages/labs/analyzer/src/lib/javascript/modules.ts +++ b/packages/labs/analyzer/src/lib/javascript/modules.ts @@ -117,7 +117,9 @@ export const getModule = ( for (const statement of sourceFile.statements) { if (ts.isClassDeclaration(statement)) { addDeclaration(getClassDeclarationInfo(statement, analyzer)); - } else if (ts.isFunctionDeclaration(statement)) { + // Ignore non-implementation signatures of overloaded functions by + // checking for `statement.body`. + } else if (ts.isFunctionDeclaration(statement) && statement.body) { addDeclaration(getFunctionDeclarationInfo(statement, analyzer)); } else if (ts.isVariableStatement(statement)) { getVariableDeclarationInfo(statement, analyzer).forEach(addDeclaration); diff --git a/packages/labs/analyzer/src/lib/model.ts b/packages/labs/analyzer/src/lib/model.ts index eaa76ea218..cd62477c1d 100644 --- a/packages/labs/analyzer/src/lib/model.ts +++ b/packages/labs/analyzer/src/lib/model.ts @@ -330,15 +330,29 @@ export interface FunctionLikeInit extends DeprecatableDescribed { name: string; parameters?: Parameter[] | undefined; return?: Return | undefined; + overloads?: FunctionOverloadDeclaration[] | undefined; } export class FunctionDeclaration extends Declaration { parameters?: Parameter[] | undefined; return?: Return | undefined; + overloads?: FunctionOverloadDeclaration[] | undefined; constructor(init: FunctionLikeInit) { super(init); this.parameters = init.parameters; this.return = init.return; + this.overloads = init.overloads; + } +} + +export interface FunctionLikeOverloadInit extends FunctionLikeInit { + overloads?: undefined; +} + +export class FunctionOverloadDeclaration extends FunctionDeclaration { + override overloads: undefined; + constructor(init: FunctionLikeOverloadInit) { + super(init); } } diff --git a/packages/labs/analyzer/src/test/javascript/functions_test.ts b/packages/labs/analyzer/src/test/javascript/functions_test.ts index da4aacdd22..4382ec279b 100644 --- a/packages/labs/analyzer/src/test/javascript/functions_test.ts +++ b/packages/labs/analyzer/src/test/javascript/functions_test.ts @@ -179,5 +179,84 @@ for (const lang of languages) { assert.equal(fn.deprecated, 'Async function deprecated'); }); + test('Overloaded function', ({module}) => { + const exportedFn = module.getResolvedExport('overloaded'); + const fn = module.getDeclaration('overloaded'); + assert.equal(fn, exportedFn); + assert.ok(fn?.isFunctionDeclaration()); + assert.equal(fn.name, 'overloaded'); + assert.equal( + fn.description, + 'This signature works with strings or numbers.' + ); + assert.equal(fn.summary, undefined); + assert.equal(fn.parameters?.length, 1); + assert.equal(fn.parameters?.[0].name, 'x'); + assert.equal( + fn.parameters?.[0].description, + 'Accepts either a string or a number.' + ); + assert.equal(fn.parameters?.[0].summary, undefined); + assert.equal(fn.parameters?.[0].type?.text, 'string | number'); + assert.equal(fn.parameters?.[0].default, undefined); + assert.equal(fn.parameters?.[0].rest, false); + assert.equal(fn.return?.type?.text, 'string | number'); + assert.equal( + fn.return?.description, + 'Returns either a string or a number.' + ); + assert.equal(fn.deprecated, undefined); + + // TODO: Run the same assertions in both languages once TS supports + // `@overload` for JSDoc in JS. + // + if (lang === 'ts') { + assert.ok(fn.overloads); + assert.equal(fn.overloads.length, 2); + + assert.equal(fn.overloads[0].name, 'overloaded'); + assert.equal( + fn.overloads[0].description, + 'This signature only works with strings.' + ); + assert.equal(fn.overloads[0].summary, undefined); + assert.equal(fn.overloads[0].parameters?.length, 1); + assert.equal(fn.overloads[0].parameters?.[0].name, 'x'); + assert.equal( + fn.overloads[0].parameters?.[0].description, + 'Accepts a string.' + ); + assert.equal(fn.overloads[0].parameters?.[0].summary, undefined); + assert.equal(fn.overloads[0].parameters?.[0].type?.text, 'string'); + assert.equal(fn.overloads[0].parameters?.[0].default, undefined); + assert.equal(fn.overloads[0].parameters?.[0].rest, false); + assert.equal(fn.overloads[0].return?.type?.text, 'string'); + assert.equal(fn.overloads[0].return?.description, 'Returns a string.'); + assert.equal(fn.overloads[0].deprecated, undefined); + + assert.equal(fn.overloads[1].name, 'overloaded'); + assert.equal( + fn.overloads[1].description, + 'This signature only works with numbers.' + ); + assert.equal(fn.overloads[1].summary, undefined); + assert.equal(fn.overloads[1].parameters?.length, 1); + assert.equal(fn.overloads[1].parameters?.[0].name, 'x'); + assert.equal( + fn.overloads[1].parameters?.[0].description, + 'Accepts a number.' + ); + assert.equal(fn.overloads[1].parameters?.[0].summary, undefined); + assert.equal(fn.overloads[1].parameters?.[0].type?.text, 'number'); + assert.equal(fn.overloads[1].parameters?.[0].default, undefined); + assert.equal(fn.overloads[1].parameters?.[0].rest, false); + assert.equal(fn.overloads[1].return?.type?.text, 'number'); + assert.equal(fn.overloads[1].return?.description, 'Returns a number.'); + assert.equal(fn.overloads[1].deprecated, undefined); + } else { + assert.equal(fn.overloads?.length ?? 0, 0); + } + }); + test.run(); } diff --git a/packages/labs/analyzer/src/test/lit-element/properties_test.ts b/packages/labs/analyzer/src/test/lit-element/properties_test.ts index 27a370f0cc..9bddd12e39 100644 --- a/packages/labs/analyzer/src/test/lit-element/properties_test.ts +++ b/packages/labs/analyzer/src/test/lit-element/properties_test.ts @@ -191,5 +191,82 @@ for (const lang of languages) { assert.equal(property.attribute, 'static-prop'); }); + test('method with an overloaded signature', ({element}) => { + const fn = Array.from(element.methods).find((m) => m.name === 'overloaded'); + assert.ok(fn?.isFunctionDeclaration()); + assert.equal(fn.name, 'overloaded'); + assert.equal( + fn.description, + 'This signature works with strings or numbers.' + ); + assert.equal(fn.summary, undefined); + assert.equal(fn.parameters?.length, 1); + assert.equal(fn.parameters?.[0].name, 'x'); + assert.equal( + fn.parameters?.[0].description, + 'Accepts either a string or a number.' + ); + assert.equal(fn.parameters?.[0].summary, undefined); + assert.equal(fn.parameters?.[0].type?.text, 'string | number'); + assert.equal(fn.parameters?.[0].default, undefined); + assert.equal(fn.parameters?.[0].rest, false); + assert.equal(fn.return?.type?.text, 'string | number'); + assert.equal( + fn.return?.description, + 'Returns either a string or a number.' + ); + assert.equal(fn.deprecated, undefined); + + // TODO: Run the same assertions in both languages once TS supports + // `@overload` for JSDoc in JS. + // + if (lang === 'ts') { + assert.ok(fn.overloads); + assert.equal(fn.overloads.length, 2); + + assert.equal(fn.overloads[0].name, 'overloaded'); + assert.equal( + fn.overloads[0].description, + 'This signature only works with strings.' + ); + assert.equal(fn.overloads[0].summary, undefined); + assert.equal(fn.overloads[0].parameters?.length, 1); + assert.equal(fn.overloads[0].parameters?.[0].name, 'x'); + assert.equal( + fn.overloads[0].parameters?.[0].description, + 'Accepts a string.' + ); + assert.equal(fn.overloads[0].parameters?.[0].summary, undefined); + assert.equal(fn.overloads[0].parameters?.[0].type?.text, 'string'); + assert.equal(fn.overloads[0].parameters?.[0].default, undefined); + assert.equal(fn.overloads[0].parameters?.[0].rest, false); + assert.equal(fn.overloads[0].return?.type?.text, 'string'); + assert.equal(fn.overloads[0].return?.description, 'Returns a string.'); + assert.equal(fn.overloads[0].deprecated, undefined); + + assert.equal(fn.overloads[1].name, 'overloaded'); + assert.equal( + fn.overloads[1].description, + 'This signature only works with numbers.' + ); + assert.equal(fn.overloads[1].summary, undefined); + assert.equal(fn.overloads[1].parameters?.length, 1); + assert.equal(fn.overloads[1].parameters?.[0].name, 'x'); + assert.equal( + fn.overloads[1].parameters?.[0].description, + 'Accepts a number.' + ); + assert.equal(fn.overloads[1].parameters?.[0].summary, undefined); + assert.equal(fn.overloads[1].parameters?.[0].type?.text, 'number'); + assert.equal(fn.overloads[1].parameters?.[0].default, undefined); + assert.equal(fn.overloads[1].parameters?.[0].rest, false); + assert.equal(fn.overloads[1].return?.type?.text, 'number'); + assert.equal(fn.overloads[1].return?.description, 'Returns a number.'); + assert.equal(fn.overloads[1].deprecated, undefined); + } else { + assert.equal(fn.overloads?.length ?? 0, 0); + } + }); + test.run(); } diff --git a/packages/labs/analyzer/test-files/js/functions/functions.js b/packages/labs/analyzer/test-files/js/functions/functions.js index 8ce866376a..c23f8b9a16 100644 --- a/packages/labs/analyzer/test-files/js/functions/functions.js +++ b/packages/labs/analyzer/test-files/js/functions/functions.js @@ -87,3 +87,16 @@ export const asyncFunction = async (a) => { await 0; return a; }; + +/** + * This signature works with strings or numbers. + * @param {string | number} x Accepts either a string or a number. + * @returns {string | number} Returns either a string or a number. + */ +export function overloaded(x) { + if (typeof x === 'string') { + return x + 'abc'; + } else { + return x + 123; + } +} diff --git a/packages/labs/analyzer/test-files/js/properties/element-a.js b/packages/labs/analyzer/test-files/js/properties/element-a.js index 47823335c1..16c1c6202a 100644 --- a/packages/labs/analyzer/test-files/js/properties/element-a.js +++ b/packages/labs/analyzer/test-files/js/properties/element-a.js @@ -56,5 +56,18 @@ export class ElementA extends LitElement { this.globalClass = document.createElement('foo'); this.staticProp = 42; } + + /** + * This signature works with strings or numbers. + * @param {string | number} x Accepts either a string or a number. + * @returns {string | number} Returns either a string or a number. + */ + overloaded(x) { + if (typeof x === 'string') { + return x + 'abc'; + } else { + return x + 123; + } + } } customElements.define('element-a', ElementA); diff --git a/packages/labs/analyzer/test-files/ts/functions/src/functions.ts b/packages/labs/analyzer/test-files/ts/functions/src/functions.ts index 02fda1b323..09fa8de124 100644 --- a/packages/labs/analyzer/test-files/ts/functions/src/functions.ts +++ b/packages/labs/analyzer/test-files/ts/functions/src/functions.ts @@ -91,3 +91,28 @@ export const asyncFunction = async (a: string) => { await 0; return a; }; + +/** + * This signature only works with strings. + * @param x Accepts a string. + * @returns Returns a string. + */ +export function overloaded(x: string): string; +/** + * This signature only works with numbers. + * @param x Accepts a number. + * @returns Returns a number. + */ +export function overloaded(x: number): number; +/** + * This signature works with strings or numbers. + * @param x Accepts either a string or a number. + * @returns Returns either a string or a number. + */ +export function overloaded(x: string | number): string | number { + if (typeof x === 'string') { + return x + 'abc'; + } else { + return x + 123; + } +} diff --git a/packages/labs/analyzer/test-files/ts/properties/src/element-a.ts b/packages/labs/analyzer/test-files/ts/properties/src/element-a.ts index 4c5f7dda22..841a52d059 100644 --- a/packages/labs/analyzer/test-files/ts/properties/src/element-a.ts +++ b/packages/labs/analyzer/test-files/ts/properties/src/element-a.ts @@ -80,4 +80,29 @@ export class ElementA extends LitElement { @property() union: LocalClass | HTMLElement | ImportedClass; + + /** + * This signature only works with strings. + * @param x Accepts a string. + * @returns Returns a string. + */ + overloaded(x: string): string; + /** + * This signature only works with numbers. + * @param x Accepts a number. + * @returns Returns a number. + */ + overloaded(x: number): number; + /** + * This signature works with strings or numbers. + * @param x Accepts either a string or a number. + * @returns Returns either a string or a number. + */ + overloaded(x: string | number): string | number { + if (typeof x === 'string') { + return x + 'abc'; + } else { + return x + 123; + } + } }