Skip to content
This repository has been archived by the owner on Mar 23, 2024. It is now read-only.

Getting duplicate warnings #89

Closed
Krinkle opened this issue Dec 6, 2013 · 3 comments
Closed

Getting duplicate warnings #89

Krinkle opened this issue Dec 6, 2013 · 3 comments

Comments

@Krinkle
Copy link
Contributor

Krinkle commented Dec 6, 2013

Given the following configuration:

{
    "requireCurlyBraces": ["if", "else", "for", "while", "do"],
    "requireSpaceAfterKeywords": ["if", "else", "for", "while", "do", "switch", "return", "function"],
    "requireSpacesInFunctionExpression": {
        "beforeOpeningCurlyBrace": true
    },
    "requireMultipleVarDecl": true,
    "requireSpacesInsideObjectBrackets": "all",
    "disallowSpaceAfterObjectKeys": true,
    "disallowLeftStickedOperators": ["?", "+", "-", "/", "*", "=", "==", "===", "!=", "!==", ">", ">=", "<", "<="],
    "disallowRightStickedOperators": ["?", "+", "/", "*", ":", "=", "==", "===", "!=", "!==", ">", ">=", "<", "<="],
    "requireRightStickedOperators": ["!"],
    "requireLeftStickedOperators": [","],
    "disallowSpaceAfterPrefixUnaryOperators": ["++", "--", "+", "-", "~"],
    "disallowSpaceBeforePostfixUnaryOperators": ["++", "--"],
    "requireSpaceBeforeBinaryOperators": ["+", "-", "/", "*", "=", "==", "===", "!=", "!=="],
    "requireSpaceAfterBinaryOperators": ["+", "-", "/", "*", "=", "==", "===", "!=", "!=="],
    "disallowKeywords": [ "with" ],
    "disallowMultipleLineBreaks": true,
    "validateLineBreaks": "LF",
    "disallowKeywordsOnNewLine": ["else"],
    "requireLineFeedAtFileEnd": true
}

For an offending line such as the following, 4 warnings are generated.

foo( n*2 );
Operator * should not stick to following expression at example.js :
Operator * should not stick to following expression at example.js :
Operator * should not stick to following expression at example.js :
Operator * should not stick to following expression at example.js :

Coming from disallowLeftStickedOperators, disallowRightStickedOperators, requireSpaceBeforeBinaryOperators and requireSpaceAfterBinaryOperators. I can understand getting two (space before and space after), though it'd be useful if the warning message mentioned it in that case.

However having all four of them trigger seems odd. So I wonder, are these (sticked operators and binary operators) overlapping sets, or is one of them a superset and as such I shouldn't enable both?

@mdevils
Copy link
Member

mdevils commented Dec 19, 2013

When using requireSpaceBeforeBinaryOperators you don't need disallowLeftStickedOperators rule for the same operators.

@mikesherov
Copy link
Contributor

@mdevils can we close this now that we have #125?

@mdevils
Copy link
Member

mdevils commented Dec 30, 2013

Yep!

@mdevils mdevils closed this as completed Dec 30, 2013
wmfgerrit pushed a commit to wikimedia/oojs-ui that referenced this issue Apr 28, 2014
* Coding style options are deprecated in jshint and scheduled
  for removal in JSHint v3 (onevar, camelcase, nomen, etc.)

  Remove these from our config and use jscs equivalents instead.
  - camelcase -> requireCamelCaseOrUpperCaseIdentifiers
  - curly -> requireCurlyBraces
  - immed -> requireParenthesesAroundIIFE
  - newcap -> requireCapitalizedConstructors
  - noempty -> disallowEmptyBlocks
  - quotmark -> validateQuoteMarks
  - trailing -> disallowTrailingWhitespace
  - onevar -> requireMultipleVarDecl
  - nomen -> camelcase -> requireCamelCaseOrUpperCaseIdentifiers

* Remove duplicative jscs rules.
  - When using requireSpaceBeforeBinaryOperators you don't need
    disallowLeftStickedOperators rule for the same operators.
    jscs-dev/node-jscs#89

Also:
* Use tabs instead of 8 spaces in .jscsrc.
* Make use of jshint's built-in config parser instead of reading
  and regex-ing out the comments manually.
* Make use of jshint's built-in config file finder instead of
  overriding it for all files at once. This way it will also
  automatically support having more than one (e.g. a more
  specific config file inside /test or /src would work naturally,
  just like it would when you run jshint from shell or in a
  text editor).
* Separate dev and dist files so that linter does not complain
  twice about the same error (once in dist/, once in src/).

Change-Id: Ibf5356fcb6df3deb2c4462d6003dd5654513895f
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants