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";
 
 /**
@@ -50,16 +50,6 @@ export interface UpgradesInfo {
   finalDbscheme: 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 {

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

@@ -16,6 +16,9 @@ export interface DatabaseInfo {
   databaseUri: string;
 }
 
+/** Arbitrary query metadata */
+export type QueryMetadata = {[key: string]: string};
+
 export interface PreviousExecution {
   queryName: string;
   time: string;
@@ -31,7 +34,6 @@ export interface Interpretation {
 
 export interface ResultsInfo {
   resultsPath: string;
-  interpretedResultsPath: string;
 }
 
 export interface SortedResultSetInfo {
@@ -56,7 +58,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.
    *
@@ -78,6 +80,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';
@@ -159,7 +160,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.
@@ -256,7 +257,7 @@ export class InterfaceManager extends DisposableObject {
       sortedResultsMap,
       database: info.database,
       shouldKeepOldResultsWhileRendering,
-      kind: info.query.metadata ? info.query.metadata.kind : undefined
+      metadata: info.query.metadata
     });
   }
 
@@ -271,7 +272,7 @@ export class InterfaceManager extends DisposableObject {
         const sourceInfo = sourceArchiveUri === undefined ?
           undefined :
           { sourceArchive: sourceArchiveUri.fsPath, sourceLocationPrefix };
-        const sarif = await interpretResults(this.cliServer, query, resultsInfo, sourceInfo);
+        const sarif = await interpretResults(this.cliServer, query.metadata, query.resultsInfo.resultsPath, 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,
@@ -295,90 +296,98 @@ export class InterfaceManager extends DisposableObject {
         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(resultsPath: 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;
+    const sourceLocationPrefix = await database.getSourceLocationPrefix(this.cliServer);
+    const sourceArchiveUri = database.sourceArchive;
+    const sourceInfo = sourceArchiveUri === undefined ?
+      undefined :
+      { sourceArchive: sourceArchiveUri.fsPath, sourceLocationPrefix };
+    const sarif = await interpretResults(this.cliServer, metadata, resultsPathOnDisk, sourceInfo);
 
-          case 'path-problem': {
-            const customResults = bqrs.createCustomResultSets<PathProblemQueryResults>(resultSets, PathProblemQueryResults);
-            await this.showProblemResultsAsDiagnostics(customResults, database);
-          }
-            break;
-
-          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(sarif, 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(results: Sarif.Log, databaseItem: DatabaseItem): Promise<void> {
+    const sourceLocationPrefix = await databaseItem.getSourceLocationPrefix(this.cliServer);
+
+
+    if (!results.runs || !results.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 results.runs[0].results) {
+      const message = result.message.text;
+      if (message === undefined) {
+        this.logger.log("Sarif had result without plaintext message")
+        continue;
       }
-      else {
-        message = problemRow.message;
+      if (!result.locations) {
+        this.logger.log("Sarif had result without location")
+        continue;
       }
-      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);
+
+      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;
+      }
+      const parsedMessage = parseSarifPlainTextMessage(message);
+      const relatedInformation: DiagnosticRelatedInformation[] = [];
+      const relatedLocationsById: { [k: number]: Sarif.Location } = {};
+
+
+      for (let loc of result.relatedLocations || []) {
+        relatedLocationsById[loc.id!] = loc;
+      }
+      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);
   }
 

extensions/ql-vscode/src/queries.ts

@@ -7,7 +7,7 @@ import * as vscode from 'vscode';
 import * as cli from './cli';
 import { DatabaseItem, getUpgradesDirectories } from './databases';
 import * as helpers from './helpers';
-import { DatabaseInfo, SortState, ResultsInfo, SortedResultSetInfo } from './interface-types';
+import { DatabaseInfo, SortState, ResultsInfo, SortedResultSetInfo, QueryMetadata } from './interface-types';
 import { logger } from './logging';
 import * as messages from './messages';
 import * as qsClient from './queryserver-client';
@@ -55,13 +55,12 @@ export class QueryInfo {
     public dbItem: DatabaseItem,
     public queryDbscheme: string, // the dbscheme file the query expects, based on library path resolution
     public quickEvalPosition?: messages.Position,
-    public metadata?: cli.QueryMetadata,
+    public metadata?: QueryMetadata,
   ) {
     this.queryId = QueryInfo.nextQueryId++;
     this.compiledQueryPath = path.join(tmpDir.name, `compiledQuery${this.queryId}.qlo`);
     this.resultsInfo = {
       resultsPath: path.join(tmpDir.name, `results${this.queryId}.bqrs`),
-      interpretedResultsPath: path.join(tmpDir.name, `interpretedResults${this.queryId}.sarif`)
     };
     this.sortedResultsInfo = new Map();
     if (dbItem.contents === undefined) {
@@ -173,11 +172,12 @@ export class QueryInfo {
 /**
  * Call cli command to interpret results.
  */
-export async function interpretResults(server: cli.CodeQLCliServer, queryInfo: QueryInfo, resultsInfo: ResultsInfo, sourceInfo?: cli.SourceInfo): Promise<sarif.Log> {
-  if (await fs.pathExists(resultsInfo.interpretedResultsPath)) {
-    return JSON.parse(await fs.readFile(resultsInfo.interpretedResultsPath, 'utf8'));
+export async function interpretResults(server: cli.CodeQLCliServer, metadata: QueryMetadata | undefined, resultsPath: string, sourceInfo?: cli.SourceInfo): Promise<sarif.Log> {
+  const interpretedResultsPath = resultsPath + ".interpreted.sarif"
+
+  if (await fs.pathExists(interpretedResultsPath)) {
+    return JSON.parse(await fs.readFile(interpretedResultsPath, 'utf8'));
   }
-  const { metadata } = queryInfo;
   if (metadata == undefined) {
     throw new Error('Can\'t interpret results without query metadata');
   }
@@ -187,14 +187,10 @@ export async function interpretResults(server: cli.CodeQLCliServer, queryInfo: Q
   }
   if (id == undefined) {
     // Interpretation per se doesn't really require an id, but the
-    // SARIF format does, so in the absence of one, we invent one
-    // based on the query path.
-    //
-    // Just to be careful, sanitize to remove '/' since SARIF (section
-    // 3.27.5 "ruleId property") says that it has special meaning.
-    id = queryInfo.program.queryPath.replace(/\//g, '-');
-  }
-  return await server.interpretBqrs({ kind, id }, resultsInfo.resultsPath, resultsInfo.interpretedResultsPath, sourceInfo);
+    // SARIF format does, so in the absence of one, we use a dummy id.
+    id = "dummy-id";
+  }
+  return await server.interpretBqrs( { kind, id }, resultsPath, interpretedResultsPath,  sourceInfo);
 }
 
 export interface EvaluationInfo {
@@ -599,7 +595,7 @@ export async function compileAndRunQueryAgainstDatabase(
   };
 
   // Read the query metadata if possible, to use in the UI.
-  let metadata: cli.QueryMetadata | undefined;
+  let metadata: QueryMetadata | undefined;
   try {
     metadata = await cliServer.resolveMetadata(qlProgram.queryPath);
   } catch (e) {

extensions/ql-vscode/src/view/result-table-utils.tsx

@@ -1,11 +1,12 @@
 import * as React from 'react';
 import { LocationValue, ResolvableLocationValue, tryGetResolvableLocation } from 'semmle-bqrs';
-import { SortState } from '../interface-types';
+import { SortState, QueryMetadata } from '../interface-types';
 import { ResultSet, vscode } from './results';
 
 export interface ResultTableProps {
   resultSet: ResultSet;
   databaseUri: string;
+  metadata?: QueryMetadata
   resultsPath: string | undefined;
   sortState?: SortState;
 }