Skip to content

Make error messages clearer for some common problems #702

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.

Sign up for GitHub

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

Merged
aeisenberg merged 1 commit into from
Dec 9, 2020
Merged

Make error messages clearer for some common problems #702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
- Fix bug when removing databases where sometimes the source folder would not be removed from the workspace or the database files would not be removed from the workspace storage location. [#692](https://github.com/github/vscode-codeql/pull/692)
- Query results with no string representation will now be displayed with placeholder text in query results. Previously, they were omitted. [#694](https://github.com/github/vscode-codeql/pull/694)
- Add a label for the language of a database in the databases view. This will only take effect for new databases created with the CodeQL CLI v2.4.1 or later. [#697](https://github.com/github/vscode-codeql/pull/697)
- Add clearer error message when running a query using a missing or invalid qlpack. [#702](https://github.com/github/vscode-codeql/pull/702)
- Add clearer error message when trying to run a command from the query history view if no item in the history is selected. [#702](https://github.com/github/vscode-codeql/pull/702)

## 1.3.7 - 24 November 2020

Expand Down
2 changes: 1 addition & 1 deletion extensions/ql-vscode/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@
},
{
"command": "codeQLQueryHistory.openQuery",
"title": "Open Query",
"title": "Open the query that produced these results",
"icon": {
"light": "media/light/edit.svg",
"dark": "media/dark/edit.svg"
Expand Down
14 changes: 14 additions & 0 deletions extensions/ql-vscode/src/query-history.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ class HistoryTreeDataProvider extends DisposableObject implements QueryHistoryDa
*/
const DOUBLE_CLICK_TIME = 500;

const NO_QUERY_SELECTED = 'No query selected. Select a query history item you have already run and try again.';
export class QueryHistoryManager extends DisposableObject {
treeDataProvider: HistoryTreeDataProvider;
treeView: vscode.TreeView<CompletedQuery>;
Expand Down Expand Up @@ -310,6 +311,10 @@ export class QueryHistoryManager extends DisposableObject {
return;
}

if (!finalSingleItem) {
throw new Error(NO_QUERY_SELECTED);
}

const textDocument = await vscode.workspace.openTextDocument(
vscode.Uri.file(finalSingleItem.query.program.queryPath)
);
Expand Down Expand Up @@ -398,6 +403,11 @@ export class QueryHistoryManager extends DisposableObject {
if (!this.assertSingleQuery(finalMultiSelect)) {
return;
}

if (!finalSingleItem) {
throw new Error(NO_QUERY_SELECTED);
}

this.treeDataProvider.setCurrentItem(finalSingleItem);

const now = new Date();
Expand Down Expand Up @@ -440,6 +450,10 @@ export class QueryHistoryManager extends DisposableObject {
return;
}

if (!singleItem) {
throw new Error(NO_QUERY_SELECTED);
}

const queryName = singleItem.queryName.endsWith('.ql')
? singleItem.queryName
: singleItem.queryName + '.ql';
Expand Down
6 changes: 5 additions & 1 deletion extensions/ql-vscode/src/run-queries.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ async function promptUserToSaveChanges(document: TextDocument): Promise<boolean>
else {
const yesItem = { title: 'Yes', isCloseAffordance: false };
const alwaysItem = { title: 'Always Save', isCloseAffordance: false };
const noItem = { title: 'No (run anyway)', isCloseAffordance: false };
const noItem = { title: 'No (run version on disk)', isCloseAffordance: false };
Copy link
Copy Markdown
Contributor

@adityasharad adityasharad Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does 'the last saved version on disk' make sense?

Copy link
Copy Markdown
Contributor Author

@aeisenberg aeisenberg Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would it ever not be the last saved version (unless you have a funky filesystem that automatically saves backup versions)?

Copy link
Copy Markdown
Contributor

@adityasharad adityasharad Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect never: what you've written is correct, I'm just wondering whether we can make the message even more obvious.

const cancelItem = { title: 'Cancel', isCloseAffordance: true };
const message = 'Query file has unsaved changes. Save now?';
const chosenItem = await window.showInformationMessage(
Expand Down Expand Up @@ -478,6 +478,10 @@ export async function compileAndRunQueryAgainstDatabase(
// Figure out the library path for the query.
const packConfig = await cliServer.resolveLibraryPath(diskWorkspaceFolders, queryPath);

if (!packConfig.dbscheme) {
throw new Error('Could not find a database scheme for this query. Please check that you have a valid qlpack.yml file for this query, which refers to a database scheme either in the `dbscheme` field or through one of its dependencies.');
}

// Check whether the query has an entirely different schema from the
// database. (Queries that merely need the database to be upgraded
// won't trigger this check)
Expand Down