Skip to content

feat: Add token methods to JSONSourceCode #112

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

feat: Add token methods to JSONSourceCode #112

wants to merge 8 commits into from

Conversation

nzakas
Copy link
Member

@nzakas nzakas commented Jun 18, 2025

Prerequisites checklist

What is the purpose of this pull request?

Adds token methods to JSONSourceCode.

What changes did you make? (Give an overview)

Added the following methods to JSONSourceCode:

  • getTokenBefore()
  • getTokenAfter()
  • getTokenOrCommentBefore()
  • getTokenOrCommentAfter()

Also added tests for these methods.

Additionally, I changed comments to a getter so it can be lazily initialized.

(The tokens array from Momoa contains both tokens and comments, so it makes sense to lazily initialize it all together.)

Related Issues

fixes #83

Is there anything you'd like reviewers to focus on?

I wasn't sure if we needed getTokenOrCommentBefore() or getTokenOrCommentAfter(). These appear as convenience methods on SourceCode in the eslint package, but getTokenBefore() and getTokenAfter() also have a second argument that lets you specify whether or not to include comments. We could go that way here and eliminate two methods, or we could keep these as-is. I'm not sure what the better path is as we'd likely want to encourage other languages to match whatever we decide on here.

Copy link
Member

@lumirlumir lumirlumir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you take a look at the CI failure? Bun CI is currently throwing an error.

Also, I’ve left comments on a few parts!

@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Jun 19, 2025
@lumirlumir lumirlumir moved this from Needs Triage to Implementing in Triage Jun 19, 2025
@lumirlumir lumirlumir requested a review from a team June 19, 2025 09:16
@nzakas
Copy link
Member Author

nzakas commented Jun 19, 2025

Bun test looks like it was a temporary issue.

Copy link
Member

@lumirlumir lumirlumir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good to me! I've left a few comments on some missing parts.

@mdjermanovic
Copy link
Member

mdjermanovic commented Jun 20, 2025

I wasn't sure if we needed getTokenOrCommentBefore() or getTokenOrCommentAfter(). These appear as convenience methods on SourceCode in the eslint package, but getTokenBefore() and getTokenAfter() also have a second argument that lets you specify whether or not to include comments. We could go that way here and eliminate two methods, or we could keep these as-is. I'm not sure what the better path is as we'd likely want to encourage other languages to match whatever we decide on here.

getTokenOrCommentBefore() and getTokenOrCommentAfter() are deprecated in the eslint package:

https://eslint.org/docs/latest/extend/custom-rules#deprecated-sourcecode-methods

So I think we shouldn't have these methods here, but an option to include comments in getTokenBefore() and getTokenAfter().

nzakas and others added 4 commits June 20, 2025 15:11
Co-authored-by: 루밀LuMir <rpfos@naver.com>
Co-authored-by: 루밀LuMir <rpfos@naver.com>
Co-authored-by: 루밀LuMir <rpfos@naver.com>
Co-authored-by: 루밀LuMir <rpfos@naver.com>
@nzakas
Copy link
Member Author

nzakas commented Jun 20, 2025

@mdjermanovic good call. Don't know how I missed that. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Implementing
Development

Successfully merging this pull request may close these issues.

Change Request: Missing TokenStore methods in JSONSourceCode breaks compatibility with ESLint utilities
3 participants