Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit more relevant error message when failing to add source folder #1021

Merged
merged 3 commits into from
Nov 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions extensions/ql-vscode/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## [UNRELEASED]

- Emit a more explicit error message when a user tries to add a database with an unzipped source folder to the workspace. [1021](https://github.com/github/vscode-codeql/pull/1021)

## 1.5.7 - 23 November 2021

- Fix the _CodeQL: Open Referenced File_ command for Windows systems. [#979](https://github.com/github/vscode-codeql/pull/979)
Expand Down
66 changes: 43 additions & 23 deletions extensions/ql-vscode/src/databases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ export interface DatabaseItem {
* Returns the root uri of the virtual filesystem for this database's source archive,
* as displayed in the filesystem explorer.
*/
getSourceArchiveExplorerUri(): vscode.Uri | undefined;
getSourceArchiveExplorerUri(): vscode.Uri;

/**
* Holds if `uri` belongs to this database's source archive.
Expand All @@ -274,6 +274,11 @@ export interface DatabaseItem {
* Gets the state of this database, to be persisted in the workspace state.
*/
getPersistedState(): PersistedDatabaseItem;

/**
* Verifies that this database item has a zipped source folder. Returns an error message if it does not.
*/
verifyZippedSources(): string | undefined;
}

export enum DatabaseEventKind {
Expand Down Expand Up @@ -459,13 +464,26 @@ export class DatabaseItemImpl implements DatabaseItem {
/**
* Returns the root uri of the virtual filesystem for this database's source archive.
*/
public getSourceArchiveExplorerUri(): vscode.Uri | undefined {
public getSourceArchiveExplorerUri(): vscode.Uri {
const sourceArchive = this.sourceArchive;
if (sourceArchive === undefined || !sourceArchive.fsPath.endsWith('.zip'))
return undefined;
if (sourceArchive === undefined || !sourceArchive.fsPath.endsWith('.zip')) {
throw new Error(this.verifyZippedSources());
}
return encodeArchiveBasePath(sourceArchive.fsPath);
}

public verifyZippedSources(): string | undefined {
const sourceArchive = this.sourceArchive;
if (sourceArchive === undefined) {
return `${this.name} has no source archive.`;
}

if (!sourceArchive.fsPath.endsWith('.zip')) {
return `${this.name} has a source folder that is unzipped.`;
}
return;
}

/**
* Holds if `uri` belongs to this database's source archive.
*/
Expand Down Expand Up @@ -603,26 +621,28 @@ export class DatabaseManager extends DisposableObject {
// This is undesirable, as we might be adding and removing many
// workspace folders as the user adds and removes databases.
const end = (vscode.workspace.workspaceFolders || []).length;
const uri = item.getSourceArchiveExplorerUri();
if (uri === undefined) {
void logger.log(`Couldn't obtain file explorer uri for ${item.name}`);

const msg = item.verifyZippedSources();
if (msg) {
void logger.log(`Could not add source folder because ${msg}`);
return;
}
else {
void logger.log(`Adding workspace folder for ${item.name} source archive at index ${end}`);
if ((vscode.workspace.workspaceFolders || []).length < 2) {
// Adding this workspace folder makes the workspace
// multi-root, which may surprise the user. Let them know
// we're doing this.
void vscode.window.showInformationMessage(`Adding workspace folder for source archive of database ${item.name}.`);
}
vscode.workspace.updateWorkspaceFolders(end, 0, {
name: `[${item.name} source archive]`,
uri,
});
// vscode api documentation says we must to wait for this event
// between multiple `updateWorkspaceFolders` calls.
await eventFired(vscode.workspace.onDidChangeWorkspaceFolders);

const uri = item.getSourceArchiveExplorerUri();
void logger.log(`Adding workspace folder for ${item.name} source archive at index ${end}`);
if ((vscode.workspace.workspaceFolders || []).length < 2) {
// Adding this workspace folder makes the workspace
// multi-root, which may surprise the user. Let them know
// we're doing this.
void vscode.window.showInformationMessage(`Adding workspace folder for source archive of database ${item.name}.`);
}
vscode.workspace.updateWorkspaceFolders(end, 0, {
name: `[${item.name} source archive]`,
uri,
});
// vscode api documentation says we must to wait for this event
// between multiple `updateWorkspaceFolders` calls.
await eventFired(vscode.workspace.onDidChangeWorkspaceFolders);
}
}

Expand Down Expand Up @@ -732,7 +752,7 @@ export class DatabaseManager extends DisposableObject {
this.updatePersistedCurrentDatabaseItem();

await vscode.commands.executeCommand('setContext', 'codeQL.currentDatabaseItem', item?.name);

this._onDidChangeCurrentDatabaseItem.fire({
item,
kind: DatabaseEventKind.Change
Expand Down