From 72221f1fa306f39ff60e5e4b20071f203b421b1f Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Fri, 22 Jul 2022 14:24:42 +0530 Subject: [PATCH 1/5] fix: improve the key width calculation in `key-spacing` rule --- lib/rules/key-spacing.js | 19 ++++++++++++++++--- package.json | 1 + tests/lib/rules/key-spacing.js | 29 +++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/lib/rules/key-spacing.js b/lib/rules/key-spacing.js index a33ef29891e..0d1ee0911d4 100644 --- a/lib/rules/key-spacing.js +++ b/lib/rules/key-spacing.js @@ -9,6 +9,7 @@ //------------------------------------------------------------------------------ const astUtils = require("./utils/ast-utils"); +const GraphemeSplitter = require("grapheme-splitter"); //------------------------------------------------------------------------------ // Helpers @@ -505,10 +506,22 @@ module.exports = { * @returns {int} Width of the key. */ function getKeyWidth(property) { - const startToken = sourceCode.getFirstToken(property); - const endToken = getLastTokenBeforeColon(property.key); + const splitter = new GraphemeSplitter(); - return endToken.range[1] - startToken.range[0]; + if (property.computed) { + const startToken = sourceCode.getFirstToken(property); + const endToken = getLastTokenBeforeColon(property.key); + const allTokens = sourceCode.getTokensBetween(startToken, endToken); + + const keyWidth = allTokens.reduce( + (width, token) => width + splitter.countGraphemes(token.value), + splitter.countGraphemes(startToken.value) + splitter.countGraphemes(endToken.value) + ); + + return keyWidth; + } + + return splitter.countGraphemes(property.key.name || property.key.raw); } /** diff --git a/package.json b/package.json index 0cac453903f..d2306de9205 100644 --- a/package.json +++ b/package.json @@ -73,6 +73,7 @@ "functional-red-black-tree": "^1.0.1", "glob-parent": "^6.0.1", "globals": "^13.15.0", + "grapheme-splitter": "^1.0.4", "ignore": "^5.2.0", "import-fresh": "^3.0.0", "imurmurhash": "^0.1.4", diff --git a/tests/lib/rules/key-spacing.js b/tests/lib/rules/key-spacing.js index fde9673a63b..45a458ea8a4 100644 --- a/tests/lib/rules/key-spacing.js +++ b/tests/lib/rules/key-spacing.js @@ -896,6 +896,35 @@ ruleTester.run("key-spacing", rule, { on: "value" } }] + }, + + // https://github.com/eslint/eslint/issues/15914 + { + code: ` + var foo = { + "a": "bar", + "𐌘": "baz" + }; + `, + options: [{ + align: { + on: "value" + } + }] + }, + { + code: ` + const foo = { + "a": "bar", + [𐌘]: "baz" + }; + `, + options: [{ + align: { + on: "value" + } + }], + parserOptions: { ecmaVersion: 6 } } ], invalid: [{ From 22e01e2024e6c03f3e3d279ebb2bbd5c69a8f6ca Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Sat, 23 Jul 2022 09:03:48 +0530 Subject: [PATCH 2/5] feat: improve the key width calculation in key-spacing rule --- lib/rules/key-spacing.js | 17 ++----- tests/lib/rules/key-spacing.js | 86 +++++++++++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 16 deletions(-) diff --git a/lib/rules/key-spacing.js b/lib/rules/key-spacing.js index 0d1ee0911d4..92017ebaeff 100644 --- a/lib/rules/key-spacing.js +++ b/lib/rules/key-spacing.js @@ -507,21 +507,10 @@ module.exports = { */ function getKeyWidth(property) { const splitter = new GraphemeSplitter(); + const startToken = sourceCode.getFirstToken(property); + const endToken = getLastTokenBeforeColon(property.key); - if (property.computed) { - const startToken = sourceCode.getFirstToken(property); - const endToken = getLastTokenBeforeColon(property.key); - const allTokens = sourceCode.getTokensBetween(startToken, endToken); - - const keyWidth = allTokens.reduce( - (width, token) => width + splitter.countGraphemes(token.value), - splitter.countGraphemes(startToken.value) + splitter.countGraphemes(endToken.value) - ); - - return keyWidth; - } - - return splitter.countGraphemes(property.key.name || property.key.raw); + return splitter.countGraphemes(sourceCode.getText().slice(startToken.range[0], endToken.range[1])); } /** diff --git a/tests/lib/rules/key-spacing.js b/tests/lib/rules/key-spacing.js index 45a458ea8a4..61ef14a1ea7 100644 --- a/tests/lib/rules/key-spacing.js +++ b/tests/lib/rules/key-spacing.js @@ -925,8 +925,21 @@ ruleTester.run("key-spacing", rule, { } }], parserOptions: { ecmaVersion: 6 } - } - ], + }, + { + code: ` + const foo = { + "abc": "bar", + [ 𐌘 ]: "baz" + }; + `, + options: [{ + align: { + on: "value" + } + }], + parserOptions: { ecmaVersion: 6 } + }], invalid: [{ code: "var a ={'key' : value };", output: "var a ={'key':value };", @@ -2232,5 +2245,74 @@ ruleTester.run("key-spacing", rule, { { messageId: "missingValue", data: { computed: "", key: "bar" }, line: 3, column: 12, type: "Literal" }, { messageId: "missingValue", data: { computed: "", key: "baz" }, line: 3, column: 20, type: "Literal" } ] + }, + { + code: ` + const foo = { + "a": "bar", + [ 𐌘 ]: "baz" + }; + `, + output: ` + const foo = { + "a": "bar", + [ 𐌘 ]: "baz" + }; + `, + options: [{ + align: { + on: "value" + } + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { messageId: "missingValue", data: { computed: "", key: "a" }, line: 3, column: 22, type: "Literal" } + ] + }, + { + code: ` + const foo = { + "a": "bar", + [ 𐌘 ]: "baz" + }; + `, + output: ` + const foo = { + "a" : "bar", + [ 𐌘 ]: "baz" + }; + `, + options: [{ + align: { + on: "colon" + } + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { messageId: "missingKey", data: { computed: "", key: "a" }, line: 3, column: 17, type: "Literal" } + ] + }, + { + code: ` + const foo = { + "a": "bar", + "𐌘": "baz" + }; + `, + output: ` + const foo = { + "a": "bar", + "𐌘": "baz" + }; + `, + options: [{ + align: { + on: "value" + } + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { messageId: "extraValue", data: { computed: "", key: "a" }, line: 3, column: 20, type: "Literal" } + ] }] }); From 31fec4a5a3574039a971fdee1f4b89fa1db30dcd Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Sun, 24 Jul 2022 06:37:56 +0530 Subject: [PATCH 3/5] test: add more cases --- tests/lib/rules/key-spacing.js | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/lib/rules/key-spacing.js b/tests/lib/rules/key-spacing.js index 61ef14a1ea7..b679f2ab21e 100644 --- a/tests/lib/rules/key-spacing.js +++ b/tests/lib/rules/key-spacing.js @@ -912,6 +912,38 @@ ruleTester.run("key-spacing", rule, { } }] }, + { + code: ` + var foo = { + "a": "bar", + "Á": "baz", + "o͂": "qux", + "mĖ…": "xyz", + "ř": "abc" + + }; + `, + options: [{ + align: { + on: "value" + } + }] + }, + { + code: ` + var foo = { + "🌷": "bar", // 2 code points + "🎁": "baz", // 2 code points + "ðŸ‡ŪðŸ‡ģ": "qux", // 4 code points + "ðŸģïļâ€ðŸŒˆ": "xyz", // 6 code points + }; + `, + options: [{ + align: { + on: "value" + } + }] + }, { code: ` const foo = { From fcdad004662c6d10b9598ba30b8fa4957067fab3 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Sun, 24 Jul 2022 06:43:02 +0530 Subject: [PATCH 4/5] test: add more cases --- tests/lib/rules/key-spacing.js | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/tests/lib/rules/key-spacing.js b/tests/lib/rules/key-spacing.js index b679f2ab21e..dd0fcbc4dcf 100644 --- a/tests/lib/rules/key-spacing.js +++ b/tests/lib/rules/key-spacing.js @@ -2346,5 +2346,34 @@ ruleTester.run("key-spacing", rule, { errors: [ { messageId: "extraValue", data: { computed: "", key: "a" }, line: 3, column: 20, type: "Literal" } ] - }] + }, + { + code: ` + var foo = { + "🌷": "bar", // 2 code points + "🎁": "baz", // 2 code points + "ðŸ‡ŪðŸ‡ģ": "qux", // 4 code points + "ðŸģïļâ€ðŸŒˆ": "xyz", // 6 code points + }; + `, + output: ` + var foo = { + "🌷": "bar", // 2 code points + "🎁": "baz", // 2 code points + "ðŸ‡ŪðŸ‡ģ": "qux", // 4 code points + "ðŸģïļâ€ðŸŒˆ": "xyz", // 6 code points + }; + `, + options: [{ + align: { + on: "value" + } + }], + errors: [ + { messageId: "extraValue", data: { computed: "", key: "🌷" }, line: 3, column: 21, type: "Literal" }, + { messageId: "extraValue", data: { computed: "", key: "🎁" }, line: 4, column: 21, type: "Literal" }, + { messageId: "extraValue", data: { computed: "", key: "ðŸ‡ŪðŸ‡ģ" }, line: 5, column: 23, type: "Literal" } + ] + } + ] }); From 269fdb510152fae559b35c1489d5848f22d31935 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Wed, 27 Jul 2022 09:26:17 +0530 Subject: [PATCH 5/5] refactor: improve performance --- lib/rules/key-spacing.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/rules/key-spacing.js b/lib/rules/key-spacing.js index 92017ebaeff..b764b7282ef 100644 --- a/lib/rules/key-spacing.js +++ b/lib/rules/key-spacing.js @@ -11,6 +11,8 @@ const astUtils = require("./utils/ast-utils"); const GraphemeSplitter = require("grapheme-splitter"); +const splitter = new GraphemeSplitter(); + //------------------------------------------------------------------------------ // Helpers //------------------------------------------------------------------------------ @@ -506,7 +508,6 @@ module.exports = { * @returns {int} Width of the key. */ function getKeyWidth(property) { - const splitter = new GraphemeSplitter(); const startToken = sourceCode.getFirstToken(property); const endToken = getLastTokenBeforeColon(property.key);