From 799391b301ebab866e392ac7f6bc49543d5ae132 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 9 Nov 2026 18:07:12 +0800 Subject: [PATCH] fix(debugger): report obsolete methods as a completed hot swap (#2330) --- .../debugger/__tests__/hotswap.test.ts | 115 +++++++++++++++++- src/features/debugger/hotswap.ts | 84 ++++++++++++- src/platform/locales/text/en.json | 4 + src/platform/locales/text/zh.json | 4 + 4 files changed, 200 insertions(+), 7 deletions(-) diff --git a/src/features/debugger/__tests__/hotswap.test.ts b/src/features/debugger/__tests__/hotswap.test.ts index dc0b2ad25..5a53de92d 100644 --- a/src/features/debugger/__tests__/hotswap.test.ts +++ b/src/features/debugger/__tests__/hotswap.test.ts @@ -1,5 +1,11 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +const executeAction = vi.hoisted(() => vi.fn()); +vi.mock("@/platform/actions", async (importOriginal) => ({ + ...(await importOriginal()), + executeAction, +})); + const ipc = vi.hoisted(() => { const ok = (data: T) => Promise.resolve({ status: "ok" as const, data }); return { @@ -18,7 +24,9 @@ vi.mock("@/platform/ipc", async (importOriginal) => { return { ...real, isTauri: () => true, commands: { ...real.commands, ...cmds } }; }); -const notes = vi.hoisted(() => [] as Array<{ level: string; title: string; body?: unknown; actions?: unknown[] }>); +const notes = vi.hoisted( + () => [] as Array<{ level: string; title: string; body?: unknown; actions?: { label: string; run: () => void }[] }>, +); vi.mock("@/workbench/contributions", async (importOriginal) => { const real = await importOriginal(); return { @@ -74,6 +82,7 @@ function session(id: number, hotCodeReplace: boolean): DebugSession { describe("HotSwap", () => { beforeEach(() => { notes.length = 0; + executeAction.mockClear(); ipc.cancelRequest.mockClear(); ipc.javaBuildWorkspace.mockClear(); ipc.debugRedefineClasses.mockReset(); @@ -226,6 +235,110 @@ describe("HotSwap", () => { expect(hotSwapMessage(3, ["boom"])).toMatchObject({ level: "error", body: "boom" }); }); + it.each([0, 1, 3])("reports obsolete methods as a reload warning even with %s returned classes", (changed) => { + expect(hotSwapMessage(changed, ["JVM contains obsolete methods"])).toMatchObject({ + level: "warning", + title: "Classes reloaded", + body: expect.stringContaining("Active frames keep running the old code"), + }); + expect(hotSwapMessage(changed, ["JVM contains obsolete methods"], true).body).toContain( + "bottom frame cannot be reset", + ); + }); + + it.each([ + "Failed to redefine classes: add method not implemented", + "Failed to redefine classes: schema change not implemented", + "Failed to redefine classes: JVM contains obsolete methods", + ])("keeps a real failure even alongside obsolete methods: %s", (failure) => { + for (const errors of [[failure], ["JVM contains obsolete methods", failure]]) { + expect(hotSwapMessage(1, errors)).toMatchObject({ level: "error", title: "Hot Swap failed" }); + } + }); + + function pausedFrames(names: string[], restartFrame = true): DebugSession { + return { + ...session(1, true), + status: "paused", + caps: { hotCodeReplace: true, restartFrame } as DebugCapabilities, + selectedThread: 1, + selectedFrame: 11, + frames: names.map((name, i) => ({ + id: 11 + i, + name, + path: "/p/App.java", + line: 11, + column: 0, + presentationHint: null, + })), + }; + } + + it.each([ + [true, ["App.main(String[])"], false], + [false, ["OrderBook.place()", "App.main(String[])"], false], + [true, ["OrderBook.place()", "Native.call()[native method]"], false], + [true, ["Native.call()[native method]", "App.main(String[])"], false], + [true, ["OrderBook.place()", "App.main(String[])"], true], + ])("offers Reset Frame only when supported and droppable (%s, %j)", async (supported, names, offered) => { + useDebug.setState({ sessions: { 1: pausedFrames(names, supported) } }); + ipc.debugRedefineClasses.mockImplementationOnce(() => + ipc.ok({ changedClasses: [], errorMessage: "JVM contains obsolete methods" }), + ); + await reloadChangedClasses(false); + const note = notes.at(-1)!; + expect(note).toMatchObject({ level: "warning", title: "Classes reloaded" }); + expect(note.actions).toHaveLength(offered ? 1 : 0); + if (offered) { + useDebug.setState({ active: 2 }); + note.actions![0]!.run(); + expect(executeAction).toHaveBeenCalledWith("debug.resetFrame"); + expect(useDebug.getState().active).toBe(1); + expect(useDebug.getState().sessions[1]?.selectedFrame).toBe(11); + } + }); + + it("does not reset a stale frame from an earlier suspension", async () => { + const s = pausedFrames(["OrderBook.place()", "App.main(String[])"]); + useDebug.setState({ sessions: { 1: s } }); + ipc.debugRedefineClasses.mockImplementationOnce(() => + ipc.ok({ changedClasses: [], errorMessage: "JVM contains obsolete methods" }), + ); + await reloadChangedClasses(false); + useDebug.setState({ sessions: { 1: { ...s, frames: [...s.frames] } } }); + notes.at(-1)!.actions![0]!.run(); + expect(executeAction).not.toHaveBeenCalled(); + }); + + it("maps java-debug's obsolete-method ERROR event to the same warning", () => { + useDebug.setState({ sessions: { 1: pausedFrames(["App.main(String[])"]) } }); + onHotCodeReplace(1 as DebugSessionId, { + kind: "hotCodeReplace", + changeType: "ERROR", + message: "JVM contains obsolete methods", + }); + expect(notes.at(-1)).toMatchObject({ + level: "warning", + title: "Classes reloaded", + body: expect.stringContaining("bottom frame cannot be reset"), + actions: [], + }); + }); + + it("does not let the obsolete-method event replace the response's real failure", async () => { + ipc.debugRedefineClasses.mockImplementationOnce(async () => { + onHotCodeReplace(1 as DebugSessionId, { + kind: "hotCodeReplace", + changeType: "ERROR", + message: "JVM contains obsolete methods", + }); + return ipc.ok({ changedClasses: [], errorMessage: "Failed to redefine classes: add method not implemented" }); + }); + await reloadChangedClasses(false); + expect(notes).toHaveLength(1); + expect(notes[0]).toMatchObject({ level: "error", title: "Hot Swap failed" }); + }); + it("translates the reload count with the sentence", async () => { await setLocale("zh"); try { diff --git a/src/features/debugger/hotswap.ts b/src/features/debugger/hotswap.ts index 38a13114b..1b319313f 100644 --- a/src/features/debugger/hotswap.ts +++ b/src/features/debugger/hotswap.ts @@ -16,8 +16,9 @@ import { type RequestId, } from "@/platform/ipc"; import { setContext } from "@/platform/context"; +import { executeAction } from "@/platform/actions"; import { maybeEditorService, notifications } from "@/workbench/contributions"; -import { isLive, useDebug, type DebugSession } from "./state"; +import { isLive, selectFrame, selectSession, useDebug, type DebugSession } from "./state"; import { tx } from "@/platform/i18n"; import type { MenuItem } from "@/ui/Menu"; @@ -48,16 +49,61 @@ export function hotSwapMenuItems(paths: readonly string[]): MenuItem[] { /** "3 classes", translated per count (the sentence around it is translated on its own). */ const classesText = (n: number) => (n === 1 ? tx`${n} class` : tx`${n} classes`); +// java-debug reports this after redefineClasses succeeds, with no changedClasses count. +const obsoleteMethods = (message: string) => message.trim() === "JVM contains obsolete methods"; + /** Message for the reload result (IDEA wording). */ export function hotSwapMessage( changed: number, errors: string[], -): { level: "info" | "error"; title: string; body?: string } { - if (errors.length) return { level: "error", title: "Hot Swap failed", body: errors.join("\n") }; + bottomFrame = false, +): { level: "info" | "warning" | "error"; title: string; body?: string } { + if (errors.some((e) => !obsoleteMethods(e))) + return { level: "error", title: "Hot Swap failed", body: errors.join("\n") }; + if (errors.length) + return { + level: "warning", + title: "Classes reloaded", + body: + tx("Active frames keep running the old code until they return. New calls use the reloaded code.") + + (bottomFrame + ? " " + + tx( + "The bottom frame cannot be reset. Re-enter the method or restart the debug session to run the new code.", + ) + : ""), + }; if (changed === 0) return { level: "info", title: "Loaded classes are up to date. Nothing to reload." }; return { level: "info", title: tx`${classesText(changed)} reloaded` }; } +function obsoleteFrameActions(sessions: DebugSession[]) { + return sessions.flatMap((s) => { + const frame = s.frames[0]; + // java-debug cannot drop the bottom frame or frames involving a native caller. + if ( + s.status !== "paused" || + !s.caps?.restartFrame || + !frame || + s.frames.length < 2 || + s.frames.slice(0, 2).some((f) => f.name.includes("[native method]")) + ) + return []; + return [ + { + label: sessions.length > 1 ? tx`Reset Frame (${s.name})` : "Reset Frame", + run: () => { + const current = useDebug.getState().sessions[s.id]; + if (current?.status !== "paused" || current.frames !== s.frames) return; + selectSession(s.id); + selectFrame(s.id, frame.id, false); + void executeAction("debug.resetFrame"); + }, + }, + ]; + }); +} + /** Compiles (unless `build` is false) and reloads changed classes into every Java debug session. */ export async function reloadChangedClasses(build = true, configId?: string): Promise { if (reloading) return; @@ -136,17 +182,31 @@ export async function reloadChangedClasses(build = true, configId?: string): Pro } const changed = new Set(); const errors: string[] = []; + const obsolete: DebugSessionId[] = []; for (const s of sessions) { try { const r = await call(commands.debugRedefineClasses(s.id)); r.changedClasses.forEach((c) => changed.add(c)); - if (r.errorMessage) errors.push(r.errorMessage); + if (r.errorMessage) { + errors.push(r.errorMessage); + if (obsoleteMethods(r.errorMessage)) obsolete.push(s.id); + } } catch (e) { errors.push(e instanceof Error ? e.message : String(e)); } } - const m = hotSwapMessage(changed.size, errors); - notifications().notify({ group: "run", key: KEY, ...m }); + const affected = obsolete.flatMap((id) => useDebug.getState().sessions[id] ?? []); + const m = hotSwapMessage( + changed.size, + errors, + affected.some((s) => s.status === "paused" && s.frames.length === 1), + ); + notifications().notify({ + group: "run", + key: KEY, + ...m, + ...(m.level === "warning" ? { sticky: true, actions: obsoleteFrameActions(affected) } : {}), + }); } finally { reloading = false; quietUntil = Date.now() + 2_000; @@ -170,6 +230,18 @@ export function onHotCodeReplace(session: DebugSessionId, event: Extract