diff --git a/vscode/src/analysis/__tests__/mergeRuleSets.test.ts b/vscode/src/analysis/__tests__/mergeRuleSets.test.ts index de18bfe..f98f048 100644 --- a/vscode/src/analysis/__tests__/mergeRuleSets.test.ts +++ b/vscode/src/analysis/__tests__/mergeRuleSets.test.ts @@ -1,6 +1,6 @@ import { RuleSet } from "@editor-extensions/shared"; import { produce } from "immer"; -import { mergeRuleSets } from "../mergeRuleSets"; +import { mergeRuleSetsWithStringPaths as mergeRuleSets } from "../mergeRuleSets"; import expect from "expect"; import { FOO, BAR, DISCOVERY } from "./data"; @@ -74,3 +74,33 @@ describe("mergeRuleSets() adds new violations/incidents in the chosen files", () expect(foo.violations?.["foo-01"].incidents[0].message).toBe("new message"); }); }); + +describe("mergeRuleSets() workarounds", () => { + it("ignores files outside of included_paths received in analysis response", () => { + const responseWithUnknownFile = produce(FOO, (draft: RuleSet) => { + draft.violations!["foo-01"].incidents = [ + { + uri: "file:///src/UnknownFile.java", + message: "new message", + }, + ]; + }); + const merged: RuleSet[] = produce(BASE_STATE, (draft) => + mergeRuleSets(draft, [responseWithUnknownFile], SAVED_FILES), + ); + expect(merged).toHaveLength(3); + const [foo] = merged; + + expect(foo.violations?.["foo-01"].incidents).toHaveLength(0); + }); + it("takes the whole partial response if there is no base response in the state", () => { + const merged: RuleSet[] = produce([] as RuleSet[], (draft) => + mergeRuleSets(draft, PARTIAL_SINGLE_INCIDENT, SAVED_FILES), + ); + expect(merged).toHaveLength(1); + const [foo] = merged; + + expect(foo.violations?.["foo-01"].incidents).toHaveLength(1); + expect(foo.violations?.["foo-01"].incidents[0].message).toBe("new message"); + }); +}); diff --git a/vscode/src/analysis/mergeRuleSets.ts b/vscode/src/analysis/mergeRuleSets.ts index 1bc4744..2be2d89 100644 --- a/vscode/src/analysis/mergeRuleSets.ts +++ b/vscode/src/analysis/mergeRuleSets.ts @@ -1,10 +1,28 @@ import { RuleSet, Violation } from "@editor-extensions/shared"; - +import { Immutable } from "immer"; +import { Uri } from "vscode"; export const mergeRuleSets = ( + draft: RuleSet[], + received: RuleSet[], + fileUris: Uri[], +): RuleSet[] => { + // use the same path representation as in the response + // which is file:///some/path/File.java + const filePaths = fileUris.map((uri) => uri.toString()); + return mergeRuleSetsWithStringPaths(draft, received, filePaths); +}; + +export const mergeRuleSetsWithStringPaths = ( draft: RuleSet[], received: RuleSet[], filePaths: string[], ): RuleSet[] => { + if (draft.length === 0) { + // there were no full analysis yet or it's results were deleted + // nothing to merge - take the whole partial analysis response + draft.push(...received); + return draft; + } // remove old incidents in the files included in the partial analysis draft .flatMap((it) => [...Object.values(it.violations ?? {})]) @@ -21,7 +39,10 @@ export const mergeRuleSets = ( ]) .map(([name, violations]): [string, [string, Violation][]] => [ name, - violations.filter(([, violation]) => violation.incidents?.length), + // reject incidents outside of requested scope + violations.filter( + ([, violation]) => violation.incidents?.filter((it) => filePaths.includes(it.uri))?.length, + ), ]) .filter(([, violations]) => violations.length) // remaining violations contain incidents @@ -57,3 +78,9 @@ export const mergeRuleSets = ( return draft; }; + +export const countIncidentsOnPaths = (ruleSets: Immutable, filePaths: string[]) => + ruleSets + .flatMap((r) => Object.values(r.violations ?? {})) + .flatMap((v) => v?.incidents ?? []) + .filter((it) => filePaths.includes(it.uri)).length; diff --git a/vscode/src/analysis/runAnalysis.ts b/vscode/src/analysis/runAnalysis.ts index 18b0dec..d233e05 100644 --- a/vscode/src/analysis/runAnalysis.ts +++ b/vscode/src/analysis/runAnalysis.ts @@ -1,12 +1,14 @@ import { getConfigAnalyzeOnSave } from "../utilities"; import { ExtensionState } from "../extensionState"; -import { TextDocument, commands, window } from "vscode"; +import { TextDocument, Uri, commands, window } from "vscode"; export const partialAnalysisTrigger = (textDoc: TextDocument) => { - commands.executeCommand("konveyor.partialAnalysis", [textDoc.fileName]); + if (textDoc.uri.scheme === "file") { + commands.executeCommand("konveyor.partialAnalysis", [textDoc.uri]); + } }; -export const runPartialAnalysis = async (state: ExtensionState, filePaths: string[]) => { +export const runPartialAnalysis = async (state: ExtensionState, filePaths: Uri[]) => { if (!getConfigAnalyzeOnSave()) { return; } diff --git a/vscode/src/client/analyzerClient.ts b/vscode/src/client/analyzerClient.ts index 852566b..5f0eb2b 100644 --- a/vscode/src/client/analyzerClient.ts +++ b/vscode/src/client/analyzerClient.ts @@ -24,7 +24,11 @@ import { getConfigMaxIterations, getConfigMaxPriority, getConfigKaiDemoMode, + isAnalysisResponse, } from "../utilities"; +import { allIncidents } from "../issueView"; +import { Immutable } from "immer"; +import { countIncidentsOnPaths } from "../analysis"; export class AnalyzerClient { private kaiRpcServer: ChildProcessWithoutNullStreams | null = null; @@ -42,6 +46,7 @@ export class AnalyzerClient { constructor( private extContext: vscode.ExtensionContext, mutateExtensionData: (recipe: (draft: ExtensionData) => void) => void, + private getExtStateData: () => Immutable, ) { this.fireStateChange = (state: ServerState) => mutateExtensionData((draft) => { @@ -365,7 +370,7 @@ export class AnalyzerClient { * * Will only run if the sever state is: `running` */ - public async runAnalysis(filePaths?: string[]): Promise { + public async runAnalysis(filePaths?: vscode.Uri[]): Promise { // TODO: Ensure serverState is running if (!this.rpcConnection) { @@ -386,7 +391,7 @@ export class AnalyzerClient { const requestParams = { label_selector: getConfigLabelSelector(), - included_paths: filePaths, + included_paths: filePaths?.map((uri) => uri.fsPath), }; this.outputChannel.appendLine( @@ -407,7 +412,7 @@ export class AnalyzerClient { }); }); - const { response, isCancelled }: any = await Promise.race([ + const { response: rawResponse, isCancelled }: any = await Promise.race([ this.rpcConnection!.sendRequest("analysis_engine.Analyze", requestParams).then( (response) => ({ response }), ), @@ -420,17 +425,47 @@ export class AnalyzerClient { this.fireAnalysisStateChange(false); return; } - this.outputChannel.appendLine(`Response: ${JSON.stringify(response)}`); + const isResponseWellFormed = isAnalysisResponse(rawResponse?.Rulesets); + const ruleSets: RuleSet[] = isResponseWellFormed ? rawResponse?.Rulesets : []; + const summary = isResponseWellFormed + ? { + wellFormed: true, + rawIncidentCount: ruleSets + .flatMap((r) => Object.values(r.violations ?? {})) + .flatMap((v) => v.incidents ?? []).length, + incidentCount: allIncidents(ruleSets).length, + partialAnalysis: filePaths + ? { + incidentsBefore: countIncidentsOnPaths( + this.getExtStateData().ruleSets, + filePaths.map((uri) => uri.toString()), + ), + incidentsAfter: countIncidentsOnPaths( + ruleSets, + filePaths.map((uri) => uri.toString()), + ), + } + : {}, + } + : { wellFormed: false }; + + this.outputChannel.appendLine(`Response received. Summary: ${JSON.stringify(summary)}`); // Handle the result - const rulesets = response?.Rulesets as RuleSet[]; - if (!rulesets || rulesets.length === 0) { + if (!isResponseWellFormed) { + vscode.window.showErrorMessage( + "Analysis completed, but received results are not well formed.", + ); + this.fireAnalysisStateChange(false); + return; + } + if (ruleSets.length === 0) { vscode.window.showInformationMessage("Analysis completed, but no RuleSets were found."); this.fireAnalysisStateChange(false); return; } - vscode.commands.executeCommand("konveyor.loadRuleSets", rulesets, filePaths); + vscode.commands.executeCommand("konveyor.loadRuleSets", ruleSets, filePaths); progress.report({ message: "Results processed!" }); vscode.window.showInformationMessage("Analysis completed successfully!"); } catch (err: any) { diff --git a/vscode/src/commands.ts b/vscode/src/commands.ts index ee6602f..cd32620 100644 --- a/vscode/src/commands.ts +++ b/vscode/src/commands.ts @@ -371,7 +371,8 @@ const commandsMap: (state: ExtensionState) => { // Update the user settings await updateLabelSelector(modifiedLabelSelector); }, - "konveyor.loadRuleSets": async (ruleSets: RuleSet[]) => loadRuleSets(state, ruleSets), + "konveyor.loadRuleSets": async (ruleSets: RuleSet[], filePaths: Uri[]) => + loadRuleSets(state, ruleSets, filePaths), "konveyor.cleanRuleSets": () => cleanRuleSets(state), "konveyor.loadStaticResults": loadStaticResults, "konveyor.loadResultsFromDataFolder": loadResultsFromDataFolder, @@ -403,7 +404,7 @@ const commandsMap: (state: ExtensionState) => { "konveyor.diffView.applyBlockInline": applyBlock, "konveyor.diffView.applySelection": applyBlock, "konveyor.diffView.applySelectionInline": applyBlock, - "konveyor.partialAnalysis": async (filePaths: string[]) => runPartialAnalysis(state, filePaths), + "konveyor.partialAnalysis": async (filePaths: Uri[]) => runPartialAnalysis(state, filePaths), "konveyor.configureGetSolutionParams": async () => { const maxPriorityInput = await window.showInputBox({ prompt: "Enter max_priority for getSolution", diff --git a/vscode/src/data/analyzerResults.ts b/vscode/src/data/analyzerResults.ts index 3a4df61..83be83d 100644 --- a/vscode/src/data/analyzerResults.ts +++ b/vscode/src/data/analyzerResults.ts @@ -36,7 +36,7 @@ function getSeverityFromCategory(category: Category | undefined): vscode.Diagnos export const processIncidents = ( ruleSets: Immutable, -): ReadonlyArray<[vscode.Uri, vscode.Diagnostic[] | undefined]> => +): ReadonlyArray<[vscode.Uri, vscode.Diagnostic[]]> => ruleSets .flatMap((ruleSet) => Object.values(ruleSet.violations ?? {})) .flatMap((violation): [vscode.DiagnosticSeverity, Incident][] => { diff --git a/vscode/src/data/loadResults.ts b/vscode/src/data/loadResults.ts index 7000bd6..679a9d8 100644 --- a/vscode/src/data/loadResults.ts +++ b/vscode/src/data/loadResults.ts @@ -9,7 +9,7 @@ import { processIncidents } from "./analyzerResults"; import { ExtensionState } from "src/extensionState"; import { deleteAllDataFilesByPrefix, writeDataFile } from "./storage"; import { toLocalChanges, writeSolutionsToMemFs } from "./virtualStorage"; -import { window } from "vscode"; +import { Diagnostic, Uri, window } from "vscode"; import { KONVEYOR_SCHEME, MERGED_RULE_SET_DATA_FILE_PREFIX, @@ -20,10 +20,28 @@ import { import { castDraft, Immutable } from "immer"; import { mergeRuleSets } from "../analysis"; +const diagnosticsForPaths = ( + filePaths: Uri[], + diagnostics: Immutable<[Uri, Diagnostic[]][]>, +): [Uri, Diagnostic[]][] => { + const paths = new Set(filePaths.map((it) => it.toString())); + return diagnostics.filter(([uri]) => paths.has(uri.toString())) as [Uri, Diagnostic[]][]; +}; + +const countDiagnosticPerPath = (diagnostics: [Uri, readonly Diagnostic[] | undefined][]) => + diagnostics.reduce( + (acc, [uri, items]) => { + const path = uri.toString(); + acc[path] = (acc[path] ?? 0) + (items?.length ?? 0); + return acc; + }, + {} as { [key: string]: number }, + ); + export const loadRuleSets = async ( state: ExtensionState, receivedRuleSets: RuleSet[], - filePaths?: string[], + filePaths?: Uri[], ) => { const isPartial = !!filePaths; if (isPartial) { @@ -42,10 +60,28 @@ export const loadRuleSets = async ( ? mergeRuleSets(draft.ruleSets, receivedRuleSets, filePaths) : receivedRuleSets; }); + const diagnosticTuples = processIncidents(data.ruleSets); if (isPartial) { await writeDataFile(data.ruleSets, MERGED_RULE_SET_DATA_FILE_PREFIX); + console.log( + "Diagnostics on the selected paths before update", + countDiagnosticPerPath(filePaths.map((uri) => [uri, state.diagnosticCollection.get(uri)])), + ); + const scopedChange = [ + // remove existing markers by passing undefined first + ...filePaths.map((uri): [Uri, undefined] => [uri, undefined]), + // set new values + ...diagnosticsForPaths(filePaths, diagnosticTuples), + ]; + state.diagnosticCollection.set(scopedChange); + console.log( + "Diagnostics on the selected paths after update", + countDiagnosticPerPath(filePaths.map((uri) => [uri, state.diagnosticCollection.get(uri)])), + ); + } else { + state.diagnosticCollection.clear(); + state.diagnosticCollection.set(diagnosticTuples); } - state.diagnosticCollection.set(processIncidents(data.ruleSets)); }; export const cleanRuleSets = (state: ExtensionState) => { diff --git a/vscode/src/diffView/register.ts b/vscode/src/diffView/register.ts index 0ebc0e4..51867df 100644 --- a/vscode/src/diffView/register.ts +++ b/vscode/src/diffView/register.ts @@ -60,11 +60,11 @@ export function registerDiffView({ .map((change, index): [LocalChange, number] => [change, index]) .filter(([change, index]) => hasChanged(change, index)) .filter(([{ state }]) => state === "applied" || state === "discarded") - .map(([change, index]): [LocalChange, number, vscode.Uri[], string] => [ + .map(([change, index]): [LocalChange, number, vscode.Uri[], vscode.Uri | undefined] => [ change, index, copyFromTo(change), - change.state === "applied" ? change.originalUri.fsPath : "", + change.state === "applied" ? change.originalUri : undefined, ]) .map(([change, index, [fromUri, toUri], filePath]) => vscode.workspace.fs.copy(fromUri, toUri, { overwrite: true }).then(() => { @@ -74,7 +74,7 @@ export function registerDiffView({ ), ); - const appliedPaths = allModifiedPaths.filter(Boolean); + const appliedPaths = allModifiedPaths.filter(Boolean).filter((uri) => uri?.scheme === "file"); if (appliedPaths.length && getConfigAnalyzeOnSave()) { return vscode.commands.executeCommand("konveyor.partialAnalysis", appliedPaths); } diff --git a/vscode/src/extension.ts b/vscode/src/extension.ts index b1883d1..395d5ee 100644 --- a/vscode/src/extension.ts +++ b/vscode/src/extension.ts @@ -45,7 +45,7 @@ class VsCodeExtension { }; this.state = { - analyzerClient: new AnalyzerClient(context, mutateData), + analyzerClient: new AnalyzerClient(context, mutateData, getData), webviewProviders: new Map(), extensionContext: context, diagnosticCollection: vscode.languages.createDiagnosticCollection("konveyor"), diff --git a/vscode/src/utilities/typeGuards.ts b/vscode/src/utilities/typeGuards.ts index 879c499..b6b11a4 100644 --- a/vscode/src/utilities/typeGuards.ts +++ b/vscode/src/utilities/typeGuards.ts @@ -38,15 +38,21 @@ export function isAnalysis(obj: unknown): obj is RuleSet { skipped: "object", }; + const knownKeysAsString = knownKeys as Record; + return ( isObject(obj) && !isEmpty(obj) && Object.entries(obj).every( - ([key, value]) => typeof value === (knownKeys as Record)[key], + ([key, value]) => !knownKeysAsString[key] || typeof value === knownKeysAsString[key], ) ); } +export function isAnalysisResponse(obj: unknown[]): obj is RuleSet[] { + return Array.isArray(obj) && obj.every((item) => isAnalysis(item)); +} + export function isUri(obj: unknown): obj is Uri { if (!isObject(obj)) { return false;