Typescript: autoimport should care about semicolons

[mjbvz]

_From @phra on November 9, 2017 20:11_

i usually write my ts/js code without semicolons and there is now way to configure vscode to avoid adding semicolons to auto imports. it would be nice an automatic parsing of the tslint config or at least a configurable option in the settings.

• VSCode Version: 1.18
• OS Version: Linux

Steps to Reproduce:

1. use new autoimport feature
2. imports are always with semicolons

_Copied from original issue: Microsoft/vscode#37991_

[andy-ms]

It looks like FormatCodeSettings only controls whitespace, so this would have to be an emitter option?

[mhegazy]

We could add a new set of CodeFixSettings that include semicolon and quotes as well as any other things that would be needed by code fixes..

alternatively, we can teach the formatter about semi colons.

[andy-ms]

It's probably better to keep all of those settings in one place. So your second suggestion sounds best.

[Lxxyx]

I like ts importer. You can configure semicon, space by youself.

https://github.com/pmneo/ts-importer

[phra]

yep, i think that updating the formatter can be the best option too.

[timc13]

just to add my 2 cents, a few lint rules can probably fixed at the same time. Here are the rules that are breaking for me when I auto fix an import:

• ordered-imports
• semicolon
• quotemark

[ksmutny]

Why doesn't the TS AutoImport just respect TS Lint settings?

[phra]

any news?

[mhegazy]

With https://github.com/Microsoft/TypeScript/pull/22236 added, we can add a configuration option for this feature. we would be happy to take a PR for this.

[chenasraf]

I'd be happy to work on this, can anyone point me to the general direction of where I should look?
Where is the auto import logic stored?

[mhegazy]

https://github.com/Microsoft/TypeScript/blob/master/src/services/codefixes/importFixes.ts

[andy-ms]

I don't think this issue is actually specific to importFixes -- what is needed is a formatter option to remove unnecessary semicolons. (This would then apply to all text changes, such as extract-to-constant which currently adds a semicolon at the end of the new constant.) Currently the formatter only affects whitespace. See formatting.ts.

[chemzqm]

Both auto import and organize imports feature have this problem, would be great if there's an option to disable this.

[demisx]

Would be great just to turn off semicolons for all TypeScript files globally and have each extension honor it. We don't use them at all except those very rare occasions where we have no choice.

[ananiy]

hope add a setting option to remove the trailing semicolon... 👍

[ivawzh]

So sad this issue is not moving.

[Christilut]

This problem is somehow solved for me now. I noticed a difference in tslint.json in another project and even though it makes no sense, the following makes auto import work and it removes semicolons:

