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

no-unused-args: ignore parameters with name matching a regex #2321

Closed
mgol opened this Issue Apr 15, 2015 · 13 comments

Comments

Projects
None yet
4 participants
@mgol
Contributor

mgol commented Apr 15, 2015

In AngularJS when using dependency injection the order of parameters is not important, therefore it makes sense if all of them are used so that to not leave an unused dependency in parameter list. However, sometimes one has to define a function with unused first parameter, e.g. if a function is passed two arguments but we need only the second one.

How about adding an option that'd behave more or less as {"args": "all"} but that'd ignore parameter names that match a certain regex/start with something. This would allow to write:

function onClick(_ev, data) {
    send(data);
}

and have it not fail on ESLint because _ev starts with _ but would still allow to use ESLint to find unused parameters where they're not left on purpose.

@nzakas

This comment has been minimized.

Member

nzakas commented Apr 15, 2015

You can already write that today using the rule. Can you provide an example that isn't possible today that you'd life to support?

@nzakas nzakas added the triage label Apr 15, 2015

@mgol

This comment has been minimized.

Contributor

mgol commented Apr 15, 2015

I'd like the following:

function onClick(_ev, data) { // no warning about an unused _ev
    console.log(data);
}

function job(name, title, source) { // warning about unused title
    jobs[name] = source;
};

Now I can either have an error in both those cases with:

"no-unused-vars": [2, {"vars": "all", "args": "all"}]

or nowhere with:

"no-unused-vars": [2, {"vars": "all", "args": "after-used"}]
@nzakas

This comment has been minimized.

Member

nzakas commented Apr 16, 2015

Ah I see...so you're saying that you want to be able to annotate parameters to say, "don't check this one"? Is a naming convention the best way to do that? What about using a comment?

@mgol

This comment has been minimized.

Contributor

mgol commented Apr 25, 2015

Ah I see...so you're saying that you want to be able to annotate parameters to say, "don't check this one"?

Exactly.

Is a naming convention the best way to do that? What about using a comment?

You mean to wrap it in proper /* eslint-enable ... */ & /* eslint-disable */? I guess that would work but it's a little more tedious; also, this doesn't allow to disable checking for specific parameters but for the whole function. In cases like that I usually have to define a first unused parameter but I do care if I don't introduce new excessive ones after that one.

@mgol

This comment has been minimized.

Contributor

mgol commented May 8, 2015

@nzakas any thoughts about my ideas?

@nzakas

This comment has been minimized.

Member

nzakas commented May 8, 2015

I suppose I can see it. What would you expect the options to look like?

@mgol

This comment has been minimized.

Contributor

mgol commented May 11, 2015

I see a couple of options. Either a specific comment:

function f(a /* unused */, b) {
    return b;
}

configured normally:

"no-unused-vars": [2, {"vars": "all", "args": "all"}]

or (my preferred but the previous one would be ok as well):

function f(_a, b) {
    return b;
}

configured like:

"no-unused-vars": [2, {"vars": "all", "args": {"all-except-prefixes": ["_"]}}]

or:

"no-unused-vars": [2, {"vars": "all", "args": {"all-except-regex": ["/^_/"]}}]
@andy-hanson

This comment has been minimized.

andy-hanson commented May 18, 2015

mzgol's idea would be great. 99% of the time I intend to use every parameter, so having to type an underscore for ignored parameters is fine.

@gyandeeps

This comment has been minimized.

Member

gyandeeps commented May 22, 2015

@nzakas I think we have all the info to make a decision and add the appropriate labels here.

@nzakas

This comment has been minimized.

Member

nzakas commented May 29, 2015

I think we need a better option name for using regular exclusions, but otherwise, looks good.

@mgol

This comment has been minimized.

Contributor

mgol commented Aug 25, 2015

Is this supposed to work with ESLint 1.2.1? The following config:

{
    "root": true,
    "rules": {
        "no-unused-vars": [2, {"args": "all", "argsIgnorePattern": "^_"}]
    }
}

still prints an error for the file:

function foo(_a) { } foo();

Error:

a.js
  1:14  error  _a is defined but never used  no-unused-vars
@gyandeeps

This comment has been minimized.

Member

gyandeeps commented Aug 25, 2015

Correction: This issue has been fixed but not yet released

@mgol

This comment has been minimized.

Contributor

mgol commented Aug 25, 2015

@gyandeeps Sorry, I got confused by https://github.com/eslint/eslint/commits/master showing 1.2.1 as a newer commit than 91fc1c5 so I erroneously assumed it includes it. That's what happens when you flatten a tree. :)

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.