Merge pull request #491 from jcreedcmu/jcreed/cleanup

Remove pagination feature flag

Author: jcreedcmu

extensions/ql-vscode/src/adapt.ts

@@ -103,30 +103,7 @@ export function adaptBqrs(schema: AdaptedSchema, page: DecodedBqrsChunk): RawRes
   };
 }
 
-/**
- * This type has two branches; we are in the process of changing from
- * one to the other. The old way is to parse them inside the webview,
- * the new way is to parse them in the extension. The main motivation
- * for this transition is to make pagination possible in such a way
- * that only one page needs to be sent from the extension to the webview.
- */
-export type ParsedResultSets = ExtensionParsedResultSets | WebviewParsedResultSets;
-
-/**
- * The old method doesn't require any nontrivial information to be included here,
- * just a tag to indicate that it is being used.
- */
-export interface WebviewParsedResultSets {
-  t: 'WebviewParsed';
-  selectedTable?: string; // when undefined, means 'show default table'
-}
-
-/**
- * The new method includes which bqrs page is being sent, and the
- * actual results parsed on the extension side.
- */
-export interface ExtensionParsedResultSets {
-  t: 'ExtensionParsed';
+export interface ParsedResultSets {
   pageNumber: number;
   numPages: number;
   numInterpretedPages: number;

extensions/ql-vscode/src/config.ts

@@ -39,18 +39,6 @@ class Setting {
 
 const ROOT_SETTING = new Setting('codeQL');
 
-// Enable experimental features
-
-/**
- * Any settings below are deliberately not in package.json so that
- * they do not appear in the settings ui in vscode itself. If users
- * want to enable experimental features, they can add them directly in
- * their vscode settings json file.
- */
-
-/* Advanced setting: used to enable bqrs parsing in the cli instead of in the webview. */
-export const EXPERIMENTAL_BQRS_SETTING = new Setting('experimentalBqrsParsing', ROOT_SETTING);
-
 // Distribution configuration
 
 const DISTRIBUTION_SETTING = new Setting('cli', ROOT_SETTING);

extensions/ql-vscode/src/interface.ts

@@ -41,7 +41,6 @@ import {
   ParsedResultSets,
   RawResultSet,
 } from './adapt';
-import { EXPERIMENTAL_BQRS_SETTING } from './config';
 import {
   WebviewReveal,
   fileUriToWebviewUri,
@@ -335,40 +334,33 @@ export class InterfaceManager extends DisposableObject {
     }
 
     const getParsedResultSets = async (): Promise<ParsedResultSets> => {
-      if (EXPERIMENTAL_BQRS_SETTING.getValue()) {
-        const resultSetSchemas = await this.getResultSetSchemas(results);
-        const resultSetNames = resultSetSchemas.map(schema => schema.name);
-
-        // This may not wind up being the page we actually show, if there are interpreted results,
-        // but speculatively send it anyway.
-        const selectedTable = getDefaultResultSetName(resultSetNames);
-        const schema = resultSetSchemas.find(
-          (resultSet) => resultSet.name == selectedTable
-        )!;
-        if (schema === undefined) {
-          return { t: 'WebviewParsed' };
-        }
 
-        const chunk = await this.cliServer.bqrsDecode(
-          results.query.resultsPaths.resultsPath,
-          schema.name,
-          RAW_RESULTS_PAGE_SIZE,
-          schema.pagination?.offsets[0]
-        );
-        const adaptedSchema = adaptSchema(schema);
-        const resultSet = adaptBqrs(adaptedSchema, chunk);
-        return {
-          t: 'ExtensionParsed',
-          pageNumber: 0,
-          numPages: numPagesOfResultSet(resultSet),
-          numInterpretedPages: numInterpretedPages(this._interpretation),
-          resultSet: { t: 'RawResultSet', ...resultSet },
-          selectedTable: undefined,
-          resultSetNames,
-        };
-      } else {
-        return { t: 'WebviewParsed' };
-      }
+      const resultSetSchemas = await this.getResultSetSchemas(results);
+      const resultSetNames = resultSetSchemas.map(schema => schema.name);
+
+      // This may not wind up being the page we actually show, if there are interpreted results,
+      // but speculatively send it anyway.
+      const selectedTable = getDefaultResultSetName(resultSetNames);
+      const schema = resultSetSchemas.find(
+        (resultSet) => resultSet.name == selectedTable
+      )!;
+
+      const chunk = await this.cliServer.bqrsDecode(
+        results.query.resultsPaths.resultsPath,
+        schema.name,
+        RAW_RESULTS_PAGE_SIZE,
+        schema.pagination?.offsets[0]
+      );
+      const adaptedSchema = adaptSchema(schema);
+      const resultSet = adaptBqrs(adaptedSchema, chunk);
+      return {
+        pageNumber: 0,
+        numPages: numPagesOfResultSet(resultSet),
+        numInterpretedPages: numInterpretedPages(this._interpretation),
+        resultSet: { t: 'RawResultSet', ...resultSet },
+        selectedTable: undefined,
+        resultSetNames,
+      };
     };
 
     await this.postMessage({
@@ -461,7 +453,6 @@ export class InterfaceManager extends DisposableObject {
     const resultSet = adaptBqrs(adaptedSchema, chunk);
 
     const parsedResultSets: ParsedResultSets = {
-      t: 'ExtensionParsed',
       pageNumber,
       resultSet: { t: 'RawResultSet', ...resultSet },
       numPages: numPagesOfResultSet(resultSet),

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

@@ -30,7 +30,7 @@ export class RawTable extends React.Component<RawTableProps, {}> {
     const tableRows = dataRows.map((row: ResultRow, rowIndex: number) =>
       <RawTableRow
         key={rowIndex}
-        rowIndex={rowIndex}
+        rowIndex={rowIndex + this.props.offset}
         row={row}
         databaseUri={databaseUri}
       />

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

@@ -15,7 +15,7 @@ import {
 import { PathTable } from './alert-table';
 import { RawTable } from './raw-results-table';
 import { ResultTableProps, tableSelectionHeaderClassName, toggleDiagnosticsClassName, alertExtrasClassName } from './result-table-utils';
-import { ParsedResultSets, ExtensionParsedResultSets } from '../adapt';
+import { ParsedResultSets } from '../adapt';
 import { vscode } from './vscode-api';
 
 
@@ -90,52 +90,23 @@ export class ResultTables
   }
 
   private getResultSetNames(resultSets: ResultSet[]): string[] {
-    if (this.props.parsedResultSets.t === 'ExtensionParsed') {
-      return this.props.parsedResultSets.resultSetNames.concat([ALERTS_TABLE_NAME]);
-    }
-    else {
-      return resultSets.map(resultSet => resultSet.schema.name);
-    }
-  }
-
-  /**
-   * Holds if we have a result set obtained from the extension that came
-   * from the ExtensionParsed branch of ParsedResultSets. This is evidence
-   * that the user has the experimental flag turned on that allows extension-side
-   * bqrs parsing.
-   */
-  paginationAllowed(): boolean {
-    return this.props.parsedResultSets.t === 'ExtensionParsed';
+    return this.props.parsedResultSets.resultSetNames.concat([ALERTS_TABLE_NAME]);
   }
 
   constructor(props: ResultTablesProps) {
     super(props);
     const selectedTable = props.parsedResultSets.selectedTable || getDefaultResultSet(this.getResultSets());
-    let selectedPage: string;
-
-    switch (props.parsedResultSets.t) {
-      case 'ExtensionParsed':
-        selectedPage = (props.parsedResultSets.pageNumber + 1) + '';
-        break;
-      case 'WebviewParsed':
-        selectedPage = '';
-        break;
-    }
+    const selectedPage = (props.parsedResultSets.pageNumber + 1) + '';
     this.state = { selectedTable, selectedPage };
   }
 
   private onTableSelectionChange = (event: React.ChangeEvent<HTMLSelectElement>): void => {
     const selectedTable = event.target.value;
-
-    if (this.paginationAllowed()) {
-      vscode.postMessage({
-        t: 'changePage',
-        pageNumber: 0,
-        selectedTable
-      });
-    }
-    else
-      this.setState({ selectedTable });
+    vscode.postMessage({
+      t: 'changePage',
+      pageNumber: 0,
+      selectedTable
+    });
   }
 
   private alertTableExtras(): JSX.Element | undefined {
@@ -164,15 +135,11 @@ export class ResultTables
 
   getOffset(): number {
     const { parsedResultSets } = this.props;
-    switch (parsedResultSets.t) {
-      case 'ExtensionParsed':
-        return parsedResultSets.pageNumber * RAW_RESULTS_PAGE_SIZE;
-      case 'WebviewParsed':
-        return 0;
-    }
+    return parsedResultSets.pageNumber * RAW_RESULTS_PAGE_SIZE;
   }
 
-  renderPageButtons(resultSets: ExtensionParsedResultSets): JSX.Element {
+  renderPageButtons(): JSX.Element {
+    const { parsedResultSets } = this.props;
     const selectedTable = this.state.selectedTable;
 
     // FIXME: The extension, not the view, should be in charge of deciding whether to initially show
@@ -181,7 +148,7 @@ export class ResultTables
     // not interpreted pages, because the extension doesn't know the view will default to showing alerts
     // instead.
     const numPages = selectedTable == ALERTS_TABLE_NAME ?
-      resultSets.numInterpretedPages : resultSets.numPages;
+      parsedResultSets.numInterpretedPages : parsedResultSets.numPages;
 
     const onChange = (e: React.ChangeEvent<HTMLInputElement>) => {
       this.setState({ selectedPage: e.target.value });
@@ -201,14 +168,14 @@ export class ResultTables
     const prevPage = (e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => {
       vscode.postMessage({
         t: 'changePage',
-        pageNumber: Math.max(resultSets.pageNumber - 1, 0),
+        pageNumber: Math.max(parsedResultSets.pageNumber - 1, 0),
         selectedTable,
       });
     };
     const nextPage = (e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => {
       vscode.postMessage({
         t: 'changePage',
-        pageNumber: Math.min(resultSets.pageNumber + 1, numPages - 1),
+        pageNumber: Math.min(parsedResultSets.pageNumber + 1, numPages - 1),
         selectedTable,
       });
     };
@@ -230,13 +197,6 @@ export class ResultTables
     </span>;
   }
 
-  renderButtons(): JSX.Element {
-    if (this.props.parsedResultSets.t === 'ExtensionParsed' && this.paginationAllowed())
-      return this.renderPageButtons(this.props.parsedResultSets);
-    else
-      return <span />;
-  }
-
   render(): React.ReactNode {
     const { selectedTable } = this.state;
     const resultSets = this.getResultSets();
@@ -250,7 +210,7 @@ export class ResultTables
       resultSetNames.map(name => <option key={name} value={name}>{name}</option>);
 
     return <div>
-      {this.renderButtons()}
+      {this.renderPageButtons()}
       <div className={tableSelectionHeaderClassName}>
         <select value={selectedTable} onChange={this.onTableSelectionChange}>
           {resultSetOptions}