Update linting rules

Add the `@typescript-eslint/no-floating-promises` rule with an allowance
for floating promises if `void` is used.

This increases safety and ensures that we are explicit when we avoid
awaiting a promise. I already caught a few bugish locations.

In general, we don't need to await the results of logging calls.

databases-ui, we were using a deprecated method for removing a
directory. `fs.rmdir` instead of `fs.remove`.

Author: aeisenberg

extensions/ql-vscode/.eslintrc.js

@@ -24,6 +24,7 @@ module.exports = {
     "@typescript-eslint/explicit-function-return-type": "off",
     "@typescript-eslint/no-non-null-assertion": "off",
     "@typescript-eslint/no-explicit-any": "off",
+    "@typescript-eslint/no-floating-promises": [ "error", { ignoreVoid: true } ],
     "prefer-const": ["warn", { destructuring: "all" }],
     indent: "off",
     "@typescript-eslint/indent": "off",

extensions/ql-vscode/src/archive-filesystem-provider.ts

@@ -115,7 +115,7 @@ class InvalidSourceArchiveUriError extends Error {
 export function decodeSourceArchiveUri(uri: vscode.Uri): ZipFileReference {
   if (!uri.authority) {
     // Uri is malformed, but this is recoverable
-    logger.log(`Warning: ${new InvalidSourceArchiveUriError(uri).message}`);
+    void logger.log(`Warning: ${new InvalidSourceArchiveUriError(uri).message}`);
     return {
       pathWithinSourceArchive: '/',
       sourceArchiveZipPath: uri.path
@@ -141,7 +141,7 @@ function ensureFile(map: DirectoryHierarchyMap, file: string) {
   const dirname = path.dirname(file);
   if (dirname === '.') {
     const error = `Ill-formed path ${file} in zip archive (expected absolute path)`;
-    logger.log(error);
+    void logger.log(error);
     throw new Error(error);
   }
   ensureDir(map, dirname);

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

@@ -18,7 +18,7 @@ export async function getCodeQlCliVersion(codeQlPath: string, logger: Logger): P
   } catch (e) {
     // Failed to run the version command. This might happen if the cli version is _really_ old, or it is corrupted.
     // Either way, we can't determine compatibility.
-    logger.log(`Failed to run 'codeql version'. Reason: ${e.message}`);
+    void logger.log(`Failed to run 'codeql version'. Reason: ${e.message}`);
     return undefined;
   }
 }

extensions/ql-vscode/src/cli.ts

@@ -183,15 +183,15 @@ export class CodeQLCliServer implements Disposable {
   killProcessIfRunning(): void {
     if (this.process) {
       // Tell the Java CLI server process to shut down.
-      this.logger.log('Sending shutdown request');
+      void this.logger.log('Sending shutdown request');
       try {
         this.process.stdin.write(JSON.stringify(['shutdown']), 'utf8');
         this.process.stdin.write(this.nullBuffer);
-        this.logger.log('Sent shutdown request');
+        void this.logger.log('Sent shutdown request');
       } catch (e) {
         // We are probably fine here, the process has already closed stdin.
-        this.logger.log(`Shutdown request failed: process stdin may have already closed. The error was ${e}`);
-        this.logger.log('Stopping the process anyway.');
+        void this.logger.log(`Shutdown request failed: process stdin may have already closed. The error was ${e}`);
+        void this.logger.log('Stopping the process anyway.');
       }
       // Close the stdin and stdout streams.
       // This is important on Windows where the child process may not die cleanly.
@@ -271,7 +271,7 @@ export class CodeQLCliServer implements Disposable {
       // Compute the full args array
       const args = command.concat(LOGGING_FLAGS).concat(commandArgs);
       const argsString = args.join(' ');
-      this.logger.log(`${description} using CodeQL CLI: ${argsString}...`);
+      void this.logger.log(`${description} using CodeQL CLI: ${argsString}...`);
       try {
         await new Promise<void>((resolve, reject) => {
           // Start listening to stdout
@@ -298,7 +298,7 @@ export class CodeQLCliServer implements Disposable {
         const fullBuffer = Buffer.concat(stdoutBuffers);
         // Make sure we remove the terminator;
         const data = fullBuffer.toString('utf8', 0, fullBuffer.length - 1);
-        this.logger.log('CLI command succeeded.');
+        void this.logger.log('CLI command succeeded.');
         return data;
       } catch (err) {
         // Kill the process if it isn't already dead.
@@ -311,7 +311,7 @@ export class CodeQLCliServer implements Disposable {
         newError.stack += (err.stack || '');
         throw newError;
       } finally {
-        this.logger.log(Buffer.concat(stderrBuffers).toString('utf8'));
+        void this.logger.log(Buffer.concat(stderrBuffers).toString('utf8'));
         // Remove the listeners we set up.
         process.stdout.removeAllListeners('data');
         process.stderr.removeAllListeners('data');
@@ -371,7 +371,7 @@ export class CodeQLCliServer implements Disposable {
       }
       if (logger !== undefined) {
         // The human-readable output goes to stderr.
-        logStream(child.stderr!, logger);
+        void logStream(child.stderr!, logger);
       }
 
       for await (const event of await splitStreamAtSeparators(child.stdout!, ['\0'])) {
@@ -813,7 +813,7 @@ export function spawnServer(
   if (progressReporter !== undefined) {
     progressReporter.report({ message: `Starting ${name}` });
   }
-  logger.log(`Starting ${name} using CodeQL CLI: ${base} ${argsString}`);
+  void logger.log(`Starting ${name} using CodeQL CLI: ${base} ${argsString}`);
   const child = child_process.spawn(base, args);
   if (!child || !child.pid) {
     throw new Error(`Failed to start ${name} using command ${base} ${argsString}.`);
@@ -829,7 +829,7 @@ export function spawnServer(
   if (progressReporter !== undefined) {
     progressReporter.report({ message: `Started ${name}` });
   }
-  logger.log(`${name} started on PID: ${child.pid}`);
+  void logger.log(`${name} started on PID: ${child.pid}`);
   return child;
 }
 
@@ -858,10 +858,10 @@ export async function runCodeQlCliCommand(
     if (progressReporter !== undefined) {
       progressReporter.report({ message: description });
     }
-    logger.log(`${description} using CodeQL CLI: ${codeQlPath} ${argsString}...`);
+    void logger.log(`${description} using CodeQL CLI: ${codeQlPath} ${argsString}...`);
     const result = await promisify(child_process.execFile)(codeQlPath, args);
-    logger.log(result.stderr);
-    logger.log('CLI command succeeded.');
+    void logger.log(result.stderr);
+    void logger.log('CLI command succeeded.');
     return result.stdout;
   } catch (err) {
     throw new Error(`${description} failed: ${err.stderr || err}`);
@@ -976,7 +976,7 @@ const lineEndings = ['\r\n', '\r', '\n'];
  */
 async function logStream(stream: Readable, logger: Logger): Promise<void> {
   for await (const line of await splitStreamAtSeparators(stream, lineEndings)) {
-    logger.log(line);
+    await logger.log(line);
   }
 }
 

extensions/ql-vscode/src/commandRunner.ts

@@ -126,16 +126,16 @@ export function commandRunner(
       if (e instanceof UserCancellationException) {
         // User has cancelled this action manually
         if (e.silent) {
-          logger.log(errorMessage);
+          void logger.log(errorMessage);
         } else {
-          showAndLogWarningMessage(errorMessage);
+          void showAndLogWarningMessage(errorMessage);
         }
       } else {
         // Include the full stack in the error log only.
         const fullMessage = e.stack
           ? `${errorMessage}\n${e.stack}`
           : errorMessage;
-        showAndLogErrorMessage(errorMessage, {
+        void showAndLogErrorMessage(errorMessage, {
           fullMessage
         });
       }
@@ -177,16 +177,16 @@ export function commandRunnerWithProgress<R>(
       if (e instanceof UserCancellationException) {
         // User has cancelled this action manually
         if (e.silent) {
-          logger.log(errorMessage);
+          void logger.log(errorMessage);
         } else {
-          showAndLogWarningMessage(errorMessage);
+          void showAndLogWarningMessage(errorMessage);
         }
       } else {
         // Include the full stack in the error log only.
         const fullMessage = e.stack
           ? `${errorMessage}\n${e.stack}`
           : errorMessage;
-        showAndLogErrorMessage(errorMessage, {
+        void showAndLogErrorMessage(errorMessage, {
           fullMessage
         });
       }

extensions/ql-vscode/src/compare/compare-interface.ts

@@ -173,7 +173,7 @@ export class CompareInterfaceManager extends DisposableObject {
         break;
 
       case 'changeCompare':
-        this.changeTable(msg.newResultSetName);
+        await this.changeTable(msg.newResultSetName);
         break;
 
       case 'viewSourceFile':
@@ -267,11 +267,11 @@ export class CompareInterfaceManager extends DisposableObject {
     return resultsDiff(fromResults, toResults);
   }
 
-  private openQuery(kind: 'from' | 'to') {
+  private async openQuery(kind: 'from' | 'to') {
     const toOpen =
       kind === 'from' ? this.comparePair?.from : this.comparePair?.to;
     if (toOpen) {
-      this.showQueryResultsCallback(toOpen);
+      await this.showQueryResultsCallback(toOpen);
     }
   }
 }

extensions/ql-vscode/src/config.ts

@@ -226,7 +226,7 @@ export class QueryServerConfigListener extends ConfigListener implements QuerySe
       return undefined;
     }
     if (memory == 0 || typeof (memory) !== 'number') {
-      logger.log(`Ignoring value '${memory}' for setting ${MEMORY_SETTING.qualifiedName}`);
+      void logger.log(`Ignoring value '${memory}' for setting ${MEMORY_SETTING.qualifiedName}`);
       return undefined;
     }
     return memory;

extensions/ql-vscode/src/contextual/queryResolver.ts

@@ -37,7 +37,7 @@ export async function resolveQueries(cli: CodeQLCliServer, qlpack: string, keyTy
 
   const queries = await cli.resolveQueriesInSuite(suiteFile, helpers.getOnDiskWorkspaceFolders());
   if (queries.length === 0) {
-    helpers.showAndLogErrorMessage(
+    void helpers.showAndLogErrorMessage(
       `No ${nameOfKeyType(keyType)} queries (tagged "${tagOfKeyType(keyType)}") could be found in the current library path. \
       Try upgrading the CodeQL libraries. If that doesn't work, then ${nameOfKeyType(keyType)} queries are not yet available \
       for this language.`

extensions/ql-vscode/src/databaseFetcher.ts

@@ -51,8 +51,8 @@ export async function promptImportInternetDatabase(
   );
 
   if (item) {
-    commands.executeCommand('codeQLDatabases.focus');
-    showAndLogInformationMessage('Database downloaded and imported successfully.');
+    await commands.executeCommand('codeQLDatabases.focus');
+    void showAndLogInformationMessage('Database downloaded and imported successfully.');
   }
   return item;
 
@@ -91,8 +91,8 @@ export async function promptImportLgtmDatabase(
         token
       );
       if (item) {
-        commands.executeCommand('codeQLDatabases.focus');
-        showAndLogInformationMessage('Database downloaded and imported successfully.');
+        await commands.executeCommand('codeQLDatabases.focus');
+        void showAndLogInformationMessage('Database downloaded and imported successfully.');
       }
       return item;
     }
@@ -125,8 +125,8 @@ export async function importArchiveDatabase(
       token
     );
     if (item) {
-      commands.executeCommand('codeQLDatabases.focus');
-      showAndLogInformationMessage('Database unzipped and imported successfully.');
+      await commands.executeCommand('codeQLDatabases.focus');
+      void showAndLogInformationMessage('Database unzipped and imported successfully.');
     }
     return item;
   } catch (e) {
@@ -433,7 +433,7 @@ export async function convertToDatabaseUrl(lgtmUrl: string) {
       language,
     ].join('/')}`;
   } catch (e) {
-    logger.log(`Error: ${e.message}`);
+    void logger.log(`Error: ${e.message}`);
     throw new Error(`Invalid LGTM URL: ${lgtmUrl}`);
   }
 }

extensions/ql-vscode/src/databases-ui.ts

@@ -234,7 +234,7 @@ export class DatabaseUI extends DisposableObject {
   }
 
   init() {
-    logger.log('Registering database panel commands.');
+    void logger.log('Registering database panel commands.');
     this.push(
       commandRunnerWithProgress(
         'codeQL.setCurrentDatabase',
@@ -369,20 +369,20 @@ export class DatabaseUI extends DisposableObject {
     try {
       return await this.chooseAndSetDatabase(true, progress, token);
     } catch (e) {
-      showAndLogErrorMessage(e.message);
+      void showAndLogErrorMessage(e.message);
       return undefined;
     }
   };
 
   handleRemoveOrphanedDatabases = async (): Promise<void> => {
-    logger.log('Removing orphaned databases from workspace storage.');
+    void logger.log('Removing orphaned databases from workspace storage.');
     let dbDirs = undefined;
 
     if (
       !(await fs.pathExists(this.storagePath)) ||
       !(await fs.stat(this.storagePath)).isDirectory()
     ) {
-      logger.log('Missing or invalid storage directory. Not trying to remove orphaned databases.');
+      void logger.log('Missing or invalid storage directory. Not trying to remove orphaned databases.');
       return;
     }
 
@@ -403,7 +403,7 @@ export class DatabaseUI extends DisposableObject {
     dbDirs = await asyncFilter(dbDirs, isLikelyDatabaseRoot);
 
     if (!dbDirs.length) {
-      logger.log('No orphaned databases found.');
+      void logger.log('No orphaned databases found.');
       return;
     }
 
@@ -412,8 +412,8 @@ export class DatabaseUI extends DisposableObject {
     await Promise.all(
       dbDirs.map(async dbDir => {
         try {
-          logger.log(`Deleting orphaned database '${dbDir}'.`);
-          await fs.rmdir(dbDir, { recursive: true } as any);  // typings doesn't recognize the options argument
+          void logger.log(`Deleting orphaned database '${dbDir}'.`);
+          await fs.remove(dbDir);
         } catch (e) {
           failures.push(`${path.basename(dbDir)}`);
         }
@@ -422,7 +422,7 @@ export class DatabaseUI extends DisposableObject {
 
     if (failures.length) {
       const dirname = path.dirname(failures[0]);
-      showAndLogErrorMessage(
+      void showAndLogErrorMessage(
         `Failed to delete unused databases (${
         failures.join(', ')
         }).\nTo delete unused databases, please remove them manually from the storage folder ${dirname}.`
@@ -438,7 +438,7 @@ export class DatabaseUI extends DisposableObject {
     try {
       return await this.chooseAndSetDatabase(false, progress, token);
     } catch (e) {
-      showAndLogErrorMessage(e.message);
+      void showAndLogErrorMessage(e.message);
       return undefined;
     }
   };
@@ -617,7 +617,7 @@ export class DatabaseUI extends DisposableObject {
     });
 
     if (newName) {
-      this.databaseManager.renameDatabaseItem(databaseItem, newName);
+      await this.databaseManager.renameDatabaseItem(databaseItem, newName);
     }
   };