Skip to content

Commit cbc6b73

Browse files
authored
Wire up processing model evaluation run results and showing alerts (#3503)
1 parent 5c43018 commit cbc6b73

7 files changed

Lines changed: 224 additions & 7 deletions

File tree

extensions/ql-vscode/src/common/interface-types.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,11 @@ interface SetModelAlertsViewStateMessage {
732732
viewState: ModelAlertsViewState;
733733
}
734734

735+
interface SetReposResultsMessage {
736+
t: "setReposResults";
737+
reposResults: VariantAnalysisScannedRepositoryResult[];
738+
}
739+
735740
interface OpenModelPackMessage {
736741
t: "openModelPack";
737742
path: string;
@@ -749,7 +754,8 @@ interface StopEvaluationRunMessage {
749754
export type ToModelAlertsMessage =
750755
| SetModelAlertsViewStateMessage
751756
| SetVariantAnalysisMessage
752-
| SetRepoResultsMessage;
757+
| SetRepoResultsMessage
758+
| SetReposResultsMessage;
753759

754760
export type FromModelAlertsMessage =
755761
| CommonFromViewMessages
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import type { AnalysisAlert } from "../../variant-analysis/shared/analysis-result";
2+
import type { ModeledMethod } from "../modeled-method";
3+
import { EndpointType } from "../method";
4+
import type { ModelAlerts } from "./model-alerts";
5+
6+
/**
7+
* Calculate which model has contributed to each alert.
8+
* @param alerts The alerts to process.
9+
* @returns The alerts grouped by modeled method.
10+
*/
11+
export function calculateModelAlerts(alerts: AnalysisAlert[]): ModelAlerts[] {
12+
// Temporary logging to use alerts variable.
13+
console.log(`Processing ${alerts.length} alerts`);
14+
15+
// For now we just return some mock data, but once we have provenance information
16+
// we'll be able to calculate this properly based on the alerts that are passed in
17+
// and potentially some other information.
18+
return [
19+
{
20+
model: createModeledMethod(),
21+
alerts: [createMockAlert()],
22+
},
23+
];
24+
}
25+
26+
function createModeledMethod(): ModeledMethod {
27+
return {
28+
libraryVersion: "1.6.0",
29+
signature: "org.sql2o.Connection#createQuery(String)",
30+
endpointType: EndpointType.Method,
31+
packageName: "org.sql2o",
32+
typeName: "Connection",
33+
methodName: "createQuery",
34+
methodParameters: "(String)",
35+
type: "sink",
36+
input: "Argument[0]",
37+
kind: "path-injection",
38+
provenance: "manual",
39+
};
40+
}
41+
42+
function createMockAlert(): AnalysisAlert {
43+
return {
44+
message: {
45+
tokens: [
46+
{
47+
t: "text",
48+
text: "This is an empty block.",
49+
},
50+
],
51+
},
52+
shortDescription: "This is an empty block.",
53+
fileLink: {
54+
fileLinkPrefix:
55+
"https://github.com/expressjs/express/blob/33e8dc303af9277f8a7e4f46abfdcb5e72f6797b",
56+
filePath: "test/app.options.js",
57+
},
58+
severity: "Warning",
59+
codeSnippet: {
60+
startLine: 10,
61+
endLine: 14,
62+
text: " app.del('/', function(){});\n app.get('/users', function(req, res){});\n app.put('/users', function(req, res){});\n\n request(app)\n",
63+
},
64+
highlightedRegion: {
65+
startLine: 12,
66+
startColumn: 41,
67+
endLine: 12,
68+
endColumn: 43,
69+
},
70+
codeFlows: [],
71+
};
72+
}

extensions/ql-vscode/src/model-editor/model-alerts/model-alerts-view.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,15 @@ export class ModelAlertsView extends AbstractWebview<
4848
this.onEvaluationRunStopClickedEventEmitter.event;
4949
}
5050

51-
public async showView() {
51+
public async showView(
52+
reposResults: VariantAnalysisScannedRepositoryResult[],
53+
) {
5254
const panel = await this.getPanel();
5355
panel.reveal(undefined, true);
5456

5557
await this.waitForPanelLoaded();
5658
await this.setViewState();
59+
await this.updateReposResults(reposResults);
5760
}
5861

5962
protected async getPanelConfig(): Promise<WebviewPanelConfig> {
@@ -139,6 +142,19 @@ export class ModelAlertsView extends AbstractWebview<
139142
});
140143
}
141144

145+
public async updateReposResults(
146+
reposResults: VariantAnalysisScannedRepositoryResult[],
147+
): Promise<void> {
148+
if (!this.isShowingPanel) {
149+
return;
150+
}
151+
152+
await this.postMessage({
153+
t: "setReposResults",
154+
reposResults,
155+
});
156+
}
157+
142158
public async focusView(): Promise<void> {
143159
this.panel?.reveal();
144160
}

extensions/ql-vscode/src/model-editor/model-evaluator.ts

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
UserCancellationException,
1414
withProgress,
1515
} from "../common/vscode/progress";
16+
import { VariantAnalysisScannedRepositoryDownloadStatus } from "../variant-analysis/shared/variant-analysis";
1617
import type { VariantAnalysis } from "../variant-analysis/shared/variant-analysis";
1718
import type { CancellationToken } from "vscode";
1819
import { CancellationTokenSource } from "vscode";
@@ -126,7 +127,6 @@ export class ModelEvaluator extends DisposableObject {
126127
this.dbItem,
127128
this.extensionPack,
128129
);
129-
await this.modelAlertsView.showView();
130130

