Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
morganwdavis opened this issue Sep 20, 2020 · 5 comments · May be fixed by #40942
Open

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

morganwdavis opened this issue Sep 20, 2020 · 5 comments · May be fixed by #40942

Comments

@morganwdavis
Copy link

@morganwdavis morganwdavis commented Sep 20, 2020

TS Template added by @mjbvz

TypeScript Version: 4.1.0-dev.20200916

Search Terms

  • convert to template string
  • refactoring / code action

(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 mjbvz changed the title "Convert to template string" not offered in certain conditions and interactions Enable "Convert to template string" on expressions that don't start with a string Sep 21, 2020
@mjbvz
Copy link
Contributor

@mjbvz mjbvz commented Sep 21, 2020

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 mjbvz transferred this issue from microsoft/vscode Sep 21, 2020
@mjbvz
Copy link
Contributor

@mjbvz mjbvz commented Sep 21, 2020

Self contained example:

const obj = { prop: '123' };
const z = obj.prop + 'b' + 'c'
  1. Select obj.prop and try triggering code actions
@DanielRosenwasser
Copy link
Member

@DanielRosenwasser DanielRosenwasser commented Sep 21, 2020

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
Copy link
Author

@morganwdavis morganwdavis commented Sep 21, 2020

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
Copy link

@dbartholomae dbartholomae commented Sep 27, 2020

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.