Skip to content
Permalink
Browse files

Fix: `keyword-spacing` false positive in template strings (fixes #5043)

  • Loading branch information...
mysticatea committed Jan 24, 2016
1 parent 346a135 commit b0f21a0582a998a0391e80d7651ecafea0d0a4c1
Showing with 72 additions and 0 deletions.
  1. +30 −0 lib/rules/keyword-spacing.js
  2. +42 −0 tests/lib/rules/keyword-spacing.js
@@ -22,6 +22,8 @@ var PREV_TOKEN = /^[\)\]\}>]$/;
var NEXT_TOKEN = /^(?:[\(\[\{<~!]|\+\+?|--?)$/;
var PREV_TOKEN_M = /^[\)\]\}>*]$/;
var NEXT_TOKEN_M = /^[\{*]$/;
var TEMPLATE_OPEN_PAREN = /\$\{$/;
var TEMPLATE_CLOSE_PAREN = /^\}/;
var CHECK_TYPE = /^(?:JSXElement|RegularExpression|String|Template)$/;
var KEYS = keywords.concat(["as", "await", "from", "get", "let", "of", "set", "yield"]);

@@ -35,6 +37,30 @@ var KEYS = keywords.concat(["as", "await", "from", "get", "let", "of", "set", "y
}
}());

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

/**
* Checks whether or not a given token is a "Template" token ends with "${".
*
* @param {Token} token - A token to check.
* @returns {boolean} `true` if the token is a "Template" token ends with "${".
*/
function isOpenParenOfTemplate(token) {
return token.type === "Template" && TEMPLATE_OPEN_PAREN.test(token.value);
}

