kellyselden / broccoli-jscs

Broccoli plugin for jscs
MIT License
16 stars 16 forks source link

JS files starting with import Ember from 'ember' cause error with disallowParenthesesAroundArrowParam #42

Closed adam-knights closed 9 years ago

adam-knights commented 9 years ago

A number of my jscs tests fail when running ember test. I am using the broccoli-jscs node package to generate and run them.

The issue does not appear when the tests are run in chrome but does appear when ran through phantomjs and ember test > results.tap appears in both Chrome and phantomJS via ember test > results.tap. Not all of my jscs tests fail, only those that have first line:

import Ember from 'ember';

Example file that causes jscs to fall over:

import Ember from 'ember';

// Inputs
// none

export default Ember.TextField.extend({
  didInsertElement: function() {
    Ember.run(() => {
      this.$().focus();
    });
  }
});

The stack trace:

components/forms/focus-input.js should pass jscs. Error running rule disallowParenthesesAroundArrowParam: 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 TypeError: Cannot read property 'type' of undefined at /var/lib/jenkins/workspace/Harrier Frontend (Feature Branch)/node_modules/broccoli-jscs/node_modules/jscs/lib/rules/disallow-parentheses-around-arrow-param.js:62:59 at Array.forEach (native) at Object.JsFile.iterateNodesByType (/var/lib/jenkins/workspace/Harrier Frontend (Feature Branch)/node_modules/broccoli-jscs/node_modules/jscs/lib/js-file.js:488:42) at Object.module.exports.check (/var/lib/jenkins/workspace/Harrier Frontend (Feature Branch)/node_modules/broccoli-jscs/node_modules/jscs/lib/rules/disallow-parentheses-around-arrow-param.js:57:14) at null. (/var/lib/jenkins/workspace/Harrier Frontend (Feature Branch)/node_modules/broccoli-jscs/node_modules/jscs/lib/string-checker.js:165:22) at Array.forEach (native) at StringChecker._checkJsFile (/var/lib/jenkins/workspace/Harrier Frontend (Feature Branch)/node_modules/broccoli-jscs/node_modules/jscs/lib/string-checker.js:161:31) at StringChecker.checkString (/var/lib/jenkins/workspace/Harrier Frontend (Feature Branch)/node_modules/broccoli-jscs/node_modules/jscs/lib/string-checker.js:106:14) at JSCSFilter.processString (/var/lib/jenkins/workspace/Harrier Frontend (Feature Branch)/node_modules/broccoli-jscs/index.js:72:31) at /var/lib/jenkins/workspace/Harrier Frontend (Feature Branch)/node_modules/broccoli-jscs/node_modules/broccoli-persistent-filter/lib/strategies/persistent.js:31:42 at components/forms/focus-input.js : 1 |import Ember from 'ember'; --------^ 2 | 3 |// Inputs

And finally my jscsrc file:

{
  "disallowKeywords": ["with"],
  "disallowMixedSpacesAndTabs": true,
  "disallowMultipleVarDecl": "exceptUndefined",
  "disallowNewlineBeforeBlockStatements": true,
  "disallowQuotedKeysInObjects": true,
  "disallowSpaceAfterObjectKeys": true,
  "disallowSpaceAfterPrefixUnaryOperators": true,
  "disallowSpacesInsideParentheses": true,
  "disallowTrailingWhitespace": true,
  "disallowParenthesesAroundArrowParam": true,
  "esnext": true,
  "maximumLineLength": 160,
  "requireCamelCaseOrUpperCaseIdentifiers": true,
  "requireCapitalizedComments": true,
  "requireCapitalizedConstructors": true,
  "requireCurlyBraces": true,
  "requireSpaceAfterKeywords": [
    "if",
    "else",
    "for",
    "while",
    "do",
    "switch",
    "case",
    "return",
    "try",
    "catch",
    "typeof"
  ],
  "requireSpaceAfterLineComment": true,
  "requireSpaceAfterBinaryOperators": true,
  "requireSpaceBeforeBinaryOperators": true,
  "requireSpaceBeforeBlockStatements": true,
  "requireSpaceBeforeObjectValues": true,
  "requireSpaceBetweenArguments": true,
  "requireSpacesInFunction": {
    "beforeOpeningCurlyBrace": true
  },
  "requireShorthandArrowFunctions": true,
  "validateIndentation": 2,
  "validateParameterSeparator": ", ",
  "validateQuoteMarks": "'"
}

I wasn't seeing this issue with version 1.0.0 so I think it was introduced by 1.1.0, I am happy to post this against jscs if it is their issue but looking at the commits between those versions I saw changes by @rwjblue in persistent filter and that is in the stack trace so thought best to post issue here first.

adam-knights commented 9 years ago

I tried reverting back to 1.0.0 and still see the issue, but it could still be a dependency behind the scenes that has versioned up since my original install. Happy to report against jscs if you think it is there issue.

This happens both locally on Windows 7 and on our jenkins CI running on CentOS.

rwjblue commented 9 years ago

The rule disallowParenthesesAroundArrowParam seens broken (it is flagging as failed even if no arguments are used). A fix was submitted in https://github.com/jscs-dev/node-jscs/pull/1748 a few hours ago.

adam-knights commented 9 years ago

That would make sense npm list jscs shows v2.1.1 under the broccoli-jscs tree which was only released 8 days ago. I'll close this for now and reopen if https://github.com/jscs-dev/node-jscs/issues/1747 doesn't fix things, but I think now this is a jscs issue.

hzoo commented 9 years ago

I merged https://github.com/jscs-dev/node-jscs/commit/5448a1fb71452c80f997cc1ed6cbb5e03cf987f9 which should fix it (tested with your code snippet)

kellyselden commented 9 years ago

Thanks all!