-
Notifications
You must be signed in to change notification settings - Fork 226
Enable the @typescript-eslint/restrict-template-expressions rule #4111
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
Changes from 1 commit
3511e8a
527519d
b305a21
24626c8
25ba228
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,7 +89,7 @@ export async function askForLanguage( | |
| if (!isQueryLanguage(language)) { | ||
| void showAndLogErrorMessage( | ||
| extLogger, | ||
| `Language '${language}' is not supported. Only languages ${Object.values( | ||
| `Language '${language as QueryLanguage}' is not supported. Only languages ${Object.values( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an interesting case because the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think casting it to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with casting to |
||
| QueryLanguage, | ||
| ).join(", ")} are supported.`, | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -68,9 +68,7 @@ function eventFired<T>( | |||||
| ): Promise<T | undefined> { | ||||||
| return new Promise((res, _rej) => { | ||||||
| const timeout = setTimeout(() => { | ||||||
| void extLogger.log( | ||||||
| `Waiting for event ${event} timed out after ${timeoutMs}ms`, | ||||||
| ); | ||||||
| void extLogger.log(`Waiting for event timed out after ${timeoutMs}ms`); | ||||||
|
||||||
| void extLogger.log(`Waiting for event timed out after ${timeoutMs}ms`); | |
| void extLogger.log(`Waiting for event ${String(event)} timed out after ${timeoutMs}ms`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The vscode.Event<T> type is just an interface with a "listen" method. So I don't know if there is any good way we could stringify the event to find out what it was. 🫤
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we just pass a string to this function that gives a "friendly name" for the event? It seems like there's only 1 caller anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. I thought it would be hard to do but you're right it's only called from one place and it's really easy to pass in an event name arg.
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -80,11 +80,11 @@ class AstViewerDataProvider | |||||||
| const treeItem = new TreeItem(item.label || "", state); | ||||||||
| treeItem.description = line ? `Line ${line}` : ""; | ||||||||
| treeItem.id = String(item.id); | ||||||||
| treeItem.tooltip = `${treeItem.description} ${treeItem.label}`; | ||||||||
| treeItem.tooltip = `${treeItem.description} ${typeof treeItem.label === "string" ? treeItem.label : (treeItem.label?.label ?? "")}`; | ||||||||
|
||||||||
| treeItem.tooltip = `${treeItem.description} ${typeof treeItem.label === "string" ? treeItem.label : (treeItem.label?.label ?? "")}`; | |
| const labelText = typeof treeItem.label === "string" ? treeItem.label : (treeItem.label?.label ?? ""); | |
| treeItem.tooltip = `${treeItem.description} ${labelText}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit ugly but the type of treeItem.label is string | TreeItemLabel | undefined and it doesn't seem that TreeItemLabel implements toString().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will item.location.toString() return? It looks like item.location is an object, so would that just give [Object object]?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good spot. I just saw UrlValue as the type and assumed it would stringify nicely.
Also I've been trying to test this to see what it looks like and I can't work out how to get this value to show up. It's not the hover text of the tree item but it's the tooltip of the on-click command. I'm not convinced it shows up anywhere so I've just changed it to a static "Go to Code" like the title.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -99,7 +99,7 @@ async function previewQueryHelp( | |||||
| telemetryListener, | ||||||
| errorMessage, | ||||||
| { | ||||||
| fullMessage: `${errorMessage}\n${getErrorMessage(e)}`, | ||||||
| fullMessage: `${errorMessage.fullMessage}\n${getErrorMessage(e)}`, | ||||||
|
||||||
| fullMessage: `${errorMessage.fullMessage}\n${getErrorMessage(e)}`, | |
| fullMessage: `${errorMessage}\n${getErrorMessage(e)}`, |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -45,7 +45,10 @@ class EvalLogDataProvider | |||||
| ? TreeItemCollapsibleState.Collapsed | ||||||
| : TreeItemCollapsibleState.None; | ||||||
| const treeItem = new TreeItem(element.label || "", state); | ||||||
| treeItem.tooltip = `${treeItem.label} || ''}`; | ||||||
| treeItem.tooltip = | ||||||
| typeof treeItem.label === "string" | ||||||
| ? treeItem.label | ||||||
| : (treeItem.label?.label ?? ""); | ||||||
|
||||||
| : (treeItem.label?.label ?? ""); | |
| treeItem.tooltip = element.label || ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
commandvariable appears to be a string in the original code, not an array. Calling.join(" ")on a string will cause a runtime error. This should likely be just${command}or the variable type needs to be verified.