Compiler warnings / errors for common automatic semi-colon insertion (ASI) issues

[jeffmay]

Personally, I like the look (and simplicity) of ECMA6 / TypeScript without semi-colons. The only problem is that there are some known danger-zones with automatic semi-colon insertion. Could we have compiler warnings to prevent these from slipping through the cracks?

Example:

function dangerous(): string {
    return
        "watch the semi-colons!"
}
dangerous()

Compiles to the following with no warnings or compiler errors (even though the function actually doesn't return a string):

function dangerous() {
    return;
    "watch the semi-colons!";
}
dangerous();

This specific case is kind of scary / annoying, but it might be worth looking into others.

Thoughts?

[basarat]

Good suggestion 👍 Seems more like a tslint thing for me though.

My 2c:
I find not having semicolons throws off formatting and code analysis in a bunch of JS tools so I insert them whenever.

[jeffmay]

@basarat do the code analysis tools apply to TypeScript or the resulting JavaScript? Because TypeScript will always insert the semi-colons on the resulting JavaScript, so it shouldn't affect any JavaScript tools.

[mhegazy]

@basarat was referring to tslin: https://github.com/palantir/tslint, and that is a typescript linter.

[antoinerousseau]

+1

another scary example:

var foo = true
// some comment
['a', 'b'].forEach((l: string) => {/**/})

compiles into

var foo = true['a', 'b'].forEach(function (l) {/**/})

...

[jeffmay]

@mhegazy I'm more interested in removing semi-colons from the language. A linter to require semi-colons would be going the other direction.

Personally, I feel like the compiler should be able to detect when a missing semi-colon would fail to satisfy the type system (like in the examples given above) and give an appropriate warning or error without adding a separate build and lint step. Any time a statement without a semi-colon would fail to satisfy the expected type, it should be a compiler error and not a silent runtime failure.

[basarat]

I feel like the compiler should be able to detect when a missing semi-colon would fail to satisfy the type system

Since undefined is compatible with anything an empty return being effectively return undefined does satisfy the type system requirement.

But I do get what you are asking for. Just don't have a spec for it :)

[jeffmay]

Also, @mhegazy, I'm glad you mentioned the linter, I think it is important for the community to maintain a standard on style. I am just personally against semi-colons. They feel more and more clumsy these days. Especially when combined with the more functional style of fat-arrow functions, functional apis, chaining, etc.

[basarat]

. I am just personally against semi-colons

Don't. Code analysis tools will have a hard time formatting. And as pointed out, ASI is not pretty. It is okay to be against semi-colons if all whitespace was always significant, but it isn't and JavaScript has weird edge cases as pointed out by @antoinerousseau

[basarat]

You think it would add a semi-colon but it will not. And I can't know what you think. Perhaps you meant return undefined and didn't bother to write it?

[jeffmay]

return undefined is such a strange thing to do. I would rather that we have a compiler error on return than allow it to default to return undefined. And if you want to return undefined, you should have to explicitly state return undefined or return;. As far as code-analysis, I understand how it would be an issue, but again, I don't see why the compiler couldn't just insert the semi-colons for you when it isn't dangerously ambiguous and otherwise raise an error. Maybe the tooling could call into this phase of the compiler before processing the TypeScript / JavaScript?

I can't think of any case where an ambiguous statement due to an absent semi-colon is not just bad style that should either require an explicit semi-colon or be a compiler error. I can understand always adding it when interpreting the code line by line, because you don't know when to stop interpreting, but a compiler could reasonably bail out when a semi-colon cannot be safely added.

[basarat]

couldn't just insert the semi-colons for you when it isn't dangerously ambiguous

Definitely not. You don't want to deviate from JS semantics. That would make porting JS code hard.

otherwise raise an error

This is what you have been requested and I support that ❤️

Again, perhaps this is better suited by an additional option in tslint.

[DanielRosenwasser]

return undefined is such a strange thing to do.

Yup. Some people don't want to write undefined the same way some people don't want to write semicolons.

[antoinerousseau]

OK so the best would be to implement warnings on edge cases like these, we just have to choose between the TypeScript compiler and the linters like tslint

[DanielRosenwasser]

tslint is honestly probably the better place for this. I'm already inclined to say this is somewhat out of scope since our errors are effectively warnings, and the only checking we do at the whim of a flag is with noImplicitAny. I think it would be a awkward to create a flag tailored specifically to this.

[basarat]

I would guide this by : compiler errors are independent of developer coding style

Linter errors are driven by coding style.

If this were added will definitely need a new compiler flag. Those should not be added lightly.

[jeffmay]

@DanielRosenwasser

Some people don't want to write undefined

You wouldn't be stopped from putting return;

[ghost]

A quick question, as I can't find it in the spec - are semicolons intended to be optional in TypeScript?

I too would prefer not to use semicolons, as I'm coming from Scala and TypeScript seems to otherwise be the closest Javascript derivative.

[antoinerousseau]

They are optional in Javascript (see ASI), thus optional in TypeScript, since it is a superset of Javascript

[jeffmay]

I understand the TypeScript (the language) is a superset of JavaScript and should remain as close to that as possible. However, TypeScript (the compiler) could be a little more helpful on catching bugs that would slip through the cracks. I don't think finding bugs is a matter of style. Although, I suppose I can just pay for WebStorm to solve the issue.

[ghost]

I agree with @jeffmay. WebStorm is awesome (the IDE for javascript IMHO), but not sure how it solves the issue? are you referring to the dynamic code inspector in WS?

@antoinerousseau I like that semicolons are optional in TypeScript just like they are in Scala.

[mrkaspa]

+1 for removing semicolons

[andreas-bergstrom]

+1 for no semicolons.

[basarat]

This specific case is kind of scary / annoying

There is already a tslint rule that errors in this case (early return), Add the following to tslint.json:

"no-unreachable" : true

🌹

[DanielRosenwasser]

I think @vladima's CFA work covers this (i.e. reachability analysis). We should close it when appropriate.

[ytechie]

Just wanted to throw in my 2 cents - this problem bit me in the ass today. Spent a decent amount of time figuring it out, and then found this issue.

[mhegazy]

the original issue for return statements will now be flagged as error with #4788. please reopen/refile if there are other issues that are not covered by the reachability checks reported by the compiler in TypeScript 1.8.

[mnpenner]

@mhegazy What about @antoinerousseau's scenario?

[cogitaria-admin]

I don't think "scary" or "dangerous" are good characterizations. In particular, the dangerous() function example will never produce the desired result. It would never pass an automatic or a manual test. For those who do not use unnecessary semicolons, the code jumps off of the page as an error. It's no more "dangerous" than for(...); { ... } really.

Having said that, the dangerous() example will actually produce a typical lint warning: unreachable code.