In tslint.json you can have the "semicolon": false line. If you remove it all together (don't set it to true or false) then auto imports get their semicolons removed.

This probably has something to do with my VSCode settings too, so here are the relevant parts of my settings.json. Hopefully someone can use this!

json "tslint.autoFixOnSave": true, "editor.codeActionsOnSave": { "source.organizeImports": true }, "eslint.autoFixOnSave": true, "editor.formatOnSave": true

[demisx]

If I remove "semicolon": [true, "never"] line from tslint.json, then I get tons of "Missing semicolon" warnings from TSLint. Will not work for me, unfortunately.

[GaryChangCN]

Same question here

[mjomble]

I've looked into this a bit, but I'm not sure if/when I might have the time to submit a proper PR.

However, if anyone else is interested, here's what I've found so far:

This line in textChanges.getNonFormattedText is used to generate the import code.
It creates a new Printer with options { newLine, neverAsciiEscape: true }.
Fortunately, PrinterOptions already has an omitTrailingSemicolon: boolean member that does exactly what is needed here.
So the tricky part is getting the right boolean value to the right place.
Like other methods of textChanges, getNonFormattedText should probably take a formatContext: formatting.FormatContext parameter, from which you could get formatContext.options which is a FormatCodeSettings object.
Then we'd just need to add a new option to FormatCodeSettings and hook it up to a VS Code setting.

Another tricky part is coming up with a good name for the new setting, as getNonFormattedText is also used for other purposes than auto imports.
And a generic "use / don't use semicolons in all automatically generated code" setting would probably need more changes across the project.

But I'm hoping this will provide a good starting point for someone interested in implementing this :)

[mjomble]

As a temporary workaround, you can modify your local TS server that VS Code uses.

On Windows, you need to edit %LOCALAPPDATA%\Programs\Microsoft VS Code\resources\app\extensions\node_modules\typescript\lib\tsserver.js (if you're on a different OS or using workspace TS, the location will be different).

Find function getNonformattedText( and a few lines after that, replace

ts.createPrinter({ newLine: newLine, neverAsciiEscape: true }

with

ts.createPrinter({ newLine: newLine, neverAsciiEscape: true, omitTrailingSemicolon: true }

Then, if it's running, restart VS Code (or use the TypeScript: Restart TS Server command in it).

Note that this is a temporary hack and if VS Code updates to a newer TS version, it will probably stop working and you need to apply this change again.

[lppedd]

@mjomble I'm sorry to say it doesn't work anymore, apparently.
EDIT: my bad, I had set the Workspace TS.
Thanks you!

[mjomble]

Looks like this workaround breaks in TypeScript 3.2

tl;dr: You can apply an even more horrible hack as a temporary fix :)

Perform the same change as in the earlier comment and also add a new line before the ts.createPrinter(... line:

for (var key in writer) writer[key] = writer[key];

Or, if you want to play a little safer, use these lines instead of the for-loop:

writer.write = writer.write;
writer.setLastNonTriviaPosition = writer.setLastNonTriviaPosition;

---

Details:

getNonformattedText creates a writer with new Writer(...)

The Writer class extends EmitTextWriter, so it compiles to older JS in such a way that EmitTextWriter methods are added to the Writer prototype, but not directly to instances of Writer.

Later, if the omitTrailingSemicolon setting is added, this Writer object gets wrapped by getTrailingSemicolonOmittingWriter() due to this change

The new writer is created via object spread as seen here

Prototype methods are lost when using object spread, so at runtime, we now have an EmitTextWriter without a write() method.
This crashes with TypeError: writer.write is not a function.

In VS Code, the effect is that no auto-import suggestions are provided.

@rbuckton I'm pinging you just in case, so you would be aware of the getTrailingSemicolonOmittingWriter and Writer extends EmitTextWriter combo that results in the loss of prototype methods.
On the one hand, I realise this is internal code and there's no obligation for TypeScript to support hacks like the above. On the other hand, this could potentially lead to other bugs, so it might deserve some attention.

[ekifox]

Any news?

[Hatko]

It would be really nice to have this setting

[khaosdoctor]

I'm trying to find this piece of code and set it up in a PR, however it is a bit foggy for my about in what project is this problem related to. It'd be nice to have a VSCode setting about this, however this seems to be in the TS server.

[mjomble]

@khaosdoctor we should probably first add a new settings for it in the TS server (this repo) and then make a change in the VS Code repo that would use this setting.

More thoughts and suggestions in this comment

[khaosdoctor]

Yeah, I just talked to some people here and they have explained me this. We need to add the logic to read this setting in the TS server and another PR to VSCode to create this setting altogether.

[rdewolff]

I keep cleaning our projects files' import as VSCode is adding semicolon automatically and we always forget some.

Any plan to do this?

[mlshv]

I use VSCode for more than 2 years (and really enjoy it, btw). One of the first bugs I've noticed was this: when you use autoimport it always adds a semicolon in the end of the line. Even if you don't want it and have all kinds of settings for it. I thought "Ok, it's a pretty obvious and simple thing, they gotta fix it really soon". I was waiting for ages. And today I finally decided to find out what's going on.

Is it so complicated? Does somebody work on this issue? Is there any way to help?

[khaosdoctor]

@mlshv It's pretty tricky since you have to add two settings, one in the TS Language Server, to allow the use of semicolons, and another one in the VSCode to read the previous one.

We discuss this in some previous comments

[andrewbranch]

There's a milestone _and_ an assignee now → It ✨ should ✨ get done for 3.6 🙂

[cancerberoSgx]

as @mjomble said, Printer doesn't seem to be respecting omitTrailingSemicolon, np matter its value it always print trailing semicolon, would be awesome if this is also fixed, thanks

[rob2d]

I noticed that there was an algorithm for detecting semi colons, but in this fix (that has been closed), does it only apply this to import checks? the reason I ask is I have seen it very common in certain codebases (especially React) not to include semi colons for import/export, but to do so for all other code. If all that was considered was the entire file having at least 3/4 semi colons on each statement, it would not detect this sort of scenario properly or intelligently.

[andrewbranch]

@rob2d no, the fix considers any statements that normally have semicolons but don’t require them.

If all that was considered was the entire file having at least 3/4 semi colons on each statement

That said, it starts looking from the top of the file and quits once it sees a handful of examples to work with, so if you had a bunch of semicolon-less imports at the top of your file, it should infer that you don’t want semicolons. So in that case, auto-imports will do what you want, and other code insertions won’t.

I have seen it very common in certain codebases (especially React) not to include semi colons for import/export, but to do so for all other code

Before joining TypeScript, I worked heavily in React for several years, and I’ve never heard of such a thing. Can you send some examples?

[andrewbranch]

@rob2d Unless I’m really missing something, every example you just sent doesn’t use semicolons _at all_.

In my memory, eslint-config-standard, which prefers no semicolons, was really popular with many React folks in the early days, and more recently eslint-config-react-app I think has become a de-facto choice.

To be clear, I have absolutely zero judgement on how people format their code—I want you to put semicolons wherever they make you happy, no more and no less—it’s just that I feel like we’re unlikely to add additional heuristics to TypeScript to support preferences that are atypical and/or more work to test for. That’s why I asked for examples—not because I don’t believe you or think that’s a weird preference to have, but rather because I want to try to form an idea of how common this is by looking at the size of projects that adopt such a convention.

[rob2d]

well, I put some links there? Seems you did not click them if you are making this statement. I mean obviously in terms of that, check the React code and not the webpack code or course.

Edit: no, you definitely clicked.

[andrewbranch]

I went into each repo you linked and found a random JS file, so possibly we’re looking at different files. Can you send a specific file where you see this convention? Here’s what I looked at:

• https://github.com/reduxjs/redux/blob/master/src/createStore.js
• https://github.com/reduxjs/react-redux/blob/master/test/components/Provider.spec.js
• https://github.com/cssinjs/jss/blob/master/packages/jss-plugin-compose/src/index.js
• https://github.com/ReactTraining/react-router/blob/v3/modules/IndexRedirect.js

[blikblum]

Seems you did not click them if you are making this statement.

I clicked on the links and they are not using semicolons at all

[rob2d]

Edit: upon further reflection, I hadn't noticed that they were either all or nothing, and I am an idiot 🙃 my whole life has just changed a bit thanks to this thread.

TLDR: you all have a very good point. Sorry about that.

[cancerberoSgx]

@andrewbranch - Thanks for working on the issue, I'm testing latest version to see if this issue was solved in PrinterOptions which has a property exactly for this which was working in the past so we developers can have control . Will update with results here. Left some questions / opinions https://github.com/microsoft/TypeScript/pull/31801#issuecomment-518718313 thanks

[mjomble]

It might make more sense to have this discussion on #31801

[RyanCavanaugh]

@cancerberoSgx replying to your pre-edit content, our belief was that implementing this as a heuristic would actually be the best behavior, since the tool would just "do the right thing" with no necessary configuration from the user. It seems it'd be better if TS simply inserted semicolons in files that seemed to use semicolons so that users didn't have to go find the configuration option.

[cancerberoSgx]

@RyanCavanaugh I give my feedback https://github.com/microsoft/TypeScript/pull/31801 so there's less noise, thanks - ("do the right thing" sounds strange particularly if in a platform extendible by third parties with their own opinions- I think "the right thing" is to consistently support any code style that the JavaScript language specification allows any user to write their code with (since there's no standard for that and I don't think airbnb style guide or any other is even a defacto standard on this regard.Currently semicolons behavior and settings API is inconsistent with the rest of code format related semantics)