jslint-org/jslint

Allow detect when function argument shadows existing variable

dvhx opened this issue · 4 comments

dvhx commented

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 :)
  • 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

  1. 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.
  2. when they represent a domain entity which has a single character name const e = 2.71828