From 86a1073a37bbe1a0015588cb171153a0a42df5e4 Mon Sep 17 00:00:00 2001 From: sebthom Date: Thu, 30 Oct 2025 14:08:19 +0100 Subject: [PATCH 1/2] fix: correct ResourceOperationTypeAdapter dispatch and add guard - Replace reversed `isAssignableFrom` checks with `instanceof` to correctly handle subclasses (e.g., `CreateFile` derivatives) in write(...) - Add explicit `JsonParseException` for unknown `ResourceOperation` subtypes in write(...) to mirror read(...) and avoid silent malformed JSON - Improves protocol correctness and failure transparency when encountering unsupported operations Tests: add `ResourceOperationTypeAdapterTest` covering subclass serialization and unknown subtype failure --- .../ResourceOperationTypeAdapter.java | 18 ++--- .../ResourceOperationTypeAdapterTest.java | 71 +++++++++++++++++++ 2 files changed, 81 insertions(+), 8 deletions(-) create mode 100644 org.eclipse.lsp4j/src/test/java/org/eclipse/lsp4j/test/adapters/ResourceOperationTypeAdapterTest.java diff --git a/org.eclipse.lsp4j/src/main/java/org/eclipse/lsp4j/adapters/ResourceOperationTypeAdapter.java b/org.eclipse.lsp4j/src/main/java/org/eclipse/lsp4j/adapters/ResourceOperationTypeAdapter.java index 2c661ee6a..9ecf7ddf3 100644 --- a/org.eclipse.lsp4j/src/main/java/org/eclipse/lsp4j/adapters/ResourceOperationTypeAdapter.java +++ b/org.eclipse.lsp4j/src/main/java/org/eclipse/lsp4j/adapters/ResourceOperationTypeAdapter.java @@ -1,12 +1,12 @@ /****************************************************************************** * Copyright (c) 2018 Microsoft Corporation and others. - * + * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0 which is available at * http://www.eclipse.org/legal/epl-2.0, * or the Eclipse Distribution License v. 1.0 which is available at * http://www.eclipse.org/org/documents/edl-v10.php. - * + * * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause ******************************************************************************/ @@ -46,9 +46,9 @@ public TypeAdapter create(Gson gson, TypeToken type) { private static class InnerResourceOperationTypeAdapter extends TypeAdapter { - TypeAdapter createFileAdapter; - TypeAdapter deleteFileAdapter; - TypeAdapter renameFileAdapter; + final TypeAdapter createFileAdapter; + final TypeAdapter deleteFileAdapter; + final TypeAdapter renameFileAdapter; InnerResourceOperationTypeAdapter(TypeAdapterFactory factory, Gson gson) { createFileAdapter = gson.getDelegateAdapter(factory, TypeToken.get(CreateFile.class)); @@ -58,12 +58,14 @@ private static class InnerResourceOperationTypeAdapter extends TypeAdapter gson.toJson(op, ResourceOperation.class)); + } +} From d6336338520fa8d22de88772e9e25f7828d54791 Mon Sep 17 00:00:00 2001 From: Vladimir Piskarev Date: Thu, 30 Oct 2025 16:34:18 +0300 Subject: [PATCH 2/2] Minor updates to `ResourceOperationTypeAdapterTest` --- .../ResourceOperationTypeAdapterTest.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/org.eclipse.lsp4j/src/test/java/org/eclipse/lsp4j/test/adapters/ResourceOperationTypeAdapterTest.java b/org.eclipse.lsp4j/src/test/java/org/eclipse/lsp4j/test/adapters/ResourceOperationTypeAdapterTest.java index a962d6ff3..869244ce8 100644 --- a/org.eclipse.lsp4j/src/test/java/org/eclipse/lsp4j/test/adapters/ResourceOperationTypeAdapterTest.java +++ b/org.eclipse.lsp4j/src/test/java/org/eclipse/lsp4j/test/adapters/ResourceOperationTypeAdapterTest.java @@ -16,15 +16,14 @@ import org.eclipse.lsp4j.CreateFile; import org.eclipse.lsp4j.ResourceOperation; +import org.eclipse.lsp4j.adapters.ResourceOperationTypeAdapter; import org.junit.Test; import com.google.gson.Gson; import com.google.gson.GsonBuilder; /** - * Failing tests that demonstrate issues in ResourceOperationTypeAdapter: - * - reversed isAssignableFrom dispatch prevents handling subclasses - * - missing guard for unknown subtypes silently produces invalid output + * Tests for {@link ResourceOperationTypeAdapter}. */ public class ResourceOperationTypeAdapterTest { @@ -35,8 +34,7 @@ static class CustomCreateFile extends CreateFile { } /** - * Verify that a subclass of CreateFile serializes like CreateFile. - * Current buggy implementation fails to serialize any output for subclasses. + * Verify that a subclass of {@link CreateFile} serializes like {@link CreateFile} itself. */ @Test public void write_subclassOfCreateFile_serializes() { @@ -44,12 +42,12 @@ public void write_subclassOfCreateFile_serializes() { ResourceOperation op = new CustomCreateFile("file:///tmp/x"); String json = gson.toJson(op, ResourceOperation.class); // Expected: kind=create and uri present in JSON - assertTrue("Expected kind=create in JSON but was: " + json, json != null && json.contains("\"kind\":\"create\"")); - assertTrue("Expected uri in JSON but was: " + json, json != null && json.contains("\"uri\":\"file:///tmp/x\"")); + assertTrue("Expected kind=create in JSON but was: " + json, json.contains("\"kind\":\"create\"")); + assertTrue("Expected uri in JSON but was: " + json, json.contains("\"uri\":\"file:///tmp/x\"")); } /** - * An unknown ResourceOperation subtype to demonstrate that the adapter + * An unknown {@link ResourceOperation} subtype to demonstrate that the adapter * should fail fast instead of producing empty/invalid output. */ static class UnknownOp extends ResourceOperation { @@ -59,8 +57,8 @@ static class UnknownOp extends ResourceOperation { } /** - * Verify that an unknown subtype causes a IllegalArgumentException during writing. - * Current buggy implementation writes nothing and does not signal an error. + * Verify that an unknown subtype of {@link ResourceOperation} + * causes an {@link IllegalArgumentException} during writing. */ @Test public void write_unknownSubtype_throws() {