eslint / eslint

Find and fix problems in your JavaScript code.
https://eslint.org
MIT License
24.55k stars 4.43k forks source link

False positive for no-implicit-coercion #9638

Closed LJNeon closed 6 years ago

LJNeon commented 6 years ago

Tell us about your environment

What parser (default, Babel-ESLint, etc.) are you using? ECMA2017 Please show your full configuration:

Configuration ```js { "env": { "es6": true, "node": true }, "extends": "eslint:recommended", "rules": { "accessor-pairs": 2, "array-bracket-newline": [2, "consistent"], "array-bracket-spacing": 2, "array-callback-return": 2, "array-element-newline": [2, "never"], "arrow-body-style": 2, "arrow-parens": [2, "as-needed"], "arrow-spacing": 2, "block-scoped-var": 2, "block-spacing": [2, "never"], "brace-style": 2, "comma-dangle": 2, "comma-spacing": 2, "comma-style": 2, "computed-property-spacing": 2, "curly": 2, "dot-notation": 2, "dot-location": [2, "property"], "eol-last": 2, "eqeqeq": 2, "for-direction": 2, "func-call-spacing": 2, "func-name-matching": 2, "func-style": 2, "function-paren-newline": 2, "getter-return": 2, "key-spacing": 2, "keyword-spacing": [2, { "before": false, "after": true, "overrides": { "of": {"before": true}, "break": {"after": false}, "case": {"after": false}, "catch": {"after": false}, "continue": {"after": false}, "debugger": {"after": false}, "default": {"after": false}, "do": {"after": false}, "else": {"after": false}, "extends": {"before": true}, "finally": {"after": false}, "for": {"after": false}, "if": {"after": false}, "in": {"before": true}, "instanceof": {"before": true}, "super": {"after": false}, "switch": {"after": false}, "this": {"after": false}, "while": {"after": false} } }], "line-comment-position": 2, "lines-between-class-members": [2, "never"], "linebreak-style": 2, "multiline-comment-style": [2, "bare-block"], "multiline-ternary": [2, "never"], "newline-per-chained-call": [2, {"ignoreChainWithDepth": 3}], "new-parens": 2, "no-array-constructor": 2, "no-bitwise": 2, "no-catch-shadow": 2, "no-else-return": 2, "no-empty-function": [2, {"allow": ["arrowFunctions"]}], "no-extend-native": 2, "no-extra-bind": 2, "no-extra-boolean-cast": 2, "no-extra-label": 2, "no-extra-parens": 2, "no-extra-semi": 2, "no-floating-decimal": 2, "no-global-assign": 2, "no-implicit-coercion": 2, "no-implicit-globals": 2, "no-implied-eval": 2, "no-inline-comments": 2, "no-invalid-this": 2, "no-label-var": 2, "no-lone-blocks": 2, "no-lonely-if": 2, "no-loop-func": 2, "no-mixed-spaces-and-tabs": 2, "no-multiple-empty-lines": [2, { "max": 0, "maxEOF": 1 }], "no-multi-spaces": 2, "no-multi-str": 2, "no-negated-condition": 2, "no-new": 2, "no-new-wrappers": 2, "no-octal-escape": 2, "no-path-concat": 2, "no-param-reassign": 2, "no-plusplus": [2, {"allowForLoopAfterthoughts": true}], "no-return-await": 2, "no-self-assign": [2, {"props": true}], "no-self-compare": 2, "no-shadow": 2, "no-shadow-restricted-names": 2, "no-template-curly-in-string": 2, "no-trailing-spaces": 2, "no-undef-init": 2, "no-unmodified-loop-condition": 2, "no-unneeded-ternary": 2, "no-unused-expressions": 2, "no-useless-call": 2, "no-useless-computed-key": 2, "no-useless-concat": 2, "no-useless-constructor": 2, "no-useless-rename": 2, "no-useless-return": 2, "no-use-before-define": 2, "no-var": 2, "no-void": 2, "no-whitespace-before-property": 2, "no-with": 2, "object-curly-newline": [2, {"consistent": true}], "object-curly-spacing": 2, "object-shorthand": 2, "one-var": 2, "one-var-declaration-per-line": 2, "operator-assignment": 2, "operator-linebreak": [2, "before"], "padded-blocks": [2, "never"], "padding-line-between-statements": [2, { "blankLine": "never", "prev": "*", "next": "*" }], "prefer-arrow-callback": 2, "prefer-const": 2, "prefer-destructuring": 2, "prefer-promise-reject-errors": 2, "prefer-rest-params": 2, "prefer-spread": 2, "quote-props": [2, "as-needed"], "quotes": [2, "double"], "radix": 2, "rest-spread-spacing": 2, "require-await": 2, "semi": [2, "always"], "semi-spacing": 2, "semi-style": 2, "sort-keys": [2, "asc", { "caseSensitive": false, "natural": true }], "sort-vars": [2, {"ignoreCase": true}], "space-before-blocks": 2, "space-before-function-paren": [2, { "anonymous": "never", "asyncArrow": "always", "named": "never" }], "space-in-parens": 2, "space-infix-ops": 2, "space-unary-ops": [2, { "words": true, "nonwords": false }], "spaced-comment": 2, "strict": [2, "global"], "symbol-description": 2, "wrap-iife": 2, "yoda": 2 }, "parserOptions": {"ecmaVersion": 2017} } ```

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

for(let i = 0; i < this.guessed.length; i++) {
    const amount = 3 - 1 * i, // line 88
    {id} = this.guessed[i];
    if(this.points.hasOwnProperty(id)) {
        this.points[id] += amount;
    }else{
        this.points[id] = amount;
    }
}
eslint src/**

What did you expect to happen? No errors would be thrown as i is already a number. What actually happened? Please include the actual, raw output from ESLint.

  88:28  error  use `Number(i)` instead  no-implicit-coercion
LJNeon commented 6 years ago

And yes I am fully aware that the 1 * part isn't needed and that I could do 3 - i. My point here was that i is a number yet it still errors.

not-an-aardvark commented 6 years ago

Thanks for the report. This is happening because it's not possible in general to statically determine what types your variables will have at runtime.

For example, if your code looked like this:

const i = someUnknownFunction() ? 'a string' : 5;

console.log(1 * i);

...i would possibly be a string and possibly be a number, and ESLint wouldn't be able to statically determine which is the case. So instead the rule assumes that 1 * x is always used to coerce a value to a number, because it wouldn't be useful to do 1 * x if x was already a number.

LJNeon commented 6 years ago

Couldn't it check for conditionals like that statically? If there's any uncertainty on whether or not a variable is a number assume that it isn't, otherwise it's fine.

not-an-aardvark commented 6 years ago

Type checking is generally out of scope for a linter -- if you want to enforce static type checking, you should use a dedicated type checker like TypeScript. In this case I don't think implementing type checking would be worth the very minor benefit that it would provide for the no-implicit-coercion rule.

ilyavolodin commented 6 years ago

Closing this issue as this is working as intended. Unfortunately, ESLint is not capable of checking types and validating the example provided.