Skip to content
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

fix(semantic): Refactor jsdoc finding #2437

Merged
merged 17 commits into from
Feb 24, 2024
Merged

fix(semantic): Refactor jsdoc finding #2437

merged 17 commits into from
Feb 24, 2024

Conversation

leaysgur
Copy link
Contributor

@leaysgur leaysgur commented Feb 19, 2024

Partial fix for #168

  • Fix general finding behavior for leading comments
  • Accept multiple jsdoc comments per node
  • Provide get_one and also get_all
  • Add iter_all() for non-node related usage
  • Limit AST node kinds to parse

@github-actions github-actions bot added the A-semantic Area - Semantic label Feb 19, 2024
@Boshen
Copy link
Member

Boshen commented Feb 19, 2024

Thank you so much for trying to implement one of the hardest part of this project ❤️

You are free to rewrite everything if you think it's necessary. What I wrote is just a prototype to initiate eslint-plugin-jsdoc.

You have write permission, you may try graphite with stacked PRs.

@github-actions github-actions bot added the A-linter Area - Linter label Feb 19, 2024
Copy link

codspeed-hq bot commented Feb 19, 2024

CodSpeed Performance Report

Merging #2437 will not alter performance

Comparing jsdoc-parse (cd575d8) with main (6aa8c2d)

Summary

✅ 27 untouched benchmarks

@leaysgur
Copy link
Contributor Author

leaysgur commented Feb 19, 2024

one of the hardest part of this project

This is so true! 😂 The more I looked into it, the more I got confused...

Although there are many usecases for JSDoc comments and also implementations(I've learned JSDoc TS, jsdoc.app, eslint-plugin-jsdoc are all differently implemented 🫠), if we set primary goal to implement eslint-plugin-jsdoc, I'm going to try to identify again what kind of API are needed.

@Boshen
Copy link
Member

Boshen commented Feb 20, 2024

Yeah the first goal is to get eslint-plugin-jsdoc tests passing. jsdoc doesn't have a formal spec so it's a bit hard to figure out which implementation to go for.

By looking at the dependencies of eslint-plugin-jsdoc, I think we can split the task into:

@leaysgur
Copy link
Contributor Author

Thank you so much~! I'm just now deciphering that code base... 🕵🏻‍♂️
(Sorry the PR is still half done.)

@Boshen
Copy link
Member

Boshen commented Feb 23, 2024

Feels like I can merge this as is after looking at the code.

@leaysgur
Copy link
Contributor Author

leaysgur commented Feb 23, 2024

There are a few things I want to fix here.

  • Decide how to handle (): ParenthesizedExpression
    • jsdoccomment does not attach JSDoc to this, but TS does(and this is intuitive tho)
  • Return the nearest comment only or split API for this
    • Still keeping them in array
  • Note about min, maxLines settings does not supported
    • jsdoccomment and eslint-plugin-jsdoc has options for this
    • Blank lines are configurable between a node and its attached JSDoc
      • e.g. if 2 lines are open, do not attach.
      • e.g. Ignore it if on the same line
      • (I'm not sure what this for...?)
  • Collect all non-attached JSDoc, in particular, those located at the EOF and not attached to any node

(Of course, feel free to merge first. I'll send a PR for this later.)

@leaysgur leaysgur requested a review from Boshen February 24, 2024 06:10
@leaysgur leaysgur marked this pull request as ready for review February 24, 2024 09:02
@Boshen
Copy link
Member

Boshen commented Feb 24, 2024

Thank you so much for working on this, we'll get there eventually 😁

It seems like we're a little bit closer to https://github.com/import-js/eslint-plugin-import/blob/v2.29.1/docs/rules/no-deprecated.md

Let me setup the code for it.

@Boshen Boshen merged commit bc22ae5 into main Feb 24, 2024
22 checks passed
@Boshen Boshen deleted the jsdoc-parse branch February 24, 2024 09:24
@leaysgur
Copy link
Contributor Author

Oh, JSDoc everywhere! 😮

Now I can understand this kind of things easily...

Wish you luck. 🚀

Boshen pushed a commit that referenced this pull request Feb 24, 2024
IWANABETHATGUY pushed a commit to IWANABETHATGUY/oxc that referenced this pull request May 29, 2024
Partial fix for oxc-project#168 

- [x] Fix general finding behavior for leading comments
- [x] Accept multiple jsdoc comments per node
- [x] Provide `get_one` and also `get_all` 
- [x] Add `iter_all()` for non-node related usage
- [x] Limit AST node kinds to parse
IWANABETHATGUY pushed a commit to IWANABETHATGUY/oxc that referenced this pull request May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter A-semantic Area - Semantic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants