Enable the @typescript-eslint/restrict-template-expressions rule #4111

robertbrignull · 2025-08-14T12:04:19Z

Enable the @typescript-eslint/restrict-template-expressions rule. This was "disabled" in the eslint 9 migration, though actually I don't think this rule was enabled before so really this is a new rule.

This PR will make behavioural changes to the extension, but I think all the changes are for the better 🤞🏼

The purpose of this rule is to protect us from trying to stringify something that won't look nice. It should stop the classic [Object object] problem by forcing us to manually convert everything to a string first so we're being deliberate about how that transformation happens. This can be a little bit tedious but enabling it for our repo didn't turn out to be too hard.

To be clear, when you try to include a value in a string it'll call .toString() for you which is defined on all objects. Often this is fine, and many values (e.g. the URI class) have implemented a good toString method, but generic objects and arrays have no and they do not stringify well.

The most common cases we have are errors and URIs and both of those are easy to fix and shouldn't change the behaviour, even if calling .toString() on all URIs is a little annoying. But this rule did end up surfacing quite a few cases where I think we did have a bug and if that code had been hit (often it's an error message) then it wouldn't have looked pretty and wouldn't have been useful to us.

I haven't split this up into separate commits, but each code change is very localised and you can consider each of them separately when reviewing.

Copilot

Pull Request Overview

This PR enables the @typescript-eslint/restrict-template-expressions rule to prevent the stringification of objects that don't provide meaningful string representations (like [Object object]). The changes systematically address violations by explicitly converting values to strings using methods like toString(), getErrorMessage(), and JSON.stringify().

Replaces implicit string coercion with explicit string conversion methods
Uses getErrorMessage() helper for error objects to ensure consistent error handling
Adds explicit .toString() calls for URI objects, version objects, and other complex types

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 5 comments.

File	Description
eslint.config.mjs	Enables the `@typescript-eslint/restrict-template-expressions` rule
Multiple source files	Adds explicit string conversion for error objects, URIs, versions, and other complex types in template expressions

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

Copilot · 2025-08-14T12:05:29Z

extensions/ql-vscode/src/query-evaluation-logging/eval-log-viewer.ts

+    treeItem.tooltip =
+      typeof treeItem.label === "string"
+        ? treeItem.label
+        : (treeItem.label?.label ?? "");


The original line 47 uses element.label || "" but the tooltip logic assumes treeItem.label could be non-string. This creates an inconsistency - if element.label is falsy, treeItem.label becomes an empty string, making the type check unnecessary. Either use element.label directly in the tooltip or handle the falsy case consistently.

Suggested change

: (treeItem.label?.label ?? "");

treeItem.tooltip = element.label || "";

Copilot · 2025-08-14T12:05:30Z

extensions/ql-vscode/src/language-support/ast-viewer/ast-viewer.ts

@@ -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 ?? "")}`;


[nitpick] This line is extremely long and hard to read. Consider extracting the label logic into a variable: const labelText = typeof treeItem.label === "string" ? treeItem.label : (treeItem.label?.label ?? ""); treeItem.tooltip = \${treeItem.description} ${labelText}`;`

Suggested change

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}`;

Copilot · 2025-08-14T12:05:30Z

extensions/ql-vscode/src/language-support/query-editor.ts

