"no-unused-vars" cannot report all parameters of function

[g-plane]

Tell us about your environment

• ESLint Version: 4.13.1
• Node Version: 8.9.3

What parser (default, Babel-ESLint, etc.) are you using? default

Please show your full configuration:

Configuration

```json { "parserOptions": { "ecmaVersion": 2017, "sourceType": "module", "ecmaFeatures": { "jsx": true } }, "env": { "es6": true, "node": true }, "plugins": [ "import", "eslint-comments" ], "rules": { "no-compare-neg-zero": 2, "no-cond-assign": [2, "except-parens"], "no-constant-condition": 2, "no-dupe-args": 2, "no-dupe-keys": 2, "no-duplicate-case": 2, "no-ex-assign": 2, "no-extra-boolean-cast": 2, "no-extra-parens": [2, "all", { "conditionalAssign": false, "returnAssign": false, "nestedBinaryExpressions": false, "ignoreJSX": "multi-line", "enforceForArrowConditionals": false }], "no-extra-semi": 2, "no-func-assign": 2, "no-inner-declarations": [2, "functions"], "no-obj-calls": 2, "no-regex-spaces": 1, "no-sparse-arrays": 2, "no-template-curly-in-string": 1, "no-unexpected-multiline": 2, "no-unreachable": 2, "no-unsafe-negation": 2, "use-isnan": 2, "valid-jsdoc": 1, "valid-typeof": 2, "accessor-pairs": 1, "array-callback-return": 2, "curly": 2, "default-case": 1, "dot-location": [2, "property"], "dot-notation": [2, { "allowKeywords": false }], "eqeqeq": 2, "guard-for-in": 2, "no-alert": 2, "no-caller": 2, "no-case-declarations": 2, "no-empty-function": 2, "no-empty-pattern": 2, "no-eq-null": 2, "no-eval": 2, "no-extend-native": 2, "no-extra-bind": 2, "no-extra-label": 2, "no-fallthrough": 2, "no-floating-decimal": 2, "no-global-assign": 2, "no-implicit-coercion": [2, { "allow": ["!!", "~"] }], "no-implied-eval": 2, "no-iterator": 2, "no-loop-func": 2, "no-multi-spaces": [2, { "ignoreEOLComments": true }], "no-multi-str": 2, "no-new": 2, "no-new-func": 2, "no-new-wrappers": 2, "no-octal": 2, "no-param-reassign": 2, "no-proto": 2, "no-redeclare": 2, "no-return-assign": 2, "no-self-assign": 2, "no-self-compare": 2, "no-throw-literal": 2, "no-unmodified-loop-condition": 2, "no-unused-expressions": [2, { "allowShortCircuit": true, "allowTernary": true }], "no-unused-labels": 2, "no-useless-call": 2, "no-useless-concat": 2, "no-useless-escape": 2, "no-useless-return": 2, "no-with": 2, "radix": 2, "require-await": 2, "wrap-iife": [2, "inside"], "yoda": [2, "never"], "no-catch-shadow": 2, "no-shadow": 2, "no-shadow-restricted-names": 2, "no-undef": 2, "no-undef-init": 2, "no-undefined": 1, "no-unused-vars": 1, "no-use-before-define": 2, "handle-callback-err": 1, "no-buffer-constructor": 2, "no-new-require": 2, "no-path-concat": 1, "array-bracket-newline": [2, "consistent"], "array-bracket-spacing": [2, "never"], "block-spacing": [2, "always"], "brace-style": [2, "1tbs"], "camelcase": 2, "comma-dangle": [2, "only-multiline"], "comma-spacing": 2, "comma-style": 2, "computed-property-spacing": [2, "never"], "eol-last": 2, "func-call-spacing": [2, "never"], "func-name-matching": 2, "function-paren-newline": [2, "consistent"], "implicit-arrow-linebreak": [2, "beside"], "indent": [2, 2], "jsx-quotes": [2, "prefer-double"], "key-spacing": [2, { "mode": "strict" }], "keyword-spacing": 2, "linebreak-style": [2, "unix"], "lines-between-class-members": [2, "always"], "max-len": [2, { "code": 80 }], "max-params": [2, 3], "max-statements-per-line": [2, { "max": 1 }], "multiline-ternary": [2, "always-multiline"], "new-cap": 2, "new-parens": 2, "newline-per-chained-call": 2, "no-array-constructor": 2, "no-bitwise": 2, "no-lonely-if": 2, "no-mixed-operators": [2, { "allowSamePrecedence": true }], "no-mixed-spaces-and-tabs": 2, "no-multiple-empty-lines": 2, "no-new-object": 2, "no-trailing-spaces": [2, { "ignoreComments": true }], "no-unneeded-ternary": 2, "no-whitespace-before-property": 2, "object-curly-newline": [2, { "consistent": true }], "object-curly-spacing": [2, "always", { "arraysInObjects": true, "objectsInObjects": true }], "operator-assignment": [1, "always"], "operator-linebreak": [2, "before"], "quote-props": [2, "as-needed", { "keywords": true }], "quotes": [2, "single", { "avoidEscape": true }], "semi": [2, "never"], "semi-spacing": 2, "space-before-blocks": [2, "always"], "space-before-function-paren": [2, "always"], "space-in-parens": [2, "never"], "space-infix-ops": 2, "space-unary-ops": 2, "spaced-comment": 2, "switch-colon-spacing": 2, "template-tag-spacing": [2, "never"], "arrow-body-style": [2, "as-needed"], "arrow-parens": [2, "as-needed"], "arrow-spacing": 2, "constructor-super": 2, "no-class-assign": 2, "no-confusing-arrow": [2, { "allowParens": true }], "no-const-assign": 2, "no-dupe-class-members": 2, "no-duplicate-imports": 2, "no-new-symbol": 2, "no-this-before-super": 2, "no-useless-computed-key": 2, "no-useless-constructor": 2, "no-useless-rename": 2, "no-var": 2, "object-shorthand": 2, "prefer-arrow-callback": 2, "prefer-const": 2, "prefer-destructuring": 2, "prefer-numeric-literals": 2, "prefer-rest-params": 2, "prefer-spread": 2, "prefer-template": 1, "rest-spread-spacing": [2, "never"], "template-curly-spacing": [2, "never"], "import/no-unresolved": [2, { "ignore": ["^@"] }], "import/named": 1, "import/default": 2, "import/namespace": 2, "import/no-absolute-path": 2, "import/no-dynamic-require": 1, "import/export": 2, "import/no-named-as-default": 2, "import/no-named-as-default-member": 1, "import/no-deprecated": 1, "import/no-extraneous-dependencies": [2, { "devDependencies": ["**/*.test.js", "**/*.spec.js"] }], "import/first": 2, "import/exports-last": 1, "import/newline-after-import": 2, "import/prefer-default-export": 2, "eslint-comments/disable-enable-pair": [1, { "allowWholeFile": true }], "eslint-comments/no-aggregating-enable": 1, "eslint-comments/no-duplicate-disable": 1, "eslint-comments/no-unused-disable": 1, "eslint-comments/no-unused-enable": 1 } } ```

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

