From fb49d1d0a8fc39dde68d5ffd1f8e7f7aa7a31ac3 Mon Sep 17 00:00:00 2001 From: Joop van de Pol Date: Tue, 1 Oct 2024 16:27:19 +0200 Subject: [PATCH] Fix opening github link --- src/codeMarker.ts | 80 +++++++++++++++++++++++++++++------------------ src/types.ts | 11 ++----- 2 files changed, 53 insertions(+), 38 deletions(-) diff --git a/src/codeMarker.ts b/src/codeMarker.ts index 21405e4..27bc8db 100644 --- a/src/codeMarker.ts +++ b/src/codeMarker.ts @@ -18,11 +18,10 @@ import { FullPath, Location, FullLocation, - LocationEntry, FullLocationEntry, - isOldLocationEntry, isLocationEntry, isEntry, + isOldEntry, Repository, PathOrganizerEntry, createDefaultEntryDetails, @@ -1789,38 +1788,58 @@ export class CodeMarker implements vscode.TreeDataProvider { this.workspaces.setupRepositories(); }); - vscode.commands.registerCommand("weAudit.openGithubIssue", (entry: Entry | LocationEntry) => { - const actualEntry: Entry = isOldLocationEntry(entry) ? entry.parentEntry : entry; + /** + * Open a github issue. Warning: this command is used by Sarif Explorer and should at least accept Entry types. + * Sarif explorer will always provide absolute paths as location paths, so it should be possible to find the corresponding workspace root. + * */ + vscode.commands.registerCommand("weAudit.openGithubIssue", (entry: Entry | FullEntry | FullLocationEntry) => { + let actualEntry: FullEntry; + if (isOldEntry(entry)) { + // This is the Sarif Explorer case. Location paths are absolute paths. - // First check that all locations are inside one of the workspace roots: - for (const loc of actualEntry.locations) { - const [wsRoot, _relativePath] = this.workspaces.getCorrespondingRootAndPath(loc.path); - if (wsRoot === undefined) { - vscode.window.showErrorMessage(`Failed to open a GitHub issue. The file ${loc.path} is not in any workspace root.`); - return; + // First check that all locations are inside one of the workspace roots: + for (const loc of entry.locations) { + const [wsRoot, _relativePath] = this.workspaces.getCorrespondingRootAndPath(loc.path); + if (wsRoot === undefined) { + vscode.window.showErrorMessage(`Failed to open a GitHub issue. The file ${loc.path} is not in any workspace root.`); + return; + } + } + + actualEntry = { + label: entry.label, + entryType: entry.entryType, + author: entry.author, + details: entry.details, + locations: entry.locations.map((loc) => { + // transform absolute paths to relative paths to the workspace path + const [wsRoot, relativePath] = this.workspaces.getCorrespondingRootAndPath(loc.path); + return { + path: relativePath, + startLine: loc.startLine, + endLine: loc.endLine, + label: loc.label, + description: loc.description, + rootPath: wsRoot!.rootPath, // We checked this in the earlier for loop + } as FullLocation; + }), + } as FullEntry; + } else { + // This is the weAudit internal case, entries are either FullEntry or FullLocationEntry + actualEntry = isLocationEntry(entry) ? entry.parentEntry : entry; + + // First check that all locations are inside one of the workspace roots: + for (const loc of actualEntry.locations) { + const fullPath = path.join(loc.rootPath, loc.path); + const [wsRoot, _relativePath] = this.workspaces.getCorrespondingRootAndPath(loc.rootPath); + if (wsRoot === undefined) { + vscode.window.showErrorMessage(`Failed to open a GitHub issue. The file ${fullPath} is not in any workspace root.`); + return; + } } } - const actualFullEntry = { - label: actualEntry.label, - entryType: actualEntry.entryType, - author: actualEntry.author, - details: actualEntry.details, - locations: actualEntry.locations.map((loc) => { - // transform absolute paths to relative paths to the workspace path - const [wsRoot, relativePath] = this.workspaces.getCorrespondingRootAndPath(loc.path); - return { - path: relativePath, - startLine: loc.startLine, - endLine: loc.endLine, - label: loc.label, - description: loc.description, - rootPath: wsRoot!.rootPath, // We checked this in the earlier for loop - } as FullLocation; - }), - } as FullEntry; - - this.openGithubIssue(actualFullEntry); + this.openGithubIssue(actualEntry); }); // This command takes a configuration file, toggles its current selection, and shows/hides the corresponding findings @@ -1898,6 +1917,7 @@ export class CodeMarker implements vscode.TreeDataProvider { this.showMarkedFilesDayLog(); }); + // This command is only used by Sarif Explorer, which will provide a location with an absolute path vscode.commands.registerCommand("weAudit.getClientPermalink", (location: Location) => { const [wsRoot, relativePath] = this.workspaces.getCorrespondingRootAndPath(location.path); if (wsRoot === undefined) { diff --git a/src/types.ts b/src/types.ts index 2a369ae..0691a31 100644 --- a/src/types.ts +++ b/src/types.ts @@ -524,15 +524,10 @@ export function isEntry(treeEntry: TreeEntry): treeEntry is FullEntry { } /** - * TreeEntry union type. - * This was used to represent the tree entries in the finding tree. - * It is maintained for backwards compatibility with sarif-explorer. - * - Entry: a finding or a note, are used when there is a single location - * - LocationEntry: are used to represent additional locations + * Type predicate for backwards compatibility purposes */ -export type OldTreeEntry = Entry | LocationEntry; -export function isOldLocationEntry(treeEntry: OldTreeEntry): treeEntry is LocationEntry { - return (treeEntry as LocationEntry).parentEntry !== undefined; +export function isOldEntry(entry: Entry | FullEntry | FullLocationEntry): entry is Entry { + return (entry as FullEntry).locations[0]?.rootPath === undefined && (entry as FullLocationEntry).location.rootPath === undefined; } export interface ConfigurationEntry {