Typescript: Catch dependencies of functions used before assignment

[OliverJAsh]

    foo()
    const x = 1;
    function foo() { x; }

or

const foo = () => x;
foo();
const x = 1;

This will produce a ReferenceError. Should TypeScript be catching this? foo can be called before its dependencies have been defined.

[mohsen1]

I don't think TS can track that. For instance this is valid code:

  foo()
    const x = 1;
    function foo() { typeof x; }

[aluanhaddad]

@mohsen1 your example throws on node 7, and in the current versions of Chrome, Edge, Firefox, and Opera, with Firefox giving the most precise error
ReferenceError: can't access lexical declaration 'x' before initialization

@OliverJAsh I was going to recommend the tslint rule no-use-before-declare, but it does not catch this either.

[DanielRosenwasser]

@aluanhaddad correct - the code itself has to be executed before you get the error. The following won't error unless you actually call f.

function f() {
    foo()
    const x = 1;
    function foo() { typeof x; }
}

In other words, the error isn't what ECMAScript calls an early error - it's not given by analyzing the code structure. It's an error caused by actually executing the code.

[aluanhaddad]

@DanielRosenwasser Yeah, the code I was actually testing with was

(function () {
    foo();
    const x = 1;
    function foo() { x; }
}())

The difference is interesting since the reference to foo is perfectly valid and the capture of x is also perfectly valid but the error arises from when the call to foo is made relative to the initialization of x.

[aluanhaddad]

I think that suggests it actually should _not_ be an error since

(function () {
    try {
        foo();
    } catch (e) {
        console.error(e);
    }
    const x = 1;
    function foo() { console.log(x); };
    foo();
}());

is valid.

[OliverJAsh]

ESLint's no-use-before-define rule catches this. Perhaps it should live in tslint, as I suggested at https://github.com/palantir/tslint/issues/1537

[rotemdan]

Bear in mind that any rule that's added to TSLint might in practice get drowned by hundreds of warnings about missing semicolons and braces etc., especially in a large project. I personally don't feel combining messages about coding conventions and the identification of actual potential software bugs to be a great idea.

[mohsen1]

Actually linting is more about catching common runtime errors that are impossible or hard to find at compile time.

[aluanhaddad]

@OliverJAsh Thank you for opening that issue with tslint. I always use this rule and wish it caught this.

Bear in mind that any rule that's added to TSLint might in practice get drowned by hundreds of warnings about missing semicolons and braces etc., especially in a large project. I personally don't feel combining messages about coding conventions and the identification of actual potential software bugs to be a great idea.

The key thing is to agree on a convention with your team. This can be very difficult to do but it is worthwhile. Then clean up your code. There might be a few commits that have a deceptively large scope but it's worth it. If people don't bother to lint the code before they check it in, then get a broom and sweep up behind them 😄 . Even better would be to use a pre-commit hook but some people will be very frustrated if you do that.

[RyanCavanaugh]

Correctly detecting this in all cases would require all function calls to trace out their call graphs, which would be pretty expensive relative to the number of bugs we'd actually detect (which are all fast errors, so you're not suffering silent undefineds anyway).

[rotemdan]

@aluanhaddad @OliverJAsh

Why did you support a conscious decision of the the team to reject the idea that a reasonable check would be performed, and which might actually catch _many_ bugs (made by developers less experienced than yourselves, probably). Is it your belief that it is not important? Is it the belief that programmers should be "self disciplined" not to make mistakes and not need a compiler? That everyone should become "educated" about the concept of "hoisting" otherwise they shouldn't be writing any code in the first place? In that case you should probably recommend them to use plain Javascript? Why would anyone need type checking anyway?

Maybe you bought the idea that it's "too complex" for the TypeScript team but not for the TSLint developers? Here's an idea, maybe the TSLint developers should implement it and contribute it to TypeScript instead? After all, they are both open source projects?