131131
this.modelAlertsView.onEvaluationRunStopClicked(async () => {
132132
await this.stopEvaluation();
@@ -148,6 +148,12 @@ export class ModelEvaluator extends DisposableObject {
148148
throw new Error("No variant analysis available");
149149
}
150150

151+
const reposResults =
152+
this.variantAnalysisManager.getLoadedResultsForVariantAnalysis(
153+
variantAnalysis.id,
154+
);
155+
await this.modelAlertsView.showView(reposResults);
156+
151157
await this.modelAlertsView.updateVariantAnalysis(variantAnalysis);
152158
}
153159
}
@@ -260,6 +266,21 @@ export class ModelEvaluator extends DisposableObject {
260266
),
261267
);
262268

269+
this.push(
270+
this.variantAnalysisManager.onRepoStatesUpdated(async (e) => {
271+
if (
272+
e.variantAnalysisId === variantAnalysisId &&
273+
e.repoState.downloadStatus ===
274+
VariantAnalysisScannedRepositoryDownloadStatus.Succeeded
275+
) {
276+
await this.readAnalysisResults(
277+
variantAnalysisId,
278+
e.repoState.repositoryId,
279+
);
280+
}
281+
}),
282+
);
283+
263284
this.push(
264285
this.variantAnalysisManager.onRepoResultsLoaded(async (e) => {
265286
if (e.variantAnalysisId === variantAnalysisId) {
@@ -268,4 +289,39 @@ export class ModelEvaluator extends DisposableObject {
268289
}),
269290
);
270291
}
292+
293+
private async readAnalysisResults(
294+
variantAnalysisId: number,
295+
repositoryId: number,
296+
) {
297+
const variantAnalysis =
298+
this.variantAnalysisManager.tryGetVariantAnalysis(variantAnalysisId);
299+
if (!variantAnalysis) {
300+
void this.app.logger.log(
301+
`Could not find variant analysis with id ${variantAnalysisId}`,
302+
);
303+
throw new Error(
304+
"There was an error when trying to retrieve variant analysis information",
305+
);
306+
}
307+
308+
const repository = variantAnalysis.scannedRepos?.find(
309+
(r) => r.repository.id === repositoryId,
310+
);
311+
if (!repository) {
312+
void this.app.logger.log(
313+
`Could not find repository with id ${repositoryId} in scanned repos`,
314+
);
315+
throw new Error(
316+
"There was an error when trying to retrieve repository information",
317+
);
318+
}
319+
320+
// Trigger loading the results for the repository. This will trigger a
321+
// onRepoResultsLoaded event that we'll process.
322+
await this.variantAnalysisManager.loadResults(
323+
variantAnalysisId,
324+
repository.repository.fullName,
325+
);
326+
}
271327
}

extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,17 @@ export class VariantAnalysisManager
635635
);
636636
}
637637

