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

Fix Spurious Compilation Errors #3845

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import { telemetryListener } from "../../common/vscode/telemetry";
import type { LanguageContextStore } from "../../language-context-store";
import type { DatabaseOrigin } from "./database-origin";
import { ensureZippedSourceLocation } from "./database-contents";
import type { LanguageClient } from "vscode-languageclient/node";

/**
* The name of the key in the workspaceState dictionary in which we
Expand Down Expand Up @@ -118,6 +119,7 @@ export class DatabaseManager extends DisposableObject {
private readonly app: App,
private readonly qs: QueryRunner,
private readonly cli: CodeQLCliServer,
private readonly langClient: LanguageClient,
private readonly languageContext: LanguageContextStore,
public logger: Logger,
) {
Expand Down Expand Up @@ -407,6 +409,7 @@ export class DatabaseManager extends DisposableObject {
const qlPackGenerator = new QlPackGenerator(
databaseItem.language,
this.cli,
this.langClient,
qlpackStoragePath,
qlpackStoragePath,
);
Expand Down
8 changes: 5 additions & 3 deletions extensions/ql-vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -792,12 +792,16 @@ async function activateWithInstalledDistribution(
const languageSelectionPanel = new LanguageSelectionPanel(languageContext);
ctx.subscriptions.push(languageSelectionPanel);

void extLogger.log("Initializing CodeQL language server.");
const languageClient = createLanguageClient(qlConfigurationListener);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this changes the initialization order of things because we now inject the language client into sub-components. It seems to work fine for me locally.


void extLogger.log("Initializing database manager.");
const dbm = new DatabaseManager(
ctx,
app,
qs,
cliServer,
languageClient,
languageContext,
extLogger,
);
Expand Down Expand Up @@ -961,16 +965,14 @@ async function activateWithInstalledDistribution(

ctx.subscriptions.push(tmpDirDisposal);

void extLogger.log("Initializing CodeQL language server.");
const languageClient = createLanguageClient(qlConfigurationListener);

const localQueries = new LocalQueries(
app,
qs,
qhm,
dbm,
databaseFetcher,
cliServer,
languageClient,
databaseUI,
localQueryResultsView,
queryStorageDir,
Expand Down
3 changes: 3 additions & 0 deletions extensions/ql-vscode/src/local-queries/local-queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import { tryGetQueryLanguage } from "../common/query-language";
import type { LanguageContextStore } from "../language-context-store";
import type { ExtensionApp } from "../common/vscode/extension-app";
import type { DatabaseFetcher } from "../databases/database-fetcher";
import type { LanguageClient } from "vscode-languageclient/node";

export enum QuickEvalType {
None,
Expand All @@ -72,6 +73,7 @@ export class LocalQueries extends DisposableObject {
private readonly databaseManager: DatabaseManager,
private readonly databaseFetcher: DatabaseFetcher,
private readonly cliServer: CodeQLCliServer,
private readonly langClient: LanguageClient,
private readonly databaseUI: DatabaseUI,
private readonly localQueryResultsView: ResultsView,
private readonly queryStorageDir: string,
Expand Down Expand Up @@ -324,6 +326,7 @@ export class LocalQueries extends DisposableObject {
const language = this.languageContextStore.selectedLanguage;
const skeletonQueryWizard = new SkeletonQueryWizard(
this.cliServer,
this.langClient,
progress,
this.app,
this.databaseManager,
Expand Down
25 changes: 25 additions & 0 deletions extensions/ql-vscode/src/local-queries/qlpack-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ import type { CodeQLCliServer } from "../codeql-cli/cli";
import type { QueryLanguage } from "../common/query-language";
import { getOnDiskWorkspaceFolders } from "../common/vscode/workspace-folders";
import { basename } from "../common/path";
import {
DidChangeWatchedFilesNotification,
FileChangeType,
} from "vscode-languageclient";
import type { LanguageClient } from "vscode-languageclient/node";

export class QlPackGenerator {
private qlpackName: string | undefined;
Expand All @@ -17,6 +22,7 @@ export class QlPackGenerator {
constructor(
private readonly queryLanguage: QueryLanguage,
private readonly cliServer: CodeQLCliServer,
private readonly langClient: LanguageClient,
private readonly storagePath: string,
private readonly queryStoragePath: string,
private readonly includeFolderNameInQlpackName: boolean = false,
Expand Down Expand Up @@ -114,5 +120,24 @@ select f, "Hello, world!"

private async createCodeqlPackLockYaml() {
await this.cliServer.packAdd(this.folderUri.fsPath, this.queryLanguage);
// when the language pack has not been available locally before, the
// packAdd command will download it. This will trigger a pack change that
// the language server needs to be notified of:
await this.notifyPackChanged(this.folderUri.fsPath, this.langClient);
}

private async notifyPackChanged(
packFileDir: string,
ideServer: LanguageClient,
) {
const packFilePath = join(packFileDir, this.qlpackFileName);
await ideServer.sendNotification(DidChangeWatchedFilesNotification.type, {
changes: [
{
type: FileChangeType.Changed,
uri: Uri.file(packFilePath).toString(),
},
],
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import type { QueryTreeViewItem } from "../queries-panel/query-tree-view-item";
import { containsPath, pathsEqual } from "../common/files";
import { getQlPackFilePath } from "../common/ql";
import { getQlPackLanguage } from "../common/qlpack-language";
import type { LanguageClient } from "vscode-languageclient/node";

type QueryLanguagesToDatabaseMap = Record<string, string>;

Expand All @@ -56,6 +57,7 @@ export class SkeletonQueryWizard {

constructor(
private readonly cliServer: CodeQLCliServer,
private readonly langClient: LanguageClient,
private readonly progress: ProgressCallback,
private readonly app: App,
private readonly databaseManager: DatabaseManager,
Expand Down Expand Up @@ -443,6 +445,7 @@ export class SkeletonQueryWizard {
return new QlPackGenerator(
this.language,
this.cliServer,
this.langClient,
this.qlPackStoragePath,
this.queryStoragePath,
includeFolderNameInQlpackName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ import {
createQueryTreeFolderItem,
} from "../../../../src/queries-panel/query-tree-view-item";
import { dump } from "js-yaml";
import type { LanguageClient } from "vscode-languageclient/node";

describe("SkeletonQueryWizard", () => {
let mockCli: CodeQLCliServer;
let mockLanguageClient: LanguageClient;
let mockApp: App;
let wizard: SkeletonQueryWizard;
let mockDatabaseManager: DatabaseManager;
Expand Down Expand Up @@ -88,6 +90,11 @@ describe("SkeletonQueryWizard", () => {
]),
resolveQlpacks: resolveQlpacksMock,
});

mockLanguageClient = mockedObject<LanguageClient>({
sendNotification: jest.fn().mockResolvedValue([]),
});

mockApp = createMockApp();

mockDatabaseManager = mockedObject<DatabaseManager>({
Expand Down Expand Up @@ -142,6 +149,7 @@ describe("SkeletonQueryWizard", () => {

wizard = new SkeletonQueryWizard(
mockCli,
mockLanguageClient,
jest.fn(),
mockApp,
mockDatabaseManager,
Expand All @@ -165,6 +173,7 @@ describe("SkeletonQueryWizard", () => {
beforeEach(() => {
wizard = new SkeletonQueryWizard(
mockCli,
mockLanguageClient,
jest.fn(),
mockApp,
mockDatabaseManager,
Expand Down Expand Up @@ -313,6 +322,7 @@ describe("SkeletonQueryWizard", () => {

wizard = new SkeletonQueryWizard(
mockCli,
mockLanguageClient,
jest.fn(),
mockApp,
mockDatabaseManagerWithItems,
Expand Down Expand Up @@ -362,6 +372,7 @@ describe("SkeletonQueryWizard", () => {

wizard = new SkeletonQueryWizard(
mockCli,
mockLanguageClient,
jest.fn(),
mockApp,
mockDatabaseManagerWithItems,
Expand Down Expand Up @@ -466,6 +477,7 @@ describe("SkeletonQueryWizard", () => {

wizard = new SkeletonQueryWizard(
mockCli,
mockLanguageClient,
jest.fn(),
mockApp,
mockDatabaseManager,
Expand Down Expand Up @@ -687,6 +699,7 @@ describe("SkeletonQueryWizard", () => {

wizard = new SkeletonQueryWizard(
mockCli,
mockLanguageClient,
jest.fn(),
mockApp,
mockDatabaseManager,
Expand Down Expand Up @@ -716,6 +729,7 @@ describe("SkeletonQueryWizard", () => {

wizard = new SkeletonQueryWizard(
mockCli,
mockLanguageClient,
jest.fn(),
mockApp,
mockDatabaseManager,
Expand Down Expand Up @@ -749,6 +763,7 @@ describe("SkeletonQueryWizard", () => {

wizard = new SkeletonQueryWizard(
mockCli,
mockLanguageClient,
jest.fn(),
mockApp,
mockDatabaseManager,
Expand Down Expand Up @@ -792,6 +807,7 @@ describe("SkeletonQueryWizard", () => {

wizard = new SkeletonQueryWizard(
mockCli,
mockLanguageClient,
jest.fn(),
mockApp,
mockDatabaseManager,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
} from "../../../factories/databases/databases";
import { findSourceArchive } from "../../../../src/databases/local-databases/database-resolver";
import { LanguageContextStore } from "../../../../src/language-context-store";
import type { LanguageClient } from "vscode-languageclient/node";

describe("local databases", () => {
let databaseManager: DatabaseManager;
Expand Down Expand Up @@ -109,6 +110,7 @@ describe("local databases", () => {
resolveDatabase: resolveDatabaseSpy,
packAdd: packAddSpy,
}),
mockedObject<LanguageClient>({}),
new LanguageContextStore(mockApp),
mockedObject<Logger>({
log: logSpy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { mockedObject } from "../utils/mocking.helpers";
import { ensureDir, readFile } from "fs-extra";
import { load } from "js-yaml";
import type { QlPackFile } from "../../../src/packaging/qlpack-file";
import type { LanguageClient } from "vscode-languageclient/node";

describe("QlPackGenerator", () => {
let packFolderPath: string;
Expand All @@ -23,6 +24,7 @@ describe("QlPackGenerator", () => {
typeof CodeQLCliServer.prototype.resolveQlpacks
>;
let mockCli: CodeQLCliServer;
let mockLangClient: LanguageClient;
let dir: DirResult;

beforeEach(async () => {
Expand All @@ -45,9 +47,14 @@ describe("QlPackGenerator", () => {
resolveQlpacks: resolveQlpacksSpy,
});

mockLangClient = mockedObject<LanguageClient>({
sendNotification: jest.fn().mockResolvedValue([]),
});

generator = new QlPackGenerator(
language as QueryLanguage,
mockCli,
mockLangClient,
packFolderPath,
packFolderPath,
);
Expand Down Expand Up @@ -131,6 +138,7 @@ describe("QlPackGenerator", () => {
generator = new QlPackGenerator(
language as QueryLanguage,
mockCli,
mockLangClient,
packFolderPath,
packFolderPath,
true,
Expand Down Expand Up @@ -165,6 +173,7 @@ describe("QlPackGenerator", () => {
generator = new QlPackGenerator(
language as QueryLanguage,
mockCli,
mockLangClient,
packFolderPath,
packFolderPath,
true,
Expand Down Expand Up @@ -200,6 +209,7 @@ describe("QlPackGenerator", () => {
generator = new QlPackGenerator(
language as QueryLanguage,
mockCli,
mockLangClient,
packFolderPath,
packFolderPath,
true,
Expand Down
Loading