Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option Proposal: for no-unused-vars, add exceptions and exceptionsIgnorePattern options #3837

Closed
SpadeAceman opened this issue Sep 17, 2015 · 8 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint help wanted The team would welcome a contribution from the community for this issue rule Relates to ESLint's core rules

Comments

@SpadeAceman
Copy link

Following from the discussion in #3543, I propose the addition of options to the no-unused-vars rule, allowing us to specify how we want to handle exception variables defined in the catch() clause of a try/catch statement.

Currently, the documentation doesn't address this, and the rule allows caught exception variables to remain unused, even when the "all" settings are used for the current options (tested via ESLint 1.4.3, though this behavior was also present in previous versions):

/*eslint no-unused-vars: [2, {"vars": "all", "args": "all"}]*/
function example(){
  try {
    doSomething();
  } catch (err) { // unused exception variable isn't flagged as an error
    return false;
  }
  return true;
}

Therefore, I propose the addition of exceptions and exceptionsIgnorePattern options, mirroring the functionality of the vars/varsIgnorePattern and args/argsIgnorePattern options. Valid settings for exceptions would be:

  • none - the default, matches the current behavior of not reporting any unused exception variables
  • all - reports all unused exception variables, aside from those matching exceptionsIgnorePattern
/*eslint no-unused-vars: [2, {"exceptions": "all", "exceptionsIgnorePattern": "^ignore"}]*/
function hypotheticalExample(){
  try {
    doSomething();
  } catch (err) { // unused exception variable IS flagged as an error
    return false;
  }
  try {
    doSomethingElse();
  } catch (ignoreErr) { // unused exception variable matches ignore pattern, is NOT flagged as an error
    return false;
  }
  return true;
}

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@eslintbot
Copy link

Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Sep 17, 2015
@ilyavolodin ilyavolodin added rule Relates to ESLint's core rules feature This change adds a new feature to ESLint and removed triage An ESLint team member will look at this issue soon labels Sep 17, 2015
@nzakas nzakas added enhancement This change enhances an existing feature of ESLint accepted There is consensus among the team that this change meets the criteria for inclusion and removed feature This change adds a new feature to ESLint labels Sep 18, 2015
@nzakas
Copy link
Member

nzakas commented Sep 18, 2015

Looks good, let's do it.

@gyandeeps
Copy link
Member

@nzakas Should we just make the default behavior to flag args inside catch statements and People who do not want to flag that can use argsIgnorePattern option. Since err is also an argument to the catch block. I never knew this rule was missing this as I would expect it to flag things inside catch statement also, unless I am missing something which I am not thinking of.
We can release this with 2.0 with under the breaking section.

@nzakas
Copy link
Member

nzakas commented Dec 29, 2015

I don't see a reason to introduce a breaking change for this. It flags things inside catch it just didn't flag the exception because it's required.

@mischah
Copy link

mischah commented Feb 18, 2016

I know you hate those +1 comments.

… but I love to see this 😘

@platinumazure
Copy link
Member

@mischah Issue is accepted, so if you want to write a pull request, have at it! It's like somebody said (I forget who, maybe Gandhi?), "Be the change you wish to see in the world". So he probably would have also said "If you want something, write it yourself" 😁

@nzakas nzakas added the help wanted The team would welcome a contribution from the community for this issue label Feb 19, 2016
@pvamshi
Copy link
Contributor

pvamshi commented Mar 18, 2016

I am working on this , will create a pull request for review shortly

pvamshi pushed a commit to pvamshi/eslint that referenced this issue Mar 18, 2016
pvamshi pushed a commit to pvamshi/eslint that referenced this issue Mar 18, 2016
pvamshi pushed a commit to pvamshi/eslint that referenced this issue Mar 18, 2016
pvamshi pushed a commit to pvamshi/eslint that referenced this issue Mar 18, 2016
pvamshi pushed a commit to pvamshi/eslint that referenced this issue Mar 18, 2016
pvamshi pushed a commit to pvamshi/eslint that referenced this issue Mar 18, 2016
pvamshi pushed a commit to pvamshi/eslint that referenced this issue Mar 23, 2016
pvamshi pushed a commit to pvamshi/eslint that referenced this issue Mar 23, 2016
@nzakas nzakas closed this as completed in 48ad5fe Mar 25, 2016
nzakas added a commit that referenced this issue Mar 25, 2016
Update: Add 'caughtErrors' to rule no-unused-vars (fixes #3837)
@ryanbriscall
Copy link

Thanks for this feature, but I needed this for function names. It works for variables, given the provided "catch (ignoreErr)" example. But doesn't work for functions, say for example: function ignoreTest() { }. I was expecting this "no-unused-vars" exceptions solution to work for functions too, since eslint is showing my unused function error as a "no-unused-vars" error.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 7, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint help wanted The team would welcome a contribution from the community for this issue rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

9 participants