Define a function which it has several unused parameters.

function fn (a, b, c) {
  // do nothing
}
fn()

What did you expect to happen?

For the example above, ESLint should report all parameters is unused.

What actually happened? Please include the actual, raw output from ESLint.

ESLint just reported the last parameter (for the example above that is c) is unused, previous parameters haven't been reported.

[aladdin-add]

this is working as intended. You can use option args: "all" to achieve this. refs: https://github.com/eslint/eslint/blob/master/docs/rules/no-unused-vars.md#args

[j-f1]

@Aladdin-ADD I feel like it should report unused parameters if:

• they are the last parameter, or
• the parameter after them is being reported as unused.

[g-plane]

@Aladdin-ADD Oh, thanks a lot! Sorry for missing that detail.

[platinumazure]

I think we might have a bug here.

It's true that the default setting for this rule is to only report unused args after the last used arg. However, we need to make sure all of them are reported.

function one(a, b, c) {
    // none used: Should report a, b, and c (even with the "after-used" setting)
}

function two(a, b, c, d, e) {
    doSomething(c);
    // a/b/d/e unused: Should report a/b/d/e in "all" setting, and d/e in "after-used" setting)
}

function three(a, b, c) {
    doSomething(c);
    // a/b unused: Should report a/b in "all" setting, and should not report in "after-used" setting)
}

