Make suggested changes, build currently failing

Author: marcnjaramillo

extensions/ql-vscode/CHANGELOG.md

@@ -2,9 +2,8 @@
 
 ## [UNRELEASED]
 
-- Fix the _CodeQL: Open Referenced File_ command for Windows systems. [#979](https://github.com/github/vscode-codeql/pull/979)
 - Fix the _CodeQL: Open Referenced File_ command for Windows systems. [#979](https://github.com/github/vscode-codeql/pull/979) 
-- Fix a bug that causes VSCode to crash when handling large SARIF files (>4GB) [#1004](https://github.com/github/vscode-codeql/pull/1004)
+- Support large SARIF results files (>4GB) without crashing VS Code. [#1004](https://github.com/github/vscode-codeql/pull/1004)
 - Fix a bug that shows 'Set current database' when hovering over the currently selected database in the databases view. [#976](https://github.com/github/vscode-codeql/pull/976)
 - Fix a bug with importing large databases. Databases over 4GB can now be imported directly from LGTM or from a zip file. This functionality is only available when using CodeQL CLI version 2.6.0 or later. [#971](https://github.com/github/vscode-codeql/pull/971)
 - Replace certain control codes (`U+0000` - `U+001F`) with their corresponding control labels (`U+2400` - `U+241F`) in the results view. [#963](https://github.com/github/vscode-codeql/pull/963)

extensions/ql-vscode/package-lock.json

@@ -969,6 +969,7 @@
     "react": "^16.8.6",
     "react-dom": "^16.8.6",
     "semver": "~7.3.2",
+    "stream": "^0.0.2",
     "stream-chain": "~2.2.4",
     "stream-json": "~1.7.3",
     "tmp": "^0.1.0",

extensions/ql-vscode/package.json

@@ -1,11 +1,6 @@
 import * as cpp from 'child-process-promise';
 import * as child_process from 'child_process';
-import * as fs from 'fs-extra';
 import * as path from 'path';
-import { parser } from 'stream-json';
-import { pick } from 'stream-json/filters/Pick';
-import Assembler = require('stream-json/Assembler');
-import { chain } from 'stream-chain';
 import * as sarif from 'sarif';
 import { SemVer } from 'semver';
 import { Readable } from 'stream';
@@ -21,6 +16,7 @@ import { assertNever } from './pure/helpers-pure';
 import { QueryMetadata, SortDirection } from './pure/interface-types';
 import { Logger, ProgressReporter } from './logging';
 import { CompilationMessage } from './pure/messages';
+import { parseSarif } from './pure/sarif-utils';
 import { dbSchemeToLanguage } from './helpers';
 
 /**
@@ -38,8 +34,6 @@ const CSV_FORMAT = 'csv';
  */
 const LOGGING_FLAGS = ['-v', '--log-to-stderr'];
 
-const DUMMY_TOOL : sarif.Tool = {driver: {name: ''}};
-
 /**
  * The expected output of `codeql resolve library-path`.
  */
@@ -582,46 +576,6 @@ export class CodeQLCliServer implements Disposable {
     }
   }
 
-  static async parseSarif(interpretedResultsPath: string) : Promise<sarif.Log> {
-    try {
-      // Parse the SARIF file into token streams, filtering out only the results array.
-      const p = parser();
-      const pipeline = chain([
-        fs.createReadStream(interpretedResultsPath),
-        p,
-        pick({filter: 'runs.0.results'})
-      ]);
-
-      // Creates JavaScript objects from the token stream
-      const asm = Assembler.connectTo(pipeline);
-
-      // Returns a constructed Log object with the results or an empty array if no results were found.
-      // If the parser fails for any reason, it will reject the promise.
-      return await new Promise((resolve, reject) => {
-        pipeline.on('error', (error) => {
-          reject(error);
-        });
-        
-        asm.on('done', (asm) => {
-
-          const log : sarif.Log = {
-            version: '2.1.0', 
-            runs: [
-              { 
-                tool: DUMMY_TOOL, 
-                results: asm.current ?? []
-              }
-            ]
-          };
-          
-          resolve(log);
-        });
-      });
-    } catch (err) {
-      throw new Error(`Parsing output of interpretation failed: ${err.stderr || err}`);
-    }
-  }
-
   /**
    * Gets the metadata for a query.
    * @param queryPath The path to the query.
@@ -728,7 +682,7 @@ export class CodeQLCliServer implements Disposable {
 
   async interpretBqrs(metadata: QueryMetadata, resultsPath: string, interpretedResultsPath: string, sourceInfo?: SourceInfo): Promise<sarif.Log> {
     await this.runInterpretCommand(SARIF_FORMAT, metadata, resultsPath, interpretedResultsPath, sourceInfo);
-    return await CodeQLCliServer.parseSarif(interpretedResultsPath);
+    return await parseSarif(interpretedResultsPath);
   }
 
   async generateResultsCsv(metadata: QueryMetadata, resultsPath: string, csvPath: string, sourceInfo?: SourceInfo): Promise<void> {

extensions/ql-vscode/src/cli.ts

@@ -1,6 +1,14 @@
 import * as Sarif from 'sarif';
+import * as fs from 'fs-extra';
+import { parser } from 'stream-json';
+import { pick } from 'stream-json/filters/Pick';
+import Assembler = require('stream-json/Assembler');
+import { chain } from 'stream-chain';
+
 import { ResolvableLocationValue } from './bqrs-cli-types';
 
+const DUMMY_TOOL : Sarif.Tool = {driver: {name: ''}};
+
 export interface SarifLink {
   dest: number;
   text: string;
@@ -156,6 +164,46 @@ export function parseSarifLocation(
   }
 }
 
+export async function parseSarif(interpretedResultsPath: string) : Promise<Sarif.Log> {
+  try {
+    // Parse the SARIF file into token streams, filtering out only the results array.
+    const p = parser();
+    const pipeline = chain([
+      fs.createReadStream(interpretedResultsPath),
+      p,
+      pick({filter: 'runs.0.results'})
+    ]);
+
+    // Creates JavaScript objects from the token stream
+    const asm = Assembler.connectTo(pipeline);
+
+    // Returns a constructed Log object with the results or an empty array if no results were found.
+    // If the parser fails for any reason, it will reject the promise.
+    return await new Promise((resolve, reject) => {
+      pipeline.on('error', (error) => {
+        reject(error);
+      });
+      
+      asm.on('done', (asm) => {
+
+        const log : Sarif.Log = {
+          version: '2.1.0', 
+          runs: [
+            { 
+              tool: DUMMY_TOOL, 
+              results: asm.current ?? []
+            }
+          ]
+        };
+        
+        resolve(log);
+      });
+    });
+  } catch (err) {
+    throw new Error(`Parsing output of interpretation failed: ${err.stderr || err}`);
+  }
+}
+
 export function isNoLocation(loc: ParsedSarifLocation): loc is NoLocation {
   return 'hint' in loc;
 }

extensions/ql-vscode/src/pure/sarif-utils.ts

@@ -1,16 +1,20 @@
 import 'mocha';
 import { expect } from 'chai';
 import * as Sarif from 'sarif';
+import * as path from 'path';
 
 import {
   getPathRelativeToSourceLocationPrefix,
   parseSarifLocation,
   parseSarifPlainTextMessage,
-  unescapeSarifText
+  unescapeSarifText,
+  parseSarif
 } from '../../src/pure/sarif-utils';
 
 
 describe('parsing sarif', () => {
+  const sarifDir = path.join(path.dirname(__dirname), 'sarif');
+
   it('should be able to parse a simple message from the spec', async function() {
     const message = 'Tainted data was used. The data came from [here](3).';
     const results = parseSarifPlainTextMessage(message);
@@ -60,6 +64,20 @@ describe('parsing sarif', () => {
       .to.eq('file:/a/b/c/?x=test');
   });
 
+  it('should parse a valid SARIF file', async () => {
+    const result = await parseSarif(path.join(sarifDir, 'validSarif.sarif'));
+    expect(result.version).to.exist;
+    expect(result.runs).to.exist;
+    expect(result.runs[0].tool).to.exist;
+    expect(result.runs[0].tool.driver).to.exist;
+    expect(result.runs.length).to.be.at.least(1);
+  });
+
+  it('should return an empty array if there are no results', async () => {
+    const result = await parseSarif(path.join(sarifDir, 'emptyResultsSarif.sarif'));
+    expect(result.runs[0].results).to.be.empty;
+  });
+
   describe('parseSarifLocation', () => {
     it('should parse a sarif location with "no location"', () => {
       expect(parseSarifLocation({}, '')).to.deep.equal({