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

reporting 'unused variable' for a globally defined function #659

Closed
cargowisebenwoodcock opened this issue Sep 26, 2012 · 17 comments
Closed

Comments

@cargowisebenwoodcock
Copy link

function foo() {
}

If the above is the entire contents of a file then I get a JSHint warning

'Unused Variable 'foo'.

How would I go about allowing this to occur without disabling the unused rule. Such that

function foo() {
    var bar = true;
}

reports unused variable 'bar' but not unused variable 'foo'?

The function in question is used by other files just not within the current file.

@changemewtf
Copy link

The solution to this might be similar to a solution that works for the issue I just posted, #660.

@valueof
Copy link
Member

valueof commented Sep 26, 2012

Yes, this is a real problem but I don't have any good solution in mind. Suggestions are welcomed.

@changemewtf
Copy link

@antonkovalyov, can you take another look at the suggestions I have in #660? I have an ulterior motive in that I think #660 is still relevant even though jshint already ignores unused function arguments followed by used ones, but a couple of the solutions might be practical for both issues.

In particular, an /*unusedOK*/-type directive might be a nice and clear way to handle this. Perhaps a directive similar to /*global*/ that lived at the top of a file:

/*exported foo*/
function foo() {
    ...
}

@cargowisebenwoodcock
Copy link
Author

I had a look at #660. While similar I do believe these are different; though I agree with @antonkovalyov comments on that topic, keeping the function declarations the same is important.

For this issue though I am liking the idea of the /*exported foo*/ idea. It would mean explicitly declaring that this function is needed and is known not to be used in this file.

@cargowisebenwoodcock
Copy link
Author

Not sure how I closed that, sorry.

@rwaldron
Copy link
Member

How would the global scope ever know about foo?

results in the warning, "'foo' is defined but never used."

Because it's not used, nor "exported"

@huwendy
Copy link

huwendy commented Jul 24, 2014

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.

@JaredCubilla
Copy link

@huwendy I second that motion.

@rwaldron
Copy link
Member

@huwendy @JaredCubilla do either of you have an idea of what this might look like, if not the existing /* exported ...*/ directive?

@wonbyte
Copy link

wonbyte commented Sep 5, 2014

Is there an option for "exported" in the jshintrc? Or does it always have to be per file?

@rwaldron
Copy link
Member

rwaldron commented Sep 5, 2014

exported
A directive for telling JSHint about global variables that are defined in the current file but used elsewhere.

If this were allowed in global options, it would be ambiguous.

@wonbyte
Copy link

wonbyte commented Sep 5, 2014

Makes sense 👍

@huwendy
Copy link

huwendy commented Sep 6, 2014

@rwaldron
I think a global config param in jshintrc that tells jshint to not complain about unused functions would be awesome!

Eg ignoreunusedfunc : true or something like that

@rwaldron
Copy link
Member

rwaldron commented Sep 6, 2014

@huwendy like this: /*jshint unused:false */?

@huwendy
Copy link

huwendy commented Sep 7, 2014

@rwaldron
To clarify, the ask was to not complain about unused functions, but still complain about other kinds of unused variables

@rwaldron
Copy link
Member

rwaldron commented Sep 7, 2014

Yes, you're right—I read that but misinterpreted it, thanks for clarifying.

jugglinmike pushed a commit to jugglinmike/jshint that referenced this issue Oct 21, 2014
Sometimes, especially in the browser world, people "export" variables
by leaving them global and then concatenating files together. But
JSHint doesn't know about that and complains about unused variables.

This patch (initially started by @circusbred in jshintGH-726) introduces a
new directive, /*exported varA, varB,.. */ that can be used to tell
JSHint that variables varA and varB are intentionally global and will
be used in other files even if they are unused in the current one.

Closes jshintGH-726.
Closes jshintGH-659.
@georgir
Copy link

georgir commented Nov 20, 2014

Just a thought, while I'm waiting for jslint and other derivatives to catch up and get /*exported xxx*/, I decided that writing window.xxx=... instead of var xxx=... (or even var xxx=... followed by window.xxx=xxx at the end of the file, so you can use xxx if you decide) is a clearer way to define exported globals, as it makes the code work when wrapped in a namespacing function or eval-ed in some non-global context as might happen with certain js loaders.
A problem with this is that window is not always the global object... i.e. it won't work in worker thread scripts or server-side environments like node.js. Using self works in worker threads and normal browser context but not in any server environments I think... Using this can do the trick if running in global context but not in a function in strict mode, with some inidrect eval hacks it is possible to get the global object when it is not this, but it gets ugly... But at least if you know your target environment you can pick an option.

As to those wanting to make a distinction between unused functions and other types of unused variables, that is mostly useless in my opinion - first because modules might want to export non-function values, and second because even function values may be defined with a var xxx=..., and not even just with a simple function expression but with a more complicated function-returning call that is impossible to detect syntactically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants