Skip to content

feat: add parsing inline tags #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 5, 2023
Merged

Conversation

kraenhansen
Copy link
Contributor

This solves #11 by adding a second regexp based parse step on top of the description of a comment block as well as the description of any of the tags on a comment block.

@kraenhansen kraenhansen requested a review from brettz9 May 3, 2023 22:11
@brettz9
Copy link
Contributor

brettz9 commented May 3, 2023

I've been really burning the candle at both ends, but eager to check it out. Hope to take a look tomorrow.

@brettz9
Copy link
Contributor

brettz9 commented May 4, 2023

Was able to take a look just now, and I see it all works very well, but there is one issue. I would like for us to have the AST working in ESTree format so that selectors can target the contents (whether in eslint-plugin-jsdoc contexts with the comment property or for anyone using jsdoc-eslint-parser).

The file src/commentParserToESTree.js is the particular file where we'd ideally be adapting the comment-parser AST (and your AST if it is being added prior to this file, such as would be the case with it now). Currently that file looks at jsdoc.source as the sole authority for building the ESTree-compatible AST (it's the only place where one can get at all parts of the block). I would think perhaps the inline tag AST could be added to descriptionLine (I guess inline tags wouldn't span line breaks? If they would, then we'd need to add them directly on JsdocBlock and JsdocTag directly, but I think that'd be less than ideal).

(FWIW, there is also src/estreeToString.js, but the inline tag AST is not necessary for serialization, so we can ignore that file.)

@kraenhansen
Copy link
Contributor Author

I guess inline tags wouldn't span line breaks?

Do you have a link for the JSDoc spec, besides https://jsdoc.app/? I can't seem to find something that specifies the grammer. According to https://github.com/jsdoc/jsdoc/blob/main/packages/jsdoc-tag/lib/inline.js#L63 the text can contain new lines and on the tsdoc playground I'm also able to make the text span multiple lines, where:

/**
 * I'm experimenting with {@link something | A
 * multiline
 * text}.
 */

produce the following AST:

- Comment
  - Section
    - Paragraph
      - PlainText
        * PlainText="I'm experimenting with "
      - LinkTag
        * InlineTag_OpeningDelimiter="{"
        * InlineTag_TagName="@link"
        * Spacing=" "
        - DeclarationReference
          - MemberReference
            - MemberIdentifier
              * MemberIdentifier_Identifier="something"
            * Spacing=" "
        * LinkTag_Pipe="|"
        * Spacing=" "
        * LinkTag_LinkText="A\nmultiline\ntext"
        * InlineTag_ClosingDelimiter="}"
      - PlainText
        * PlainText="."
      - SoftBreak
        * SoftBreak="\n"

Therefore it does seem to me that the source is not the ideal place to place the array of inlineTags. And in the context of that, I'd suggest new JsdocInlineTag AST node type gets introduced and inlineTags gets added on the JsdocTag and JsdocBlock nodes. What do you think?

Currently that file looks at jsdoc.source as the sole authority for building the ESTree-compatible AST

In the context of the above, would it be a dealbreaker if I started accessing the inlineTags directly off jsdoc in commentParserToESTree.js?

@brettz9
Copy link
Contributor

brettz9 commented May 4, 2023

Do you have a link for the JSDoc spec, besides https://jsdoc.app/? I can't seem to find something that specifies the grammer.

No, it sounds like you've really found the "specs"--the docs, source, and TSDoc (though I haven't really used the latter to date myself).

Therefore it does seem to me that the source is not the ideal place to place the array of inlineTags.

Agreed.

And in the context of that, I'd suggest new JsdocInlineTag AST node type gets introduced and inlineTags gets added on the JsdocTag and JsdocBlock nodes. What do you think?

Yes, sounds good.

Currently that file looks at jsdoc.source as the sole authority for building the ESTree-compatible AST

In the context of the above, would it be a dealbreaker if I started accessing the inlineTags directly off jsdoc in commentParserToESTree.js?

You can access it however is reliable but since the code depends on source and since there are no absolute offsets, I don't think we can know for sure when the JsdocInlineTag can be built if we don't analyze source. Maybe you could map a current count of tags on source to inlineTags, but you'd still need to do so in the context of processing source as far as I can tell.

@brettz9
Copy link
Contributor

brettz9 commented May 5, 2023

It appears to be working from the eslint-plugin-jsdoc side now too. Do you have any other changes to make?

@kraenhansen
Copy link
Contributor Author

kraenhansen commented May 5, 2023

I believe this is ready for a final review 👍

@brettz9 brettz9 merged commit 895a7f1 into es-joy:main May 5, 2023
@brettz9
Copy link
Contributor

brettz9 commented May 5, 2023

Thanks for the great contribution!

@brettz9
Copy link
Contributor

brettz9 commented May 5, 2023

Released in v0.38.0 (and in eslint-plugin-jsdoc with v43.2.0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants