Skip to content

Do not warn unreachable code for variable declarations #2240

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

Open
mttcr opened this issue Jan 16, 2017 · 5 comments
Open

Do not warn unreachable code for variable declarations #2240

mttcr opened this issue Jan 16, 2017 · 5 comments

Comments

@mttcr
Copy link

mttcr commented Jan 16, 2017

From https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Statements/var :

var hoisting
Because variable declarations (and declarations in general) are processed before any code is executed, declaring a variable anywhere in the code is equivalent to declaring it at the top. This also means that a variable can appear to be used before it's declared. This behavior is called "hoisting", as it appears that the variable declaration is moved to the top of the function or global code.

If there is a return statement in a function, variable declarations written after should not warn for unreachable code.
I did not intentionnally write a statement like that, it's from typescript transpilation:

export class A
{
    public a: A;
    private v( ...params: string[] )
    {

    }
    public test()
    {
        return this.a.v( ...[] );
    }
}

Is transpiled in:

var A = (function () {
    function A() {
    }
    A.prototype.v = function () {
        var params = [];
        for (var _i = 0; _i < arguments.length; _i++) {
            params[_i] = arguments[_i];
        }
    };
    A.prototype.test = function () {
        return (_a = this.a).v.apply(_a, []);
        var _a;
    };
    return A;
}());
exports.A = A;
@MatrixFrog
Copy link
Contributor

@mprobst do you have tools that shift things around so that the var statement comes before the return, or something similar that solves this?

@mprobst
Copy link
Contributor

mprobst commented Jan 18, 2017

@evmar any intuition on how hard this'd be to solve in tsickle (e.g. assuming we generate a duplicate declarations at the top of the file)?

@evmar
Copy link
Contributor

evmar commented Jan 18, 2017

I experimented a bit.

Even this code:

    let x = this.a.v( ...[] );
    return x;

is transpiled as

       var x = (_a = this.a).v.apply(_a, []);
        return x;
        var _a;

We currently do a pass over the compilation output (to rewrite require statements as goog.module), so it's possible for us to transform this as well. It'd definitely be inconvenient; we're not transforming AST so much as moving text around. On the other hand I dunno if it makes sense for Closure compiler to accept this code because it's pretty suspicious looking.

@Dominator008
Copy link
Contributor

Cross referencing microsoft/TypeScript#7017

I thought TypeScript fixed this in 2.0 but apparently not.

@ChadKillingsworth
Copy link
Collaborator

I would prefer we NOT accept such code. Encouraging users to take advantage of hoisting with variables and functions defined after an exit point is not something I want to see.

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

6 participants