jslint-org / jslint

JSLint, The JavaScript Code Quality and Coverage Tool
https://www.jslint.com
The Unlicense
3.62k stars 462 forks source link

function hoisting not allowed #272

Closed bunglegrind closed 3 years ago

bunglegrind commented 4 years ago

Dear Mr. Crockford, I'm aware that there is already an issue concerning this topic (it's #33 ) but it's 9 years old and in the meantime the language and JSLint have evolved.

Specifically, JSLint requires all the functions to be declared before their usage, which leads to two consequences in the coding style:

function foo() {
   console.log("foo");
   setTimeout(bar, 1000);
}

function bar() {
   console.log("bar");
   setTimeout(foo, 1000);
}

foo();

So, only for the function declarations, can you reconsider to relax this constraint?

Anyway, many thanks for your software!

Cheers

mauskin commented 4 years ago

Reading is a concern. When we’re talking about code it is less confusing to read it as an article on a complex topic—further concepts are based on the ones described at the beginning—as opposed to a novel with several plot twists.

A function though could reference another one declared below in case the latter one calls the former.

function first() {
    return second;
}

function second() {
    return first();
}

I’ve stumbled upon it while reading include.js.

bunglegrind commented 4 years ago

Yes, this is the "bottom-up" approach which I was referring to.

Btw, I haven't noticed that that kind of declaration is allowed. I don't understand why, though...

kaizhu256 commented 4 years ago

the following kludge will pass jslint (which i've used in past).

/*jslint browser devel*/
let foo;
let bar;

foo = function () {
    console.log("foo");
    setTimeout(bar, 1000);
};

bar = function () {
    console.log("bar");
    setTimeout(foo, 1000);
};

foo();

i say kludge, b/c having functions cyclically call each other is error-prone and difficult to debug/trace.

jslint eventually trained me to avoid it and look for better-ways that are easier to qa, e.g.:

/*jslint browser devel*/
function foo() {
    let ii = 0;
    setInterval(function () {
        switch (ii) {
        case 0:
            console.log("bar");
            break;
        case 1:
            console.log("foo");
            break;
        }
        ii = (ii + 1) % 2;
    }, 1000);
}

foo();
bunglegrind commented 4 years ago

I agree, it's a kludge, you're tricking JSLint. The main issue is in the impossibilty to have a top-down style of writing, though; the two functions calling each other are a secondary issue. You can usually (but not always) refactor. Anyway, a switch-case (or if-else) as an alternative is IMHO even worse, since you're basically writing two functions inside one - resorting to some kind of polymorphism (if possible, of course) should be preferred.

bunglegrind commented 4 years ago

A function though could reference another one declared below in case the latter one calls the former.

By inspecting jslint code, I see that line 3947 is the one that allows such kind of declaration and this exception was added in June 2018. It looks to me really like an ad hoc exception in order to allow some edge case codes...

FrankFang commented 3 years ago

You could declare an initialize/main/run function, and put it at the top, and run it at the bottom.

kaizhu256 commented 3 years ago

i just noticed jslint.js has 2 functions recursively calling each-other, with no warnings. suspect its a bug, but maybe could be turned into a feature as well.

// https://github.com/jslint-org/jslint/blob/v2021.5.30/jslint.js#L4573
function walk_expression(thing) {
    if (thing) {
        if (Array.isArray(thing)) {
...
        } else {
...
                // walk_statement() seems to be magically hoisted by jslint
                walk_statement(thing.block);
...
}

// https://github.com/jslint-org/jslint/blob/v2021.5.30/jslint.js#L4613
function walk_statement(thing) {
    if (thing) {
        if (Array.isArray(thing)) {
...
        } else {
            preamble(thing);
            walk_expression(thing.expression);
...
}
douglascrockford commented 3 years ago

JSLint allows

function a() {
    return b();
}

function b() {
    return a();
}
kaizhu256 commented 3 years ago

oh nice to know : ) but if calls are wrapped in setTimeout (in original poster's example) jslint gives out-of-scope-warning. verified behavior in both current and old version v2020.11.6 @ https://www.jslint.com/branch-v2020.11.6/index.html

/*jslint browser*/
function a() {
    return b();
}

function b() {
    setTimeout(a);
}

// warning
// 1 'b' is out of scope. // line 3, column 12
//     return b();

image