feat: implement autofix for no-unnormalized-keys#151
feat: implement autofix for no-unnormalized-keys#151lumirlumir wants to merge 13 commits intomainfrom
no-unnormalized-keys#151Conversation
lumirlumir
force-pushed
the
feat-support-autofix-for-no-unnormalized-keys
branch
from
dccd118 to
42c637a
Compare
lumirlumir
force-pushed
the
feat-support-autofix-for-no-unnormalized-keys
branch
from
7177f2f to
78c0ce5
Compare
nzakas
left a comment
nzakas
left a comment
There was a problem hiding this comment.
LGTM. Would like @mdjermanovic to review before merging.
| fix(fixer) { | ||
| return fixer.replaceTextRange( | ||
| name.type === "String" | ||
| ? [name.range[0] + 1, name.range[1] - 1] | ||
| : name.range, | ||
| normalizedKey, | ||
| ); | ||
| }, |
There was a problem hiding this comment.
This would produce incorrect fix if there are escape sequences, e.g., \n:
/* eslint json/no-unnormalized-keys: [2, { form: "NFD" }] */
{
"\u1E9B\u0323\n": 42
}Fixed to invalid JSON:
/* eslint json/no-unnormalized-keys: [2, { form: "NFD" }] */
{
"ẛ̣
": 42
}Also, I'm not sure if autofixing \u1E9B\u0323 to ẛ̣ instead of a sequence with \uXXXX would be desirable.
There was a problem hiding this comment.
Your suggestion also makes sense to me, but if that's the case, I have two questions:
-
Should
data.keybe updated accordingly?It seems
data.keyis displayed the same way:
-
Is there a JavaScript (or other) rule I can refer to?
I don't have much experience with JSON (and JavaScript) rules, so I think I'm missing some edge cases while implementing the rule. Do you have any recommendations I could refer to when implementing the rule? If so, that would be very helpful.
There was a problem hiding this comment.
- Should
data.keybe updated accordingly?
I think it would be better to show the raw key representation (i.e., as it appears in the linted source code) in the error message.
There was a problem hiding this comment.
2. Is there a JavaScript (or other) rule I can refer to?
I think the most similar thing we have in JS rules is a utility that parses string literals:
https://github.com/eslint/eslint/blob/main/lib/rules/utils/char-source.js
So we could try making something similar for JSON and use it to check how individual characters were represented in the original key and then try preserving the same form (a character directly inserted into the source code or an escape sequence) in the fixed key. A problem that might be difficult to solve in this particular rule is how to map characters from the fixed key to characters in the original key since the normalizations produce strings with different lengths.
Another option is to limit the autofix to keys that don't have escape sequences only.
There was a problem hiding this comment.
Sorry for the delay. Your reference was very helpful, but because of the difficulty of handling edge cases, I've chosen a simpler approach: "Another option is to limit the autofix to keys that don't have escape sequences only".
I've addressed all suggestions here.
…rt-autofix-for-no-unnormalized-keys
There was a problem hiding this comment.
Pull request overview
This PR implements autofix functionality for the no-unnormalized-keys rule, which checks that JSON object keys are properly normalized according to Unicode normalization forms (NFC, NFD, NFKC, or NFKD). The implementation automatically fixes unnormalized keys by replacing them with their normalized equivalents, but intelligently skips autofix when escape sequences are present in the raw key to avoid breaking JSON syntax.
Changes:
- Added
fixable: "code"metadata and fix function to theno-unnormalized-keysrule - Modified error reporting to use raw key text for better clarity when escape sequences are present
- Added comprehensive test coverage for autofix with different quote types and normalization forms, plus tests verifying no autofix occurs when escape sequences are present
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/rules/no-unnormalized-keys.js | Implemented autofix logic with escape sequence detection, imported getRawKey utility, and updated error message to show raw keys |
| tests/rules/no-unnormalized-keys.test.js | Added output expectations for fixable cases, new valid test cases for escaped forms, and invalid test cases verifying escape sequences prevent autofix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3 participants
Prerequisites checklist
What is the purpose of this pull request?
In this PR, I've implemented an autofix for
no-unnormalized-keys.This PR closes #141.
What changes did you make? (Give an overview)
In this PR, I've implemented an autofix for
no-unnormalized-keys.Related Issues
Closes: #141
Is there anything you'd like reviewers to focus on?
N/A