Use sarif for problems view over doing it manually.

Author: alexet

extensions/ql-vscode/src/cli.ts

@@ -6,7 +6,7 @@ import * as util from 'util';
 import { Logger, ProgressReporter } from "./logging";
 import { Disposable } from "vscode";
 import { DistributionProvider } from "./distribution";
-import { SortDirection } from "./interface-types";
+import { SortDirection, QueryMetadata } from "./interface-types";
 import { assertNever } from "./helpers-pure";
 
 /**
@@ -55,16 +55,6 @@ export interface UpgradesInfo {
  */
 export type QlpacksInfo = { [name: string]: string[] };
 
-/**
- * The expected output of `codeql resolve metadata`.
- */
-export interface QueryMetadata {
-  name?: string,
-  description?: string,
-  id?: string,
-  kind?: string
-}
-
 // `codeql bqrs interpret` requires both of these to be present or
 // both absent.
 export interface SourceInfo {
@@ -159,7 +149,7 @@ export class CodeQLCliServer implements Disposable {
     if (!config) {
       throw new Error("Failed to find codeql distribution")
     }
-    return spawnServer(config, "CodeQL CLI Server", ["execute", "cli-server"], [], this.logger, _data => {})
+    return spawnServer(config, "CodeQL CLI Server", ["execute", "cli-server"], [], this.logger, _data => { })
   }
 
   private async runCodeQlCliInternal(command: string[], commandArgs: string[], description: string): Promise<string> {

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

@@ -16,6 +16,14 @@ export interface DatabaseInfo {
   databaseUri: string;
 }
 
+/** Arbitrary query metadata */
+export interface QueryMetadata {	
+  name?: string,	
+  description?: string,	
+  id?: string,	
+  kind?: string	
+}
+
 export interface PreviousExecution {
   queryName: string;
   time: string;
@@ -31,7 +39,6 @@ export interface Interpretation {
 
 export interface ResultsInfo {
   resultsPath: string;
-  interpretedResultsPath: string;
 }
 
 export interface SortedResultSetInfo {
@@ -56,7 +63,7 @@ export interface SetStateMsg {
   sortedResultsMap: SortedResultsMap;
   interpretation: undefined | Interpretation;
   database: DatabaseInfo;
-  kind?: string;
+  metadata?: QueryMetadata
   /**
    * Whether to keep displaying the old results while rendering the new results.
    *
@@ -86,6 +93,7 @@ interface ViewSourceFileMsg {
 interface ToggleDiagnostics {
   t: 'toggleDiagnostics';
   databaseUri: string;
+  metadata?: QueryMetadata
   resultsPath: string;
   visible: boolean;
   kind?: string;

extensions/ql-vscode/src/interface.ts

@@ -1,8 +1,9 @@
 import * as crypto from 'crypto';
 import * as path from 'path';
-import * as bqrs from 'semmle-bqrs';
-import { CustomResultSets, FivePartLocation, LocationStyle, LocationValue, PathProblemQueryResults, ProblemQueryResults, ResolvableLocationValue, tryGetResolvableLocation, WholeFileLocation } from 'semmle-bqrs';
-import { FileReader } from 'semmle-io-node';
+import * as cli from './cli';
+import * as Sarif from 'sarif';
+import { parseSarifLocation, parseSarifPlainTextMessage }  from './sarif-utils';
+import { FivePartLocation, LocationValue, ResolvableLocationValue, WholeFileLocation, tryGetResolvableLocation, LocationStyle } from 'semmle-bqrs';
 import { DisposableObject } from 'semmle-vscode-utils';
 import * as vscode from 'vscode';
 import { Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, languages, Location, Position, Range, Uri, window as Window, workspace } from 'vscode';
@@ -11,7 +12,7 @@ import { DatabaseItem, DatabaseManager } from './databases';
 import * as helpers from './helpers';
 import { showAndLogErrorMessage } from './helpers';
 import { assertNever } from './helpers-pure';
-import { FromResultsViewMsg, Interpretation, IntoResultsViewMsg, ResultsInfo, SortedResultSetInfo, SortedResultsMap, INTERPRETED_RESULTS_PER_RUN_LIMIT } from './interface-types';
+import { FromResultsViewMsg, Interpretation, IntoResultsViewMsg, ResultsInfo, SortedResultSetInfo, SortedResultsMap, INTERPRETED_RESULTS_PER_RUN_LIMIT, QueryMetadata } from './interface-types';
 import { Logger } from './logging';
 import * as messages from './messages';
 import { EvaluationInfo, interpretResults, QueryInfo, tmpDir } from './queries';
@@ -165,7 +166,7 @@ export class InterfaceManager extends DisposableObject {
         if (msg.visible) {
           const databaseItem = this.databaseManager.findDatabaseItem(Uri.parse(msg.databaseUri));
           if (databaseItem !== undefined) {
-            await this.showResultsAsDiagnostics(msg.resultsPath, msg.kind, databaseItem);
+            await this.showResultsAsDiagnostics(msg.resultsPath, msg.metadata, databaseItem);
           }
         } else {
           // TODO: Only clear diagnostics on the same database.
@@ -262,10 +263,31 @@ export class InterfaceManager extends DisposableObject {
       sortedResultsMap,
       database: info.database,
       shouldKeepOldResultsWhileRendering,
-      kind: info.query.metadata ? info.query.metadata.kind : undefined
+      metadata: info.query.metadata
     });
   }
 
+private async getTruncatedResults(metadata : QueryMetadata | undefined ,resultsPathOnDisk: string, sourceInfo : cli.SourceInfo  | undefined, sourceLocationPrefix : string ) : Promise<Interpretation> {
+  const sarif = await interpretResults(this.cliServer, metadata, resultsPathOnDisk, sourceInfo);
+  // For performance reasons, limit the number of results we try
+  // to serialize and send to the webview. TODO: possibly also
+  // limit number of paths per result, number of steps per path,
+  // or throw an error if we are in aggregate trying to send
+  // massively too much data, as it can make the extension
+  // unresponsive.
+  let numTruncatedResults = 0;
+  sarif.runs.forEach(run => {
+    if (run.results !== undefined) {
+      if (run.results.length > INTERPRETED_RESULTS_PER_RUN_LIMIT) {
+        numTruncatedResults += run.results.length - INTERPRETED_RESULTS_PER_RUN_LIMIT;
+        run.results = run.results.slice(0, INTERPRETED_RESULTS_PER_RUN_LIMIT);
+      }
+    }
+  });
+  return { sarif, sourceLocationPrefix, numTruncatedResults };
+  ;
+}
+
   private async interpretResultsInfo(query: QueryInfo, resultsInfo: ResultsInfo): Promise<Interpretation | undefined> {
     let interpretation: Interpretation | undefined = undefined;
     if (query.hasInterpretedResults()
@@ -277,114 +299,107 @@ export class InterfaceManager extends DisposableObject {
         const sourceInfo = sourceArchiveUri === undefined ?
           undefined :
           { sourceArchive: sourceArchiveUri.fsPath, sourceLocationPrefix };
-        const sarif = await interpretResults(this.cliServer, query, resultsInfo, sourceInfo);
-        // For performance reasons, limit the number of results we try
-        // to serialize and send to the webview. TODO: possibly also
-        // limit number of paths per result, number of steps per path,
-        // or throw an error if we are in aggregate trying to send
-        // massively too much data, as it can make the extension
-        // unresponsive.
-        let numTruncatedResults = 0;
-        sarif.runs.forEach(run => {
-          if (run.results !== undefined) {
-            if (run.results.length > INTERPRETED_RESULTS_PER_RUN_LIMIT) {
-              numTruncatedResults += run.results.length - INTERPRETED_RESULTS_PER_RUN_LIMIT;
-              run.results = run.results.slice(0, INTERPRETED_RESULTS_PER_RUN_LIMIT);
-            }
-          }
-        });
-        interpretation = { sarif, sourceLocationPrefix, numTruncatedResults };
+        interpretation = await this.getTruncatedResults(query.metadata, resultsInfo.resultsPath, sourceInfo, sourceLocationPrefix);
       }
       catch (e) {
         // If interpretation fails, accept the error and continue
         // trying to render uninterpreted results anyway.
         this.logger.log(`Exception during results interpretation: ${e.message}. Will show raw results instead.`);
       }
     }
-
     return interpretation;
   }
 
-  private async showResultsAsDiagnostics(resultsPath: string, kind: string | undefined,
-    database: DatabaseItem) {
 
+  private async showResultsAsDiagnostics(webviewResultsUri: string, metadata: QueryMetadata | undefined, database: DatabaseItem) {
     // URIs from the webview have the vscode-resource scheme, so convert into a filesystem URI first.
-    const resultsPathOnDisk = webviewUriToFileUri(resultsPath).fsPath;
-    const fileReader = await FileReader.open(resultsPathOnDisk);
-    try {
-      const resultSets = await bqrs.open(fileReader);
-      try {
-        switch (kind || 'problem') {
-          case 'problem': {
-            const customResults = bqrs.createCustomResultSets<ProblemQueryResults>(resultSets, ProblemQueryResults);
-            await this.showProblemResultsAsDiagnostics(customResults, database);
-          }
-            break;
-
-          case 'path-problem': {
-            const customResults = bqrs.createCustomResultSets<PathProblemQueryResults>(resultSets, PathProblemQueryResults);
-            await this.showProblemResultsAsDiagnostics(customResults, database);
-          }
-            break;
+    const resultsPathOnDisk = webviewUriToFileUri(webviewResultsUri).fsPath;
+    const sourceLocationPrefix = await database.getSourceLocationPrefix(this.cliServer);
+    const sourceArchiveUri = database.sourceArchive;
+    const sourceInfo = sourceArchiveUri === undefined ?
+      undefined :
+      { sourceArchive: sourceArchiveUri.fsPath, sourceLocationPrefix };
+    const interpretation = await this.getTruncatedResults(metadata, resultsPathOnDisk, sourceInfo, sourceLocationPrefix);
 
-          default:
-            throw new Error(`Unrecognized query kind '${kind}'.`);
-        }
-      }
-      catch (e) {
-        const msg = e instanceof Error ? e.message : e.toString();
-        this.logger.log(`Exception while computing problem results as diagnostics: ${msg}`);
-        this._diagnosticCollection.clear();
-      }
+    try {
+      await this.showProblemResultsAsDiagnostics(interpretation, database);
     }
-    finally {
-      fileReader.dispose();
+    catch (e) {
+      const msg = e instanceof Error ? e.message : e.toString();
+      this.logger.log(`Exception while computing problem results as diagnostics: ${msg}`);
+      this._diagnosticCollection.clear();
     }
+
   }
 
-  private async showProblemResultsAsDiagnostics(results: CustomResultSets<ProblemQueryResults>,
-    databaseItem: DatabaseItem): Promise<void> {
+  private async showProblemResultsAsDiagnostics(interpretation : Interpretation, databaseItem: DatabaseItem): Promise<void> {
+    const { sarif, sourceLocationPrefix } = interpretation;
+
+
+    if (!sarif.runs || !sarif.runs[0].results) {
+      this.logger.log("Didn't find a run in the sarif results. Error processing sarif?")
+      return;
+    }
 
     const diagnostics: [Uri, ReadonlyArray<Diagnostic>][] = [];
-    for await (const problemRow of results.problems.readTuples()) {
-      const codeLocation = resolveLocation(problemRow.element.location, databaseItem);
-      let message: string;
-      const references = problemRow.references;
-      if (references) {
-        let referenceIndex = 0;
-        message = problemRow.message.replace(/\$\@/g, sub => {
-          if (referenceIndex < references.length) {
-            const replacement = references[referenceIndex].text;
-            referenceIndex++;
-            return replacement;
-          }
-          else {
-            return sub;
-          }
-        });
+
+    for (const result of sarif.runs[0].results) {
+      const message = result.message.text;
+      if (message === undefined) {
+        this.logger.log("Sarif had result without plaintext message")
+        continue;
+      }
+      if (!result.locations) {
+        this.logger.log("Sarif had result without location")
+        continue;
+      }
+
+      const sarifLoc = parseSarifLocation(result.locations[0], sourceLocationPrefix);
+      if (sarifLoc.t == "NoLocation") {
+        continue;
+      }
+      const resultLocation = tryResolveLocation(sarifLoc, databaseItem)
+      if (!resultLocation) {
+        this.logger.log("Sarif location was not resolvable " + sarifLoc)
+        continue;
       }
-      else {
-        message = problemRow.message;
+      const parsedMessage = parseSarifPlainTextMessage(message);
+      const relatedInformation: DiagnosticRelatedInformation[] = [];
+      const relatedLocationsById: { [k: number]: Sarif.Location } = {};
+
+
+      for (let loc of result.relatedLocations || []) {
+        relatedLocationsById[loc.id!] = loc;
       }
-      const diagnostic = new Diagnostic(codeLocation.range, message, DiagnosticSeverity.Warning);
-      if (problemRow.references) {
-        const relatedInformation: DiagnosticRelatedInformation[] = [];
-        for (const reference of problemRow.references) {
-          const referenceLocation = tryResolveLocation(reference.element.location, databaseItem);
+      let resultMessageChunks: string[] = [];
+      for (const section of parsedMessage) {
+        if (typeof section === "string") {
+          resultMessageChunks.push(section);
+        } else {
+          resultMessageChunks.push(section.text);
+          const sarifChunkLoc = parseSarifLocation(relatedLocationsById[section.dest], sourceLocationPrefix);
+          if (sarifChunkLoc.t == "NoLocation") {
+            continue;
+          }
+          const referenceLocation = tryResolveLocation(sarifChunkLoc, databaseItem);
+
+
           if (referenceLocation) {
             const related = new DiagnosticRelatedInformation(referenceLocation,
-              reference.text);
+              section.text);
             relatedInformation.push(related);
           }
         }
-        diagnostic.relatedInformation = relatedInformation;
       }
+      const diagnostic = new Diagnostic(resultLocation.range, resultMessageChunks.join(""), DiagnosticSeverity.Warning);
+      diagnostic.relatedInformation = relatedInformation;
+
       diagnostics.push([
-        codeLocation.uri,
+        resultLocation.uri,
         [diagnostic]
       ]);
-    }
 
+    }
     this._diagnosticCollection.set(diagnostics);
   }
 
@@ -479,22 +494,6 @@ function resolveWholeFileLocation(loc: WholeFileLocation, databaseItem: Database
   return new Location(databaseItem.resolveSourceFile(loc.file), range);
 }
 
-/**
- * Resolve the specified CodeQL location to a URI into the source archive.
- * @param loc CodeQL location to resolve
- * @param databaseItem Database in which to resolve the file location.
- */
-function resolveLocation(loc: LocationValue | undefined, databaseItem: DatabaseItem): Location {
-  const resolvedLocation = tryResolveLocation(loc, databaseItem);
-  if (resolvedLocation) {
-    return resolvedLocation;
-  }
-  else {
-    // Return a fake position in the source archive directory itself.
-    return new Location(databaseItem.resolveSourceFile(undefined), new Position(0, 0));
-  }
-}
-
 /**
  * Try to resolve the specified CodeQL location to a URI into the source archive. If no exact location
  * can be resolved, returns `undefined`.