"no-shadowed-variable": vars shouldn't be shadowed by later declared vars

[benny-medflyt]

Bug Report

• TSLint version: 5.5.0
• TypeScript version: 2.4.2
• Running TSLint via: CLI

TypeScript code being linted

export function foo(wam: boolean) {
    if (wam) {
        const now = new Date();
        return now.getTime();
    }

    const now = new Date();
    return now.getTime() + 1000;
}

with tslint.json configuration:

{
    "no-shadowed-variable": true
}

Actual behavior

ERROR: 3:15     no-shadowed-variable  Shadowed name: 'now'

Expected behavior

Even though the inner now variable is inside an inner scope where a variable of the same name resides, I don't believe that there is actually any shadowing.

Indeed the inner block cannot access the outer now variable. If we try (by commenting out line 3) then we get this tsc compile error:

error TS2448: Block-scoped variable 'now' used before its declaration

Code in the style above is pretty common IMHO and is readable, and should not trigger "no-shadowed-variable" (as I have shown there is technically no shadowing)

[ajafff]

You're right that it doesn't make much sense in your example.

As soon as you have a closure in your block, that a different story:

export function foo(wam: boolean) {
    if (wam) {
        const now = new Date(); // if you remove this line, the next line will use `now` from the outer scope
        setTimeout(() => console.log(now));
    }

    const now = new Date();
    setTimeout(() => console.log(now));
}

If there is enough demand from the community, we could add an option to ignore variables that are shadowed in their temporal dead zone, i.e. shadowed before the declaration.

[benny-medflyt]

If you remove that line, then the code won't pass tslint due to "no-use-before-declare"

[shelacek]

I think, this is valid concern. Please add option that take temporal dead zone into account. We have such great keywords like let and const, but they are devalued by this rule.

[adidahiya]

If you remove that line, then the code won't pass tslint due to "no-use-before-declare"

Note that we don't really take this rule into account when making TSLint design decisions; it's not very useful with let / const syntax, it's slow to compute, and it's not enabled in the built-in configuration presets.