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

Allow detect when function argument shadows existing variable #443

Open
dvhx opened this issue Jul 23, 2023 · 4 comments
Open

Allow detect when function argument shadows existing variable #443

dvhx opened this issue Jul 23, 2023 · 4 comments

Comments

@dvhx
Copy link

dvhx commented Jul 23, 2023

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);
}
@douglascrockford
Copy link
Collaborator

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
Copy link
Collaborator

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.

@kaizhu256
Copy link
Member

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

@bunglegrind
Copy link

  • 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

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

No branches or pull requests

5 participants
@douglascrockford @kaizhu256 @dvhx @bunglegrind and others