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.

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

Jump to bottom

Autodetect what language a query is for #915

Merged
merged 3 commits into from
Aug 4, 2021
Merged

Autodetect what language a query is for #915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1aef089
Autodetect language using "resolve queries"
shati-patel Jul 26, 2021
83c68ec
Address review comments
shati-patel Aug 4, 2021
ad4ff84
Don't show empty list of DBs
shati-patel Aug 4, 2021
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
1 change: 1 addition & 0 deletions extensions/ql-vscode/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## [UNRELEASED]

- Add a command _CodeQL: Run Query on Multiple Databases_, which lets users select multiple databases to run a query on. [#898](https://github.com/github/vscode-codeql/pull/898)
- Autodetect what language a query targets. This refines the _CodeQL: Run Query on Multiple Databases_ command to only show relevant databases. [#915](https://github.com/github/vscode-codeql/pull/915)

## 1.5.2 - 13 July 2021

Expand Down
39 changes: 38 additions & 1 deletion extensions/ql-vscode/src/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { Readable } from 'stream';
import { StringDecoder } from 'string_decoder';
import * as tk from 'tree-kill';
import { promisify } from 'util';
import { CancellationToken, Disposable } from 'vscode';
import { CancellationToken, Disposable, Uri } from 'vscode';

import { BQRSInfo, DecodedBqrsChunk } from './pure/bqrs-cli-types';
import { CliConfig } from './config';
Expand Down Expand Up @@ -43,6 +43,16 @@ export interface QuerySetup {
compilationCache?: string;
}

/**
* The expected output of `codeql resolve queries --format bylanguage`.
*/
export interface QueryInfoByLanguage {
// Using `unknown` as a placeholder. For now, the value is only ever an empty object.
byLanguage: Record<string, Record<string, unknown>>;
noDeclaredLanguage: Record<string, unknown>;
multipleDeclaredLanguages: Record<string, unknown>;
}

/**
* The expected output of `codeql resolve database`.
*/
Expand Down Expand Up @@ -71,6 +81,11 @@ export interface UpgradesInfo {
*/
export type QlpacksInfo = { [name: string]: string[] };

/**
* The expected output of `codeql resolve languages`.
*/
export type LanguagesInfo = { [name: string]: string[] };

/**
* The expected output of `codeql resolve qlref`.
*/
Expand Down Expand Up @@ -482,6 +497,20 @@ export class CodeQLCliServer implements Disposable {
return await this.runJsonCodeQlCliCommand<QuerySetup>(['resolve', 'library-path'], subcommandArgs, 'Resolving library paths');
}

/**
* Resolves the language for a query.
* @param queryUri The URI of the query
*/
async resolveQueryByLanguage(workspaces: string[], queryUri: Uri): Promise<QueryInfoByLanguage> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you asked about tests:

  • extensions/ql-vscode/src/vscode-tests/cli-integration/run-cli.test.ts is a good place to add integration tests for the resolveQueryByLanguage and resolveLanguages functions.
  • You can use the existing tests as an example.
  • resolveLanguages is probably easier to write a test for because there's no query as input.
  • resolveQueryByLanguage will require a test query: there are some examples in the data folders, but as far as I can tell none of them depend on an actual language. So you may need to add one, or save this test for later.

const subcommandArgs = [
'--format', 'bylanguage',
queryUri.fsPath,
'--additional-packs',
workspaces.join(path.delimiter)
];
return JSON.parse(await this.runCodeQlCliCommand(['resolve', 'queries'], subcommandArgs, 'Resolving query by language'));
}

/**
* Finds all available QL tests in a given directory.
* @param testPath Root of directory tree to search for tests.
Expand Down Expand Up @@ -724,6 +753,14 @@ export class CodeQLCliServer implements Disposable {
);
}

/**
* Gets information about the available languages.
* @returns A dictionary mapping language name to the directory it comes from
*/
async resolveLanguages(): Promise<LanguagesInfo> {
return await this.runJsonCodeQlCliCommand<LanguagesInfo>(['resolve', 'languages'], [], 'Resolving languages');
}

/**
* Gets information about queries in a query suite.
* @param suite The suite to resolve.
Expand Down
22 changes: 18 additions & 4 deletions extensions/ql-vscode/src/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ import {
import { CodeQlStatusBarHandler } from './status-bar';

import { Credentials } from './authentication';
import runRemoteQuery from './run-remote-query';
import { runRemoteQuery, findLanguage } from './run-remote-query';

/**
* extension.ts
Expand Down Expand Up @@ -570,7 +570,21 @@ async function activateWithInstalledDistribution(
token: CancellationToken,
uri: Uri | undefined
) => {
const quickPickItems = dbm.databaseItems.map<DatabaseQuickPickItem>(dbItem => (
let filteredDBs = dbm.databaseItems;
if (filteredDBs.length === 0) {
void helpers.showAndLogErrorMessage('No databases found. Please add a suitable database to your workspace.');
return;
}
// If possible, only show databases with the right language (otherwise show all databases).
const queryLanguage = await findLanguage(cliServer, uri);
if (queryLanguage) {
filteredDBs = dbm.databaseItems.filter(db => db.language === queryLanguage);
if (filteredDBs.length === 0) {
void helpers.showAndLogErrorMessage(`No databases found for language ${queryLanguage}. Please add a suitable database to your workspace.`);
return;
}
}
const quickPickItems = filteredDBs.map<DatabaseQuickPickItem>(dbItem => (
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that if the user has no databases and doesn't pick a language when prompted, we try to display this prompt with nothing in it. There should probably be an extra check to display an error in this case and return, similarly to what happens with the filtered list above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Woops, good point! Thanks 😄

{
databaseItem: dbItem,
label: dbItem.name,
Expand All @@ -582,7 +596,7 @@ async function activateWithInstalledDistribution(
*/
const quickpick = await window.showQuickPick<DatabaseQuickPickItem>(
quickPickItems,
{ canPickMany: true }
{ canPickMany: true, ignoreFocusOut: true }
);
if (quickpick !== undefined) {
// Collect all skipped databases and display them at the end (instead of popping up individual errors)
Expand Down Expand Up @@ -707,7 +721,7 @@ async function activateWithInstalledDistribution(
) => {
if (isCanary()) {
const credentials = await Credentials.initialize(ctx);
await runRemoteQuery(credentials, uri || window.activeTextEditor?.document.uri);
await runRemoteQuery(cliServer, credentials, uri || window.activeTextEditor?.document.uri);
}
})
);
Expand Down
47 changes: 42 additions & 5 deletions extensions/ql-vscode/src/run-remote-query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,53 @@
import { Uri } from 'vscode';
import { Uri, window } from 'vscode';
import * as yaml from 'js-yaml';
import * as fs from 'fs-extra';
import { showAndLogErrorMessage, showAndLogInformationMessage } from './helpers';
import { getOnDiskWorkspaceFolders, showAndLogErrorMessage, showAndLogInformationMessage } from './helpers';
import { Credentials } from './authentication';
import * as cli from './cli';
import { logger } from './logging';

interface Config {
repositories: string[];
ref?: string;
language: string;
language?: string;
}

// Test "controller" repository and workflow.
const OWNER = 'dsp-testing';
const REPO = 'qc-controller';

export default async function runRemoteQuery(credentials: Credentials, uri?: Uri) {
/**
* Finds the language that a query targets.
* If it can't be autodetected, prompt the user to specify the language manually.
*/
export async function findLanguage(
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you asked about tests: I agree that having tests for this function would be handy, and give you confidence in its behaviour. However we don't yet have a test module for run-remote-query due to the experimental nature of the entire feature, so adding a test requires some upfront work. (In future it will be easier :) )

I'll record some pointers here about how you could create those tests, but I'm not expecting you to do it in this PR. We can look at test coverage together after this PR.

  • For integration tests (which use a real query and actually run the underlying commands), copy extensions/ql-vscode/src/vscode-tests/cli-integration/queries.test.ts as a template. I think it would make sense to have:
    • one test function that runs findLanguage on a query with a real language dependency and identifies the language
    • one test function that runs it on a dummy query and returns undefined
    • any other cases you can think of
  • For unit tests (which don't run the actual commands, but simulate the underlying behaviour), use extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts as a template. Those tests mock the behaviour of the CLI server.

cliServer: cli.CodeQLCliServer,
queryUri: Uri | undefined
): Promise<string | undefined> {
const uri = queryUri || window.activeTextEditor?.document.uri;
if (uri !== undefined) {
try {
const queryInfo = await cliServer.resolveQueryByLanguage(getOnDiskWorkspaceFolders(), uri);
const language = (Object.keys(queryInfo.byLanguage))[0];
void logger.log(`Detected query language: ${language}`);
return language;
} catch (e) {
void logger.log('Could not autodetect query language. Select language manually.');
}
}
const availableLanguages = Object.keys(await cliServer.resolveLanguages());
const language = await window.showQuickPick(
availableLanguages,
{ placeHolder: 'Select target language for your query', ignoreFocusOut: true }
);
if (!language) {
// This only happens if the user cancels the quick pick.
void showAndLogErrorMessage('Language not found. Language must be specified manually.');
shati-patel marked this conversation as resolved.
Show resolved Hide resolved
}
return language;
}

export async function runRemoteQuery(cliServer: cli.CodeQLCliServer, credentials: Credentials, uri?: Uri) {
if (!uri?.fsPath.endsWith('.ql')) {
return;
}
Expand All @@ -34,9 +67,13 @@ export default async function runRemoteQuery(credentials: Credentials, uri?: Uri
const config = yaml.safeLoad(await fs.readFile(repositoriesFile, 'utf8')) as Config;

const ref = config.ref || 'main';
const language = config.language;
const language = config.language || await findLanguage(cliServer, uri);
const repositories = config.repositories;

if (!language) {
return; // No error message needed, since `findlanguage` already displays one.
}

try {
await octokit.request(
'POST /repos/:owner/:repo/code-scanning/codeql/queries',
Expand Down