Update/jsdoc package #295

lmores · 2021-01-10T14:48:13Z

This PR contains two distinct commits, each aiming to improve the jsdoc package.
(I also have a couple more ideas I would like to implement, but I will open an issue to discuss them before)

First commit
feat(jsdoc,nunjucks): update espree and nunjucks packages

Update nunjucks package to ^3.2.2
Move nunjucks 'throwOnUndefined' option to nunjucks Environment config (as per nunjucks docs https://mozilla.github.io/nunjucks/api.html#environment)
Update espree package to ^7.3.1 (this enable to parse new js keywords like 'async/await')
Update the espree config object as per https://github.com/eslint/espree#options
Make the jsParser service fully configurable via the jsParser.config object
Note: since espree 7.3.1 does no more throw exceptions with the 'description' property, the error messages generated in jsdoc/file-readers/jsdoc.js:19 are a bit less descriptive (see https://github.com/eslint/espree/blob/3b4ca9e3141514ffac93bb7fef6c1329370df310/lib/espree.js#L188).

Relates to issue #293

Second commit
feat(jsdoc): add support for more jsdoc tags

Add support for the following jsdoc tags: async, constant, enum, since
Improve support for the following jsdoc tags: deprecated, module
Add the booleanTagTransform() service for jsdoc tags like '@async' and '@deprecated'
Add the moduleTagTransform() service for the jsdoc tag '@module' to cope with jsdoc behaviour when no value is provided (see https://jsdoc.app/tags-module.html)

google-cla · 2021-01-10T14:48:17Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

googlebot · 2021-01-10T14:48:17Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

lmores · 2021-01-10T14:50:51Z

@googlebot I signed it!

googlebot · 2021-01-10T14:50:54Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-10T14:50:55Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot · 2021-01-10T15:04:51Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-10T15:04:52Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

lmores · 2021-01-10T15:05:12Z

@googlebot I fixed it.

petebacondarwin

Thanks for this PR @sir-ion. Generally looks good. I would like to avoid the breaking change to the API.

petebacondarwin · 2021-01-10T15:35:14Z

jsdoc/services/jsParser.js

-    });
+  var _config = { ...OPTIONAL_ESPREE_CONFIG, ... MANDATORY_ESPREEE_CONFIG };
+
+  return {


Can we avoid the breaking change (where the user must now call jsParser.parse() rather than just jsParser()) by attaching config to the function itself?

Also I would avoid the setter/getter since it doesn't make it clear that you cannot mutate the config object directly and expect it to affect the parse() function.

E.g.:

function parse(code) { return jsParserImpl.parse(code, _config); } parse.setConfig(config) { ... }; parse.getConfig() { ... }; return parse;

Actually, even better. Let's make the jsParser config an injectable service of its own, which the jsParser depends upon.

module.exports = function jsParser(jsParserConfig) { return function(code) { return jsParserImpl.parse(code, jsParserConfig); }; };

petebacondarwin · 2021-01-10T15:42:01Z

jsdoc/tag-defs/deprecated.js

+  return {
+    name: 'deprecated',
+    transforms: [ booleanTagTransform ]


Deprecated is not actually a boolean jsdoc tag - https://jsdoc.app/tags-deprecated.html - it can contain a description of the deprecation.

Whoops, my bad. Reverted!

petebacondarwin · 2021-01-10T15:47:18Z

jsdoc/services/jsParser.spec.js

+    return new Dgeni.Package('testJsParserPackage', [mockPackage()])
+    .config(function(jsParser) {
+      jsParser.config = {
+        comment: false,              // this value should NOT override the default (true)


I don't quite understand the comment on this line. Should this override the default or not?

Are you saying that even if you set the config for comment it will not affect the value passed to the parser?

Yes, that was my goal. Since the current implementation of the jsdoc package requires javascript comments to be attached to the espree AST in order to work properly, I forbade the user to override such options. (However now I realize that this choice goes against the general goal to provide a fully customizable tool to the users)

Anyway, in my last commit I moved the config to the new jsParserConfig service.

jsdoc/file-readers/jsdoc.js

jsdoc/services/code-name.js

petebacondarwin

Great! The only thing I wish for now is a spec file for the new moduleTagTransform service 🙏

lmores · 2021-01-12T15:05:33Z

Great! The only thing I wish for now is a spec file for the new moduleTagTransform service

You are totally right.

Actually, looking at jsdoc module page I realized that the implementation of transformers for the module tagDef could be improved. Have a look and tell me if you agree with the changes I made (unfortunately they are not fully backward compatitle...).

petebacondarwin

Thanks for the work here @sir-ion.

I have two requests:

the first is that I am about to land a bit refactoring that updates the code to more "modern" standards. In particular use of const and let rather than var; and also use of fat-arrow functions rather than anonymous function() {...} style functions. Could you rebase on top of that work when it lands and ensure that you update the code here to match?
the second is that I have some concerns over this new @module tag definition (or more specifically the transform). Could we pull that change out of this PR into its own so that we can land the other changes here without delay? Then we can discuss the update to @module separately.

jsdoc/services/transforms/extract-module-name.js

petebacondarwin · 2021-01-12T20:25:18Z

jsdoc/services/transforms/extract-module-name.spec.js

+    var newValue = transform(doc, tag, value);
+
+    expect(newValue).toEqual('');
+    expect(tag).toEqual({ tagName: 'module', name: 'module:customName' });


From my reading of the JSDOC docs the name in this case should just be customName. I.e. the module: prefix is not part of the name.

petebacondarwin · 2021-01-12T20:26:51Z

jsdoc/services/transforms/extract-module-name.js

+ */
+module.exports = function extractModuleNameTransform(log, createDocMessage) {
+  return function(doc, tag, value) {
+    if (tag.tagName == 'module') {


I don't think it is necessary to guard this transform, since we only apply it to the module tagDef.

petebacondarwin · 2021-01-12T20:30:19Z

jsdoc/services/transforms/extract-module-name.js

+      doc.codeNode = null;
+      doc.codeAncestors = null;


I am worried that this will break other jsdoc tags...

For example, consider the following code:

/** * @module */ /** * Description of foo(). */ function foo() {}

Then I think that both comment blocks will be processed with respect to a single doc that has the foo() function as its codeNode.

Now the @module tag will cause this doc to lose connection to the foo() function.

The fact that we don't have a good way to distinguish JSDOC comments that should apply to the file only, is one reason why I have not properly implemented this before.

petebacondarwin · 2021-01-12T20:31:42Z

jsdoc/services/transforms/extract-module-name.js

+        if (remainder) {
+          log.warn(createDocMessage(`Some text remained after parsing module name: ${remainder}`, doc));
+        }


I would also not warn about this either, since it makes the transform less usable elsewhere.
If we want to ensure that that there is nothing else on the line for the @module tagDef then we could add another transform that did this check.

googlebot · 2021-01-13T07:56:23Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-13T07:56:23Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-13T07:59:04Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

googlebot · 2021-01-13T07:59:04Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

lmores · 2021-01-13T08:03:25Z

the first is that I am about to land a bit refactoring that updates the code to more "modern" standards. In particular use of const and let rather than var; and also use of fat-arrow functions rather than anonymous function() {...} style functions. Could you rebase on top of that work when it lands and ensure that you update the code here to match?

Sure! Actually, I was tempted to already adopt those more "modern" standards, but in the end I chose to apply the "never been here" approach.

the second is that I have some concerns over this new @module tag definition (or more specifically the transform). Could we pull that change out of this PR into its own so that we can land the other changes here without delay? Then we can discuss the update to @module separately.

Done, last commit reverts all changes to the module tagDef so that it matches the one in the current master.

- Update nunjucks package to ^3.2.2 - Move nunjucks 'throwOnUndefined' option to nunjucks Environment config (as per nunjucks docs https://mozilla.github.io/nunjucks/api.html#environment). - Update espree package to ^7.3.1 (this enable parsing of new js keywords like `async/await`). - Update the espree config object as per https://github.com/eslint/espree#options Note: since espree 7.3.1 no longer throws exceptions with the 'description' property, the error messages generated in jsdoc/file-readers/jsdoc.js:19 are a bit less descriptive (see https://github.com/eslint/espree/blob/3b4ca9e3141514ffac93bb7fef6c1329370df310/lib/espree.js#L188). Relates to issue angular#293

- Add support for the following jsdoc tags: `async`, `constant`, `enum`, `since`. - Improve support for the following jsdoc tags: deprecated, module. - Add the booleanTagTransform() service for jsdoc tags like '@async'. - Add jsParserConfig service for `jsParser`.

googlebot · 2021-01-13T09:58:01Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-13T09:58:01Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

petebacondarwin · 2021-01-13T09:58:24Z

I just rebased and force-pushed this PR. I will now merge it into master.

google-cla bot added the cla: no label Jan 10, 2021

lmores force-pushed the update/jsdoc-package branch from 2d8eaa2 to 01cba85 Compare January 10, 2021 15:04

googlebot removed the cla: no label Jan 10, 2021

google-cla bot added the cla: yes label Jan 10, 2021

petebacondarwin suggested changes Jan 10, 2021

View reviewed changes

petebacondarwin reviewed Jan 11, 2021

View reviewed changes

petebacondarwin suggested changes Jan 12, 2021

View reviewed changes

google-cla bot removed the cla: yes label Jan 13, 2021

googlebot added the cla: no label Jan 13, 2021

lmores force-pushed the update/jsdoc-package branch from 6d4538d to 853f005 Compare January 13, 2021 07:58

google-cla bot added cla: yes and removed cla: no labels Jan 13, 2021

googlebot added the cla: yes label Jan 13, 2021

lmores mentioned this pull request Jan 13, 2021

Feature proposal: enhance tagDefs #298

Open

petebacondarwin mentioned this pull request Jan 13, 2021

Test from the typescript package fails #297

Closed

petebacondarwin force-pushed the update/jsdoc-package branch from 853f005 to c0b077b Compare January 13, 2021 09:57

googlebot added cla: no and removed cla: yes labels Jan 13, 2021

docs(package.json): add Lorenzo as a contributor

c4f4d3b

petebacondarwin merged commit 8b673a8 into angular:master Jan 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update/jsdoc package #295

Update/jsdoc package #295

lmores commented Jan 10, 2021

google-cla bot commented Jan 10, 2021

googlebot commented Jan 10, 2021

lmores commented Jan 10, 2021

googlebot commented Jan 10, 2021

google-cla bot commented Jan 10, 2021

googlebot commented Jan 10, 2021

google-cla bot commented Jan 10, 2021

lmores commented Jan 10, 2021

petebacondarwin left a comment

petebacondarwin Jan 10, 2021

petebacondarwin Jan 10, 2021

lmores Jan 11, 2021

petebacondarwin Jan 10, 2021

lmores Jan 11, 2021

petebacondarwin Jan 10, 2021

petebacondarwin Jan 10, 2021

lmores Jan 11, 2021

petebacondarwin left a comment

lmores commented Jan 12, 2021

petebacondarwin left a comment

petebacondarwin Jan 12, 2021

petebacondarwin Jan 12, 2021

petebacondarwin Jan 12, 2021

petebacondarwin Jan 12, 2021

googlebot commented Jan 13, 2021

google-cla bot commented Jan 13, 2021

google-cla bot commented Jan 13, 2021

googlebot commented Jan 13, 2021

lmores commented Jan 13, 2021

googlebot commented Jan 13, 2021

google-cla bot commented Jan 13, 2021

petebacondarwin commented Jan 13, 2021

Update/jsdoc package #295

Update/jsdoc package #295

Conversation

lmores commented Jan 10, 2021

google-cla bot commented Jan 10, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

googlebot commented Jan 10, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

lmores commented Jan 10, 2021

googlebot commented Jan 10, 2021

google-cla bot commented Jan 10, 2021

googlebot commented Jan 10, 2021

google-cla bot commented Jan 10, 2021

lmores commented Jan 10, 2021

petebacondarwin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petebacondarwin left a comment

Choose a reason for hiding this comment

lmores commented Jan 12, 2021

petebacondarwin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

googlebot commented Jan 13, 2021

google-cla bot commented Jan 13, 2021

google-cla bot commented Jan 13, 2021

googlebot commented Jan 13, 2021

lmores commented Jan 13, 2021

googlebot commented Jan 13, 2021

google-cla bot commented Jan 13, 2021

petebacondarwin commented Jan 13, 2021