From c49aa8e05e975ec3687744525361952dce70744d Mon Sep 17 00:00:00 2001 From: marcnjaramillo Date: Mon, 15 Nov 2021 14:42:26 -0800 Subject: [PATCH 01/10] Fix issue with large SARIF files crashing view Authored by: Marc Jaramillo marcnjaramillo@github.com Authored by: Musab Guma'a mgsium@github.com --- extensions/ql-vscode/src/cli.ts | 66 +++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 12 deletions(-) diff --git a/extensions/ql-vscode/src/cli.ts b/extensions/ql-vscode/src/cli.ts index 8c1515a28a3..c2a2ec6f633 100644 --- a/extensions/ql-vscode/src/cli.ts +++ b/extensions/ql-vscode/src/cli.ts @@ -2,6 +2,11 @@ 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 { verifier } from 'stream-json/utils/Verifier'; +import Assembler = require('stream-json/Assembler'); +import { chain } from 'stream-chain'; import * as sarif from 'sarif'; import { SemVer } from 'semver'; import { Readable } from 'stream'; @@ -682,19 +687,56 @@ export class CodeQLCliServer implements Disposable { async interpretBqrs(metadata: QueryMetadata, resultsPath: string, interpretedResultsPath: string, sourceInfo?: SourceInfo): Promise { await this.runInterpretCommand(SARIF_FORMAT, metadata, resultsPath, interpretedResultsPath, sourceInfo); - - let output: string; - try { - output = await fs.readFile(interpretedResultsPath, 'utf8'); - } catch (e) { - const rawMessage = e.stderr || e.message; - const errorMessage = rawMessage.startsWith('Cannot create a string') - ? `SARIF too large. ${rawMessage}` - : rawMessage; - throw new Error(`Reading output of interpretation failed: ${errorMessage}`); - } try { - return JSON.parse(output) as sarif.Log; + // 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'}), + verifier() + ]); + + // 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 dummyTool : sarif.Tool = {driver: {name: ''}}; + if (asm.current) { + const log : sarif.Log = { + version: '2.1.0', + runs: [ + { + tool: dummyTool, + results: asm.current + } + ] + }; + + resolve(log); + } else { + const log : sarif.Log = { + version: '2.1.0', + runs: [ + { + tool: dummyTool, + results: [] + } + ] + }; + + resolve(log); + } + + }); + }); } catch (err) { throw new Error(`Parsing output of interpretation failed: ${err.stderr || err}`); } From 4374f409a89b5890658a898bfb8507cc0054875e Mon Sep 17 00:00:00 2001 From: marcnjaramillo Date: Mon, 15 Nov 2021 16:08:17 -0800 Subject: [PATCH 02/10] Add changelog entry and add missing dependencies --- extensions/ql-vscode/CHANGELOG.md | 2 + extensions/ql-vscode/package-lock.json | 68 ++++++++++++++++++++++++++ extensions/ql-vscode/package.json | 4 ++ extensions/ql-vscode/src/cli.ts | 2 +- 4 files changed, 75 insertions(+), 1 deletion(-) diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index ad0225ff29e..96b9a3096fe 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -3,6 +3,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) - 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) diff --git a/extensions/ql-vscode/package-lock.json b/extensions/ql-vscode/package-lock.json index a055803e1c7..fa7eb6e56e0 100644 --- a/extensions/ql-vscode/package-lock.json +++ b/extensions/ql-vscode/package-lock.json @@ -21,6 +21,8 @@ "react": "^16.8.6", "react-dom": "^16.8.6", "semver": "~7.3.2", + "stream-chain": "^2.2.4", + "stream-json": "^1.7.3", "tmp": "^0.1.0", "tmp-promise": "~3.0.2", "tree-kill": "~1.2.2", @@ -55,6 +57,8 @@ "@types/semver": "~7.2.0", "@types/sinon": "~7.5.2", "@types/sinon-chai": "~3.2.3", + "@types/stream-chain": "^2.0.1", + "@types/stream-json": "^1.7.1", "@types/through2": "^2.0.36", "@types/tmp": "^0.1.0", "@types/unzipper": "~0.10.1", @@ -852,6 +856,25 @@ "integrity": "sha512-K5K+yml8LTo9bWJI/rECfIPrGgxdpeNbj+d53lwN4QjW1MCwlkhUms+gtdzigTeUyBr09+u8BwOIY3MXvHdcsA==", "dev": true }, + "node_modules/@types/stream-chain": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/@types/stream-chain/-/stream-chain-2.0.1.tgz", + "integrity": "sha512-D+Id9XpcBpampptkegH7WMsEk6fUdf9LlCIX7UhLydILsqDin4L0QT7ryJR0oycwC7OqohIzdfcMHVZ34ezNGg==", + "dev": true, + "dependencies": { + "@types/node": "*" + } + }, + "node_modules/@types/stream-json": { + "version": "1.7.1", + "resolved": "https://registry.npmjs.org/@types/stream-json/-/stream-json-1.7.1.tgz", + "integrity": "sha512-BNIK/ix6iJvWvoXbDVVJhw5LNG1wie/rXcUo7jw4hBqY3FhIrg0e+RMXFN5UreKclBIStl9FDEHNSDLuuQ9/MQ==", + "dev": true, + "dependencies": { + "@types/node": "*", + "@types/stream-chain": "*" + } + }, "node_modules/@types/tapable": { "version": "1.0.6", "resolved": "https://registry.npmjs.org/@types/tapable/-/tapable-1.0.6.tgz", @@ -9720,12 +9743,25 @@ "node": ">=0.10.0" } }, + "node_modules/stream-chain": { + "version": "2.2.4", + "resolved": "https://registry.npmjs.org/stream-chain/-/stream-chain-2.2.4.tgz", + "integrity": "sha512-9lsl3YM53V5N/I1C2uJtc3Kavyi3kNYN83VkKb/bMWRk7D9imiFyUPYa0PoZbLohSVOX1mYE9YsmwObZUsth6Q==" + }, "node_modules/stream-exhaust": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/stream-exhaust/-/stream-exhaust-1.0.2.tgz", "integrity": "sha512-b/qaq/GlBK5xaq1yrK9/zFcyRSTNxmcZwFLGSTG0mXgZl/4Z6GgiyYOXOvY7N3eEvFRAG1bkDRz5EPGSvPYQlw==", "dev": true }, + "node_modules/stream-json": { + "version": "1.7.3", + "resolved": "https://registry.npmjs.org/stream-json/-/stream-json-1.7.3.tgz", + "integrity": "sha512-Y6dXn9KKWSwxOqnvHGcdZy1PK+J+7alBwHCeU3W9oRqm4ilLRA0XSPmd1tWwhg7tv9EIxJTMWh7KF15tYelKJg==", + "dependencies": { + "stream-chain": "^2.2.4" + } + }, "node_modules/stream-shift": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/stream-shift/-/stream-shift-1.0.1.tgz", @@ -12526,6 +12562,25 @@ "integrity": "sha512-K5K+yml8LTo9bWJI/rECfIPrGgxdpeNbj+d53lwN4QjW1MCwlkhUms+gtdzigTeUyBr09+u8BwOIY3MXvHdcsA==", "dev": true }, + "@types/stream-chain": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/@types/stream-chain/-/stream-chain-2.0.1.tgz", + "integrity": "sha512-D+Id9XpcBpampptkegH7WMsEk6fUdf9LlCIX7UhLydILsqDin4L0QT7ryJR0oycwC7OqohIzdfcMHVZ34ezNGg==", + "dev": true, + "requires": { + "@types/node": "*" + } + }, + "@types/stream-json": { + "version": "1.7.1", + "resolved": "https://registry.npmjs.org/@types/stream-json/-/stream-json-1.7.1.tgz", + "integrity": "sha512-BNIK/ix6iJvWvoXbDVVJhw5LNG1wie/rXcUo7jw4hBqY3FhIrg0e+RMXFN5UreKclBIStl9FDEHNSDLuuQ9/MQ==", + "dev": true, + "requires": { + "@types/node": "*", + "@types/stream-chain": "*" + } + }, "@types/tapable": { "version": "1.0.6", "resolved": "https://registry.npmjs.org/@types/tapable/-/tapable-1.0.6.tgz", @@ -19683,12 +19738,25 @@ } } }, + "stream-chain": { + "version": "2.2.4", + "resolved": "https://registry.npmjs.org/stream-chain/-/stream-chain-2.2.4.tgz", + "integrity": "sha512-9lsl3YM53V5N/I1C2uJtc3Kavyi3kNYN83VkKb/bMWRk7D9imiFyUPYa0PoZbLohSVOX1mYE9YsmwObZUsth6Q==" + }, "stream-exhaust": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/stream-exhaust/-/stream-exhaust-1.0.2.tgz", "integrity": "sha512-b/qaq/GlBK5xaq1yrK9/zFcyRSTNxmcZwFLGSTG0mXgZl/4Z6GgiyYOXOvY7N3eEvFRAG1bkDRz5EPGSvPYQlw==", "dev": true }, + "stream-json": { + "version": "1.7.3", + "resolved": "https://registry.npmjs.org/stream-json/-/stream-json-1.7.3.tgz", + "integrity": "sha512-Y6dXn9KKWSwxOqnvHGcdZy1PK+J+7alBwHCeU3W9oRqm4ilLRA0XSPmd1tWwhg7tv9EIxJTMWh7KF15tYelKJg==", + "requires": { + "stream-chain": "^2.2.4" + } + }, "stream-shift": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/stream-shift/-/stream-shift-1.0.1.tgz", diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index 9028f1e1fe4..3db03dab9c6 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -969,6 +969,8 @@ "react": "^16.8.6", "react-dom": "^16.8.6", "semver": "~7.3.2", + "stream-chain": "^2.2.4", + "stream-json": "^1.7.3", "tmp": "^0.1.0", "tmp-promise": "~3.0.2", "tree-kill": "~1.2.2", @@ -1003,6 +1005,8 @@ "@types/semver": "~7.2.0", "@types/sinon": "~7.5.2", "@types/sinon-chai": "~3.2.3", + "@types/stream-chain": "^2.0.1", + "@types/stream-json": "^1.7.1", "@types/through2": "^2.0.36", "@types/tmp": "^0.1.0", "@types/unzipper": "~0.10.1", diff --git a/extensions/ql-vscode/src/cli.ts b/extensions/ql-vscode/src/cli.ts index c2a2ec6f633..d5d445ad9e5 100644 --- a/extensions/ql-vscode/src/cli.ts +++ b/extensions/ql-vscode/src/cli.ts @@ -1185,7 +1185,7 @@ export class CliVersionConstraint { /** * CLI version where database registration was introduced - */ + */ public static CLI_VERSION_WITH_DB_REGISTRATION = new SemVer('2.4.1'); /** From 63129236d0a339904ad8a440ba544b932cd27acd Mon Sep 17 00:00:00 2001 From: marcnjaramillo Date: Wed, 17 Nov 2021 12:48:56 -0800 Subject: [PATCH 03/10] Work on tests for new behavior --- extensions/ql-vscode/package.json | 8 +- extensions/ql-vscode/src/cli.ts | 97 +++++++++---------- .../src/vscode-tests/no-workspace/cli.test.ts | 24 +++++ .../data/sarif/emptyResultsSarif.sarif | 35 +++++++ .../data/sarif/invalidSarif.sarif | 33 +++++++ .../no-workspace/data/sarif/validSarif.sarif | 57 +++++++++++ 6 files changed, 197 insertions(+), 57 deletions(-) create mode 100644 extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts create mode 100644 extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif create mode 100644 extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif create mode 100644 extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/validSarif.sarif diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index 3db03dab9c6..188a3cbedce 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -969,8 +969,8 @@ "react": "^16.8.6", "react-dom": "^16.8.6", "semver": "~7.3.2", - "stream-chain": "^2.2.4", - "stream-json": "^1.7.3", + "stream-chain": "~2.2.4", + "stream-json": "~1.7.3", "tmp": "^0.1.0", "tmp-promise": "~3.0.2", "tree-kill": "~1.2.2", @@ -1005,8 +1005,8 @@ "@types/semver": "~7.2.0", "@types/sinon": "~7.5.2", "@types/sinon-chai": "~3.2.3", - "@types/stream-chain": "^2.0.1", - "@types/stream-json": "^1.7.1", + "@types/stream-chain": "~2.0.1", + "@types/stream-json": "~1.7.1", "@types/through2": "^2.0.36", "@types/tmp": "^0.1.0", "@types/unzipper": "~0.10.1", diff --git a/extensions/ql-vscode/src/cli.ts b/extensions/ql-vscode/src/cli.ts index d5d445ad9e5..670519d800f 100644 --- a/extensions/ql-vscode/src/cli.ts +++ b/extensions/ql-vscode/src/cli.ts @@ -39,6 +39,8 @@ 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`. */ @@ -581,6 +583,47 @@ export class CodeQLCliServer implements Disposable { } } + static async parseSarif(interpretedResultsPath: string) : Promise { + 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'}), + verifier() + ]); + + // 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. @@ -687,59 +730,7 @@ export class CodeQLCliServer implements Disposable { async interpretBqrs(metadata: QueryMetadata, resultsPath: string, interpretedResultsPath: string, sourceInfo?: SourceInfo): Promise { await this.runInterpretCommand(SARIF_FORMAT, metadata, resultsPath, interpretedResultsPath, sourceInfo); - 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'}), - verifier() - ]); - - // 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 dummyTool : sarif.Tool = {driver: {name: ''}}; - if (asm.current) { - const log : sarif.Log = { - version: '2.1.0', - runs: [ - { - tool: dummyTool, - results: asm.current - } - ] - }; - - resolve(log); - } else { - const log : sarif.Log = { - version: '2.1.0', - runs: [ - { - tool: dummyTool, - results: [] - } - ] - }; - - resolve(log); - } - - }); - }); - } catch (err) { - throw new Error(`Parsing output of interpretation failed: ${err.stderr || err}`); - } + return await CodeQLCliServer.parseSarif(interpretedResultsPath); } async generateResultsCsv(metadata: QueryMetadata, resultsPath: string, csvPath: string, sourceInfo?: SourceInfo): Promise { diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts new file mode 100644 index 00000000000..b0a8445c765 --- /dev/null +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts @@ -0,0 +1,24 @@ +import * as chai from 'chai'; +import * as chaiAsPromised from 'chai-as-promised'; + +import { CodeQLCliServer } from '../../cli'; + +chai.use(chaiAsPromised); +const expect = chai.expect; + +describe.only('cliServerTests', () => { + it('should parse a valid SARIF file', async () => { + const result = await CodeQLCliServer.parseSarif(__dirname + '/data/sarif/validSarif.sarif'); + expect(result.runs.length).to.eq(1); + }); + + it('should return an empty array if there are no results', async () => { + const result = await CodeQLCliServer.parseSarif(__dirname + '/data/sarif/emptyResultsSarif.sarif'); + expect(result.runs[0].results?.length).to.eq(0); + }); + + it('should throw an error if the file fails to parse', async () => { + const result = await CodeQLCliServer.parseSarif(__dirname + '/data/sarif/invalidSarif.sarif'); + await expect(result).to.rejectedWith(/Error: Parsing output of interpretation failed: /); + }); +}); \ No newline at end of file diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif new file mode 100644 index 00000000000..3b4b4292d0d --- /dev/null +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif @@ -0,0 +1,35 @@ +{ + "version": "2.1.0", + "$schema": "http://json.schemastore.org/sarif-2.1.0-rtm.4", + "runs": [ + { + "tool": { + "driver": { + "name": "ESLint", + "informationUri": "https://eslint.org", + "rules": [ + { + "id": "no-unused-vars", + "shortDescription": { + "text": "disallow unused variables" + }, + "helpUri": "https://eslint.org/docs/rules/no-unused-vars", + "properties": { + "category": "Variables" + } + } + ] + } + }, + "artifacts": [ + { + "location": { + "uri": "file:///C:/dev/sarif/sarif-tutorials/samples/Introduction/simple-example.js" + } + } + ], + "results": [], + ] + } + ] +} \ No newline at end of file diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif new file mode 100644 index 00000000000..aece31dc9ab --- /dev/null +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif @@ -0,0 +1,33 @@ +{ + "version": "2.1.0", + "$schema": "http://json.schemastore.org/sarif-2.1.0-rtm.4", + "runs": [ + { + "tool": { + "driver": { + "name": "ESLint", + "informationUri": "https://eslint.org", + "rules": [ + { + "id": "no-unused-vars", + "shortDescription": { + "text": "disallow unused variables" + }, + "helpUri": "https://eslint.org/docs/rules/no-unused-vars", + "properties": { + "category": "Variables" + } + } + ] + } + }, + "artifacts": [ + { + "location": { + "uri": "file:///C:/dev/sarif/sarif-tutorials/samples/Introduction/simple-example.js" + } + } + ], + } + ] +} \ No newline at end of file diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/validSarif.sarif b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/validSarif.sarif new file mode 100644 index 00000000000..2d2fa5cacfa --- /dev/null +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/validSarif.sarif @@ -0,0 +1,57 @@ +{ + "version": "2.1.0", + "$schema": "http://json.schemastore.org/sarif-2.1.0-rtm.4", + "runs": [ + { + "tool": { + "driver": { + "name": "ESLint", + "informationUri": "https://eslint.org", + "rules": [ + { + "id": "no-unused-vars", + "shortDescription": { + "text": "disallow unused variables" + }, + "helpUri": "https://eslint.org/docs/rules/no-unused-vars", + "properties": { + "category": "Variables" + } + } + ] + } + }, + "artifacts": [ + { + "location": { + "uri": "file:///C:/dev/sarif/sarif-tutorials/samples/Introduction/simple-example.js" + } + } + ], + "results": [ + { + "level": "error", + "message": { + "text": "'x' is assigned a value but never used." + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "file:///C:/dev/sarif/sarif-tutorials/samples/Introduction/simple-example.js", + "index": 0 + }, + "region": { + "startLine": 1, + "startColumn": 5 + } + } + } + ], + "ruleId": "no-unused-vars", + "ruleIndex": 0 + } + ] + } + ] +} \ No newline at end of file From a66661928934f93eaee7169da9230982dd52d018 Mon Sep 17 00:00:00 2001 From: marcnjaramillo Date: Wed, 17 Nov 2021 13:55:34 -0800 Subject: [PATCH 04/10] Remove error handling for now --- extensions/ql-vscode/src/cli.ts | 7 +--- .../src/vscode-tests/no-workspace/cli.test.ts | 7 +--- .../data/sarif/emptyResultsSarif.sarif | 2 +- .../data/sarif/invalidSarif.sarif | 33 ------------------- 4 files changed, 3 insertions(+), 46 deletions(-) delete mode 100644 extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif diff --git a/extensions/ql-vscode/src/cli.ts b/extensions/ql-vscode/src/cli.ts index 670519d800f..8e759c9d677 100644 --- a/extensions/ql-vscode/src/cli.ts +++ b/extensions/ql-vscode/src/cli.ts @@ -4,7 +4,6 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import { parser } from 'stream-json'; import { pick } from 'stream-json/filters/Pick'; -import { verifier } from 'stream-json/utils/Verifier'; import Assembler = require('stream-json/Assembler'); import { chain } from 'stream-chain'; import * as sarif from 'sarif'; @@ -591,7 +590,6 @@ export class CodeQLCliServer implements Disposable { fs.createReadStream(interpretedResultsPath), p, pick({filter: 'runs.0.results'}), - verifier() ]); // Creates JavaScript objects from the token stream @@ -599,10 +597,7 @@ export class CodeQLCliServer implements Disposable { // 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); - }); + return await new Promise((resolve) => { asm.on('done', (asm) => { diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts index b0a8445c765..1713ecedcfc 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts @@ -14,11 +14,6 @@ describe.only('cliServerTests', () => { it('should return an empty array if there are no results', async () => { const result = await CodeQLCliServer.parseSarif(__dirname + '/data/sarif/emptyResultsSarif.sarif'); - expect(result.runs[0].results?.length).to.eq(0); - }); - - it('should throw an error if the file fails to parse', async () => { - const result = await CodeQLCliServer.parseSarif(__dirname + '/data/sarif/invalidSarif.sarif'); - await expect(result).to.rejectedWith(/Error: Parsing output of interpretation failed: /); + expect(result.runs[0].results).to.be.empty; }); }); \ No newline at end of file diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif index 3b4b4292d0d..99ebef48e84 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif @@ -28,7 +28,7 @@ } } ], - "results": [], + "results": [] ] } ] diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif deleted file mode 100644 index aece31dc9ab..00000000000 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif +++ /dev/null @@ -1,33 +0,0 @@ -{ - "version": "2.1.0", - "$schema": "http://json.schemastore.org/sarif-2.1.0-rtm.4", - "runs": [ - { - "tool": { - "driver": { - "name": "ESLint", - "informationUri": "https://eslint.org", - "rules": [ - { - "id": "no-unused-vars", - "shortDescription": { - "text": "disallow unused variables" - }, - "helpUri": "https://eslint.org/docs/rules/no-unused-vars", - "properties": { - "category": "Variables" - } - } - ] - } - }, - "artifacts": [ - { - "location": { - "uri": "file:///C:/dev/sarif/sarif-tutorials/samples/Introduction/simple-example.js" - } - } - ], - } - ] -} \ No newline at end of file From ca5e5e23e6df11e1e1c101a37a66d699e3a21068 Mon Sep 17 00:00:00 2001 From: marcnjaramillo Date: Wed, 17 Nov 2021 15:59:12 -0800 Subject: [PATCH 05/10] Finish tests --- extensions/ql-vscode/src/cli.ts | 9 +++-- .../src/vscode-tests/no-workspace/cli.test.ts | 4 ++- .../data/sarif/emptyResultsSarif.sarif | 1 - .../data/sarif/invalidSarif.sarif | 33 +++++++++++++++++++ 4 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif diff --git a/extensions/ql-vscode/src/cli.ts b/extensions/ql-vscode/src/cli.ts index 8e759c9d677..7c5ed685b7c 100644 --- a/extensions/ql-vscode/src/cli.ts +++ b/extensions/ql-vscode/src/cli.ts @@ -589,7 +589,7 @@ export class CodeQLCliServer implements Disposable { const pipeline = chain([ fs.createReadStream(interpretedResultsPath), p, - pick({filter: 'runs.0.results'}), + pick({filter: 'runs.0.results'}) ]); // Creates JavaScript objects from the token stream @@ -597,8 +597,11 @@ export class CodeQLCliServer implements Disposable { // 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) => { - + return await new Promise((resolve, reject) => { + pipeline.on('error', (error) => { + reject(error); + }); + asm.on('done', (asm) => { const log : sarif.Log = { diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts index 1713ecedcfc..c3c732431c5 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts @@ -6,7 +6,9 @@ import { CodeQLCliServer } from '../../cli'; chai.use(chaiAsPromised); const expect = chai.expect; -describe.only('cliServerTests', () => { +describe.only('cliServerTests', function() { + this.timeout(10000); + it('should parse a valid SARIF file', async () => { const result = await CodeQLCliServer.parseSarif(__dirname + '/data/sarif/validSarif.sarif'); expect(result.runs.length).to.eq(1); diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif index 99ebef48e84..b61d130f827 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif @@ -29,7 +29,6 @@ } ], "results": [] - ] } ] } \ No newline at end of file diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif new file mode 100644 index 00000000000..a5ec27484ad --- /dev/null +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif @@ -0,0 +1,33 @@ +{ + "version": "2.1.0", + "$schema": "http://json.schemastore.org/sarif-2.1.0-rtm.4", + "runs": [ + { + "tool": { + "driver": { + "name": "ESLint", + "informationUri": "https://eslint.org", + "rules": [ + { + "id": "no-unused-vars", + "shortDescription": { + "text": "disallow unused variables" + }, + "helpUri": "https://eslint.org/docs/rules/no-unused-vars", + "properties": { + "category": "Variables" + } + } + ] + } + }, + "artifacts": [ + { + "location": { + "uri": "file:///C:/dev/sarif/sarif-tutorials/samples/Introduction/simple-example.js" + } + } + ] + } + ] +} \ No newline at end of file From c3eca5b1b7e893eddccfd6f346c153aed164ee14 Mon Sep 17 00:00:00 2001 From: marcnjaramillo Date: Thu, 18 Nov 2021 16:05:31 -0800 Subject: [PATCH 06/10] Update test for valid SARIF file --- .../ql-vscode/src/vscode-tests/no-workspace/cli.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts index c3c732431c5..89131838921 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts @@ -7,11 +7,13 @@ chai.use(chaiAsPromised); const expect = chai.expect; describe.only('cliServerTests', function() { - this.timeout(10000); it('should parse a valid SARIF file', async () => { const result = await CodeQLCliServer.parseSarif(__dirname + '/data/sarif/validSarif.sarif'); - expect(result.runs.length).to.eq(1); + 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; }); it('should return an empty array if there are no results', async () => { From d53abd815d357dd3d4216de50b87945bd6e8c2e1 Mon Sep 17 00:00:00 2001 From: marcnjaramillo Date: Fri, 19 Nov 2021 16:01:18 -0800 Subject: [PATCH 07/10] Make suggested changes, build currently failing --- extensions/ql-vscode/CHANGELOG.md | 3 +- extensions/ql-vscode/package-lock.json | 36 +++++++++++-- extensions/ql-vscode/package.json | 1 + extensions/ql-vscode/src/cli.ts | 50 +------------------ extensions/ql-vscode/src/pure/sarif-utils.ts | 48 ++++++++++++++++++ .../test/pure-tests/sarif-utils.test.ts | 20 +++++++- .../sarif/emptyResultsSarif.sarif | 0 .../data => test}/sarif/invalidSarif.sarif | 0 .../data => test}/sarif/validSarif.sarif | 0 9 files changed, 103 insertions(+), 55 deletions(-) rename extensions/ql-vscode/{src/vscode-tests/no-workspace/data => test}/sarif/emptyResultsSarif.sarif (100%) rename extensions/ql-vscode/{src/vscode-tests/no-workspace/data => test}/sarif/invalidSarif.sarif (100%) rename extensions/ql-vscode/{src/vscode-tests/no-workspace/data => test}/sarif/validSarif.sarif (100%) diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index 96b9a3096fe..6e8cad8b227 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/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) diff --git a/extensions/ql-vscode/package-lock.json b/extensions/ql-vscode/package-lock.json index fa7eb6e56e0..6f8e5814362 100644 --- a/extensions/ql-vscode/package-lock.json +++ b/extensions/ql-vscode/package-lock.json @@ -21,8 +21,9 @@ "react": "^16.8.6", "react-dom": "^16.8.6", "semver": "~7.3.2", - "stream-chain": "^2.2.4", - "stream-json": "^1.7.3", + "stream": "^0.0.2", + "stream-chain": "~2.2.4", + "stream-json": "~1.7.3", "tmp": "^0.1.0", "tmp-promise": "~3.0.2", "tree-kill": "~1.2.2", @@ -57,8 +58,8 @@ "@types/semver": "~7.2.0", "@types/sinon": "~7.5.2", "@types/sinon-chai": "~3.2.3", - "@types/stream-chain": "^2.0.1", - "@types/stream-json": "^1.7.1", + "@types/stream-chain": "~2.0.1", + "@types/stream-json": "~1.7.1", "@types/through2": "^2.0.36", "@types/tmp": "^0.1.0", "@types/unzipper": "~0.10.1", @@ -3566,6 +3567,11 @@ "integrity": "sha512-EGuiJW4yBPOTj2NtWGZcX93ZE8IGj33HJAx4d3ouE2zOfW2trbWU+t1e0yzLr1qQIw81++txbM3BH52QwSRE6Q==", "dev": true }, + "node_modules/emitter-component": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/emitter-component/-/emitter-component-1.1.1.tgz", + "integrity": "sha1-Bl4tvtaVm/RwZ57avq95gdEAOrY=" + }, "node_modules/emitter-listener": { "version": "1.1.2", "resolved": "https://registry.npmjs.org/emitter-listener/-/emitter-listener-1.1.2.tgz", @@ -4738,6 +4744,7 @@ "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-1.2.13.tgz", "integrity": "sha512-oWb1Z6mkHIskLzEJ/XWX0srkpkTQ7vaopMQkyaEIoq0fmtFVxOthb8cCxeT+p3ynTdkk/RZwbgG4brR5BeWECw==", "dev": true, + "hasInstallScript": true, "optional": true, "os": [ "darwin" @@ -9743,6 +9750,14 @@ "node": ">=0.10.0" } }, + "node_modules/stream": { + "version": "0.0.2", + "resolved": "https://registry.npmjs.org/stream/-/stream-0.0.2.tgz", + "integrity": "sha1-f1Nj8Ff2WSxVlfALyAon9c7B8O8=", + "dependencies": { + "emitter-component": "^1.1.1" + } + }, "node_modules/stream-chain": { "version": "2.2.4", "resolved": "https://registry.npmjs.org/stream-chain/-/stream-chain-2.2.4.tgz", @@ -14783,6 +14798,11 @@ "integrity": "sha512-EGuiJW4yBPOTj2NtWGZcX93ZE8IGj33HJAx4d3ouE2zOfW2trbWU+t1e0yzLr1qQIw81++txbM3BH52QwSRE6Q==", "dev": true }, + "emitter-component": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/emitter-component/-/emitter-component-1.1.1.tgz", + "integrity": "sha1-Bl4tvtaVm/RwZ57avq95gdEAOrY=" + }, "emitter-listener": { "version": "1.1.2", "resolved": "https://registry.npmjs.org/emitter-listener/-/emitter-listener-1.1.2.tgz", @@ -19738,6 +19758,14 @@ } } }, + "stream": { + "version": "0.0.2", + "resolved": "https://registry.npmjs.org/stream/-/stream-0.0.2.tgz", + "integrity": "sha1-f1Nj8Ff2WSxVlfALyAon9c7B8O8=", + "requires": { + "emitter-component": "^1.1.1" + } + }, "stream-chain": { "version": "2.2.4", "resolved": "https://registry.npmjs.org/stream-chain/-/stream-chain-2.2.4.tgz", diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index 188a3cbedce..4d5d36bd556 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.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", diff --git a/extensions/ql-vscode/src/cli.ts b/extensions/ql-vscode/src/cli.ts index 7c5ed685b7c..70398058fd8 100644 --- a/extensions/ql-vscode/src/cli.ts +++ b/extensions/ql-vscode/src/cli.ts @@ -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 { - 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 { 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 { diff --git a/extensions/ql-vscode/src/pure/sarif-utils.ts b/extensions/ql-vscode/src/pure/sarif-utils.ts index e4b651ea2c3..308ca6c2fa3 100644 --- a/extensions/ql-vscode/src/pure/sarif-utils.ts +++ b/extensions/ql-vscode/src/pure/sarif-utils.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 { + 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; } diff --git a/extensions/ql-vscode/test/pure-tests/sarif-utils.test.ts b/extensions/ql-vscode/test/pure-tests/sarif-utils.test.ts index 6976217a73d..bbc5f0c4d4a 100644 --- a/extensions/ql-vscode/test/pure-tests/sarif-utils.test.ts +++ b/extensions/ql-vscode/test/pure-tests/sarif-utils.test.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({ diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif b/extensions/ql-vscode/test/sarif/emptyResultsSarif.sarif similarity index 100% rename from extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif rename to extensions/ql-vscode/test/sarif/emptyResultsSarif.sarif diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif b/extensions/ql-vscode/test/sarif/invalidSarif.sarif similarity index 100% rename from extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif rename to extensions/ql-vscode/test/sarif/invalidSarif.sarif diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/validSarif.sarif b/extensions/ql-vscode/test/sarif/validSarif.sarif similarity index 100% rename from extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/validSarif.sarif rename to extensions/ql-vscode/test/sarif/validSarif.sarif From 0bb1501e72f0a64823016f457cee1db6621c8abe Mon Sep 17 00:00:00 2001 From: marcnjaramillo Date: Fri, 19 Nov 2021 17:21:42 -0800 Subject: [PATCH 08/10] Move sarif parser and tests, build completing --- extensions/ql-vscode/src/cli.ts | 4 +- extensions/ql-vscode/src/pure/sarif-utils.ts | 48 ------------------- extensions/ql-vscode/src/sarif-parser.ts | 48 +++++++++++++++++++ .../sarif/emptyResultsSarif.sarif | 0 .../no-workspace}/sarif/invalidSarif.sarif | 0 .../no-workspace}/sarif/validSarif.sarif | 0 .../{cli.test.ts => sarifParser.test.ts} | 12 +++-- .../test/pure-tests/sarif-utils.test.ts | 17 ------- 8 files changed, 57 insertions(+), 72 deletions(-) create mode 100644 extensions/ql-vscode/src/sarif-parser.ts rename extensions/ql-vscode/{test => src/vscode-tests/no-workspace}/sarif/emptyResultsSarif.sarif (100%) rename extensions/ql-vscode/{test => src/vscode-tests/no-workspace}/sarif/invalidSarif.sarif (100%) rename extensions/ql-vscode/{test => src/vscode-tests/no-workspace}/sarif/validSarif.sarif (100%) rename extensions/ql-vscode/src/vscode-tests/no-workspace/{cli.test.ts => sarifParser.test.ts} (55%) diff --git a/extensions/ql-vscode/src/cli.ts b/extensions/ql-vscode/src/cli.ts index 70398058fd8..a73299fcb37 100644 --- a/extensions/ql-vscode/src/cli.ts +++ b/extensions/ql-vscode/src/cli.ts @@ -16,7 +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 { sarifParser } from './sarif-parser'; import { dbSchemeToLanguage } from './helpers'; /** @@ -682,7 +682,7 @@ export class CodeQLCliServer implements Disposable { async interpretBqrs(metadata: QueryMetadata, resultsPath: string, interpretedResultsPath: string, sourceInfo?: SourceInfo): Promise { await this.runInterpretCommand(SARIF_FORMAT, metadata, resultsPath, interpretedResultsPath, sourceInfo); - return await parseSarif(interpretedResultsPath); + return await sarifParser(interpretedResultsPath); } async generateResultsCsv(metadata: QueryMetadata, resultsPath: string, csvPath: string, sourceInfo?: SourceInfo): Promise { diff --git a/extensions/ql-vscode/src/pure/sarif-utils.ts b/extensions/ql-vscode/src/pure/sarif-utils.ts index 308ca6c2fa3..e4b651ea2c3 100644 --- a/extensions/ql-vscode/src/pure/sarif-utils.ts +++ b/extensions/ql-vscode/src/pure/sarif-utils.ts @@ -1,14 +1,6 @@ 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; @@ -164,46 +156,6 @@ export function parseSarifLocation( } } -export async function parseSarif(interpretedResultsPath: string) : Promise { - 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; } diff --git a/extensions/ql-vscode/src/sarif-parser.ts b/extensions/ql-vscode/src/sarif-parser.ts new file mode 100644 index 00000000000..eed080b4639 --- /dev/null +++ b/extensions/ql-vscode/src/sarif-parser.ts @@ -0,0 +1,48 @@ +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'; + +const DUMMY_TOOL : Sarif.Tool = {driver: {name: ''}}; + +export async function sarifParser(interpretedResultsPath: string) : Promise { + 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}`); + } +} \ No newline at end of file diff --git a/extensions/ql-vscode/test/sarif/emptyResultsSarif.sarif b/extensions/ql-vscode/src/vscode-tests/no-workspace/sarif/emptyResultsSarif.sarif similarity index 100% rename from extensions/ql-vscode/test/sarif/emptyResultsSarif.sarif rename to extensions/ql-vscode/src/vscode-tests/no-workspace/sarif/emptyResultsSarif.sarif diff --git a/extensions/ql-vscode/test/sarif/invalidSarif.sarif b/extensions/ql-vscode/src/vscode-tests/no-workspace/sarif/invalidSarif.sarif similarity index 100% rename from extensions/ql-vscode/test/sarif/invalidSarif.sarif rename to extensions/ql-vscode/src/vscode-tests/no-workspace/sarif/invalidSarif.sarif diff --git a/extensions/ql-vscode/test/sarif/validSarif.sarif b/extensions/ql-vscode/src/vscode-tests/no-workspace/sarif/validSarif.sarif similarity index 100% rename from extensions/ql-vscode/test/sarif/validSarif.sarif rename to extensions/ql-vscode/src/vscode-tests/no-workspace/sarif/validSarif.sarif diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/sarifParser.test.ts similarity index 55% rename from extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts rename to extensions/ql-vscode/src/vscode-tests/no-workspace/sarifParser.test.ts index 89131838921..ca49cd15e66 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/cli.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/sarifParser.test.ts @@ -1,23 +1,25 @@ +import * as path from 'path'; import * as chai from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; -import { CodeQLCliServer } from '../../cli'; +import { sarifParser } from '../../sarif-parser'; chai.use(chaiAsPromised); const expect = chai.expect; -describe.only('cliServerTests', function() { - +describe.only('sarif parser', function() { + const sarifDir = path.join(path.dirname(__dirname), 'sarif'); it('should parse a valid SARIF file', async () => { - const result = await CodeQLCliServer.parseSarif(__dirname + '/data/sarif/validSarif.sarif'); + const result = await sarifParser(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 CodeQLCliServer.parseSarif(__dirname + '/data/sarif/emptyResultsSarif.sarif'); + const result = await sarifParser(path.join(sarifDir, 'emptyResultsSarif.sarif')); expect(result.runs[0].results).to.be.empty; }); }); \ No newline at end of file diff --git a/extensions/ql-vscode/test/pure-tests/sarif-utils.test.ts b/extensions/ql-vscode/test/pure-tests/sarif-utils.test.ts index bbc5f0c4d4a..39e1be0b812 100644 --- a/extensions/ql-vscode/test/pure-tests/sarif-utils.test.ts +++ b/extensions/ql-vscode/test/pure-tests/sarif-utils.test.ts @@ -1,19 +1,16 @@ import 'mocha'; import { expect } from 'chai'; import * as Sarif from 'sarif'; -import * as path from 'path'; import { getPathRelativeToSourceLocationPrefix, parseSarifLocation, parseSarifPlainTextMessage, 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).'; @@ -64,20 +61,6 @@ 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({ From 9d9f48bcf85ca00c4a39cda9eaad286c35f397c4 Mon Sep 17 00:00:00 2001 From: marcnjaramillo Date: Fri, 19 Nov 2021 20:43:22 -0800 Subject: [PATCH 09/10] Fix tests for sarif parser --- .../ql-vscode/src/vscode-tests/no-workspace/sarifParser.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/sarifParser.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/sarifParser.test.ts index ca49cd15e66..3776c54c89d 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/sarifParser.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/sarifParser.test.ts @@ -8,7 +8,7 @@ chai.use(chaiAsPromised); const expect = chai.expect; describe.only('sarif parser', function() { - const sarifDir = path.join(path.dirname(__dirname), 'sarif'); + const sarifDir = path.join(__dirname, 'sarif'); it('should parse a valid SARIF file', async () => { const result = await sarifParser(path.join(sarifDir, 'validSarif.sarif')); expect(result.version).to.exist; From ad8112726725ad0aedf975439a947f5ca3763348 Mon Sep 17 00:00:00 2001 From: marcnjaramillo Date: Mon, 22 Nov 2021 11:49:40 -0800 Subject: [PATCH 10/10] Move test files into data directory --- .../no-workspace/{ => data}/sarif/emptyResultsSarif.sarif | 0 .../no-workspace/{ => data}/sarif/invalidSarif.sarif | 0 .../vscode-tests/no-workspace/{ => data}/sarif/validSarif.sarif | 0 .../ql-vscode/src/vscode-tests/no-workspace/sarifParser.test.ts | 2 +- 4 files changed, 1 insertion(+), 1 deletion(-) rename extensions/ql-vscode/src/vscode-tests/no-workspace/{ => data}/sarif/emptyResultsSarif.sarif (100%) rename extensions/ql-vscode/src/vscode-tests/no-workspace/{ => data}/sarif/invalidSarif.sarif (100%) rename extensions/ql-vscode/src/vscode-tests/no-workspace/{ => data}/sarif/validSarif.sarif (100%) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/sarif/emptyResultsSarif.sarif b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif similarity index 100% rename from extensions/ql-vscode/src/vscode-tests/no-workspace/sarif/emptyResultsSarif.sarif rename to extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/emptyResultsSarif.sarif diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/sarif/invalidSarif.sarif b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif similarity index 100% rename from extensions/ql-vscode/src/vscode-tests/no-workspace/sarif/invalidSarif.sarif rename to extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/invalidSarif.sarif diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/sarif/validSarif.sarif b/extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/validSarif.sarif similarity index 100% rename from extensions/ql-vscode/src/vscode-tests/no-workspace/sarif/validSarif.sarif rename to extensions/ql-vscode/src/vscode-tests/no-workspace/data/sarif/validSarif.sarif diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/sarifParser.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/sarifParser.test.ts index 3776c54c89d..f9b2fedf36e 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/sarifParser.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/sarifParser.test.ts @@ -8,7 +8,7 @@ chai.use(chaiAsPromised); const expect = chai.expect; describe.only('sarif parser', function() { - const sarifDir = path.join(__dirname, 'sarif'); + const sarifDir = path.join(__dirname, 'data/sarif'); it('should parse a valid SARIF file', async () => { const result = await sarifParser(path.join(sarifDir, 'validSarif.sarif')); expect(result.version).to.exist;