Skip to content
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

feat: add rule no-unnormalized-keys #38

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bmeck
Copy link

@bmeck bmeck commented Oct 7, 2024

Prerequisites checklist

What is the purpose of this pull request?

To add a rule to check for JSON keys that are not normalized.

What changes did you make? (Give an overview)

Added a rule to check for JSON keys that are not normalized.

Related Issues

Fixes #32

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

Couldn't get options working on invalid test cases?

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

Can you add the rule to the plugin (otherwise, it could not be used):

json/src/index.js

Lines 29 to 32 in cac8847

rules: {
"no-duplicate-keys": noDuplicateKeys,
"no-empty-keys": noEmptyKeys,
},

And, also add it to the docs:

json/README.md

Lines 153 to 156 in cac8847

## Rules
- `no-duplicate-keys` - warns when there are two keys in an object with the same text.
- `no-empty-keys` - warns when there is a key in an object that is an empty string or contains only whitespace (note: `package-lock.json` uses empty keys intentionally)

@@ -0,0 +1,61 @@
/**
* @fileoverview Tests for no-empty-keys rule.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @fileoverview Tests for no-empty-keys rule.
* @fileoverview Tests for no-unnormalized-keys rule.

Comment on lines +18 to +25
schema: {
type: "array",
minItems: 0,
maxItems: 1,
items: {
enum: ["NFC", "NFD", "NFKC", "NFKD"],
},
},
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
schema: {
type: "array",
minItems: 0,
maxItems: 1,
items: {
enum: ["NFC", "NFD", "NFKC", "NFKD"],
},
},
schema: [
{
enum: ["NFC", "NFD", "NFKC", "NFKD"],
},
],

This can be simplified by using the shorthand form (https://eslint.org/docs/latest/extend/custom-rules#options-schemas, the first format).

Comment on lines +34 to +37
const key =
node.name.type === "String"
? node.name.value
: node.name.name;
Copy link
Member

Choose a reason for hiding this comment

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

Can you add an invalid test where node.name is an Identifier node?

@mdjermanovic mdjermanovic changed the title feat: rule no-unormalized-keys feat: add rule no-unnormalized-keys Oct 13, 2024
@nzakas
Copy link
Member

nzakas commented Oct 18, 2024

@bmeck there's feedback for you here.

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

Successfully merging this pull request may close these issues.

New rule: Normalize string keys
3 participants