(This is not sarcasm. I'm serious)

[OliverJAsh]

@rotemdan I won't pretend to understand the performance implications of such a check, but I can imagine it would be very expensive.

[rotemdan]

@OliverJAsh

If every function had an associated cached list of potentially uninitialized or out-of-scope captured variables it unconditionally uses, and list of functions it unconditionally calls, one can recursively iterate the list of called functions and accumulate all the matching captured variables, and cache that list as well (cycles are ignored). Then at every function call site check if those variables might be potentially uninitialized or out-of-scope based on that cached list (_edit: the actual formulation is a bit more subtle than that, but I couldn't make a more thorough analysis in the limited time I have_).

I can't see this being too "difficult" (there are many things that are much more "difficult" that the compiler already does). It might only catch 80% of cases (it couldn't reliably check for passed callbacks, for example), but the increased safety would definitely be valuable. There are plenty of existing checks in the language that only catch 80% or even only 50% or 20% of potential errors. No one suddenly claims they are not valuable.

In any case I was saying I couldn't see why a perceived technical challenge should somehow convince people that somehow a "linter" can magically do this but a compiler can't, or that because it is "complex" it somehow magically means that people should convince themselves it's not necessary or important. It's a pattern I see a lot here, and I feel it's quite unfortunate.

_Edits: tried to correct some inaccuracies in the very rough algorithm outline I gave_.

[rotemdan]

Another aspect is that the way the issue is closed makes it look like they are _never going to do this_, _forever_. What about TS 3.0? 4.0? Do they mean, even in 2024 this would be "Too Complex"? (or maybe just an embarrassing artifact or the "unwise" past?)

[zpdDG4gta8XKpMCd]

what it comes down to is that TS needs a standalone thorough type checker
that takes the TS syntax but runs its own strict inference and verification
algorithms

this effort doesnt have to do anything with TS team and/or the original
codebase

volunteers are welcome

On Jan 10, 2017 6:06 AM, "Rotem Dan" [e-mail] wrote:

Another aspect is that the way the issue is closed makes it look like they
are never going to do this, forever. What about TS 3.0? 4.0? Do they
mean, even in 2024 this would be "Too Complex"? or maybe just an
embarrassing artifact or the "unwise" past?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/Microsoft/TypeScript/issues/13320#issuecomment-271548089,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA5PzUnZFxJ2cAfmqiu8Cn4s56rwbA_kks5rQ2Y8gaJpZM4Lck4L
.

[rotemdan]

@aleksey-bykov

That's a very interesting idea. What you are describing sounds like the equivalent of a secondary type checker - not an entire compiler, as the emit stage can still go through the regular compiler.

I feel something like this shouldn't be described as a "linter" though. Based on Wikipedia a "linter" means:

lint or a linter is any tool that flags suspicious usage in software written in any computer language. The term lint-like behavior is sometimes applied to the process of flagging suspicious language usage. Lint-like tools generally perform static analysis of source code.

It feels a bit inaccurate describing the usage of undefined values to always be just "suspicious". There are cases where these usages can be deterministically identified (say, for the site of a particular function call) to always cause run-time errors. Of course there are other cases where it could only be seen as "suspicious", but that's not what I was personally referring to here.

_Edits: fixed some phrasing_

[zpdDG4gta8XKpMCd]

as far as linting, i am sure you know, there is tslint which is a good
start as a planform to run custom rules against TS code

it is based on TS api which guarantees it's always up to date with the
compiler,

until recently tslint could only check syntax, now it also able to run
semantic checks

On Jan 10, 2017 7:10 AM, "Rotem Dan" [e-mail] wrote:

@aleksey-bykov https://github.com/aleksey-bykov

That's a very interesting idea. What you are describing sounds like the
equivalent of a secondary type checker - not an entire compiler, as the
emit stage can still go through the regular compiler.

I feel something like this shouldn't be described as a "linter" though.
Based on Wikipedia https://en.wikipedia.org/wiki/Lint_%28software%29 a
"linter" means:

lint or a linter is any tool that flags suspicious usage in software
written in any computer language. The term lint-like behavior is sometimes
applied to the process of flagging suspicious language usage. Lint-like
tools generally perform static analysis of source code.

It feels a bit inaccurate describing the usage of uninitialized variables
to always be just "suspicious". There are cases where these usages can be
deterministically identified to always cause run-time errors. Of course
there are other cases where it could only be seen as "suspicious", but
that's not what I was personally referring to here.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/Microsoft/TypeScript/issues/13320#issuecomment-271559721,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA5PzdsZEHQdzRKMFkauANthU4C2QITgks5rQ3UUgaJpZM4Lck4L
.

[rotemdan]

@aleksey-bykov

It sounds very interesting if the compiler would export its entire type-checker (including many of its internal subroutines) as a library (without requiring forking it, I mean). Unfortunately I don't currently have time for development of that kind, but I might have tried if I did.

I still think, though, in those particular cases the linter/secondary checker (whatever it may be called) should produce an error, not a warning, in the case it can conclusively predict a run-time error. Otherwise it might just be one more warning lost in a sea of hundreds of formatting and style messages.

[OliverJAsh]

Updated the original post with an example of how this might occur without hoisting:

const foo = () => x;
foo();
const x = 1;

[rotemdan]

I wanted to mention there's a trivial way to work around the hoisting problem - by avoiding it entirely:

Instead of using function ():

    foo();
    const x = 1;
    function foo() { x; }

Using const foo = function() or let foo = function():

    foo(); // 'Block-scoped variable 'foo' used before its declaration.'
    const x = 1;
    const foo = function() { x; } // or let foo = function()

(or const foo = () = {...}, which I use a lot myself)

I could imagine a compiler flag that would prevent regular function use before definition or a linter rule that would warn about it (this may be more refined to only warn at the particular cases where captured variables are used unsafely).

It still doesn't address the problem of usage of uninitialized captured variables:

    let x: { a: number };
    const foo = function() { 
        x.a;
    } 
    foo(); // runtime error: x is undefined 
    x = { a: 1 };

So in this case, since the analysis is more complex and has been already considered and disapproved of by the team, a linter rule could mark the above usage of x.a as "suspicious". Though it would also mark this one as well:

    let x: { a: number };
    const foo = function() { 
        x.a;  // suspicious usage of potentially uninitialized variable x.
    }
    x = { a: 1 };
    foo(); // no actual run-time error

So this rule might have many false positives unless it becomes more sophisticated. I'm not sure if it would be practical at its basic form.

_Edits: fixed several mistakes in the examples_

[RyanCavanaugh]

Here's how to understand lint vs TypeScript

| | TypeScript | TSLint |
| ------------- | ------------- | --------------- |
| False Positives | Should be avoided at all costs | Generally OK |
| Configuration | Number of settings is relatively low | Extremely large configuration surface area |
| Complexity | Many settings interact with each other | Configuration settings do not interact |
| Suppression | No warning suppression mechanism | Highly-configurable suppression (per-line, per-file, per-region) |

You could imagine that TypeScript would just implement every TSLint rule and then we'd say "Ta-da! Now everyone writes their code the same correct way". We've intentionally not been hyperopinionated about how you should write your code because TS isn't supposed to be A One True Way to write typed JavaScript. I imagine that @aleksey-bykov 's codebase and @OliverJAsh have extremely little stylistic overlap - we want there to be room both for TypeScript to be Haskell Lite, and to be JavaScript But Let's Add Some Types.

[RyanCavanaugh]

Another aspect is that the way the issue is closed makes it look like they are never going to do this, forever. What about TS 3.0? 4.0?

Remember, it's all humans behind the scenes here. Nothing's forever. If this turns out to be a bigger problem than we thought, or we find some way to do it cheaply, or we run out of other problems to solve, it's on the 1,000+-item list of things we can think about doing.

[rotemdan]

@RyanCavanaugh

I personally never suggested the idea that a type-checker would try to enforce coding standards, as well as checking for heuristics like Potential unsafe usage of... I don't think that's what it's supposed to do. The idea was to look for cases where the error can be proven to happen, and it's not just a "suspicion", and leave the more speculative one for the linter, maybe. There are cases where use-before-definition can be shown with absolute confidence to be run-time errors.

By the way, due to the risks of hoisting and reassignment of functions discussed here, I'm actually in the process of converting a very large number of function f declarations (an entire code base) to const f = function() and const f = () => {}. It turns out the resulting code isn't as pretty as with function and the export const func = async (): Promise<ReturnType> => { ... } pattern looks a bit awkward and less readable when repeated 100s of times.