/**
* Checks whether or not a given token is a "Template" token starts with "}".
*
* @param {Token} token - A token to check.
* @returns {boolean} `true` if the token is a "Template" token starts with "}".
*/
function isCloseParenOfTemplate(token) {
return token.type === "Template" && TEMPLATE_CLOSE_PAREN.test(token.value);
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
@@ -56,6 +82,7 @@ module.exports = function(context) {
var prevToken = sourceCode.getTokenBefore(token);
if (prevToken &&
(CHECK_TYPE.test(prevToken.type) || pattern.test(prevToken.value)) &&
!isOpenParenOfTemplate(prevToken) &&
astUtils.isTokenOnSameLine(prevToken, token) &&
!sourceCode.isSpaceBetweenTokens(prevToken, token)
) {
@@ -84,6 +111,7 @@ module.exports = function(context) {
var prevToken = sourceCode.getTokenBefore(token);
if (prevToken &&
(CHECK_TYPE.test(prevToken.type) || pattern.test(prevToken.value)) &&
!isOpenParenOfTemplate(prevToken) &&
astUtils.isTokenOnSameLine(prevToken, token) &&
sourceCode.isSpaceBetweenTokens(prevToken, token)
) {
@@ -112,6 +140,7 @@ module.exports = function(context) {
var nextToken = sourceCode.getTokenAfter(token);
if (nextToken &&
(CHECK_TYPE.test(nextToken.type) || pattern.test(nextToken.value)) &&
!isCloseParenOfTemplate(nextToken) &&
astUtils.isTokenOnSameLine(token, nextToken) &&
!sourceCode.isSpaceBetweenTokens(token, nextToken)
) {
@@ -140,6 +169,7 @@ module.exports = function(context) {
var nextToken = sourceCode.getTokenAfter(token);
if (nextToken &&
(CHECK_TYPE.test(nextToken.type) || pattern.test(nextToken.value)) &&
!isCloseParenOfTemplate(nextToken) &&
astUtils.isTokenOnSameLine(token, nextToken) &&
sourceCode.isSpaceBetweenTokens(token, nextToken)
) {
@@ -235,6 +235,10 @@ ruleTester.run("keyword-spacing", rule, {
{code: "!class {}", parserOptions: {ecmaVersion: 6}},
{code: "! class{}", options: [NEITHER], parserOptions: {ecmaVersion: 6}},

// not conflict with `template-curly-spacing`
{code: "`${class {}}`", parserOptions: {ecmaVersion: 6}},
{code: "`${ class{}}`", options: [NEITHER], parserOptions: {ecmaVersion: 6}},

// not conflict with `jsx-curly-spacing`
{code: "<Foo onClick={class {}} />", parserOptions: {ecmaVersion: 6, ecmaFeatures: {jsx: true}}},
{code: "<Foo onClick={ class{}} />", options: [NEITHER], parserOptions: {ecmaVersion: 6, ecmaFeatures: {jsx: true}}},
@@ -362,6 +366,10 @@ ruleTester.run("keyword-spacing", rule, {
{code: "!delete(foo.a)"},
{code: "! delete (foo.a)", options: [NEITHER]},

// not conflict with `template-curly-spacing`
{code: "`${delete foo.a}`", parserOptions: {ecmaVersion: 6}},
{code: "`${ delete foo.a}`", options: [NEITHER], parserOptions: {ecmaVersion: 6}},

// not conflict with `jsx-curly-spacing`
{code: "<Foo onClick={delete foo.a} />", parserOptions: {ecmaFeatures: {jsx: true}}},
{code: "<Foo onClick={ delete foo.a} />", options: [NEITHER], parserOptions: {ecmaFeatures: {jsx: true}}},
@@ -551,6 +559,10 @@ ruleTester.run("keyword-spacing", rule, {
{code: "!function() {}"},
{code: "! function() {}", options: [NEITHER]},

// not conflict with `template-curly-spacing`
{code: "`${function() {}}`", parserOptions: {ecmaVersion: 6}},
{code: "`${ function() {}}`", options: [NEITHER], parserOptions: {ecmaVersion: 6}},

// not conflict with `jsx-curly-spacing`
{code: "<Foo onClick={function() {}} />", parserOptions: {ecmaFeatures: {jsx: true}}},
{code: "<Foo onClick={ function() {}} />", options: [NEITHER], parserOptions: {ecmaFeatures: {jsx: true}}},
@@ -702,6 +714,10 @@ ruleTester.run("keyword-spacing", rule, {
{code: "!new(foo)()"},
{code: "! new (foo)()", options: [NEITHER]},

// not conflict with `template-curly-spacing`
{code: "`${new foo()}`", parserOptions: {ecmaVersion: 6}},
{code: "`${ new foo()}`", options: [NEITHER], parserOptions: {ecmaVersion: 6}},

// not conflict with `jsx-curly-spacing`
{code: "<Foo onClick={new foo()} />", parserOptions: {ecmaFeatures: {jsx: true}}},
{code: "<Foo onClick={ new foo()} />", options: [NEITHER], parserOptions: {ecmaFeatures: {jsx: true}}},
@@ -822,6 +838,10 @@ ruleTester.run("keyword-spacing", rule, {
{code: "class A { a() { !super } }", parserOptions: {ecmaVersion: 6}},
{code: "class A { a() { ! super } }", options: [NEITHER], parserOptions: {ecmaVersion: 6}},

// not conflict with `template-curly-spacing`
{code: "class A { a() { `${super}` } }", parserOptions: {ecmaVersion: 6}},
{code: "class A { a() { `${ super }` } }", options: [NEITHER], parserOptions: {ecmaVersion: 6}},

// not conflict with `jsx-curly-spacing`
{code: "class A { a() { <Foo onClick={super} /> } }", parserOptions: {ecmaVersion: 6, ecmaFeatures: {jsx: true}}},
{code: "class A { a() { <Foo onClick={ super } /> } }", options: [NEITHER], parserOptions: {ecmaVersion: 6, ecmaFeatures: {jsx: true}}},
@@ -894,6 +914,10 @@ ruleTester.run("keyword-spacing", rule, {
{code: "!this"},
{code: "! this", options: [NEITHER]},

// not conflict with `template-curly-spacing`
{code: "`${this}`", parserOptions: {ecmaVersion: 6}},
{code: "`${ this }`", options: [NEITHER], parserOptions: {ecmaVersion: 6}},

// not conflict with `jsx-curly-spacing`
{code: "<Foo onClick={this} />", parserOptions: {ecmaFeatures: {jsx: true}}},
{code: "<Foo onClick={ this } />", options: [NEITHER], parserOptions: {ecmaFeatures: {jsx: true}}},
@@ -985,6 +1009,10 @@ ruleTester.run("keyword-spacing", rule, {
{code: "!typeof+foo"},
{code: "! typeof +foo", options: [NEITHER]},

// not conflict with `template-curly-spacing`
{code: "`${typeof foo}`", parserOptions: {ecmaVersion: 6}},
{code: "`${ typeof foo}`", options: [NEITHER], parserOptions: {ecmaVersion: 6}},

// not conflict with `jsx-curly-spacing`
{code: "<Foo onClick={typeof foo} />", parserOptions: {ecmaFeatures: {jsx: true}}},
{code: "<Foo onClick={ typeof foo} />", options: [NEITHER], parserOptions: {ecmaFeatures: {jsx: true}}},
@@ -1058,6 +1086,10 @@ ruleTester.run("keyword-spacing", rule, {
{code: "!void+foo"},
{code: "! void +foo", options: [NEITHER]},

// not conflict with `template-curly-spacing`
{code: "`${void foo}`", parserOptions: {ecmaVersion: 6}},
{code: "`${ void foo}`", options: [NEITHER], parserOptions: {ecmaVersion: 6}},

// not conflict with `jsx-curly-spacing`
{code: "<Foo onClick={void foo} />", parserOptions: {ecmaFeatures: {jsx: true}}},
{code: "<Foo onClick={ void foo} />", options: [NEITHER], parserOptions: {ecmaFeatures: {jsx: true}}},
@@ -1155,6 +1187,10 @@ ruleTester.run("keyword-spacing", rule, {
{code: "function* foo() { yield+foo }", parserOptions: {ecmaVersion: 6}},
{code: "function* foo() { yield +foo }", options: [NEITHER], parserOptions: {ecmaVersion: 6}},

// not conflict with `template-curly-spacing`
{code: "`${yield}`", parserOptions: {ecmaVersion: 6}},
{code: "`${ yield}`", options: [NEITHER], parserOptions: {ecmaVersion: 6}},

// not conflict with `jsx-curly-spacing`
{code: "function* foo() { <Foo onClick={yield} /> }", parserOptions: {ecmaVersion: 6, ecmaFeatures: {jsx: true}}},
{code: "function* foo() { <Foo onClick={ yield } /> }", options: [NEITHER], parserOptions: {ecmaVersion: 6, ecmaFeatures: {jsx: true}}}
@@ -1725,6 +1761,12 @@ ruleTester.run("keyword-spacing", rule, {
options: [override("extends", NEITHER)],
parserOptions: {ecmaVersion: 6}
},
{
code: "class Bar extends`}` {}",
output: "class Bar extends `}` {}",
errors: expectedAfter("extends"),
parserOptions: {ecmaVersion: 6}
},

//----------------------------------------------------------------------
// finally

0 comments on commit b0f21a0

Please sign in to comment.
You can’t perform that action at this time.