From eaa2cfb8076ad5eed3dd19563e052fd5bf04b60f Mon Sep 17 00:00:00 2001 From: Jeff Williams Date: Sat, 12 Dec 2020 19:24:20 -0800 Subject: [PATCH] fix: correctly track interface members When an interface is a) defined as an ES2015 class and b) assigned to a variable, JSDoc sometimes used the wrong `longname` and `memberof` for members of the interface (specifically, for instance properties). The root cause was that we weren't resolving `this` correctly within this type of interface. As a result, if you added a JSDoc comment to something like `this.foo = 'bar'`, the doclet for `this.foo` had the wrong `longname` and `memberof`. Fixing that issue uncovered another issue: When we merged the constructor's doclet with the interface's doclet, we preferred the constructor's doclet. However, the constructor's doclet used the wrong `kind` in this case; we already had code to fix up the `longname` and `memberof` of the combined doclet, but not the `kind`. The fix was to prefer the interface's doclet for all properties. Manual cherry-pick of 95e3192525310b9f1567e034c22489da3a5847a1. --- packages/jsdoc/lib/jsdoc/src/parser.js | 3 +- packages/jsdoc/lib/jsdoc/src/visitor.js | 6 +-- .../test/fixtures/interface-assignment.js | 15 +++++++ .../test/fixtures/interface-implements2.js | 23 ++++++++++ .../jsdoc/test/specs/tags/interfacetag.js | 44 +++++++++++++++++++ 5 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 packages/jsdoc/test/fixtures/interface-assignment.js create mode 100644 packages/jsdoc/test/fixtures/interface-implements2.js diff --git a/packages/jsdoc/lib/jsdoc/src/parser.js b/packages/jsdoc/lib/jsdoc/src/parser.js index dc6fc162..0f64ce56 100644 --- a/packages/jsdoc/lib/jsdoc/src/parser.js +++ b/packages/jsdoc/lib/jsdoc/src/parser.js @@ -531,7 +531,8 @@ class Parser extends EventEmitter { result = doclet.longname; } // walk up to the closest class we can find - else if (doclet.kind === 'class' || doclet.kind === 'module') { + else if (doclet.kind === 'class' || doclet.kind === 'interface' || + doclet.kind === 'module') { result = doclet.longname; } else if (node.enclosingScope) { diff --git a/packages/jsdoc/lib/jsdoc/src/visitor.js b/packages/jsdoc/lib/jsdoc/src/visitor.js index 26d28e7a..ef47307f 100644 --- a/packages/jsdoc/lib/jsdoc/src/visitor.js +++ b/packages/jsdoc/lib/jsdoc/src/visitor.js @@ -319,11 +319,9 @@ function makeConstructorFinisher(parser) { return; } + // We prefer the parent doclet because it has the correct kind, longname, and memberof. + // The child doclet might or might not have the correct kind, longname, and memberof. combined = combineDoclets(parentDoclet, eventDoclet); - combined.longname = parentDoclet.longname; - if (parentDoclet.memberof) { - combined.memberof = parentDoclet.memberof; - } parser.addResult(combined); diff --git a/packages/jsdoc/test/fixtures/interface-assignment.js b/packages/jsdoc/test/fixtures/interface-assignment.js new file mode 100644 index 00000000..e95b8ae1 --- /dev/null +++ b/packages/jsdoc/test/fixtures/interface-assignment.js @@ -0,0 +1,15 @@ +/** @namespace */ +const myCorp = {}; + +/** + * @interface + */ +myCorp.IWorker = class IWorker { + constructor(workerName) { + /** Name of the worker. */ + this.workerName = workerName; + } + + /** Interface for doing some work. */ + work() {} +}; diff --git a/packages/jsdoc/test/fixtures/interface-implements2.js b/packages/jsdoc/test/fixtures/interface-implements2.js new file mode 100644 index 00000000..d8ba33e9 --- /dev/null +++ b/packages/jsdoc/test/fixtures/interface-implements2.js @@ -0,0 +1,23 @@ +/** + * @interface + */ +class IWorker { + /** Interface for doing some work. */ + work() {} +} + +/** + * @implements {IWorker} + */ +class MyWorker { + /** Do some work. */ + work() {} + + /** Process a thing. */ + process() {} +} + +/** + * @implements {IWorker} + */ +class MyIncompleteWorker {} diff --git a/packages/jsdoc/test/specs/tags/interfacetag.js b/packages/jsdoc/test/specs/tags/interfacetag.js index b9b20f3b..af20ebc2 100644 --- a/packages/jsdoc/test/specs/tags/interfacetag.js +++ b/packages/jsdoc/test/specs/tags/interfacetag.js @@ -54,6 +54,50 @@ describe('@interface tag', () => { }); }); + describe('ES2015 classes as interfaces', () => { + const docSet2 = jsdoc.getDocSetFromFile('test/fixtures/interface-implements2.js'); + const docSet3 = jsdoc.getDocSetFromFile('test/fixtures/interface-assignment.js'); + + it('should set the correct kind on the interface', () => { + const workerInterface = docSet2.getByLongname('IWorker').filter(d => !d.undocumented)[0]; + + expect(workerInterface.kind).toBe('interface'); + }); + + it('should set the correct kind on methods in the interface', () => { + const workerInterfaceWork = docSet2.getByLongname('IWorker#work') + .filter(d => !d.undocumented)[0]; + + expect(workerInterfaceWork.kind).toBe('function'); + }); + + it('should set the correct kind on the implementing class', () => { + const workerImpl = docSet2.getByLongname('MyWorker').filter(d => !d.undocumented)[0]; + + expect(workerImpl.kind).toBe('class'); + }); + + it('should set the correct kind on an interface assigned to a variable', () => { + const workerInterface = docSet3.getByLongname('myCorp.IWorker').filter(d => !d.undocumented)[0]; + + expect(workerInterface.kind).toBe('interface'); + }); + + it('should set the correct kind on methods in an interface assigned to a variable', () => { + const workerInterfaceWork = docSet3.getByLongname('myCorp.IWorker#work') + .filter(d => !d.undocumented)[0]; + + expect(workerInterfaceWork.kind).toBe('function'); + }); + + it('should set the correct kind on other members in an interface assigned to a variable', () => { + const workerName = docSet3.getByLongname('myCorp.IWorker#workerName') + .filter(d => !d.undocumented)[0]; + + expect(workerName.kind).toBe('member'); + }); + }); + describe('Closure Compiler tags', () => { afterEach(() => { jsdoc.restoreTagDictionary();