"unused" results in unavoidable cruft output when writing API callbacks #660

mcantor opened this Issue Sep 26, 2012 · 10 comments

7 participants


This is related to the option introduced in issue #582.

Edit Note: I have updated this to reflect the inaccuracy @antonkovalyov pointed out in his first comment; my first example involved an unused argument followed by a used one, which I didn't realize jshint supported (though that is pretty neat).

Consider the following callback:

    resize: function(event, ui) {
        if (event.size.width > 500) {

This will result in the following output from jshint with unused enabled:

'ui' is defined but never used.

Removing ui from the method signature is not a good solution. Methods defined for the same callback should all have the same method signature. I know this is not an accepted standard convention, but I think it's sane enough that jshint ought to support it as a use case.

Turning off unused is not a good solution. I still want to be notified if there is an unused argument in one of my own functions, or an unused variable defined within one of them.

Ignoring the extra output is not a good solution. According to basic unix philosophy, if I run jshint on a file and there is nothing in it which I consider problematic, there should be no output, full stop.

Possible solutions:

  1. Passing a dictionary of excluded argument names as the value of unused instead of true. This isn't ideal because it would miss any arguments in non-API methods that happen to have the same name.

  2. Using comment directives within API callback method definitions to denote that unused arguments are OK.

  function(event, ui) {
  1. Passing a prefix or suffix as the value of unused which will cause jshint to ignore unused variables so suffixed/prefixed.
  "unused": {suffix: "_API"}
  function(event_API, ui_API) {

There are probably other (possibly better) solutions out there. I'm least repulsed by Solution 2.

JSHint member

Your example does not result in a warning. JSHint doesn't warn about unused arguments if they are followed by used ones. See: http://www.jshint.com/reports/808016

@valueof valueof closed this Sep 26, 2012
JSHint member

See also: 5751c5e

JSHint member

Uhm, this report is better: http://www.jshint.com/reports/814012 It doesn't have unrelated stylistic warnings.


I just realized this as I was perusing the unit tests. However, my prior point still stands:

Even if the situation were reversed and we used event without using ui, it is probably best to still have both declared in the method signature, so you can tell at a glance what sort of method it is.

I think this is a fair claim; if you have a whole bunch of callbacks and some of them look like function(event) while others are function(event, ui) because they happen to use ui, the inconsistency is confusing: they're all definitions of the same callback, so they should really all have the same method signature.

jshint supports the use case of function(event, ui){ return ui.whatever; } because of syntactic happenstance, but a better reason is that all method signatures should have the same method signature, which would support function(event, ui){ return event.whatever; } as well.

@mcantor mcantor added a commit that referenced this issue Sep 26, 2012
@mcantor mcantor Add explicit tests for unused function arguments.
While thinking about issue #660, I added these tests to solidify my
understanding of how the 'unused' option currently works. I think the
unused_second_arg test is somewhat redundant, but it is quite small, so
it shouldn't be a problem in terms of maintenance or refactoring.

At the very least, I think unused variables and unused function arguments should be separate options.


they're all definitions of the same callback, so they should really all have the same method signature.

I agree 100%. It's confusing for some functions to be defined as function(event) and others as function(event, ui) when they are both implementing the same callback. This is a very big deal from a readability point of view. Can we please reopen this issue and (as ljharb said) differentiate between unused variables and unused function arguments?

@rondevera rondevera referenced this issue in rondevera/jslintmate Jan 8, 2013

Unused variables in callbacks #21


As it stands, that unused: true forces one to remove unused arguments from the end of method signatures significantly reduces the useful of this otherwise excellent option.

In a large, multiuser codebase code readability and approachability is at a premium. If the author of a module writes a pair of related functions:

function renderFoo(type, extras) { … } // makes use of type and extras
function renderBar(type, extras) { … } // uses only type

Then consumers of this module should be able to infer that renderFoo() and renderBar() are semantically similar. If the author of the module had had unused: true set, then they would have been forced to strip extras from renderBar()’s signature, lowering readability and causing confusion. And what happens when that module author leaves their job and is longer available for questions?

As it stands, unused: true’s huge and immediate usefulness for finding declared variables that went unused is overpowered by the hindrance it enforces around function signatures.

Proposed solution: Make unused focus only on var declarations and spin the function signature examination code off into a new option called, e.g., unusedparam.


Agreed with @pdokas et al., I always list all available API params and it's saved me a trip to the docs many times, and the self-documenting aspect is one that I value as well.

For a real use-case, can you name the library this callback will be consumed by?

function(data) {}

And how about this one?

function(data, textStatus, jqXhr) {}

Currently, I can't find a way to silence these warnings at all; toggling unused only creates an error on top of the warning...


As of jshint 1.1.0 you can now achieve the separation between variables and function parameters via the unusued: "vars" parameter landed in 1cca4c2.


Can we have an option to ignore unused functions but not other kinds of unused variables?

This is especially useful for those of us writing large js libraries, and having to put a zillion /* exported xxx */ atop every file, for every function, is unmaintainable.

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