Skip to content

Commit 1cc4b3a

Browse files
feat: id-length counts graphemes instead of code units (#16321)
* fix: counts graphemes instead of code units * test: add more tests * test: add more valid tests * docs: mention about grapehemes * fix: first check if string is ASCII * fix: use regex * Update docs/src/rules/id-length.md Co-authored-by: Milos Djermanovic <[email protected]> * fix: add `max: 1` to valid tests * fix: add tests for Variation Selectors * Update tests/lib/rules/id-length.js Co-authored-by: Milos Djermanovic <[email protected]> * Update tests/lib/rules/id-length.js Co-authored-by: Milos Djermanovic <[email protected]> Co-authored-by: Milos Djermanovic <[email protected]>
1 parent f6d57fb commit 1cc4b3a

3 files changed

Lines changed: 313 additions & 2 deletions

File tree

docs/src/rules/id-length.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ var x = 5; // too short; difficult to understand its purpose without context
2020

2121
This rule enforces a minimum and/or maximum identifier length convention.
2222

23+
This rule counts [graphemes](https://unicode.org/reports/tr29/#Default_Grapheme_Cluster_Table) instead of using [`String length`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/length).
24+
2325
## Options
2426

2527
Examples of **incorrect** code for this rule with the default options:

lib/rules/id-length.js

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,45 @@
66

77
"use strict";
88

9+
//------------------------------------------------------------------------------
10+
// Requirements
11+
//------------------------------------------------------------------------------
12+
const GraphemeSplitter = require("grapheme-splitter");
13+
14+
//------------------------------------------------------------------------------
15+
// Helpers
16+
//------------------------------------------------------------------------------
17+
18+
/**
19+
* Checks if the string given as argument is ASCII or not.
20+
* @param {string} value A string that you want to know if it is ASCII or not.
21+
* @returns {boolean} `true` if `value` is ASCII string.
22+
*/
23+
function isASCII(value) {
24+
if (typeof value !== "string") {
25+
return false;
26+
}
27+
return /^[\u0020-\u007f]*$/u.test(value);
28+
}
29+
30+
/** @type {GraphemeSplitter | undefined} */
31+
let splitter;
32+
33+
/**
34+
* Gets the length of the string. If the string is not in ASCII, counts graphemes.
35+
* @param {string} value A string that you want to get the length.
36+
* @returns {number} The length of `value`.
37+
*/
38+
function getStringLength(value) {
39+
if (isASCII(value)) {
40+
return value.length;
41+
}
42+
if (!splitter) {
43+
splitter = new GraphemeSplitter();
44+
}
45+
return splitter.countGraphemes(value);
46+
}
47+
948
//------------------------------------------------------------------------------
1049
// Rule Definition
1150
//------------------------------------------------------------------------------
@@ -130,8 +169,10 @@ module.exports = {
130169
const name = node.name;
131170
const parent = node.parent;
132171

133-
const isShort = name.length < minLength;
134-
const isLong = name.length > maxLength;
172+
const nameLength = getStringLength(name);
173+
174+
const isShort = nameLength < minLength;
175+
const isLong = nameLength > maxLength;
135176

136177
if (!(isShort || isLong) || exceptions.has(name) || matchesExceptionPattern(name)) {
137178
return; // Nothing to report

tests/lib/rules/id-length.js

Lines changed: 268 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,123 @@ ruleTester.run("id-length", rule, {
113113
code: "class Foo { #abc = 1 }",
114114
options: [{ max: 3 }],
115115
parserOptions: { ecmaVersion: 2022 }
116+
},
117+
118+
// Identifier consisting of two code units
119+
{
120+
code: "var 𠮟 = 2",
121+
options: [{ min: 1, max: 1 }],
122+
parserOptions: { ecmaVersion: 6 }
123+
},
124+
{
125+
code: "var 葛󠄀 = 2", // 2 code points but only 1 grapheme
126+
options: [{ min: 1, max: 1 }],
127+
parserOptions: { ecmaVersion: 6 }
128+
},
129+
{
130+
code: "var a = { 𐌘: 1 };",
131+
options: [{ min: 1, max: 1 }],
132+
parserOptions: {
133+
ecmaVersion: 6
134+
}
135+
},
136+
{
137+
code: "(𐌘) => { 𐌘 * 𐌘 };",
138+
options: [{ min: 1, max: 1 }],
139+
parserOptions: {
140+
ecmaVersion: 6
141+
}
142+
},
143+
{
144+
code: "class 𠮟 { }",
145+
options: [{ min: 1, max: 1 }],
146+
parserOptions: {
147+
ecmaVersion: 6
148+
}
149+
},
150+
{
151+
code: "class F { 𐌘() {} }",
152+
options: [{ min: 1, max: 1 }],
153+
parserOptions: {
154+
ecmaVersion: 6
155+
}
156+
},
157+
{
158+
code: "class F { #𐌘() {} }",
159+
options: [{ min: 1, max: 1 }],
160+
parserOptions: {
161+
ecmaVersion: 2022
162+
}
163+
},
164+
{
165+
code: "class F { 𐌘 = 1 }",
166+
options: [{ min: 1, max: 1 }],
167+
parserOptions: {
168+
ecmaVersion: 2022
169+
}
170+
},
171+
{
172+
code: "class F { #𐌘 = 1 }",
173+
options: [{ min: 1, max: 1 }],
174+
parserOptions: {
175+
ecmaVersion: 2022
176+
}
177+
},
178+
{
179+
code: "function f(...𐌘) { }",
180+
options: [{ min: 1, max: 1 }],
181+
parserOptions: {
182+
ecmaVersion: 6
183+
}
184+
},
185+
{
186+
code: "function f([𐌘]) { }",
187+
options: [{ min: 1, max: 1 }],
188+
parserOptions: {
189+
ecmaVersion: 6
190+
}
191+
},
192+
{
193+
code: "var [ 𐌘 ] = a;",
194+
options: [{ min: 1, max: 1 }],
195+
parserOptions: {
196+
ecmaVersion: 6
197+
}
198+
},
199+
{
200+
code: "var { p: [𐌘]} = {};",
201+
options: [{ min: 1, max: 1 }],
202+
parserOptions: {
203+
ecmaVersion: 6
204+
}
205+
},
206+
{
207+
code: "function f({𐌘}) { }",
208+
options: [{ min: 1, max: 1 }],
209+
parserOptions: {
210+
ecmaVersion: 6
211+
}
212+
},
213+
{
214+
code: "var { 𐌘 } = {};",
215+
options: [{ min: 1, max: 1 }],
216+
parserOptions: {
217+
ecmaVersion: 6
218+
}
219+
},
220+
{
221+
code: "var { p: 𐌘} = {};",
222+
options: [{ min: 1, max: 1 }],
223+
parserOptions: {
224+
ecmaVersion: 6
225+
}
226+
},
227+
{
228+
code: "({ prop: o.𐌘 } = {});",
229+
options: [{ min: 1, max: 1 }],
230+
parserOptions: {
231+
ecmaVersion: 6
232+
}
116233
}
117234
],
118235
invalid: [
@@ -564,6 +681,157 @@ ruleTester.run("id-length", rule, {
564681
errors: [
565682
tooLongErrorPrivate
566683
]
684+
},
685+
686+
// Identifier consisting of two code units
687+
{
688+
code: "var 𠮟 = 2",
689+
parserOptions: { ecmaVersion: 6 },
690+
errors: [
691+
tooShortError
692+
]
693+
},
694+
{
695+
code: "var 葛󠄀 = 2", // 2 code points but only 1 grapheme
696+
parserOptions: { ecmaVersion: 6 },
697+
errors: [
698+
tooShortError
699+
]
700+
},
701+
{
702+
code: "var myObj = { 𐌘: 1 };",
703+
parserOptions: {
704+
ecmaVersion: 6
705+
},
706+
errors: [
707+
tooShortError
708+
]
709+
},
710+
{
711+
code: "(𐌘) => { 𐌘 * 𐌘 };",
712+
parserOptions: {
713+
ecmaVersion: 6
714+
},
715+
errors: [
716+
tooShortError
717+
]
718+
},
719+
{
720+
code: "class 𠮟 { }",
721+
parserOptions: {
722+
ecmaVersion: 6
723+
},
724+
errors: [
725+
tooShortError
726+
]
727+
},
728+
{
729+
code: "class Foo { 𐌘() {} }",
730+
parserOptions: {
731+
ecmaVersion: 6
732+
},
733+
errors: [
734+
tooShortError
735+
]
736+
},
737+
{
738+
code: "class Foo1 { #𐌘() {} }",
739+
parserOptions: {
740+
ecmaVersion: 2022
741+
},
742+
errors: [
743+
tooShortErrorPrivate
744+
]
745+
},
746+
{
747+
code: "class Foo2 { 𐌘 = 1 }",
748+
parserOptions: {
749+
ecmaVersion: 2022
750+
},
751+
errors: [
752+
tooShortError
753+
]
754+
},
755+
{
756+
code: "class Foo3 { #𐌘 = 1 }",
757+
parserOptions: {
758+
ecmaVersion: 2022
759+
},
760+
errors: [
761+
tooShortErrorPrivate
762+
]
763+
},
764+
{
765+
code: "function foo1(...𐌘) { }",
766+
parserOptions: {
767+
ecmaVersion: 6
768+
},
769+
errors: [
770+
tooShortError
771+
]
772+
},
773+
{
774+
code: "function foo([𐌘]) { }",
775+
parserOptions: {
776+
ecmaVersion: 6
777+
},
778+
errors: [
779+
tooShortError
780+
]
781+
},
782+
{
783+
code: "var [ 𐌘 ] = arr;",
784+
parserOptions: {
785+
ecmaVersion: 6
786+
},
787+
errors: [
788+
tooShortError
789+
]
790+
},
791+
{
792+
code: "var { prop: [𐌘]} = {};",
793+
parserOptions: {
794+
ecmaVersion: 6
795+
},
796+
errors: [
797+
tooShortError
798+
]
799+
},
800+
{
801+
code: "function foo({𐌘}) { }",
802+
parserOptions: {
803+
ecmaVersion: 6
804+
},
805+
errors: [
806+
tooShortError
807+
]
808+
},
809+
{
810+
code: "var { 𐌘 } = {};",
811+
parserOptions: {
812+
ecmaVersion: 6
813+
},
814+
errors: [
815+
tooShortError
816+
]
817+
},
818+
{
819+
code: "var { prop: 𐌘} = {};",
820+
parserOptions: {
821+
ecmaVersion: 6
822+
},
823+
errors: [
824+
tooShortError
825+
]
826+
},
827+
{
828+
code: "({ prop: obj.𐌘 } = {});",
829+
parserOptions: {
830+
ecmaVersion: 6
831+
},
832+
errors: [
833+
tooShortError
834+
]
567835
}
568836
]
569837
});

0 commit comments

Comments
 (0)