If the actual rule isn't following these patterns, I think we have a bug. I'm going to reopen for now.

[platinumazure]

I've run my examples through the ESLint demo page.

Everything looks good on the "all" setting.

For the "after-used" setting, I think the first two examples have a bug (only c is reported in the first example, and only e is reported in the second example).

So I'm now absolutely convinced we have a bug here.

[ilyavolodin]

That's not correct. This is behaving as expected. The reason for only last argument being reported is because you are using after-used setting. It's supposed to report only last unused argument. That's due to a standard pattern in JavaScript where if you want to get 3-rd argument only, you have to declare two unused arguments ahead of it. Documentation clearly states that in case of after-used only the last argument must be used. https://eslint.org/docs/rules/no-unused-vars#args P.S. You could argue that the name of the option is misleading, but the behavior is correct.

[platinumazure]

@ilyavolodin That's really confusing. In my examples and in the example provided by OP, there are more unused args which should be called out, but aren't. Why are we choosing to report only the last argument?

When the last argument is used, it correctly reports no arguments even if previous arguments are unused; but if there are multiple unused arguments after the last used argument, we should report all of the unused arguments.

If I need to change this to an enhancement request, I'm open to doing that. Regardless of whether the behavior is currently working as designed or not, I think the current behavior is confusing and unintuitive and we should change it.

On Dec 21, 2017 7:12 PM, "Ilya Volodin" notifications@github.com wrote:

That's not correct. This is behaving as expected. The reason for only last argument being reported is because you are using after-used setting. It's supposed to report only last unused argument. That's due to a standard pattern in JavaScript where if you want to get 3-rd argument only, you have to declare two unused arguments ahead of it. Documentation clearly states that in case of after-used only the last argument must be used. https://eslint.org/docs/rules/no-unused-vars#args

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/eslint/eslint/issues/9750#issuecomment-353497495, or mute the thread https://github.com/notifications/unsubscribe-auth/AARWegmRTsXrAt1MwrQrbzkQgVY72Hfiks5tCwHwgaJpZM4RJ44x .

[not-an-aardvark]

I think there's a reasonable argument to be made that if the last argument is unused, we should also report the previous argument. For example:

function x(foo, bar, baz) {
  return foo;
}

The rule will currently report a single error for baz, but in fact, bar is also unnecessary here. (If baz was used, then bar would be necessary, but baz is not used either.) The correct fix would be to remove both bar and baz:

function x(foo) {
  return foo;
}

That said, I think this would be an enhancement request rather than a bug in the current behavior.

[platinumazure]

I'll champion.

@eslint/eslint-team Could we get two more :+1:s? Right now, no-unused-vars reports only the last parameter that is unused, even if earlier parameters are also unused. This is confusing and inconsistent with how we generally implement other rules and I think it needs to change.

[kaicataldo]

I personally think the current behavior is fine, as it will continue warning on each last unused argument as they are removed. That being said, I'm not against a quality of life improvement to the rule that warns on all the unused arguments up front. The current behavior makes sense when you know what the configuration is and what the intention is, but I can see how it could be confusing otherwise.

[nzakas]

I think there's a good case to be made that this needs a change. The purpose of the "after-used" setting is to warn on the first unused argument that can be safely removed without affecting how the function works. If no arguments are used, then it seems like the expected behavior would be to warn on the first argument. I don't think it should warn on all of the arguments because that would be the only situation where this option would warn on more than one argument being unused.

In any event, I do think the current behavior is unexpected, and while technically it's doing what's expected, I don't think it matches user expectations.

@g-plane if this issue is accepted, are you willing to submit a pull request for the change?

[g-plane]

The purpose of the "after-used" setting is to warn on the first unused argument that can be safely removed without affecting how the function works.

@nzakas Why warns on the first argument? If first argument is removed, it will make some changes to the function.

[nzakas]

@g-plane the first argument that can be safely removed. What I mean by that is that it warns on the first argument that, when you remove it, it does not change how the function is used at all. You and I are saying the same thing.

