Add no-shadow ?

[lpatiny]

I would like to add this rule

  no-shadow:
    - error
    - builtinGlobals: true
      hoist: all
      allow: []

Those rules are active on mass-tools and I'm now adding it on ml-spectra-processing.

@targos WDYT ?

[targos]

What kind of bad code would it catch?

[targos]

If I run it in my current project, it flags 250 errors, so that would be very disruptive.

[lpatiny]

Definition of the same variable name in different scopes seems like a bad habit to me. I enforced it on ml-spectra-processing and had to fix 3 problems.

https://github.com/mljs/spectra-processing/commit/cd6c96ee0d5747629eb2012f922204572259d881

And I had to add an exception: https://github.com/mljs/spectra-processing/commit/cd6c96ee0d5747629eb2012f922204572259d881#diff-9e1ecc14c733bb1ae2e523089f1262ac6ffccbcf950487ee0984403603550e57R8

In mass-tools there was no error.

This is the kind of code I would like to avoid:

  const sum = 0;
  console.log(sum);
  for (let i = 0; i < 10; i++) {
    let sum = 0;
    sum += i;
    console.log(sum);
  }

function test() {
  for (let i = 0; i < 10; i++) {
    let sum = 0;
    sum += i;
    console.log(sum);
  }
  const sum = 0;
  console.log(sum);
}

[targos]

In react, it's quite common to do this:

const data = useMemo(() => {
  const data = ...;
  ...
  return data;
}, []);

It's annoying to have to rename the variable.

I see that the rule also flags a lot of shadowing of global variables. For example in ProFID, we use const alert = ..., but this shadows the global alert() function. I don't think this is a problem/bad habit.