diff --git a/extensions/ql-vscode/src/adapt.ts b/extensions/ql-vscode/src/adapt.ts deleted file mode 100644 index e351f98a6..000000000 --- a/extensions/ql-vscode/src/adapt.ts +++ /dev/null @@ -1,113 +0,0 @@ -import { DecodedBqrsChunk, ResultSetSchema, ColumnKind, Column, ColumnValue } from './bqrs-cli-types'; -import { LocationValue, ResultSetSchema as AdaptedSchema, ColumnSchema, ColumnType, LocationStyle } from './bqrs-types'; -import { ResultSet } from './interface-types'; - -// FIXME: This is a temporary bit of impedance matching to convert -// from the types provided by ./bqrs-cli-types, to the types used by -// the view layer. -// -// The reason that it is benign for now is that it is only used by -// feature-flag-guarded codepaths that won't be encountered by normal -// users. It is not yet guaranteed to produce correct output for raw -// results. -// -// Eventually, the view layer should be refactored to directly accept data -// of types coming from bqrs-cli-types, and this file can be deleted. - -export type ResultRow = ResultValue[]; - -export interface ResultElement { - label: string; - location?: LocationValue; -} - -export interface ResultUri { - uri: string; -} - -export type ResultValue = ResultElement | ResultUri | string; - -export interface RawResultSet { - readonly schema: AdaptedSchema; - readonly rows: readonly ResultRow[]; -} - -function adaptKind(kind: ColumnKind): ColumnType { - // XXX what about 'u'? - if (kind === 'e') { - return { type: 'e', primitiveType: 's', locationStyle: LocationStyle.FivePart, hasLabel: true }; - } - else { - return { type: kind }; - } -} - -function adaptColumn(col: Column): ColumnSchema { - return { name: col.name!, type: adaptKind(col.kind) }; -} - -export function adaptSchema(schema: ResultSetSchema): AdaptedSchema { - return { - columns: schema.columns.map(adaptColumn), - name: schema.name, - tupleCount: schema.rows, - version: 0, - }; -} - -export function adaptValue(val: ColumnValue): ResultValue { - // XXX taking a lot of incorrect shortcuts here - - if (typeof val === 'string') { - return val; - } - - if (typeof val === 'number' || typeof val === 'boolean') { - return val + ''; - } - - const url = val.url; - - if (typeof url === 'string') { - return url; - } - - if (url === undefined) { - return val.label || ''; - } - - return { - label: val.label || '', - location: { - t: LocationStyle.FivePart, - lineStart: url.startLine, - lineEnd: url.endLine, - colStart: url.startColumn, - colEnd: url.endColumn, - // FIXME: This seems definitely wrong. Should we be using - // something like the code in sarif-utils.ts? - file: url.uri.replace(/file:/, ''), - } - }; - -} - -export function adaptRow(row: ColumnValue[]): ResultRow { - return row.map(adaptValue); -} - -export function adaptBqrs(schema: AdaptedSchema, page: DecodedBqrsChunk): RawResultSet { - return { - schema, - rows: page.tuples.map(adaptRow), - }; -} - -export interface ParsedResultSets { - pageNumber: number; - numPages: number; - numInterpretedPages: number; - selectedTable?: string; // when undefined, means 'show default table' - resultSetNames: string[]; - resultSet: ResultSet; -} diff --git a/extensions/ql-vscode/src/astViewer.ts b/extensions/ql-vscode/src/astViewer.ts index 99fa8039c..592c89f9e 100644 --- a/extensions/ql-vscode/src/astViewer.ts +++ b/extensions/ql-vscode/src/astViewer.ts @@ -4,6 +4,7 @@ import { DatabaseItem } from './databases'; import { UrlValue, BqrsId } from './bqrs-cli-types'; import fileRangeFromURI from './contextual/fileRangeFromURI'; import { showLocation } from './interface-utils'; +import { isStringLoc, isWholeFileLoc, isLineColumnLoc } from './bqrs-utils'; export interface AstItem { id: BqrsId; @@ -33,7 +34,7 @@ class AstViewerDataProvider implements vscode.TreeDataProvider 1 && matches[1]) { if (isWholeFileMatch(matches)) { return { - t: LocationStyle.WholeFile, - file: matches[1], - }; + uri: matches[1], + } as WholeFileLocation; } else { return { - t: LocationStyle.FivePart, - file: matches[1], - lineStart: Number(matches[2]), - colStart: Number(matches[3]), - lineEnd: Number(matches[4]), - colEnd: Number(matches[5]), + uri: matches[1], + startLine: Number(matches[2]), + startColumn: Number(matches[3]), + endLine: Number(matches[4]), + endColumn: Number(matches[5]), }; } } else { @@ -75,8 +67,26 @@ function isWholeFileMatch(matches: RegExpExecArray): boolean { * the file path is empty. If so, we do not want to render this location * as a link. * - * @param path A file path + * @param uri A file uri */ -function isEmptyPath(path: string) { - return !path || path === '/'; +function isEmptyPath(uriStr: string) { + return !uriStr || uriStr === 'file:/'; +} + +export function isLineColumnLoc(loc: UrlValue): loc is LineColumnLocation { + return typeof loc !== 'string' + && !isEmptyPath(loc.uri) + && 'startLine' in loc + && 'startColumn' in loc + && 'endLine' in loc + && 'endColumn' in loc + && loc.endColumn > 0; +} + +export function isWholeFileLoc(loc: UrlValue): loc is WholeFileLocation { + return typeof loc !== 'string' && !isEmptyPath(loc.uri) && !isLineColumnLoc(loc); +} + +export function isStringLoc(loc: UrlValue): loc is string { + return typeof loc === 'string'; } diff --git a/extensions/ql-vscode/src/cli.ts b/extensions/ql-vscode/src/cli.ts index 9fc9a59d3..25b9ff104 100644 --- a/extensions/ql-vscode/src/cli.ts +++ b/extensions/ql-vscode/src/cli.ts @@ -525,7 +525,6 @@ export class CodeQLCliServer implements Disposable { return await this.runJsonCodeQlCliCommand(['bqrs', 'decode'], subcommandArgs, 'Reading bqrs data'); } - async interpretBqrs(metadata: { kind: string; id: string }, resultsPath: string, interpretedResultsPath: string, sourceInfo?: SourceInfo): Promise { const args = [ `-t=kind=${metadata.kind}`, diff --git a/extensions/ql-vscode/src/compare/compare-interface.ts b/extensions/ql-vscode/src/compare/compare-interface.ts index da506afa8..71877f3be 100644 --- a/extensions/ql-vscode/src/compare/compare-interface.ts +++ b/extensions/ql-vscode/src/compare/compare-interface.ts @@ -19,8 +19,7 @@ import { Logger } from '../logging'; import { CodeQLCliServer } from '../cli'; import { DatabaseManager } from '../databases'; import { getHtmlForWebview, jumpToLocation } from '../interface-utils'; -import { adaptSchema, adaptBqrs, RawResultSet } from '../adapt'; -import { BQRSInfo } from '../bqrs-cli-types'; +import { adaptBqrs, RawResultSet, BQRSInfo } from '../bqrs-cli-types'; import resultsDiff from './resultsDiff'; interface ComparePair { @@ -257,8 +256,7 @@ export class CompareInterfaceManager extends DisposableObject { resultsPath, resultSetName ); - const adaptedSchema = adaptSchema(schema); - return adaptBqrs(adaptedSchema, chunk); + return adaptBqrs(schema, chunk); } private compareResults( diff --git a/extensions/ql-vscode/src/compare/resultsDiff.ts b/extensions/ql-vscode/src/compare/resultsDiff.ts index 7f9f4f06c..60b685abd 100644 --- a/extensions/ql-vscode/src/compare/resultsDiff.ts +++ b/extensions/ql-vscode/src/compare/resultsDiff.ts @@ -1,4 +1,4 @@ -import { RawResultSet } from '../adapt'; +import { RawResultSet } from '../bqrs-cli-types'; import { QueryCompareResult } from '../interface-types'; /** diff --git a/extensions/ql-vscode/src/compare/view/CompareTable.tsx b/extensions/ql-vscode/src/compare/view/CompareTable.tsx index c26d15b5f..a7dff6ec0 100644 --- a/extensions/ql-vscode/src/compare/view/CompareTable.tsx +++ b/extensions/ql-vscode/src/compare/view/CompareTable.tsx @@ -3,7 +3,7 @@ import * as React from 'react'; import { SetComparisonsMessage } from '../../interface-types'; import RawTableHeader from '../../view/RawTableHeader'; import { className } from '../../view/result-table-utils'; -import { ResultRow } from '../../adapt'; +import { ResultRow } from '../../bqrs-cli-types'; import RawTableRow from '../../view/RawTableRow'; import { vscode } from '../../view/vscode-api'; diff --git a/extensions/ql-vscode/src/databases.ts b/extensions/ql-vscode/src/databases.ts index 47892157e..61a97a176 100644 --- a/extensions/ql-vscode/src/databases.ts +++ b/extensions/ql-vscode/src/databases.ts @@ -316,8 +316,10 @@ class DatabaseItemImpl implements DatabaseItem { } } - public resolveSourceFile(file: string | undefined): vscode.Uri { + public resolveSourceFile(uri: string | undefined): vscode.Uri { const sourceArchive = this.sourceArchive; + // FIXME: This is wrong. Should parse the uri properly first + const file = uri?.replace(/file:/, ''); if (sourceArchive === undefined) { if (file !== undefined) { // Treat it as an absolute path. diff --git a/extensions/ql-vscode/src/interface-types.ts b/extensions/ql-vscode/src/interface-types.ts index 48d40c5c4..100484613 100644 --- a/extensions/ql-vscode/src/interface-types.ts +++ b/extensions/ql-vscode/src/interface-types.ts @@ -1,10 +1,5 @@ import * as sarif from 'sarif'; -import { - ResolvableLocationValue, - ColumnSchema, - ResultSetSchema, -} from './bqrs-types'; -import { ResultRow, ParsedResultSets, RawResultSet } from './adapt'; +import { RawResultSet, ResultRow, ResultSetSchema, Column, ResolvableLocationValue } from './bqrs-cli-types'; /** * This module contains types and code that are shared between @@ -243,7 +238,7 @@ export interface SetComparisonsMessage { time: string; }; }; - readonly columns: readonly ColumnSchema[]; + readonly columns: readonly Column[]; readonly commonResultSetNames: string[]; readonly currentResultSetName: string; readonly rows: QueryCompareResult | undefined; @@ -293,3 +288,12 @@ export function getDefaultResultSetName( resultSetNames[0], ].filter((resultSetName) => resultSetNames.includes(resultSetName))[0]; } + +export interface ParsedResultSets { + pageNumber: number; + numPages: number; + numInterpretedPages: number; + selectedTable?: string; // when undefined, means 'show default table' + resultSetNames: string[]; + resultSet: ResultSet; +} diff --git a/extensions/ql-vscode/src/interface-utils.ts b/extensions/ql-vscode/src/interface-utils.ts index c5273563b..ef02aff9b 100644 --- a/extensions/ql-vscode/src/interface-utils.ts +++ b/extensions/ql-vscode/src/interface-utils.ts @@ -12,19 +12,14 @@ import { TextEditorRevealType, ThemeColor, } from 'vscode'; -import { - FivePartLocation, - LocationStyle, - LocationValue, - WholeFileLocation, - ResolvableLocationValue, -} from './bqrs-types'; import { tryGetResolvableLocation, + isLineColumnLoc } from './bqrs-utils'; import { DatabaseItem, DatabaseManager } from './databases'; import { ViewSourceFileMsg } from './interface-types'; import { Logger } from './logging'; +import { LineColumnLocation, WholeFileLocation, UrlValue, ResolvableLocationValue } from './bqrs-cli-types'; /** * This module contains functions and types that are sharedd between @@ -61,19 +56,19 @@ export function fileUriToWebviewUri( * @param databaseItem Database in which to resolve the file location. */ function resolveFivePartLocation( - loc: FivePartLocation, + loc: LineColumnLocation, databaseItem: DatabaseItem ): Location { // `Range` is a half-open interval, and is zero-based. CodeQL locations are closed intervals, and // are one-based. Adjust accordingly. const range = new Range( - Math.max(0, loc.lineStart - 1), - Math.max(0, loc.colStart - 1), - Math.max(0, loc.lineEnd - 1), - Math.max(0, loc.colEnd) + Math.max(0, loc.startLine - 1), + Math.max(0, loc.startColumn - 1), + Math.max(0, loc.endLine - 1), + Math.max(0, loc.endColumn) ); - return new Location(databaseItem.resolveSourceFile(loc.file), range); + return new Location(databaseItem.resolveSourceFile(loc.uri), range); } /** @@ -87,7 +82,7 @@ function resolveWholeFileLocation( ): Location { // A location corresponding to the start of the file. const range = new Range(0, 0, 0, 0); - return new Location(databaseItem.resolveSourceFile(loc.file), range); + return new Location(databaseItem.resolveSourceFile(loc.uri), range); } /** @@ -97,20 +92,16 @@ function resolveWholeFileLocation( * @param databaseItem Database in which to resolve the file location. */ export function tryResolveLocation( - loc: LocationValue | undefined, + loc: UrlValue | undefined, databaseItem: DatabaseItem ): Location | undefined { const resolvableLoc = tryGetResolvableLocation(loc); - if (resolvableLoc === undefined) { - return undefined; - } - switch (resolvableLoc.t) { - case LocationStyle.FivePart: - return resolveFivePartLocation(resolvableLoc, databaseItem); - case LocationStyle.WholeFile: - return resolveWholeFileLocation(resolvableLoc, databaseItem); - default: - return undefined; + if (!resolvableLoc || typeof resolvableLoc === 'string') { + return; + } else if (isLineColumnLoc(resolvableLoc)) { + return resolveFivePartLocation(resolvableLoc, databaseItem); + } else { + return resolveWholeFileLocation(resolvableLoc, databaseItem); } } diff --git a/extensions/ql-vscode/src/interface.ts b/extensions/ql-vscode/src/interface.ts index 288308ba5..18b03ad9f 100644 --- a/extensions/ql-vscode/src/interface.ts +++ b/extensions/ql-vscode/src/interface.ts @@ -36,12 +36,6 @@ import * as messages from './messages'; import { CompletedQuery, interpretResults } from './query-results'; import { QueryInfo, tmpDir } from './run-queries'; import { parseSarifLocation, parseSarifPlainTextMessage } from './sarif-utils'; -import { - adaptSchema, - adaptBqrs, - ParsedResultSets, - RawResultSet, -} from './adapt'; import { WebviewReveal, fileUriToWebviewUri, @@ -51,8 +45,8 @@ import { shownLocationLineDecoration, jumpToLocation, } from './interface-utils'; -import { getDefaultResultSetName } from './interface-types'; -import { ResultSetSchema } from './bqrs-cli-types'; +import { getDefaultResultSetName, ParsedResultSets } from './interface-types'; +import { RawResultSet, adaptBqrs, ResultSetSchema } from './bqrs-cli-types'; /** * interface.ts @@ -94,7 +88,7 @@ function sortInterpretedResults( } function numPagesOfResultSet(resultSet: RawResultSet): number { - return Math.ceil(resultSet.schema.tupleCount / RAW_RESULTS_PAGE_SIZE); + return Math.ceil(resultSet.schema.rows / RAW_RESULTS_PAGE_SIZE); } function numInterpretedPages(interpretation: Interpretation | undefined): number { @@ -378,8 +372,7 @@ export class InterfaceManager extends DisposableObject { pageSize: RAW_RESULTS_PAGE_SIZE } ); - const adaptedSchema = adaptSchema(schema); - const resultSet = adaptBqrs(adaptedSchema, chunk); + const resultSet = adaptBqrs(schema, chunk); return { pageNumber: 0, numPages: numPagesOfResultSet(resultSet), @@ -492,8 +485,7 @@ export class InterfaceManager extends DisposableObject { pageSize: RAW_RESULTS_PAGE_SIZE } ); - const adaptedSchema = adaptSchema(schema); - const resultSet = adaptBqrs(adaptedSchema, chunk); + const resultSet = adaptBqrs(schema, chunk); const parsedResultSets: ParsedResultSets = { pageNumber, @@ -684,7 +676,7 @@ export class InterfaceManager extends DisposableObject { result.locations[0], sourceLocationPrefix ); - if (sarifLoc.t == 'NoLocation') { + if ('hint' in sarifLoc) { continue; } const resultLocation = tryResolveLocation(sarifLoc, databaseItem); @@ -709,7 +701,7 @@ export class InterfaceManager extends DisposableObject { relatedLocationsById[section.dest], sourceLocationPrefix ); - if (sarifChunkLoc.t == 'NoLocation') { + if ('hint' in sarifChunkLoc) { continue; } const referenceLocation = tryResolveLocation( diff --git a/extensions/ql-vscode/src/sarif-utils.ts b/extensions/ql-vscode/src/sarif-utils.ts index a39d570d4..4a8467da4 100644 --- a/extensions/ql-vscode/src/sarif-utils.ts +++ b/extensions/ql-vscode/src/sarif-utils.ts @@ -1,21 +1,23 @@ import * as Sarif from 'sarif'; import * as path from 'path'; -import { LocationStyle, ResolvableLocationValue } from './bqrs-types'; +import { ResolvableLocationValue } from './bqrs-cli-types'; export interface SarifLink { dest: number; text: string; } +interface NoLocation { + hint: string; +} type ParsedSarifLocation = - | ResolvableLocationValue - // Resolvable locations have a `file` field, but it will sometimes include + | (ResolvableLocationValue & { userVisibleFile: string }) + // Resolvable locations have a `uri` field, but it will sometimes include // a source location prefix, which contains build-specific information the user // doesn't really need to see. We ensure that `userVisibleFile` will not contain // that, and is appropriate for display in the UI. - & { userVisibleFile: string } - | { t: 'NoLocation'; hint: string }; + | NoLocation; export type SarifMessageComponent = string | SarifLink @@ -57,68 +59,78 @@ export function parseSarifPlainTextMessage(message: string): SarifMessageCompone * @returns A string that is valid for the `.file` field of a `FivePartLocation`: * directory separators are normalized, but drive letters `C:` may appear. */ -export function getPathRelativeToSourceLocationPrefix(sourceLocationPrefix: string, sarifRelativeUui: string) { +export function getPathRelativeToSourceLocationPrefix( + sourceLocationPrefix: string, + sarifRelativeUui: string +) { const normalizedSourceLocationPrefix = sourceLocationPrefix.replace(/\\/g, '/'); - return path.join(normalizedSourceLocationPrefix, decodeURIComponent(sarifRelativeUui)); + return `file:${ + path.join(normalizedSourceLocationPrefix, decodeURIComponent(sarifRelativeUui)) + }`; } -export function parseSarifLocation(loc: Sarif.Location, sourceLocationPrefix: string): ParsedSarifLocation { +export function parseSarifLocation( + loc: Sarif.Location, + sourceLocationPrefix: string +): ParsedSarifLocation { const physicalLocation = loc.physicalLocation; if (physicalLocation === undefined) - return { t: 'NoLocation', hint: 'no physical location' }; + return { hint: 'no physical location' }; if (physicalLocation.artifactLocation === undefined) - return { t: 'NoLocation', hint: 'no artifact location' }; + return { hint: 'no artifact location' }; if (physicalLocation.artifactLocation.uri === undefined) - return { t: 'NoLocation', hint: 'artifact location has no uri' }; + return { hint: 'artifact location has no uri' }; // This is not necessarily really an absolute uri; it could either be a // file uri or a relative uri. const uri = physicalLocation.artifactLocation.uri; + // FIXME: This is probably wrong const fileUriRegex = /^file:/; - const effectiveLocation = uri.match(fileUriRegex) ? - decodeURIComponent(uri.replace(fileUriRegex, '')) : - getPathRelativeToSourceLocationPrefix(sourceLocationPrefix, uri); - const userVisibleFile = uri.match(fileUriRegex) ? - decodeURIComponent(uri.replace(fileUriRegex, '')) : - uri; + const effectiveLocation = uri.match(fileUriRegex) + ? uri + : getPathRelativeToSourceLocationPrefix(sourceLocationPrefix, uri); + const userVisibleFile = uri.match(fileUriRegex) + ? decodeURIComponent(uri.replace(fileUriRegex, '')) + : uri; if (physicalLocation.region === undefined) { // If the region property is absent, the physicalLocation object refers to the entire file. // Source: https://docs.oasis-open.org/sarif/sarif/v2.1.0/cs01/sarif-v2.1.0-cs01.html#_Toc16012638. - // TODO: Do we get here if we provide a non-filesystem URL? return { - t: LocationStyle.WholeFile, - file: effectiveLocation, - userVisibleFile, - }; + uri: effectiveLocation, + userVisibleFile + } as ParsedSarifLocation; } else { const region = physicalLocation.region; // We assume that the SARIF we're given always has startLine // This is not mandated by the SARIF spec, but should be true of // SARIF output by our own tools. - const lineStart = region.startLine!; + const startLine = region.startLine!; // These defaults are from SARIF 2.1.0 spec, section 3.30.2, "Text Regions" // https://docs.oasis-open.org/sarif/sarif/v2.1.0/cs01/sarif-v2.1.0-cs01.html#_Ref493492556 - const lineEnd = region.endLine === undefined ? lineStart : region.endLine; - const colStart = region.startColumn === undefined ? 1 : region.startColumn; + const endLine = region.endLine === undefined ? startLine : region.endLine; + const startColumn = region.startColumn === undefined ? 1 : region.startColumn; // We also assume that our tools will always supply `endColumn` field, which is // fortunate, since the SARIF spec says that it defaults to the end of the line, whose // length we don't know at this point in the code. // // It is off by one with respect to the way vscode counts columns in selections. - const colEnd = region.endColumn! - 1; + const endColumn = region.endColumn! - 1; return { - t: LocationStyle.FivePart, - file: effectiveLocation, + uri: effectiveLocation, userVisibleFile, - lineStart, - colStart, - lineEnd, - colEnd, + startLine, + startColumn, + endLine, + endColumn, }; } } + +export function isNoLocation(loc: ParsedSarifLocation): loc is NoLocation { + return 'hint' in loc; +} diff --git a/extensions/ql-vscode/src/view/RawTableHeader.tsx b/extensions/ql-vscode/src/view/RawTableHeader.tsx index ac187561b..885d5e37e 100644 --- a/extensions/ql-vscode/src/view/RawTableHeader.tsx +++ b/extensions/ql-vscode/src/view/RawTableHeader.tsx @@ -3,10 +3,10 @@ import * as React from 'react'; import { vscode } from './vscode-api'; import { RawResultsSortState, SortDirection } from '../interface-types'; import { nextSortDirection } from './result-table-utils'; -import { ColumnSchema } from '../bqrs-types'; +import { Column } from '../bqrs-cli-types'; interface Props { - readonly columns: readonly ColumnSchema[]; + readonly columns: readonly Column[]; readonly schemaName: string; readonly sortState?: RawResultsSortState; readonly preventSort?: boolean; @@ -31,9 +31,9 @@ function toggleSortStateForColumn( nextDirection === undefined ? undefined : { - columnIndex: index, - sortDirection: nextDirection, - }; + columnIndex: index, + sortDirection: nextDirection, + }; vscode.postMessage({ t: 'changeSort', resultSetName: schemaName, @@ -46,7 +46,7 @@ export default function RawTableHeader(props: Props) { {[ - ( + ( # diff --git a/extensions/ql-vscode/src/view/RawTableRow.tsx b/extensions/ql-vscode/src/view/RawTableRow.tsx index 24f4f8442..cea36c474 100644 --- a/extensions/ql-vscode/src/view/RawTableRow.tsx +++ b/extensions/ql-vscode/src/view/RawTableRow.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { ResultRow } from '../adapt'; +import { ResultRow } from '../bqrs-cli-types'; import { zebraStripe } from './result-table-utils'; import RawTableValue from './RawTableValue'; @@ -23,6 +23,6 @@ export default function RawTableRow(props: Props) { /> ))} - + ); } diff --git a/extensions/ql-vscode/src/view/RawTableValue.tsx b/extensions/ql-vscode/src/view/RawTableValue.tsx index 8f45982eb..b4ce7b341 100644 --- a/extensions/ql-vscode/src/view/RawTableValue.tsx +++ b/extensions/ql-vscode/src/view/RawTableValue.tsx @@ -1,22 +1,30 @@ import * as React from 'react'; -import { ResultValue } from '../adapt'; import { renderLocation } from './result-table-utils'; +import { ColumnValue } from '../bqrs-cli-types'; +import { isStringLoc, isWholeFileLoc, isLineColumnLoc } from '../bqrs-utils'; interface Props { - value: ResultValue; + value: ColumnValue; databaseUri: string; } export default function RawTableValue(props: Props): JSX.Element { const v = props.value; - if (typeof v === 'string') { + if (typeof v === 'string' + || typeof v === 'number' + || typeof v === 'boolean') { return {v}; } - else if ('uri' in v) { - return {v.uri}; - } - else { - return renderLocation(v.location, v.label, props.databaseUri); + + const loc = v.url; + if (!loc) { + return ; + } else if (isStringLoc(loc)) { + return {loc}; + } else if (isWholeFileLoc(loc) || isLineColumnLoc(loc)) { + return renderLocation(loc, v.label, props.databaseUri); + } else { + return ; } } diff --git a/extensions/ql-vscode/src/view/alert-table.tsx b/extensions/ql-vscode/src/view/alert-table.tsx index 1c59a84de..32cf6a348 100644 --- a/extensions/ql-vscode/src/view/alert-table.tsx +++ b/extensions/ql-vscode/src/view/alert-table.tsx @@ -2,14 +2,14 @@ import * as path from 'path'; import * as React from 'react'; import * as Sarif from 'sarif'; import * as Keys from '../result-keys'; -import { LocationStyle } from '../bqrs-types'; import * as octicons from './octicons'; import { className, renderLocation, ResultTableProps, zebraStripe, selectableZebraStripe, jumpToLocation, nextSortDirection } from './result-table-utils'; import { onNavigation, NavigationEvent } from './results'; import { PathTableResultSet } from '../interface-types'; -import { parseSarifPlainTextMessage, parseSarifLocation } from '../sarif-utils'; +import { parseSarifPlainTextMessage, parseSarifLocation, isNoLocation } from '../sarif-utils'; import { InterpretedResultsSortColumn, SortDirection, InterpretedResultsSortState } from '../interface-types'; import { vscode } from './vscode-api'; +import { isWholeFileLoc, isLineColumnLoc } from '../bqrs-utils'; export type PathTableProps = ResultTableProps & { resultSet: PathTableResultSet }; export interface PathTableState { @@ -98,20 +98,18 @@ export class PathTable extends React.Component { relatedLocationsById[loc.id!] = loc; } - const result: JSX.Element[] = []; // match things like `[link-text](related-location-id)` const parts = parseSarifPlainTextMessage(msg); - - for (const part of parts) { + return parts.map((part, i) => { if (typeof part === 'string') { - result.push({part} ); + return {part}; } else { const renderedLocation = renderSarifLocationWithText(part.text, relatedLocationsById[part.dest], undefined); - result.push({renderedLocation} ); + return {renderedLocation}; } - } return result; + }); } function renderNonLocation(msg: string | undefined, locationHint: string): JSX.Element | undefined { @@ -131,14 +129,13 @@ export class PathTable extends React.Component { function renderSarifLocationWithText(text: string | undefined, loc: Sarif.Location, pathNodeKey: Keys.PathNode | undefined): JSX.Element | undefined { const parsedLoc = parseSarifLocation(loc, sourceLocationPrefix); - switch (parsedLoc.t) { - case 'NoLocation': - return renderNonLocation(text, parsedLoc.hint); - case LocationStyle.FivePart: - case LocationStyle.WholeFile: - return renderLocation(parsedLoc, text, databaseUri, undefined, updateSelectionCallback(pathNodeKey)); + if ('hint' in parsedLoc) { + return renderNonLocation(text, parsedLoc.hint); + } else if (isWholeFileLoc(parsedLoc) || isLineColumnLoc(parsedLoc)) { + return renderLocation(parsedLoc, text, databaseUri, undefined, updateSelectionCallback(pathNodeKey)); + } else { + return undefined; } - return undefined; } /** @@ -147,18 +144,18 @@ export class PathTable extends React.Component { */ function renderSarifLocation(loc: Sarif.Location, pathNodeKey: Keys.PathNode | undefined): JSX.Element | undefined { const parsedLoc = parseSarifLocation(loc, sourceLocationPrefix); - let shortLocation, longLocation: string; - switch (parsedLoc.t) { - case 'NoLocation': - return renderNonLocation('[no location]', parsedLoc.hint); - case LocationStyle.WholeFile: - shortLocation = `${path.basename(parsedLoc.userVisibleFile)}`; - longLocation = `${parsedLoc.userVisibleFile}`; - return renderLocation(parsedLoc, shortLocation, databaseUri, longLocation, updateSelectionCallback(pathNodeKey)); - case LocationStyle.FivePart: - shortLocation = `${path.basename(parsedLoc.userVisibleFile)}:${parsedLoc.lineStart}:${parsedLoc.colStart}`; - longLocation = `${parsedLoc.userVisibleFile}`; - return renderLocation(parsedLoc, shortLocation, databaseUri, longLocation, updateSelectionCallback(pathNodeKey)); + if ('hint' in parsedLoc) { + return renderNonLocation('[no location]', parsedLoc.hint); + } else if (isWholeFileLoc(parsedLoc)) { + const shortLocation = `${path.basename(parsedLoc.userVisibleFile)}`; + const longLocation = `${parsedLoc.userVisibleFile}`; + return renderLocation(parsedLoc, shortLocation, databaseUri, longLocation, updateSelectionCallback(pathNodeKey)); + } else if (isLineColumnLoc(parsedLoc)) { + const shortLocation = `${path.basename(parsedLoc.userVisibleFile)}:${parsedLoc.startLine}:${parsedLoc.startColumn}`; + const longLocation = `${parsedLoc.userVisibleFile}`; + return renderLocation(parsedLoc, shortLocation, databaseUri, longLocation, updateSelectionCallback(pathNodeKey)); + } else { + return undefined; } } @@ -290,10 +287,14 @@ export class PathTable extends React.Component { if (nextIndex < 0 || nextIndex >= path.locations.length) return prevState; const sarifLoc = path.locations[nextIndex].location; - if (sarifLoc === undefined) return prevState; + if (sarifLoc === undefined) { + return prevState; + } const loc = parseSarifLocation(sarifLoc, this.props.resultSet.sourceLocationPrefix); - if (loc.t === 'NoLocation') return prevState; + if (isNoLocation(loc)) { + return prevState; + } jumpToLocation(loc, this.props.databaseUri); const newSelection = { ...selectedPathNode, pathNodeIndex: nextIndex }; diff --git a/extensions/ql-vscode/src/view/raw-results-table.tsx b/extensions/ql-vscode/src/view/raw-results-table.tsx index 27b22092d..d2cce964e 100644 --- a/extensions/ql-vscode/src/view/raw-results-table.tsx +++ b/extensions/ql-vscode/src/view/raw-results-table.tsx @@ -4,7 +4,7 @@ import { RAW_RESULTS_LIMIT, RawResultsSortState } from '../interface-types'; import { RawTableResultSet } from '../interface-types'; import RawTableHeader from './RawTableHeader'; import RawTableRow from './RawTableRow'; -import { ResultRow } from '../adapt'; +import { ResultRow } from '../bqrs-cli-types'; export type RawTableProps = ResultTableProps & { resultSet: RawTableResultSet; diff --git a/extensions/ql-vscode/src/view/result-table-utils.tsx b/extensions/ql-vscode/src/view/result-table-utils.tsx index 023cac670..d92e0a5f1 100644 --- a/extensions/ql-vscode/src/view/result-table-utils.tsx +++ b/extensions/ql-vscode/src/view/result-table-utils.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { LocationValue, ResolvableLocationValue } from '../bqrs-types'; +import { UrlValue, ResolvableLocationValue } from '../bqrs-cli-types'; import { tryGetResolvableLocation } from '../bqrs-utils'; import { RawResultsSortState, QueryMetadata, SortDirection } from '../interface-types'; import { assertNever } from '../helpers-pure'; @@ -60,7 +60,7 @@ export function jumpToLocation(loc: ResolvableLocationValue, databaseUri: string /** * Render a location as a link which when clicked displays the original location. */ -export function renderLocation(loc: LocationValue | undefined, label: string | undefined, +export function renderLocation(loc: UrlValue | undefined, label: string | undefined, databaseUri: string, title?: string, callback?: () => void): JSX.Element { // If the label was empty, use a placeholder instead, so the link is still clickable. diff --git a/extensions/ql-vscode/src/view/result-tables.tsx b/extensions/ql-vscode/src/view/result-tables.tsx index 8422a56d7..23daa0276 100644 --- a/extensions/ql-vscode/src/view/result-tables.tsx +++ b/extensions/ql-vscode/src/view/result-tables.tsx @@ -11,11 +11,11 @@ import { ALERTS_TABLE_NAME, SELECT_TABLE_NAME, getDefaultResultSetName, + ParsedResultSets } from '../interface-types'; import { PathTable } from './alert-table'; import { RawTable } from './raw-results-table'; import { ResultTableProps, tableSelectionHeaderClassName, toggleDiagnosticsClassName, alertExtrasClassName } from './result-table-utils'; -import { ParsedResultSets } from '../adapt'; import { vscode } from './vscode-api'; @@ -48,7 +48,7 @@ const UPDATING_RESULTS_TEXT_CLASS_NAME = 'vscode-codeql__result-tables-updating- function getResultCount(resultSet: ResultSet): number { switch (resultSet.t) { case 'RawResultSet': - return resultSet.schema.tupleCount; + return resultSet.schema.rows; case 'SarifResultSet': return resultSet.numTotalResults; } @@ -81,7 +81,11 @@ export class ResultTables // unused stubs because a SarifResultSet schema isn't used the // same way as a RawResultSet. Probably should pull `name` field // out. - schema: { name: ALERTS_TABLE_NAME, version: 0, columns: [], tupleCount: 1 }, + schema: { + name: ALERTS_TABLE_NAME, + rows: 1, + columns: [] + }, name: ALERTS_TABLE_NAME, ...this.props.interpretation, }); diff --git a/extensions/ql-vscode/src/view/results.tsx b/extensions/ql-vscode/src/view/results.tsx index 0c2b85c3a..bd53eef07 100644 --- a/extensions/ql-vscode/src/view/results.tsx +++ b/extensions/ql-vscode/src/view/results.tsx @@ -11,12 +11,10 @@ import { QueryMetadata, ResultsPaths, ALERTS_TABLE_NAME, + ParsedResultSets, } from '../interface-types'; import { EventHandlers as EventHandlerList } from './event-handler-list'; import { ResultTables } from './result-tables'; -import { - ParsedResultSets, -} from '../adapt'; import { ResultSet } from '../interface-types'; import { vscode } from './vscode-api'; @@ -113,7 +111,11 @@ class App extends React.Component<{}, ResultsViewState> { resultSet: { t: 'SarifResultSet', name: ALERTS_TABLE_NAME, - schema: { name: ALERTS_TABLE_NAME, version: 0, columns: [], tupleCount: 1 }, + schema: { + name: ALERTS_TABLE_NAME, + rows: 1, + columns: [] + }, ...msg.interpretation, }, selectedTable: ALERTS_TABLE_NAME, @@ -174,7 +176,10 @@ class App extends React.Component<{}, ResultsViewState> { resultsInfo: ResultsInfo ): Promise { const parsedResultSets = resultsInfo.parsedResultSets; - return [{ t: 'RawResultSet', ...parsedResultSets.resultSet }]; + return [{ + ...parsedResultSets.resultSet, + t: (parsedResultSets.resultSet.t ?? 'RawResultSet') as any + }]; } private async loadResults(): Promise { diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/interface-utils.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/interface-utils.test.ts index de4b58d45..fa9ca3a45 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/interface-utils.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/interface-utils.test.ts @@ -9,7 +9,6 @@ import { tryResolveLocation, } from '../../interface-utils'; import { getDefaultResultSetName } from '../../interface-types'; -import { LocationStyle } from '../../bqrs-types'; import { DatabaseItem } from '../../databases'; describe('interface-utils', () => { @@ -73,20 +72,17 @@ describe('interface-utils', () => { describe('resolveWholeFileLocation', () => { it('should resolve a whole file location', () => { const mockDatabaseItem: DatabaseItem = ({ - resolveSourceFile: sinon.stub().returns(vscode.Uri.parse('abc')), + resolveSourceFile: sinon.stub().returns(vscode.Uri.file('abc')), } as unknown) as DatabaseItem; expect( tryResolveLocation( - { - t: LocationStyle.WholeFile, - file: 'hucairz', - }, + 'file://hucairz:1:1:1:1', mockDatabaseItem ) ).to.deep.equal( new vscode.Location( - vscode.Uri.parse('abc'), - new vscode.Range(0, 0, 0, 0) + vscode.Uri.file('abc'), + new vscode.Range(0, 0, 0, 1) ) ); }); @@ -99,12 +95,11 @@ describe('interface-utils', () => { expect( tryResolveLocation( { - t: LocationStyle.FivePart, - colStart: 1, - colEnd: 3, - lineStart: 4, - lineEnd: 5, - file: 'hucairz', + startColumn: 1, + endColumn: 3, + startLine: 4, + endLine: 5, + uri: 'hucairz', }, mockDatabaseItem ) @@ -127,12 +122,11 @@ describe('interface-utils', () => { expect( tryResolveLocation( { - t: LocationStyle.FivePart, - colStart: 1, - colEnd: 3, - lineStart: 4, - lineEnd: 5, - file: '', + startColumn: 1, + endColumn: 3, + startLine: 4, + endLine: 5, + uri: '', }, mockDatabaseItem ) @@ -146,10 +140,7 @@ describe('interface-utils', () => { expect( tryResolveLocation( - { - t: LocationStyle.String, - loc: 'file://hucairz:0:0:0:0' - }, + 'file://hucairz:0:0:0:0', mockDatabaseItem ) ).to.deep.equal( @@ -170,10 +161,7 @@ describe('interface-utils', () => { expect( tryResolveLocation( - { - t: LocationStyle.String, - loc: 'file://hucairz:5:4:3:2' - }, + 'file://hucairz:5:4:3:2', mockDatabaseItem ) ).to.deep.equal( @@ -194,10 +182,7 @@ describe('interface-utils', () => { expect( tryResolveLocation( - { - t: LocationStyle.String, - loc: 'file://hucairz:x:y:z:a' - }, + 'file://hucairz:x:y:z:a', mockDatabaseItem ) ).to.be.undefined; diff --git a/extensions/ql-vscode/test/pure-tests/location.test.ts b/extensions/ql-vscode/test/pure-tests/location.test.ts index aecf3352e..992eb9d02 100644 --- a/extensions/ql-vscode/test/pure-tests/location.test.ts +++ b/extensions/ql-vscode/test/pure-tests/location.test.ts @@ -1,46 +1,33 @@ import { expect } from 'chai'; import 'mocha'; -import { LocationStyle, StringLocation } from '../../src/bqrs-types'; import { tryGetResolvableLocation } from '../../src/bqrs-utils'; -describe('processing string locations', function () { - it('should detect Windows whole-file locations', function () { - const loc: StringLocation = { - t: LocationStyle.String, - loc: 'file://C:/path/to/file.ext:0:0:0:0' - }; +describe('processing string locations', function() { + it('should detect Windows whole-file locations', function() { + const loc = 'file://C:/path/to/file.ext:0:0:0:0'; const wholeFileLoc = tryGetResolvableLocation(loc); - expect(wholeFileLoc).to.eql({t: LocationStyle.WholeFile, file: 'C:/path/to/file.ext'}); + expect(wholeFileLoc).to.eql({ uri: 'C:/path/to/file.ext' }); }); - it('should detect Unix whole-file locations', function () { - const loc: StringLocation = { - t: LocationStyle.String, - loc: 'file:///path/to/file.ext:0:0:0:0' - }; + it('should detect Unix whole-file locations', function() { + const loc = 'file:///path/to/file.ext:0:0:0:0'; const wholeFileLoc = tryGetResolvableLocation(loc); - expect(wholeFileLoc).to.eql({t: LocationStyle.WholeFile, file: '/path/to/file.ext'}); + expect(wholeFileLoc).to.eql({ uri: '/path/to/file.ext' }); }); - it('should detect Unix 5-part locations', function () { - const loc: StringLocation = { - t: LocationStyle.String, - loc: 'file:///path/to/file.ext:1:2:3:4' - }; + + it('should detect Unix 5-part locations', function() { + const loc = 'file:///path/to/file.ext:1:2:3:4'; const wholeFileLoc = tryGetResolvableLocation(loc); expect(wholeFileLoc).to.eql({ - t: LocationStyle.FivePart, - file: '/path/to/file.ext', - lineStart: 1, - colStart: 2, - lineEnd: 3, - colEnd: 4 + uri: '/path/to/file.ext', + startLine: 1, + startColumn: 2, + endLine: 3, + endColumn: 4 }); }); - it('should ignore other string locations', function () { + it('should ignore other string locations', function() { for (const loc of ['file:///path/to/file.ext', 'I am not a location']) { - const wholeFileLoc = tryGetResolvableLocation({ - t: LocationStyle.String, - loc: loc - }); + const wholeFileLoc = tryGetResolvableLocation(loc); expect(wholeFileLoc).to.be.undefined; } });