Skip to content

Conversation

TKDev7
Copy link
Contributor

@TKDev7 TKDev7 commented Jul 31, 2025

Prerequisites checklist

What is the purpose of this pull request?

This pull request aims to enhance the clarity and precision of error reporting for the no-html rule. Previously, rule violations for HTML tags would highlight only a partial segment, specifically the tag name and an extra character.

What changes did you make? (Give an overview)

I've modified the no-html rule's reporting logic to report the entire opening HTML tag, including any attributes and spaces, as long as it's on a single line. If the opening tag spans multiple lines, only the first line of the tag will be reported.

  • For <div id="foo">, it will report the full <div id="foo">.
  • For <p\n data-attribute="value">, it will report only <p.

Related Issues

Fixes #478

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

@nzakas nzakas requested a review from Copilot August 1, 2025 14:23
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the error reporting precision for the no-html rule by highlighting the entire opening HTML tag instead of just the tag name plus one character. The change enhances developer experience by providing clearer visual feedback about which portion of code violates the rule.

  • Updated regex pattern to capture complete opening HTML tags including attributes
  • Modified position calculation logic to report the full tag or first line if multi-line
  • Added comprehensive test cases covering various HTML tag scenarios

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/rules/no-html.js Updated regex pattern and position calculation to report entire opening HTML tags
tests/rules/no-html.test.js Added test cases for various HTML tag formats including attributes and multi-line scenarios

nzakas
nzakas previously approved these changes Aug 1, 2025
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! Please take a look at the Copilot comments.

@nzakas nzakas added this to Triage Aug 1, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Aug 1, 2025
@nzakas nzakas moved this from Needs Triage to Implementing in Triage Aug 1, 2025
snitin315
snitin315 previously approved these changes Aug 10, 2025
Copy link
Contributor

@snitin315 snitin315 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!

Leaving it open for @lumirlumir

@snitin315 snitin315 moved this from Implementing to Second Review Needed in Triage Aug 10, 2025
lumirlumir
lumirlumir previously approved these changes Aug 18, 2025
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.

LGTM. Would like @snitin315 and @nzakas to verify the changes before merging.

nzakas
nzakas previously approved these changes Aug 19, 2025
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
Copy link
Member

nzakas commented Aug 19, 2025

@TKDev7 can you take a look at the conflicts?

@TKDev7 TKDev7 dismissed stale reviews from nzakas and lumirlumir via 3c472a4 August 21, 2025 07:04
@nzakas nzakas merged commit 5e6e94e into eslint:main Aug 22, 2025
23 checks passed
@github-project-automation github-project-automation bot moved this from Second Review Needed to Complete in Triage Aug 22, 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.

Rule Change: Improve no-html rule's location reporting for HTML tags
4 participants