Merge pull request #232 from jcreedcmu/jcreed/sort-interpreted

Support sorting for interpreted alerts

Author: henrymercer

extensions/ql-vscode/CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - Add an icon next to any failed query runs in the query history
   view.
+- Add the ability to sort alerts by alert message.
 
 ## 1.0.4 - 24 January 2020
 

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

@@ -17,11 +17,11 @@ export interface DatabaseInfo {
 }
 
 /** Arbitrary query metadata */
-export interface QueryMetadata {	
-  name?: string,	
-  description?: string,	
-  id?: string,	
-  kind?: string	
+export interface QueryMetadata {
+  name?: string,
+  description?: string,
+  id?: string,
+  kind?: string
 }
 
 export interface PreviousExecution {
@@ -34,6 +34,11 @@ export interface PreviousExecution {
 export interface Interpretation {
   sourceLocationPrefix: string;
   numTruncatedResults: number;
+  /**
+   * sortState being undefined means don't sort, just present results in the order
+   * they appear in the sarif file.
+   */
+  sortState?: InterpretedResultsSortState;
   sarif: sarif.Log;
 }
 
@@ -44,7 +49,7 @@ export interface ResultsPaths {
 
 export interface SortedResultSetInfo {
   resultsPath: string;
-  sortState: SortState;
+  sortState: RawResultsSortState;
 }
 
 export type SortedResultsMap = { [resultSet: string]: SortedResultSetInfo };
@@ -84,7 +89,12 @@ export interface NavigatePathMsg {
 
 export type IntoResultsViewMsg = ResultsUpdatingMsg | SetStateMsg | NavigatePathMsg;
 
-export type FromResultsViewMsg = ViewSourceFileMsg | ToggleDiagnostics | ChangeSortMsg | ResultViewLoaded;
+export type FromResultsViewMsg =
+  | ViewSourceFileMsg
+  | ToggleDiagnostics
+  | ChangeRawResultsSortMsg
+  | ChangeInterpretedResultsSortMsg
+  | ResultViewLoaded;
 
 interface ViewSourceFileMsg {
   t: 'viewSourceFile';
@@ -109,13 +119,34 @@ export enum SortDirection {
   asc, desc
 }
 
-export interface SortState {
+export interface RawResultsSortState {
   columnIndex: number;
-  direction: SortDirection;
+  sortDirection: SortDirection;
+}
+
+export type InterpretedResultsSortColumn =
+  'alert-message';
+
+export interface InterpretedResultsSortState {
+  sortBy: InterpretedResultsSortColumn;
+  sortDirection: SortDirection;
 }
 
-interface ChangeSortMsg {
+interface ChangeRawResultsSortMsg {
   t: 'changeSort';
   resultSetName: string;
-  sortState?: SortState;
+  /**
+   * sortState being undefined means don't sort, just present results in the order
+   * they appear in the sarif file.
+   */
+  sortState?: RawResultsSortState;
+}
+
+interface ChangeInterpretedResultsSortMsg {
+  t: 'changeInterpretedSort';
+  /**
+   * sortState being undefined means don't sort, just present results in the order
+   * they appear in the sarif file.
+   */
+  sortState?: InterpretedResultsSortState;
 }

extensions/ql-vscode/src/interface.ts

@@ -10,7 +10,7 @@ import { CodeQLCliServer } from './cli';
 import { DatabaseItem, DatabaseManager } from './databases';
 import { showAndLogErrorMessage } from './helpers';
 import { assertNever } from './helpers-pure';
-import { FromResultsViewMsg, Interpretation, INTERPRETED_RESULTS_PER_RUN_LIMIT, IntoResultsViewMsg, QueryMetadata, ResultsPaths, SortedResultSetInfo, SortedResultsMap } from './interface-types';
+import { FromResultsViewMsg, Interpretation, INTERPRETED_RESULTS_PER_RUN_LIMIT, IntoResultsViewMsg, QueryMetadata, ResultsPaths, SortedResultSetInfo, SortedResultsMap, InterpretedResultsSortState, SortDirection } from './interface-types';
 import { Logger } from './logging';
 import * as messages from './messages';
 import { CompletedQuery, interpretResults } from './query-results';
@@ -86,6 +86,29 @@ export function webviewUriToFileUri(webviewUri: string): Uri {
   return Uri.file(path);
 }
 
+function sortMultiplier(sortDirection: SortDirection): number {
+  switch (sortDirection) {
+    case SortDirection.asc: return 1;
+    case SortDirection.desc: return -1;
+  }
+}
+
+function sortInterpretedResults(results: Sarif.Result[], sortState: InterpretedResultsSortState | undefined): void {
+  if (sortState !== undefined) {
+    const multiplier = sortMultiplier(sortState.sortDirection);
+    switch (sortState.sortBy) {
+      case 'alert-message':
+        results.sort((a, b) =>
+          a.message.text === undefined ? 0 :
+            b.message.text === undefined ? 0 :
+              multiplier * (a.message.text?.localeCompare(b.message.text)));
+        break;
+      default:
+        assertNever(sortState.sortBy);
+    }
+  }
+}
+
 export class InterfaceManager extends DisposableObject {
   private _displayedQuery?: CompletedQuery;
   private _panel: vscode.WebviewPanel | undefined;
@@ -138,6 +161,17 @@ export class InterfaceManager extends DisposableObject {
     return this._panel;
   }
 
+  private async changeSortState(update: (query: CompletedQuery) => Promise<void>): Promise<void> {
+    if (this._displayedQuery === undefined) {
+      showAndLogErrorMessage("Failed to sort results since evaluation info was unknown.");
+      return;
+    }
+    // Notify the webview that it should expect new results.
+    await this.postMessage({ t: 'resultsUpdating' });
+    await update(this._displayedQuery);
+    await this.showResults(this._displayedQuery, WebviewReveal.NotForced, true);
+  }
+
   private async handleMsgFromView(msg: FromResultsViewMsg): Promise<void> {
     switch (msg.t) {
       case 'viewSourceFile': {
@@ -179,17 +213,12 @@ export class InterfaceManager extends DisposableObject {
         this._panelLoadedCallBacks.forEach(cb => cb());
         this._panelLoadedCallBacks = [];
         break;
-      case 'changeSort': {
-        if (this._displayedQuery === undefined) {
-          showAndLogErrorMessage("Failed to sort results since evaluation info was unknown.");
-          break;
-        }
-        // Notify the webview that it should expect new results.
-        await this.postMessage({ t: 'resultsUpdating' });
-        await this._displayedQuery.updateSortState(this.cliServer, msg.resultSetName, msg.sortState);
-        await this.showResults(this._displayedQuery, WebviewReveal.NotForced, true);
+      case 'changeSort':
+        await this.changeSortState((query) => query.updateSortState(this.cliServer, msg.resultSetName, msg.sortState));
+        break;
+      case 'changeInterpretedSort':
+        await this.changeSortState((query) => query.updateInterpretedSortState(this.cliServer, msg.sortState));
         break;
-      }
       default:
         assertNever(msg);
     }
@@ -223,7 +252,7 @@ export class InterfaceManager extends DisposableObject {
       return;
     }
 
-    const interpretation = await this.interpretResultsInfo(results.query);
+    const interpretation = await this.interpretResultsInfo(results.query, results.interpretedResultsSortState);
 
     const sortedResultsMap: SortedResultsMap = {};
     results.sortedResultsInfo.forEach((v, k) =>
@@ -268,28 +297,29 @@ export class InterfaceManager extends DisposableObject {
     });
   }
 
-  private async getTruncatedResults(metadata: QueryMetadata | undefined, resultsPaths: ResultsPaths, sourceInfo: cli.SourceInfo | undefined, sourceLocationPrefix: string): Promise<Interpretation> {
-    const sarif = await interpretResults(this.cliServer, metadata, resultsPaths.interpretedResultsPath, sourceInfo);
+  private async getTruncatedResults(metadata: QueryMetadata | undefined, resultsPaths: ResultsPaths, sourceInfo: cli.SourceInfo | undefined, sourceLocationPrefix: string, sortState: InterpretedResultsSortState | undefined): Promise<Interpretation> {
+    const sarif = await interpretResults(this.cliServer, metadata, resultsPaths.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,
     // 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) {
+        sortInterpretedResults(run.results, sortState);
         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 };
-    ;
+    return { sarif, sourceLocationPrefix, numTruncatedResults, sortState };
   }
 
-  private async interpretResultsInfo(query: QueryInfo): Promise<Interpretation | undefined> {
+  private async interpretResultsInfo(query: QueryInfo, sortState: InterpretedResultsSortState | undefined): Promise<Interpretation | undefined> {
     let interpretation: Interpretation | undefined = undefined;
     if (await query.hasInterpretedResults()
       && query.quickEvalPosition === undefined // never do results interpretation if quickEval
@@ -300,7 +330,7 @@ export class InterfaceManager extends DisposableObject {
         const sourceInfo = sourceArchiveUri === undefined ?
           undefined :
           { sourceArchive: sourceArchiveUri.fsPath, sourceLocationPrefix };
-        interpretation = await this.getTruncatedResults(query.metadata, query.resultsPaths, sourceInfo, sourceLocationPrefix);
+        interpretation = await this.getTruncatedResults(query.metadata, query.resultsPaths, sourceInfo, sourceLocationPrefix, sortState);
       }
       catch (e) {
         // If interpretation fails, accept the error and continue
@@ -318,7 +348,13 @@ export class InterfaceManager extends DisposableObject {
     const sourceInfo = sourceArchiveUri === undefined ?
       undefined :
       { sourceArchive: sourceArchiveUri.fsPath, sourceLocationPrefix };
-    const interpretation = await this.getTruncatedResults(metadata, resultsInfo, sourceInfo, sourceLocationPrefix);
+    const interpretation = await this.getTruncatedResults(
+      metadata,
+      resultsInfo,
+      sourceInfo,
+      sourceLocationPrefix,
+      undefined,
+    );
 
     try {
       await this.showProblemResultsAsDiagnostics(interpretation, database);

extensions/ql-vscode/src/query-results.ts

@@ -5,7 +5,7 @@ import * as cli from './cli';
 import * as sarif from 'sarif';
 import * as fs from 'fs-extra';
 import * as path from 'path';
-import { SortState, SortedResultSetInfo, DatabaseInfo, QueryMetadata } from "./interface-types";
+import { RawResultsSortState, SortedResultSetInfo, DatabaseInfo, QueryMetadata, InterpretedResultsSortState } from "./interface-types";
 import { QueryHistoryConfig } from "./config";
 import { QueryHistoryItemOptions } from "./query-history";
 
@@ -15,11 +15,20 @@ export class CompletedQuery implements QueryWithResults {
   readonly result: messages.EvaluationResult;
   readonly database: DatabaseInfo;
   options: QueryHistoryItemOptions;
+
   /**
    * Map from result set name to SortedResultSetInfo.
    */
   sortedResultsInfo: Map<string, SortedResultSetInfo>;
 
+  /**
+   * How we're currently sorting alerts. This is not mere interface
+   * state due to truncation; on re-sort, we want to read in the file
+   * again, sort it, and only ship off a reasonable number of results
+   * to the webview. Undefined means to use whatever order is in the
+   * sarif file.
+   */
+  interpretedResultsSortState: InterpretedResultsSortState | undefined;
 
   constructor(
     evalaution: QueryWithResults,
@@ -92,7 +101,8 @@ export class CompletedQuery implements QueryWithResults {
   toString(): string {
     return this.interpolate(this.getLabel());
   }
-  async updateSortState(server: cli.CodeQLCliServer, resultSetName: string, sortState: SortState | undefined): Promise<void> {
+
+  async updateSortState(server: cli.CodeQLCliServer, resultSetName: string, sortState: RawResultsSortState | undefined): Promise<void> {
     if (sortState === undefined) {
       this.sortedResultsInfo.delete(resultSetName);
       return;
@@ -103,10 +113,13 @@ export class CompletedQuery implements QueryWithResults {
       sortState
     };
 
-    await server.sortBqrs(this.query.resultsPaths.resultsPath, sortedResultSetInfo.resultsPath, resultSetName, [sortState.columnIndex], [sortState.direction]);
+    await server.sortBqrs(this.query.resultsPaths.resultsPath, sortedResultSetInfo.resultsPath, resultSetName, [sortState.columnIndex], [sortState.sortDirection]);
     this.sortedResultsInfo.set(resultSetName, sortedResultSetInfo);
   }
 
+  async updateInterpretedSortState(_server: cli.CodeQLCliServer, sortState: InterpretedResultsSortState | undefined): Promise<void> {
+    this.interpretedResultsSortState = sortState;
+  }
 }
 
 /**

extensions/ql-vscode/src/view/alert-table.tsx

@@ -4,9 +4,10 @@ import * as Sarif from 'sarif';
 import * as Keys from '../result-keys';
 import { LocationStyle } from 'semmle-bqrs';
 import * as octicons from './octicons';
-import { className, renderLocation, ResultTableProps, zebraStripe, selectableZebraStripe, jumpToLocation } from './result-table-utils';
-import { PathTableResultSet, onNavigation, NavigationEvent } from './results';
+import { className, renderLocation, ResultTableProps, zebraStripe, selectableZebraStripe, jumpToLocation, nextSortDirection } from './result-table-utils';
+import { PathTableResultSet, onNavigation, NavigationEvent, vscode } from './results';
 import { parseSarifPlainTextMessage, parseSarifLocation } from '../sarif-utils';
+import { InterpretedResultsSortColumn, SortDirection, InterpretedResultsSortState } from '../interface-types';
 
 export type PathTableProps = ResultTableProps & { resultSet: PathTableResultSet };
 export interface PathTableState {
@@ -43,9 +44,41 @@ export class PathTable extends React.Component<PathTableProps, PathTableState> {
     e.preventDefault();
   }
 
+  sortClass(column: InterpretedResultsSortColumn): string {
+    const sortState = this.props.resultSet.sortState;
+    if (sortState !== undefined && sortState.sortBy === column) {
+      return sortState.sortDirection === SortDirection.asc ? 'sort-asc' : 'sort-desc';
+    }
+    else {
+      return 'sort-none';
+    }
+  }
+
+  getNextSortState(column: InterpretedResultsSortColumn): InterpretedResultsSortState | undefined {
+    const oldSortState = this.props.resultSet.sortState;
+    const prevDirection = oldSortState && oldSortState.sortBy === column ? oldSortState.sortDirection : undefined;
+    const nextDirection = nextSortDirection(prevDirection, true);
+    return nextDirection === undefined ? undefined :
+      { sortBy: column, sortDirection: nextDirection };
+  }
+
+  toggleSortStateForColumn(column: InterpretedResultsSortColumn): void {
+    vscode.postMessage({
+      t: 'changeInterpretedSort',
+      sortState: this.getNextSortState(column),
+    });
+  }
+
   render(): JSX.Element {
     const { databaseUri, resultSet } = this.props;
 
+    const header = <thead>
+      <tr>
+        <th colSpan={2}></th>
+        <th className={this.sortClass('alert-message') + ' vscode-codeql__alert-message-cell'} colSpan={3} onClick={() => this.toggleSortStateForColumn('alert-message')}>Message</th>
+      </tr>
+    </thead>;
+
     const rows: JSX.Element[] = [];
     const { numTruncatedResults, sourceLocationPrefix } = resultSet;
 
@@ -65,7 +98,7 @@ export class PathTable extends React.Component<PathTableProps, PathTableState> {
           result.push(<span>{part} </span>);
         } else {
           const renderedLocation = renderSarifLocationWithText(part.text, relatedLocationsById[part.dest],
-              undefined);
+            undefined);
           result.push(<span>{renderedLocation} </span>);
         }
       } return result;
@@ -93,7 +126,7 @@ export class PathTable extends React.Component<PathTableProps, PathTableState> {
           return renderNonLocation(text, parsedLoc.hint);
         case LocationStyle.FivePart:
         case LocationStyle.WholeFile:
-            return renderLocation(parsedLoc, text, databaseUri, undefined, updateSelectionCallback(pathNodeKey));
+          return renderLocation(parsedLoc, text, databaseUri, undefined, updateSelectionCallback(pathNodeKey));
       }
       return undefined;
     }
@@ -231,6 +264,7 @@ export class PathTable extends React.Component<PathTableProps, PathTableState> {
     }
 
     return <table className={className}>
+      {header}
       <tbody>{rows}</tbody>
     </table>;
   }