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 1 commit
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
13 changes: 11 additions & 2 deletions lib/rules/id-length.js
View file
Open in desktop
Expand Up @@ -6,6 +6,13 @@

"use strict";

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

const splitter = new GraphemeSplitter();

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

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

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

if (!(isShort || isLong) || exceptions.has(name) || matchesExceptionPattern(name)) {
return; // Nothing to report
Expand Down
14 changes: 14 additions & 0 deletions tests/lib/rules/id-length.js
View file
Open in desktop
Expand Up @@ -113,6 +113,13 @@ 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 }],
parserOptions: { ecmaVersion: 6 }
}
],
invalid: [
Expand Down Expand Up @@ -564,6 +571,13 @@ ruleTester.run("id-length", rule, {
errors: [
tooLongErrorPrivate
]
},
{
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

}
]
});