Change sorting interface to table header on alerts.

Author: jcreedcmu

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

@@ -34,7 +34,7 @@ export interface PreviousExecution {
 export interface Interpretation {
   sourceLocationPrefix: string;
   numTruncatedResults: number;
-  sortState: InterpretedResultsSortState;
+  sortState?: InterpretedResultsSortState;
   sarif: sarif.Log;
 }
 
@@ -120,11 +120,12 @@ export interface RawResultsSortState {
   direction: SortDirection;
 }
 
-export type InterpretedResultsSortOrder =
+export type InterpretedResultsSortColumn =
   'file-position' | 'alert-message';
 
 export interface InterpretedResultsSortState {
-  sortBy: InterpretedResultsSortOrder;
+  sortBy: InterpretedResultsSortColumn;
+  sortDirection: SortDirection;
 }
 
 interface ChangeRawResultsSortMsg {

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, InterpretedResultsSortState } 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,19 +86,28 @@ export function webviewUriToFileUri(webviewUri: string): Uri {
   return Uri.file(path);
 }
 
-function sortInterpretedResults(results: Sarif.Result[], sortState: InterpretedResultsSortState): void {
-  switch (sortState.sortBy) {
-    case 'alert-message':
-      results.sort((a, b) =>
-        a.message.text === undefined ? 0 :
-          b.message.text === undefined ? 0 :
-            a.message.text?.localeCompare(b.message.text));
-      break;
-    case 'file-position':
-      // default to the order found in the sarif file
-      break;
-    default:
-      assertNever(sortState.sortBy);
+function sortInterpretedResults(results: Sarif.Result[], sortState: InterpretedResultsSortState | undefined): void {
+  function locToString(locs: Sarif.Location[] | undefined): string {
+    if (locs === undefined) return '';
+    return JSON.stringify(locs[0]) || '';
+  }
+
+  if (sortState !== undefined) {
+    const direction = sortState.sortDirection === SortDirection.asc ? 1 : -1;
+    switch (sortState.sortBy) {
+      case 'alert-message':
+        results.sort((a, b) =>
+          a.message.text === undefined ? 0 :
+            b.message.text === undefined ? 0 :
+              direction * (a.message.text?.localeCompare(b.message.text)));
+        break;
+      case 'file-position':
+        results.sort((a, b) =>
+          direction * locToString(a.locations).localeCompare(locToString(b.locations)));
+        break;
+      default:
+        assertNever(sortState.sortBy);
+    }
   }
 }
 
@@ -295,7 +304,7 @@ export class InterfaceManager extends DisposableObject {
     });
   }
 
-  private async getTruncatedResults(metadata: QueryMetadata | undefined, resultsPaths: ResultsPaths, sourceInfo: cli.SourceInfo | undefined, sourceLocationPrefix: string, sortState: InterpretedResultsSortState): Promise<Interpretation> {
+  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
@@ -317,7 +326,7 @@ export class InterfaceManager extends DisposableObject {
     return { sarif, sourceLocationPrefix, numTruncatedResults, sortState };
   }
 
-  private async interpretResultsInfo(query: QueryInfo, sortState: InterpretedResultsSortState): 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
@@ -351,7 +360,7 @@ export class InterfaceManager extends DisposableObject {
       resultsInfo,
       sourceInfo,
       sourceLocationPrefix,
-      { sortBy: 'file-position' } // sort order doesn't matter for showing diagnostics in parallel
+      undefined,
     );
 
     try {

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

@@ -25,9 +25,10 @@ export class CompletedQuery implements QueryWithResults {
    * 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.
+   * to the webview. Undefined means to use whatever order is in the
+   * sarif file.
    */
-  interpretedResultsSortState: InterpretedResultsSortState = { sortBy: 'file-position' };
+  interpretedResultsSortState: InterpretedResultsSortState | undefined;
 
   constructor(
     evalaution: QueryWithResults,

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,40 @@ 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';
+    }
+  }
+
+  toggleSortStateForColumn(column: InterpretedResultsSortColumn): void {
+    const oldSortState = this.props.resultSet.sortState;
+    const prevDirection = oldSortState && oldSortState.sortBy === column ? oldSortState.sortDirection : undefined;
+    const nextDirection = nextSortDirection(prevDirection);
+    const sortState: InterpretedResultsSortState | undefined =
+      nextDirection === undefined ? undefined :
+        { sortBy: column, sortDirection: nextDirection };
+    vscode.postMessage({
+      t: 'changeInterpretedSort',
+      sortState,
+    });
+  }
+
   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={2} onClick={() => this.toggleSortStateForColumn('alert-message')}>Message</th>
+        <th className={this.sortClass('file-position') + ' vscode-codeql__location-cell'} onClick={() => this.toggleSortStateForColumn('file-position')}>Location</th>
+      </tr>
+    </thead>;
+
     const rows: JSX.Element[] = [];
     const { numTruncatedResults, sourceLocationPrefix } = resultSet;
 
@@ -65,7 +97,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 +125,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 +263,7 @@ export class PathTable extends React.Component<PathTableProps, PathTableState> {
     }
 
     return <table className={className}>
+      {header}
       <tbody>{rows}</tbody>
     </table>;
   }

extensions/ql-vscode/src/view/raw-results-table.tsx

@@ -1,7 +1,6 @@
 import * as React from "react";
-import { renderLocation, ResultTableProps, zebraStripe, className } from "./result-table-utils";
+import { renderLocation, ResultTableProps, zebraStripe, className, nextSortDirection } from "./result-table-utils";
 import { RawTableResultSet, ResultValue, vscode } from "./results";
-import { assertNever } from "../helpers-pure";
 import { SortDirection, RAW_RESULTS_LIMIT, RawResultsSortState } from "../interface-types";
 
 export type RawTableProps = ResultTableProps & {
@@ -84,7 +83,6 @@ export class RawTable extends React.Component<RawTableProps, {}> {
   }
 }
 
-
 /**
  * Render one column of a tuple.
  */
@@ -99,15 +97,3 @@ function renderTupleValue(v: ResultValue, databaseUri: string): JSX.Element {
     return renderLocation(v.location, v.label, databaseUri);
   }
 }
-
-function nextSortDirection(direction: SortDirection | undefined): SortDirection {
-  switch (direction) {
-    case SortDirection.asc:
-      return SortDirection.desc;
-    case SortDirection.desc:
-    case undefined:
-      return SortDirection.asc;
-    default:
-      return assertNever(direction);
-  }
-}

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

@@ -1,7 +1,8 @@
 import * as React from 'react';
 import { LocationValue, ResolvableLocationValue, tryGetResolvableLocation } from 'semmle-bqrs';
-import { RawResultsSortState, QueryMetadata } from '../interface-types';
+import { RawResultsSortState, QueryMetadata, SortDirection } from '../interface-types';
 import { ResultSet, vscode } from './results';
+import { assertNever } from '../helpers-pure';
 
 export interface ResultTableProps {
   resultSet: ResultSet;
@@ -84,3 +85,18 @@ export function selectableZebraStripe(isSelected: boolean, index: number, ...oth
     ? { className: [selectedRowClassName, ...otherClasses].join(' ') }
     : zebraStripe(index, ...otherClasses)
 }
+
+/**
+ * Returns the next sort direction when cycling through sort directions while clicking.
+ */
+export function nextSortDirection(direction: SortDirection | undefined): SortDirection {
+  switch (direction) {
+    case SortDirection.asc:
+      return SortDirection.desc;
+    case SortDirection.desc:
+    case undefined:
+      return SortDirection.asc;
+    default:
+      return assertNever(direction);
+  }
+}

extensions/ql-vscode/src/view/result-tables.tsx

@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { DatabaseInfo, Interpretation, RawResultsSortState, QueryMetadata, ResultsPaths, InterpretedResultsSortOrder, InterpretedResultsSortState } from '../interface-types';
+import { DatabaseInfo, Interpretation, RawResultsSortState, QueryMetadata, ResultsPaths, InterpretedResultsSortState } from '../interface-types';
 import { PathTable } from './alert-table';
 import { RawTable } from './raw-results-table';
 import { ResultTableProps, tableSelectionHeaderClassName, toggleDiagnosticsClassName, alertExtrasClassName } from './result-table-utils';
@@ -94,15 +94,8 @@ export class ResultTables
     this.setState({ selectedTable: event.target.value });
   }
 
-  private onSortChange = (event: React.ChangeEvent<HTMLSelectElement>): void => {
-    vscode.postMessage({
-      t: 'changeInterpretedSort',
-      sortState: { sortBy: event.target.value as InterpretedResultsSortOrder },
-    });
-  }
-
   private alertTableExtras(): JSX.Element | undefined {
-    const { database, resultsPath, metadata, origResultsPaths, interpretedSortState } = this.props;
+    const { database, resultsPath, metadata, origResultsPaths } = this.props;
 
     const displayProblemsAsAlertsToggle =
       <div className={toggleDiagnosticsClassName}>
@@ -120,15 +113,7 @@ export class ResultTables
         <label htmlFor="toggle-diagnostics">Show results in Problems view</label>
       </div>;
 
-    const interpretedResultsSortSelect = <select value={interpretedSortState?.sortBy || 'file-position'}
-      onChange={this.onSortChange}>
-      <option value={'file-position'}>Source File Position</option>
-      <option value={'alert-message'}>Alert Message</option>
-    </select>;
-
     return <div className={alertExtrasClassName}>
-      Sort:
-      {interpretedResultsSortSelect}
       {displayProblemsAsAlertsToggle}
     </div>
   }

extensions/ql-vscode/src/view/resultsView.css

@@ -112,8 +112,12 @@ td.vscode-codeql__path-index-cell {
   text-align: right;
 }
 
-td.vscode-codeql__location-cell {
-  text-align: right;
+.vscode-codeql__alert-message-cell {
+  text-align: left !important;
+}
+
+.vscode-codeql__location-cell {
+  text-align: right !important;
 }
 
 .vscode-codeql__vertical-rule {