search

jscs-dev / babel-jscs

(deprecated) JSCS has merged with ESLint, so check out babel-eslint!
https://github.com/babel/babel-eslint
MIT License
27 stars 6 forks source link

Shorthand methods don't cooperate with space checks #4

Closed mgol closed 9 years ago

mgol commented 9 years ago

The following code:

var o = {
  f() {},
};

with .jscsrc:

{
    "esprima": "./node_modules/babel-jscs",

    "requireSpacesInAnonymousFunctionExpression": {
        "beforeOpeningRoundBrace": true
    }
}

errors:

Missing space before opening round brace at app/modules/services/selection.js :
     1 |var o = {
     2 |    f() {},
--------------^
     3 |};
     4 |
hzoo commented 9 years ago

Are you on jscs master branch? Also, can you reproduce without babel-jscs?

mgol commented 9 years ago

It doesn't work for me even with:

    "jscs": "git://github.com/jscs-dev/node-jscs.git#8de3850aff54e1519bd196a0627ee1cc3b6a96a0",

I get errors for both this rule & validateQuoteMarks.

mgol commented 9 years ago

It works without babel-jscs if I set "esprima": true, both with JSCS 1.13.1 & from master.

hzoo commented 9 years ago

I can reproduce with the regular parser + esnext. So an jscs issue (just need to check node.parent.method === true) - I'l make a PR later.

hzoo commented 9 years ago

Actually @mzgol isn't the error correct anyway?

The issue to me is that f () {}, (a shorthand method with a space might also error) which I will fix

mgol commented 9 years ago

Actually @mzgol isn't the error correct anyway?

I don't think so; requireSpacesInAnonymousFunctionExpression should affect only anonymous expressions. IMO the idea behind this rule is to always separate function and the brace, not the function name and the brace.

hzoo commented 9 years ago

I guess it's because in the parser these are FunctionExpression with id === null so in that sense they are anonymous functions because the parent is type Property and parent.method === true

hzoo commented 9 years ago

I guess it depends on whether we think it should skip the rule when encountering a shorthand method (do we want to add a requireSpacesInGetter, requireSpacesInSetter, requireSpacesInShorthandMethod, requireSpacesInClassMethod and same for disallow)

@mrjoelkemp

Related is https://github.com/jscs-dev/node-jscs/issues/1066