638+
public getLoadedResultsForVariantAnalysis(variantAnalysisId: number) {
639+
const variantAnalysis = this.variantAnalyses.get(variantAnalysisId);
640+
if (!variantAnalysis) {
641+
throw new Error(`No variant analysis with id: ${variantAnalysisId}`);
642+
}
643+
644+
return this.variantAnalysisResultsManager.getLoadedResultsForVariantAnalysis(
645+
variantAnalysis,
646+
);
647+
}
648+
638649
private async variantAnalysisRecordExists(
639650
variantAnalysisId: number,
640651
): Promise<boolean> {

extensions/ql-vscode/src/variant-analysis/variant-analysis-results-manager.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { sarifParser } from "../common/sarif-parser";
1212
import { extractAnalysisAlerts } from "./sarif-processing";
1313
import type { CodeQLCliServer } from "../codeql-cli/cli";
1414
import { extractRawResults } from "./bqrs-processing";
15+
import { VariantAnalysisRepoStatus } from "./shared/variant-analysis";
1516
import type {
1617
VariantAnalysis,
1718
VariantAnalysisRepositoryTask,
@@ -305,6 +306,28 @@ export class VariantAnalysisResultsManager extends DisposableObject {
305306
}
306307
}
307308

309+
public getLoadedResultsForVariantAnalysis(
310+
variantAnalysis: VariantAnalysis,
311+
): VariantAnalysisScannedRepositoryResult[] {
312+
const scannedRepos = variantAnalysis.scannedRepos?.filter(
313+
(r) => r.analysisStatus === VariantAnalysisRepoStatus.Succeeded,
314+
);
315+
316+
if (!scannedRepos) {
317+
return [];
318+
}
319+
320+
return scannedRepos
321+
.map((scannedRepo) =>
322+
this.cachedResults.get(
323+
createCacheKey(variantAnalysis.id, scannedRepo.repository.fullName),
324+
),
325+
)
326+
.filter(
327+
(r): r is VariantAnalysisScannedRepositoryResult => r !== undefined,
328+
);
329+
}
330+
308331
public dispose(disposeHandler?: DisposeHandler) {
309332
super.dispose(disposeHandler);
310333

extensions/ql-vscode/src/view/model-alerts/ModelAlerts.tsx

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,30 @@
1-
import { useCallback, useEffect, useState } from "react";
1+
import { useCallback, useEffect, useMemo, useState } from "react";
2+
import { styled } from "styled-components";
3+
import { ModelAlertsHeader } from "./ModelAlertsHeader";
24
import type { ModelAlertsViewState } from "../../model-editor/shared/view-state";
35
import type { ToModelAlertsMessage } from "../../common/interface-types";
46
import type {
57
VariantAnalysis,
68
VariantAnalysisScannedRepositoryResult,
79
} from "../../variant-analysis/shared/variant-analysis";
810
import { vscode } from "../vscode-api";
9-
import { ModelAlertsHeader } from "./ModelAlertsHeader";
11+
import { ModelAlertsResults } from "./ModelAlertsResults";
12+
import type { ModelAlerts } from "../../model-editor/model-alerts/model-alerts";
13+
import { calculateModelAlerts } from "../../model-editor/model-alerts/alert-processor";
1014

1115
type Props = {
1216
initialViewState?: ModelAlertsViewState;
1317
variantAnalysis?: VariantAnalysis;
1418
repoResults?: VariantAnalysisScannedRepositoryResult[];
1519
};
1620

21+
const SectionTitle = styled.h3`
22+
font-size: medium;
23+
font-weight: 500;
24+
margin: 0;
25+
padding-bottom: 10px;
26+
`;
27+
1728
export function ModelAlerts({
1829
initialViewState,
1930
variantAnalysis: initialVariantAnalysis,
@@ -55,6 +66,10 @@ export function ModelAlerts({
5566
setVariantAnalysis(msg.variantAnalysis);
5667
break;
5768
}
69+
case "setReposResults": {
70+
setRepoResults(msg.reposResults);
71+
break;
72+
}
5873
case "setRepoResults": {
5974
setRepoResults((oldRepoResults) => {
6075
const newRepoIds = msg.repoResults.map((r) => r.repositoryId);
@@ -81,6 +96,16 @@ export function ModelAlerts({
8196
};
8297
}, []);
8398

99+
const modelAlerts = useMemo(() => {
100+
if (!repoResults) {
101+
return [];
102+
}
103+
104+
const alerts = repoResults.flatMap((a) => a.interpretedResults ?? []);
105+
106+
return calculateModelAlerts(alerts);
107+
}, [repoResults]);
108+
84109
if (viewState === undefined || variantAnalysis === undefined) {
85110
return <></>;
86111
}
@@ -105,8 +130,16 @@ export function ModelAlerts({
105130
stopRunClick={onStopRunClick}
106131
></ModelAlertsHeader>
107132
<div>
108-
<h3>Repo results</h3>
109-
<p>{JSON.stringify(repoResults, null, 2)}</p>
133+
<SectionTitle>Model alerts</SectionTitle>
134+
<div>
135+
{modelAlerts.map((alerts, i) => (
136+
// We're using the index as the key here which is not recommended.
137+
// but we don't have a unique identifier for models. In the future,
138+
// we may need to consider coming up with unique identifiers for models
139+
// and using those as keys.
140+
<ModelAlertsResults key={i} modelAlerts={alerts} />
141+
))}
142+
</div>
110143
</div>
111144
</>
112145
);

0 commit comments

Comments
 (0)