From 95e3192525310b9f1567e034c22489da3a5847a1 Mon Sep 17 00:00:00 2001 From: Jeff Williams Date: Sat, 19 Sep 2020 19:24:14 -0700 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. --- lib/jsdoc/src/parser.js | 2 +- lib/jsdoc/src/visitor.js | 9 ++---- test/fixtures/interface-assignment.js | 15 +++++++++ test/fixtures/interface-implements2.js | 23 ++++++++++++++ test/specs/tags/interfacetag.js | 44 ++++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 7 deletions(-) create mode 100644 test/fixtures/interface-assignment.js create mode 100644 test/fixtures/interface-implements2.js diff --git a/lib/jsdoc/src/parser.js b/lib/jsdoc/src/parser.js index 57e43fe2..cead39b7 100644 --- a/lib/jsdoc/src/parser.js +++ b/lib/jsdoc/src/parser.js @@ -539,7 +539,7 @@ 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/lib/jsdoc/src/visitor.js b/lib/jsdoc/src/visitor.js index 77d46660..c712cc81 100644 --- a/lib/jsdoc/src/visitor.js +++ b/lib/jsdoc/src/visitor.js @@ -329,12 +329,9 @@ function makeConstructorFinisher(parser) { return; } - combined = jsdoc.doclet.combine(doclet, parentDoclet); - combined.longname = parentDoclet.longname; - if (parentDoclet.memberof) { - combined.memberof = parentDoclet.memberof; - } - + // 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 = jsdoc.doclet.combine(parentDoclet, doclet); parser.addResult(combined); parentDoclet.undocumented = doclet.undocumented = true; diff --git a/test/fixtures/interface-assignment.js b/test/fixtures/interface-assignment.js new file mode 100644 index 00000000..e95b8ae1 --- /dev/null +++ b/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/test/fixtures/interface-implements2.js b/test/fixtures/interface-implements2.js new file mode 100644 index 00000000..d8ba33e9 --- /dev/null +++ b/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/test/specs/tags/interfacetag.js b/test/specs/tags/interfacetag.js index 692cdb78..338ae642 100644 --- a/test/specs/tags/interfacetag.js +++ b/test/specs/tags/interfacetag.js @@ -50,6 +50,50 @@ describe('@interface tag', () => { }); }); + describe('ES2015 classes as interfaces', () => { + const docSet2 = jasmine.getDocSetFromFile('test/fixtures/interface-implements2.js'); + const docSet3 = jasmine.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(() => { jasmine.restoreTagDictionary();