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.

Sign up for GitHub

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

Jump to bottom

feat: id-length counts graphemes instead of code units #16321

Merged
merged 11 commits into from Sep 27, 2022
Merged

feat: id-length counts graphemes instead of code units #16321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
81deaf6
fix: counts graphemes instead of code units
sosukesuzuki Sep 17, 2022
39928ab
test: add more tests
sosukesuzuki Sep 17, 2022
51508d7
test: add more valid tests
sosukesuzuki Sep 18, 2022
04802f9
docs: mention about grapehemes
sosukesuzuki Sep 23, 2022
a6beb2e
fix: first check if string is ASCII
sosukesuzuki Sep 23, 2022
61d28d1
fix: use regex
sosukesuzuki Sep 24, 2022
9249132
Update docs/src/rules/id-length.md
sosukesuzuki Sep 27, 2022
cb84c8c
fix: add `max: 1` to valid tests
sosukesuzuki Sep 27, 2022
bc16453
fix: add tests for Variation Selectors
sosukesuzuki Sep 27, 2022
cd906c0
Update tests/lib/rules/id-length.js
sosukesuzuki Sep 27, 2022
edab15b
Update tests/lib/rules/id-length.js
sosukesuzuki Sep 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/src/rules/id-length.md
View file
Open in desktop
Expand Up @@ -20,6 +20,8 @@ var x = 5; // too short; difficult to understand its purpose without context

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

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).

## Options

Examples of **incorrect** code for this rule with the default options:
Expand Down
45 changes: 43 additions & 2 deletions lib/rules/id-length.js
View file
Open in desktop
Expand Up @@ -6,6 +6,45 @@

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------
const GraphemeSplitter = require("grapheme-splitter");

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

/**
* Checks if the string given as argument is ASCII or not.
* @param {string} value A string that you want to know if it is ASCII or not.
* @returns {boolean} `true` if `value` is ASCII string.
*/
function isASCII(value) {
if (typeof value !== "string") {
return false;
}
return /^[\u0020-\u007f]*$/u.test(value);
}

/** @type {GraphemeSplitter | undefined} */
let splitter;

