jscs-dev / node-jscs

:arrow_heading_up: JavaScript Code Style checker (unmaintained)
https://jscs-dev.github.io
MIT License
4.96k stars 513 forks source link

Fix: fixup skipped requirePaddingNewlinesBeforeKeywords test #2179

Closed hzoo closed 8 years ago

hzoo commented 8 years ago

I thought this was a cst issue since I got an internal error?

But it was just removed in https://github.com/jscs-dev/node-jscs/commit/301232104111027106588f230c371672fb0ace43?

rules/require-padding-newlines-before-keywords true value should not report when returning a function:
     Expected not to have errors, but "internalError: Error running rule requirePaddingNewlinesBeforeKeywords: This is an issue with JSCS and not your codebase.
Please file an issue (with the stack trace below) at: https://github.com/jscs-dev/node-jscs/issues/new
Error: Expected end of node list but "FunctionExpression" found
    at ElementAssert.assertEnd (/Users/hzhu/dev/node-jscs/node_modules/cst/lib/elements/ElementAssert.js:208:23)
    at ReturnStatement._acceptChildren (/Users/hzhu/dev/node-jscs/node_modules/cst/lib/elements/types/ReturnStatement.js:49:22)
    at ReturnStatement._setChildren (/Users/hzhu/dev/node-jscs/node_modules/cst/lib/elements/Element.js:415:18)

@markelog

markelog commented 8 years ago

I think we should fix this on the cst side, can you port your changes over there?

hzoo commented 8 years ago

I didn't figure out why it was failing on the cst side so I looked at the original file

markelog commented 8 years ago

I'm thinking if that happened in this case it should happen elsewhere too

markelog commented 8 years ago

So what do you think? Still wanna do it here, regardless?

hzoo commented 8 years ago

Yeah although we can leave an issue open in cst

hzoo commented 8 years ago

return \n function() {} is invalid since it results in return undefined due to the newline, so merging