feat: add auto-fix suggestion for require-img-alt rule #366
Conversation
|
@yeonjuan, please review |
There was a problem hiding this comment.
Pull Request Overview
This pull request enhances the require-img-alt rule by adding an auto-fix suggestion that inserts an empty alt attribute for tags missing one. Key changes include the addition of a fixer suggestion in the rule, updates to test cases to verify the auto-fix, and modifications to the rule's meta to enable suggestions.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/eslint-plugin/tests/rules/require-img-alt.test.js | Updated test cases to incorporate auto-fix suggestion and adjusted string formatting for template literals |
| packages/eslint-plugin/lib/rules/require-img-alt.js | Modified rule metadata to reflect suggestion type and added a new suggestion to insert an empty alt attribute |
packages/eslint-plugin/tests/rules/require-img-alt.test.js
8000
Outdated
Show resolved
Hide resolved
|
Hi @simplicityf Thanks for your contribution. The format check in CI did not pass. |
|
@yeonjuan , what changes are you suggesting?? I don't understand you Have you check the recent commit? |
Yes I saw your latest commit. I propose that no suggestion auto-fix should be offered when the suggest: substitute.length ? null : [
{
messageId: MESSAGE_IDS.INSERT_EMPTY,
fix(fixer) {
return fixer.insertTextBefore(node.openEnd, ' alt=""');
},
},
], |
@yeonjuan, please check now I have made the changes, it aligns well with the rules now. |
|
@yeonjuan, I am waiting for your review |
|
@yeonjuan confirm the changes please |
|
@yeonjuan confirm the changes please |
|
@simplicityf PR hasn't been updated, could you please |
| @@ -45,6 +49,8 @@ module.exports = { | |||
| ], | |||
| messages: { | |||
| [MESSAGE_IDS.MISSING_ALT]: "Missing `alt` attribute at `<img>` tag", | |||
| [MESSAGE_IDS.EMPTY_ALT]: "Empty `alt` attribute at `<img>` tag", | |||
There was a problem hiding this comment.
@simplicityf It should be removed as unused.
There was a problem hiding this comment.
@simplicityf This comment hasn't been reflected in the PR.
| @@ -13,23 +13,27 @@ const { getRuleUrl } = require("./utils/rule"); | |||
|
|
|||
| const MESSAGE_IDS = { | |||
| MISSING_ALT: "missingAlt", | |||
| EMPTY_ALT: "emptyAlt", | |||
There was a problem hiding this comment.
@simplicityf It should be removed as unused.
|
@yeonjuan. I son't think i remeber the provious description i used. If you can point that out |
1 similar comment
|
@simplicityf I think I'm almost there, thank you. I left these three comments :) |
|
@yeonjuan, check the latest commit The code works, likewise the test case. I don't think I have the time for any changes again. Thanks :) |
|
@simplicityf Okay, thanks for the work. In addition to the behavior, I think code cleanup is also important for maintaining the project. Maybe you can work on it later when you have time, or someone else can take over this task. Thanks again! |
| @@ -77,16 +96,22 @@ module.exports = { | |||
| /** | |||
| * @param {Tag} node | |||
| * @param {string[]} substitute | |||
| * @returns | |||
| * @returns {boolean}} | |||
There was a problem hiding this comment.
| * @returns {boolean}} | |
| * @returns {boolean} |
cc @simplicityf
| @@ -13,14 +13,16 @@ const { getRuleUrl } = require("./utils/rule"); | |||
|
|
|||
| const MESSAGE_IDS = { | |||
| MISSING_ALT: "missingAlt", | |||
| EMPTY_ALT: "emptyAlt", | |||
There was a problem hiding this comment.
| EMPTY_ALT: "emptyAlt", |
cc @simplicityf
| @@ -45,6 +48,8 @@ module.exports = { | |||
| ], | |||
| messages: { | |||
| [MESSAGE_IDS.MISSING_ALT]: "Missing `alt` attribute at `<img>` tag", | |||
| [MESSAGE_IDS.EMPTY_ALT]: "Empty `alt` attribute at `<img>` tag", | |||
There was a problem hiding this comment.
| [MESSAGE_IDS.EMPTY_ALT]: "Empty `alt` attribute at `<img>` tag", |
cc @simplicityf
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #366 +/- ##
==========================================
+ Coverage 98.22% 98.25% +0.03%
==========================================
Files 77 78 +1
Lines 2419 2466 +47
Branches 660 672 +12
==========================================
+ Hits 2376 2423 +47
Misses 43 43
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Checklist
require-img-altrule #349Description
Enhances the require-img-alt rule by adding an auto-fix suggestion that inserts an empty alt="" attribute when![]()
elements are missing the alt attribute.
Changes Made
Added auto-fix suggestion: When an![]()
tag lacks an alt attribute, the rule now provides a suggestion to insert alt=""
Enhanced error reporting: Uses ESLint's suggest property to offer quick fixes
Comprehensive test coverage: Added test cases covering both standalone HTML and template literal contexts
Test Cases
Before (Invalid):
html
After (Suggested Fix):
html
Features![]()
tags
Works with self-closing and regular
Supports substitute attributes (configurable via options)
Compatible with template literals
Maintains existing functionality while adding helpful auto-fix suggestions
Implementation Details
Uses fixer.insertTextBefore(node.openEnd, ' alt="..."') to insert the attribute
Preserves existing validation logic
Follows ESLint's suggestion API patterns
Includes proper TypeScript definitions and JSDoc comments
** This enhancement improves developer experience by providing quick fixes for accessibility issues while maintaining the rule's core functionality.