Allow detect when function argument shadows existing variable
dvhx opened this issue · 4 comments
In the following code I use variable "s" to store string "foo", then I want to iterate over "data" array using forEach, I use function argument "s" which "overwrites" the parent "s". Jslint currently does not show it as warning:
var s = "foo";
var data = [1, 2, 3];
data.forEach(function (s) {
console.log(s, "long text so you don't notice arg s shadowed parent s", s);
});
Could jslint produce warning, something like:
Redefinition of 's' from line 1.
JSLINT already does it for local variables:
var s = "foo";
var data = [1, 2, 3];
function foo() {
var s = "asdf"; // JSLINT warning: Redefinition of 's' from line 1.
console.log(s);
}
Parameter names are very specific to a function, so JSLint allows redefinition of them.
I suppose we could add an option to allow/prohibit them, but I am not sure it is worth the price.
If I where to recommend any change, it would be to warn against single letter variable names. They are cryptic, confusing, and conducive to making errors like yours.
-
warning against single-letter-variable-name sounds like a good idea, and will add when time allows.
-
original issue of detecting renamed-variable is nice-to-have:
- have no objection, except high qa-cost to implement and test against regressions ^^;;
- thus no guarantee it'll be implemented in forseeable future
- however, if you want to contribute pull-request, or hints on where to modify code in jslint, i'll be happy to help :)
- have no objection, except high qa-cost to implement and test against regressions ^^;;
- warning against single-letter-variable-name sounds like a good idea, and will add when time allows.
IMHO, I think there are cases when a single letter variable can be allowed
- short (pure, math) functions:
const add = (x, y) => x + y
. Here the meaning of the function is provided by the variable name "add", the two arguments are just mute variables. - when they represent a domain entity which has a single character name
const e = 2.71828