jslint icon indicating copy to clipboard operation
jslint copied to clipboard

Allow detect when function argument shadows existing variable

Open dvhx opened this issue 2 years ago • 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);
}

dvhx avatar Jul 23 '23 06:07 dvhx

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.

douglascrockford avatar Jul 23 '23 12:07 douglascrockford

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.

douglascrockford avatar Jul 23 '23 17:07 douglascrockford

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

kaizhu256 avatar Jul 25 '23 02:07 kaizhu256

  • 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

bunglegrind avatar Jul 27 '23 21:07 bunglegrind