GeezerVison is showing github.com — Compiler warnings / errors for common automatic semi-colon insertion (ASI) issues · Issue #2575 · microsoft/TypeScript
Compiler warnings / errors for common automatic semi-colon insertion (ASI) issues #2575DescriptionPersonally, 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? Activitybasarat commented on Apr 2, 2015 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 commented on Apr 2, 2015 Basarat Ali Syed (@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 commented on Apr 2, 2015 antoinerousseau commented on Apr 13, 2015 +1
another scary example:
var foo = true
// some comment
['a', 'b'].forEach((l: string) => {/**/})compiles into
var foo = true['a', 'b'].forEach(function (l) {/**/})... Reacted by Mark Penner and Andrew Jarvis jeffmay commented on Apr 14, 2015 Mohamed Hegazy (@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 commented on Apr 14, 2015
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 commented on Apr 14, 2015 Also, Mohamed Hegazy (@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 commented on Apr 14, 2015
. 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 Antoine Rousseau (@antoinerousseau) basarat commented on Apr 14, 2015 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 commented on Apr 14, 2015 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 commented on Apr 14, 2015
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 commented on Apr 14, 2015
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 commented on Apr 14, 2015 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 commented on Apr 14, 2015 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. antoinerousseau commented on Apr 14, 2015 They are optional in Javascript (see ASI), thus optional in TypeScript, since it is a superset of Javascript jeffmay commented on Apr 14, 2015 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. mrkaspa commented on Aug 21, 2015 +1 for removing semicolons andreas-bergstrom commented on Oct 10, 2015 basarat commented on Oct 11, 2015
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:
🌹 DanielRosenwasser commented on Oct 11, 2015 ytechie commented on Oct 28, 2015 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 commented on Dec 9, 2015 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 commented on Aug 31, 2017 cogitaria-admin commented on Jan 19, 2018 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.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|