microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.77k stars 12.46k forks source link

Enable "Convert to template string" on expressions that don't start with a string #40671

Closed morganwdavis closed 3 years ago

morganwdavis commented 4 years ago

TS Template added by @mjbvz

TypeScript Version: 4.1.0-dev.20200916

Search Terms

(see comment for condensed repro of the problem)


Issue Type: Bug

Please consider the following JavaScript example:

const foo = location.href + '#' + 'bar';

The editor will raise a suggestion that the concatenation expression should be a template literal. But when you peek the problem in expectation of finding a quick fix for "Convert to template string", none appears.

image

However, if the first part of the expression is anything other than an object reference, it works as expected:

image

Additionally, Quick Fix interactions, depending on how you engage them, have inconsistent behaviors:

const url = '//xyzzy.bar';
const foo = url + '#' + 'foo';

image

If you click on the Quick Fix link...

image

...this is all you get:

image

But if you type Control + . (Period) you get the menu with the "Convert to template" option:

image

Similarly, if you click on the lightbulb icon, you also get the option:

image

VS Code version: Code 1.49.1 (58bb7b2331731bf72587010e943852e13e6fd3cf, 2020-09-16T23:27:51.792Z) OS version: Windows_NT x64 10.0.19041

System Info |Item|Value| |---|---| |CPUs|Intel(R) Core(TM) i7-10875H CPU @ 2.30GHz (16 x 2304)| |GPU Status|2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
opengl: enabled_on
protected_video_decode: enabled
rasterization: enabled
skia_renderer: disabled_off_ok
video_decode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled| |Load (avg)|undefined| |Memory (System)|31.75GB (24.86GB free)| |Process Argv|| |Screen Reader|no| |VM|0%|
Extensions (20) Extension|Author (truncated)|Version ---|---|--- Bookmarks|ale|11.3.1 vscode-intelephense-client|bme|1.5.4 sqlformatter|bre|0.0.7 bracket-pair-colorizer|Coe|1.0.61 vscode-eslint|dba|2.1.8 vscode-html-css|ecm|0.2.3 prettier-vscode|esb|5.6.0 unicode-substitutions|Gle|2.2.8 vscode-phpfmt|kok|1.0.30 vscode-apache|mrm|1.2.0 python|ms-|2020.8.109390 hexeditor|ms-|1.3.0 vscode-typescript-tslint-plugin|ms-|1.2.3 php-docblocker|nei|2.1.0 material-icon-theme|PKi|4.3.0 partial-diff|ryu|1.4.1 rewrap|stk|1.13.0 change-case|wma|1.0.0 markdown-pdf|yza|1.4.4 markdown-all-in-one|yzh|3.3.0 (1 theme extensions excluded)
mjbvz commented 4 years ago

The error/warning is coming from your linter, not VS Code. Also, Convert to template string is a refactoring not a quick fix, so it's not expected to show up in the quick fix menu.

I've update the issue title so this issue only tracks enabling convert to template string in cases where the first part of an expression is not a string literal

mjbvz commented 4 years ago

Self contained example:

const obj = { prop: '123' };
const z = obj.prop + 'b' + 'c'
  1. Select obj.prop and try triggering code actions
DanielRosenwasser commented 4 years ago

Not that familiar with the code, but sounds like this would be a matter of checking for other node kinds, (e.g. Identifier, PropertyAccess, ElementAccess, ParenthesizedExpressions), skipping those nodes until you reach something else, and checking if that node is a + expression

morganwdavis commented 4 years ago

Perhaps worth clarifying (as the subject changed and is not 100% accurate from my observations), the refactoring to template literal will work if the first part of the expression is anything other than a dotted object.property reference. In addition to string literals, simple variables, and numeric literals all work -- not just bare "strings".

const x = 'y';
const w = x + 'A' + 'B';
const z = 5 + 'a' + 'b';

It is only obj.prop at the beginning that will cause it to just shrug and make you do it manually.

Thanks @mjbvz for pointing out that it is tslint raising the refactoring suggestion. I understand tslint is being deprecated for eslint, but I've had no luck converting a large project over to eslint despite following the migration guide. I'm unable to test if eslint has this same issue. If not, perhaps the fix lies there.

And if tslint v1.2.3 is still being maintained by Microsoft, it would be nice if clicking the Quick Fix ... (Ctrl+.) link in the popup dialog worked as advertised -- that is, the behavior you get only if using the keyboard Ctrl+. option or the Show Fixes (Ctrl+.) menu.

dbartholomae commented 4 years ago

Is this a good issue to get into the TypeScript codebase? Unwound line to contribute to TypeScript during Hacktoberfest and if it makes sense would start with this issue :)