Skip to content

Conversation

lumirlumir
Copy link
Member

@lumirlumir lumirlumir commented Aug 22, 2025

Prerequisites checklist

What is the purpose of this pull request?

Hi,

In this PR, I've simplified no-bare-urls rule with ESQuery selector. This is follow-up to #495 and #498.


The code has been updated a bit to maintain the same behavior without re-traversing the AST with a for loop.

The main point of the previous logic was that URLs between HTML tags were not reported.

I've tried to keep the logic as close as possible, except that it now uses ESQuery selectors.

I also aimed to make the code cleaner and minimize the use of variables.

There may still be room for improvement, so I’d be happy to hear any suggestions!

What changes did you make? (Give an overview)

In this PR, I've simplified no-bare-urls rule with ESQuery selector. This is follow-up to #495 and #498.

Related Issues

Refs: #495, #498

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

@eslintbot eslintbot added this to Triage Aug 22, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Aug 22, 2025
src/util.js Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

I’ve just added some markers to clarify which category each util belongs to.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added additional test cases to more thoroughly verify the changes before and after.

I've confirmed that these tests pass in the previous codebase.

@lumirlumir lumirlumir marked this pull request as ready for review August 26, 2025 10:32
@lumirlumir lumirlumir requested a review from a team August 26, 2025 10:32
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

I think this look good overall. If you can add some comments around how you're making this work, that would be helpful.

@nzakas nzakas moved this from Needs Triage to Implementing in Triage Aug 26, 2025
@lumirlumir lumirlumir requested a review from nzakas August 27, 2025 09:42
@lumirlumir
Copy link
Member Author

I've added a new commit c5e2bfa

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@nzakas nzakas merged commit 651550a into main Aug 28, 2025
23 checks passed
@nzakas nzakas deleted the refactor-simplify-no-bare-urls-with-esquery-selector branch August 28, 2025 15:34
@github-project-automation github-project-automation bot moved this from Implementing to Complete in Triage Aug 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

2 participants