This repository has been archived by the owner. It is now read-only.

v2.10.0 - `requireSpaceBeforeKeywords` reports on a keyword not in its list #2135

Closed
ljharb opened this Issue Feb 15, 2016 · 8 comments

Comments

Projects
None yet
2 participants
@ljharb
Contributor

ljharb commented Feb 15, 2016

In my config file, I have:

       "requireSpaceBeforeKeywords": [
         "do",
         "for",
         "if",
         "else",
         "switch",
         "case",
         "try",
         "catch",
         "finally",
         "while",
         "with",
         "return"
       ],

However, upon upgrading to v2.10.0, I get output like:

Missing space before "typeof" keyword at index.js :
    32 |module.exports = function isCallable(value) {
    33 | if (!value) { return false; }
    34 | if (typeof value !== 'function' && typeof value !== 'object') { return false; }
-------------^
    35 | if (hasToStringTag) { return tryFunctionObject(value); }
    36 | if (isES6ClassFn(value)) { return false; }

Missing space before "typeof" keyword at test.js :
    96 |test('Typed Arrays', function (st) {
    97 | forEach(typedArrayNames, function (typedArray) {
    98 |  if (typeof global[typedArray] !== 'undefined') {
--------------^
    99 |   st.ok(isCallable(global[typedArray]), typedArray + ' is callable');
   100 |  }

Note that "typeof" is not in my array of things I want a space before, and this was not an error in v2.9.x.

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Feb 15, 2016

Member

Ok that is really weird.. I haven't checked all the commits so will look through. @markelog

Member

hzoo commented Feb 15, 2016

Ok that is really weird.. I haven't checked all the commits so will look through. @markelog

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Feb 15, 2016

Contributor

I do note in the changelog that this rule was changed - perhaps it's an unintended consequence?

Contributor

ljharb commented Feb 15, 2016

I do note in the changelog that this rule was changed - perhaps it's an unintended consequence?

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Feb 15, 2016

Member

Yeah definitely needs a fix.

Member

hzoo commented Feb 15, 2016

Yeah definitely needs a fix.

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Feb 15, 2016

Member

Ok probably #2063

Member

hzoo commented Feb 15, 2016

Ok probably #2063

@ljharb ljharb referenced this issue Feb 15, 2016

Closed

fix Issue2041 #2063

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Feb 15, 2016

Contributor

commented on the line responsible, hopefully the fix includes a regression test ;-)

Contributor

ljharb commented Feb 15, 2016

commented on the line responsible, hopefully the fix includes a regression test ;-)

zxqfox added a commit to zxqfox/node-jscs that referenced this issue Feb 15, 2016

@markelog markelog closed this in 594a4ee Feb 15, 2016

zxqfox added a commit to zxqfox/node-jscs that referenced this issue Feb 15, 2016

zxqfox added a commit to zxqfox/node-jscs that referenced this issue Feb 15, 2016

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Feb 15, 2016

Contributor

Thanks, any idea how long before a v2.10.1?

Contributor

ljharb commented Feb 15, 2016

Thanks, any idea how long before a v2.10.1?

zxqfox added a commit that referenced this issue Feb 15, 2016

zxqfox added a commit that referenced this issue Feb 15, 2016

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Feb 15, 2016

Member

Was busy, I think we were making sure it was correct

Member

hzoo commented Feb 15, 2016

Was busy, I think we were making sure it was correct

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Feb 15, 2016

Contributor

Sounds good, just looking for a timeline :-) thanks for the fast responses everyone!

Contributor

ljharb commented Feb 15, 2016

Sounds good, just looking for a timeline :-) thanks for the fast responses everyone!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.