/**
* Gets the length of the string. If the string is not in ASCII, counts graphemes.
* @param {string} value A string that you want to get the length.
* @returns {number} The length of `value`.
*/
function getStringLength(value) {
if (isASCII(value)) {
return value.length;
}
if (!splitter) {
splitter = new GraphemeSplitter();
}
return splitter.countGraphemes(value);
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -130,8 +169,10 @@ module.exports = {
const name = node.name;
const parent = node.parent;

const isShort = name.length < minLength;
const isLong = name.length > maxLength;
const nameLength = getStringLength(name);

const isShort = nameLength < minLength;
const isLong = nameLength > maxLength;

if (!(isShort || isLong) || exceptions.has(name) || matchesExceptionPattern(name)) {
return; // Nothing to report
Expand Down
268 changes: 268 additions & 0 deletions tests/lib/rules/id-length.js
View file
Open in desktop
Expand Up @@ -113,6 +113,123 @@ ruleTester.run("id-length", rule, {
code: "class Foo { #abc = 1 }",
options: [{ max: 3 }],
parserOptions: { ecmaVersion: 2022 }
},

// Identifier consisting of two code units
{
code: "var 𠮟 = 2",
options: [{ min: 1, max: 1 }],
parserOptions: { ecmaVersion: 6 }
},
{
code: "var 葛󠄀 = 2", // 2 code points but only 1 grapheme
options: [{ min: 1, max: 1 }],
parserOptions: { ecmaVersion: 6 }
},
{
code: "var a = { 𐌘: 1 };",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 6
}
},
{
code: "(𐌘) => { 𐌘 * 𐌘 };",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 6
}
},
{
code: "class 𠮟 { }",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 6
}
},
{
code: "class F { 𐌘() {} }",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 6
}
},
{
code: "class F { #𐌘() {} }",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 2022
}
},
{
code: "class F { 𐌘 = 1 }",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 2022
}
},
{
code: "class F { #𐌘 = 1 }",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 2022
}
},
{
code: "function f(...𐌘) { }",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 6
}
},
{
code: "function f([𐌘]) { }",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 6
}
},
{
code: "var [ 𐌘 ] = a;",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 6
}
},
{
code: "var { p: [𐌘]} = {};",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 6
}
},
{
code: "function f({𐌘}) { }",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 6
}
},
{
code: "var { 𐌘 } = {};",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 6
}
},
{
code: "var { p: 𐌘} = {};",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 6
}
},
{
code: "({ prop: o.𐌘 } = {});",
options: [{ min: 1, max: 1 }],
parserOptions: {
ecmaVersion: 6
}
}
],
invalid: [
Expand Down Expand Up @@ -564,6 +681,157 @@ ruleTester.run("id-length", rule, {
errors: [
tooLongErrorPrivate
]
},

// Identifier consisting of two code units
{
code: "var 𠮟 = 2",
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
Comment on lines +688 to +692
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we cover more test cases here? Playground Link 

var myObj = { 𐌘: 1 };

(𐌘) => { 𐌘 * 𐌘 };

class 𠮟 { }

class Foo { 𐌘() {} }

class Foo1 { #𐌘() {} }

class Foo2 { 𐌘 = 1 }

class Foo3 { #𐌘 = 1 }

function foo1(...𐌘) { }

function foo([𐌘]) { }

var [ 𐌘 ] = arr;

var { prop: [𐌘]} = {};

function foo({𐌘}) { }

var { 𐌘 } = {};

var { prop: 𐌘} = {};

({ prop: obj.𐌘 } = {});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! 39928ab

},
{
code: "var 葛󠄀 = 2", // 2 code points but only 1 grapheme
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
{
code: "var myObj = { 𐌘: 1 };",
parserOptions: {
ecmaVersion: 6
},
errors: [
tooShortError
]
},
{
code: "(𐌘) => { 𐌘 * 𐌘 };",
parserOptions: {
ecmaVersion: 6
},
errors: [
tooShortError
]
},
{
code: "class 𠮟 { }",
parserOptions: {
ecmaVersion: 6
},
errors: [
tooShortError
]
},
{
code: "class Foo { 𐌘() {} }",
parserOptions: {
ecmaVersion: 6
},
errors: [
tooShortError
]
},
{
code: "class Foo1 { #𐌘() {} }",
parserOptions: {
ecmaVersion: 2022
},
errors: [
tooShortErrorPrivate
]
},
{
code: "class Foo2 { 𐌘 = 1 }",
parserOptions: {
ecmaVersion: 2022
},
errors: [
tooShortError
]
},
{
code: "class Foo3 { #𐌘 = 1 }",
parserOptions: {
ecmaVersion: 2022
},
errors: [
tooShortErrorPrivate
]
},
{
code: "function foo1(...𐌘) { }",
parserOptions: {
ecmaVersion: 6
},
errors: [
tooShortError
]
},
{
code: "function foo([𐌘]) { }",
parserOptions: {
ecmaVersion: 6
},
errors: [
tooShortError
]
},
{
code: "var [ 𐌘 ] = arr;",
parserOptions: {
ecmaVersion: 6
},
errors: [
tooShortError
]
},
{
code: "var { prop: [𐌘]} = {};",
parserOptions: {
ecmaVersion: 6
},
errors: [
tooShortError
]
},
{
code: "function foo({𐌘}) { }",
parserOptions: {
ecmaVersion: 6
},
errors: [
tooShortError
]
},
{
code: "var { 𐌘 } = {};",
parserOptions: {
ecmaVersion: 6
},
errors: [
tooShortError
]
},
{
code: "var { prop: 𐌘} = {};",
parserOptions: {
ecmaVersion: 6
},
errors: [
tooShortError
]
},
{
code: "({ prop: obj.𐌘 } = {});",
parserOptions: {
ecmaVersion: 6
},
errors: [
tooShortError
]
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
}
]
});