@@ -99,7 +99,7 @@ async function previewQueryHelp(
        telemetryListener,
        errorMessage,
        {
-          fullMessage: `${errorMessage}\n${getErrorMessage(e)}`,
+          fullMessage: `${errorMessage.fullMessage}\n${getErrorMessage(e)}`,


The variable errorMessage is being treated as an object with a fullMessage property, but the original code suggests it should be a string. This change appears to introduce a bug where errorMessage.fullMessage is accessed instead of using errorMessage directly.

Suggested change

fullMessage: `${errorMessage.fullMessage}\n${getErrorMessage(e)}`,

fullMessage: `${errorMessage}\n${getErrorMessage(e)}`,

Copilot · 2025-08-14T12:05:30Z

extensions/ql-vscode/src/databases/local-databases/database-manager.ts

-      void extLogger.log(
-        `Waiting for event ${event} timed out after ${timeoutMs}ms`,
-      );
+      void extLogger.log(`Waiting for event timed out after ${timeoutMs}ms`);


The log message removes the ${event} parameter that was in the original message, making the log less informative. The event parameter should be converted to string and included: void extLogger.log(\Waiting for event ${String(event)} timed out after ${timeoutMs}ms`);`

Suggested change

void extLogger.log(`Waiting for event timed out after ${timeoutMs}ms`);

void extLogger.log(`Waiting for event ${String(event)} timed out after ${timeoutMs}ms`);

Copilot · 2025-08-14T12:05:31Z

extensions/ql-vscode/src/codeql-cli/cli.ts

@@ -748,7 +748,7 @@ export class CodeQLCliServer implements Disposable {
          } else {
            reject(
              new Error(
-                `${command} ${commandArgs.join(" ")} failed with code ${code}`,
+                `${command.join(" ")} ${commandArgs.join(" ")} failed with code ${code}`,


The command variable 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.

Suggested change

`${command.join(" ")} ${commandArgs.join(" ")} failed with code ${code}`,

`${command} ${commandArgs.join(" ")} failed with code ${code}`,

robertbrignull · 2025-08-14T12:06:07Z

extensions/ql-vscode/src/codeql-cli/query-language.ts

@@ -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(


This is an interesting case because the language variable actually has type never here. An alternative would be to cast it to string perhaps, or maybe any and then call .toString() manually, but I think we need to cast it to something.

I think casting it to string would make sense here. We know it's not a QueryLanguage, so I don't think casting it to QueryLanguage makes sense here.

robertbrignull · 2025-08-14T12:07:01Z

extensions/ql-vscode/src/language-support/ast-viewer/ast-viewer.ts

@@ -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 ?? "")}`;


This is a bit ugly but the type of treeItem.label is string | TreeItemLabel | undefined and it doesn't seem that TreeItemLabel implements toString().

robertbrignull · 2025-08-14T12:08:36Z

extensions/ql-vscode/src/databases/local-databases/database-manager.ts

-      void extLogger.log(
-        `Waiting for event ${event} timed out after ${timeoutMs}ms`,
-      );
+      void extLogger.log(`Waiting for event timed out after ${timeoutMs}ms`);


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. 🫤

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.

koesie10 · 2025-08-14T13:48:40Z

extensions/ql-vscode/src/codeql-cli/query-language.ts

@@ -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(


I think casting it to string would make sense here. We know it's not a QueryLanguage, so I don't think casting it to QueryLanguage makes sense here.

koesie10 · 2025-08-14T13:50:00Z

extensions/ql-vscode/src/databases/local-databases/database-manager.ts

-      void extLogger.log(
-        `Waiting for event ${event} timed out after ${timeoutMs}ms`,
-      );
+      void extLogger.log(`Waiting for event timed out after ${timeoutMs}ms`);


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.

koesie10 · 2025-08-14T13:52:06Z

extensions/ql-vscode/src/language-support/ast-viewer/ast-viewer.ts

    treeItem.command = {
      command: "codeQLAstViewer.gotoCode",
      title: "Go To Code",
-      tooltip: `Go To ${item.location}`,
+      tooltip: `Go To ${item.location?.toString() ?? "Code"}`,


What will item.location.toString() return? It looks like item.location is an object, so would that just give [Object object]?

Enable the @typescript-eslint/restrict-template-expressions rule

3511e8a

robertbrignull requested review from a team as code owners August 14, 2025 12:04

Copilot AI review requested due to automatic review settings August 14, 2025 12:04

Copilot AI reviewed Aug 14, 2025

View reviewed changes

robertbrignull commented Aug 14, 2025

View reviewed changes

koesie10 approved these changes Aug 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enable the @typescript-eslint/restrict-template-expressions rule #4111

Enable the @typescript-eslint/restrict-template-expressions rule #4111

Uh oh!

robertbrignull commented Aug 14, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Aug 14, 2025

Uh oh!

Copilot AI Aug 14, 2025

Uh oh!

Copilot AI Aug 14, 2025

Uh oh!

Copilot AI Aug 14, 2025

Uh oh!

Copilot AI Aug 14, 2025

Uh oh!

robertbrignull Aug 14, 2025

Uh oh!

koesie10 Aug 14, 2025

Uh oh!

robertbrignull Aug 14, 2025

Uh oh!

robertbrignull Aug 14, 2025

Uh oh!

koesie10 Aug 14, 2025

Uh oh!

koesie10 Aug 14, 2025

Uh oh!

koesie10 Aug 14, 2025

Uh oh!

koesie10 Aug 14, 2025

Uh oh!

Uh oh!

	: (treeItem.label?.label ?? "");
	treeItem.tooltip = element.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}`;

	fullMessage: `${errorMessage.fullMessage}\n${getErrorMessage(e)}`,
	fullMessage: `${errorMessage}\n${getErrorMessage(e)}`,

	void extLogger.log(`Waiting for event timed out after ${timeoutMs}ms`);
	void extLogger.log(`Waiting for event ${String(event)} timed out after ${timeoutMs}ms`);

	`${command.join(" ")} ${commandArgs.join(" ")} failed with code ${code}`,
	`${command} ${commandArgs.join(" ")} failed with code ${code}`,

Enable the @typescript-eslint/restrict-template-expressions rule #4111

Are you sure you want to change the base?

Enable the @typescript-eslint/restrict-template-expressions rule #4111

Uh oh!

Conversation

robertbrignull commented Aug 14, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!