[g-plane]

@nzakas So should it just report the first unused argument or the rest?

For example:

function fn(a, b, c, d, e) {
  a  // used
  c  // used
}
// ESLint reports: `d` is unused

or

function fn(a, b, c, d, e) {
  a  // used
  c  // used
}
// ESLint reports: `d` and `e` are unused

Which one is better (as expected behavior)? IMHO, the latter one may be better.

[nzakas]

According to the docs, after-used behaves like this:

only the last argument must be used. This allows you, for instance, to have two named parameters to a function and as long as you use the second argument, ESLint will not warn you about the first.

So the key is whether or not the last argument is used. So @g-plane, I believe the fix in your example would be to warn that e is unused.

[platinumazure]

I feel that it would make more sense from a UX standpoint to consider "all" and "after-used" as keywords to define what potentially unused arguments should be reported (i.e., report all of them, or only the ones after the last used argument?). Then we should report all such arguments that are unused and that match the constraint set by all/after-used.

Reporting only a subset doesn't make sense to me. It's not a great experience to get a warning for one unused argument, then remove it and rerun the linter to get another warning, etc. Might as well report all of them at once so the user only has to look at the parameter list once, holistically.

@nzakas Please let me know if I'm missing or misunderstanding something.

On Jan 29, 2018 9:26 AM, "Nicholas C. Zakas" notifications@github.com wrote:

According to the docs https://eslint.org/docs/rules/no-unused-vars., after-used behaves like this:

only the last argument must be used. This allows you, for instance, to have two named parameters to a function and as long as you use the second argument, ESLint will not warn you about the first.

So the key is whether or not the last argument is used. So @g-plane https://github.com/g-plane, I believe the fix in your example would be to warn that e is unused.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/eslint/eslint/issues/9750#issuecomment-361280433, or mute the thread https://github.com/notifications/unsubscribe-auth/AARWeswbuGW4dqIimSSeq0fNdA4KSZ0Aks5tPeMigaJpZM4RJ44x .

[g-plane]

warn that e is unused

That is how ESLint behaves now.

[platinumazure]

I've created #9909 to focus on the specific rule change proposal I have for this issue, which is to report on all unused arguments after the last used argument when using args: "after-used" configuration. That way, we can get definitive :+1:/:-1: votes from the team on a specific proposal.

If anyone has a strong reason for why we should only report one parameter instead of all, that might be a good place to discuss it.

Thanks @g-plane for your patience as we hash this out.

[nzakas]

@g-plane okay, I think I misunderstood what you were saying. I thought you were saying that this rule should warn on all parameters when no parameters are used. Now it sounds like you're saying you want all unused parameters to be flagged all the time. Is that correct?

@platinumazure I don't think it's a good idea to create a new issue, as it splits the conversation into two places and makes it more difficult for people to follow (including me, I was very confused when I saw two notifications this morning). If your proposal is to address this issue, I think it should be stated on this issue. People can always :+1: :-1: your individual comment to show support or not. As it is, I'm not sure where I should continue this conversation now.

[platinumazure]

@nzakas We've asked people in the past to focus on the first issue post so that we know what proposal should be accepted (i.e., if there are multiple proposals in later comments and each gets consensus, it's not clear which one we should accept). This issue originally came in as a bug report but there was confusion on whether this was a bug or working as designed. I did comment in here with an enhancement proposal, but it didn't get a lot of traction. So... perhaps this could have been handled differently, but I don't immediately see how. I tried to explain in the new issue why I created it; if that explanation was unclear, your feedback is welcome.

I've asked @g-plane whether this issue could be closed now that the other one is accepted, so that would resolve the question of where the conversation should continue.

I apologize for any confusion. If there is anything that definitely should have been differently, I would love for that to be clarified in the maintainer guide.

[nzakas]

@platinumazure I think the approach of opening a new issue makes sense when there are a lot of participants with different proposals being considered. When there's an issue like this, with just a few participants and the conversation has spawned some questions and a bit of back-and-forth, it's much more beneficial to keep everything in one place.

In general, I think the approach should be to favor having one issue and one place to discuss until that becomes unwieldy. In this case, I think it would have been fine to continue here.