Skip to content

Commit bd14a6d

Browse files
timsueberkruebtimsueberkrueb
authored
Fix serialization of hole info (#1002)
LSP4j uses the Gson Java library to (de)serialize JSON as part of the language server protocol. As a Java library, Gson doesn't special-case the Scala types Option and List (or any other Scala types for that matter), leading to undesirable output for these based on the default runtime-reflection based (de)serialization logic. This PR fixes it by registering custom type adapters for scala `List` and `Option` with Gson.
1 parent 940be9f commit bd14a6d

File tree

4 files changed

+146
-57
lines changed

4 files changed

+146
-57
lines changed

‎effekt/jvm/src/main/scala/effekt/GsonUtil.scala‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package effekt
2+
3+
import com.google.gson._
4+
import com.google.gson.reflect.TypeToken
5+
import java.lang.reflect.{ParameterizedType, Type}
6+
import scala.jdk.CollectionConverters._
7+
8+
class ScalaOptionTypeAdapter extends JsonSerializer[Option[Any]] with JsonDeserializer[Option[Any]] {
9+
override def serialize(src: Option[Any], typeOfSrc: Type, context: JsonSerializationContext): JsonElement = {
10+
src match {
11+
case Some(value) => context.serialize(value)
12+
case None => JsonNull.INSTANCE
13+
}
14+
}
15+
16+
override def deserialize(json: JsonElement, typeOfT: Type, context: JsonDeserializationContext): Option[Any] = {
17+
val innerType = typeOfT match {
18+
case pt: ParameterizedType => pt.getActualTypeArguments.head
19+
case _ => classOf[Object]
20+
}
21+
22+
if (json.isJsonNull) None
23+
else Some(context.deserialize(json, innerType))
24+
}
25+
}
26+
27+
class ScalaListTypeAdapter extends JsonSerializer[scala.collection.immutable.List[Any]] with JsonDeserializer[scala.collection.immutable.List[Any]] {
28+
override def serialize(src: scala.collection.immutable.List[Any], typeOfSrc: Type, context: JsonSerializationContext): JsonElement = {
29+
val javaList: java.util.List[Any] = src.asJava
30+
context.serialize(javaList)
31+
}
32+
33+
override def deserialize(json: JsonElement, typeOfT: Type, context: JsonDeserializationContext): scala.collection.immutable.List[Any] = {
34+
val elemType = typeOfT match {
35+
case pt: ParameterizedType => pt.getActualTypeArguments.head
36+
case _ => classOf[Object]
37+
}
38+
39+
val listType = TypeToken.getParameterized(classOf[java.util.List[_]], elemType).getType
40+
val javaList = context.deserialize[java.util.List[Any]](json, listType)
41+
42+
javaList.asScala.toList
43+
}
44+
}
45+
46+
/**
47+
* Make the Scala Option[_] and List[_] types (de)serialize correctly with Gson.
48+
*
49+
* As a Java library, Gson does not special case any Scala types. This leads to unexpected (de)serialization by default.
50+
* By default, Some(x) serializes to {"value":x} and None serializes to {}.
51+
* List serializes to nested JSON objects.
52+
*
53+
* This method adds custom (de)serializers such that
54+
* - Some(x) serializes to x and None serializes to null
55+
* - List serializes to a flat JSON array.
56+
*/
57+
extension (builder: GsonBuilder) def withScalaSupport: GsonBuilder =
58+
builder
59+
.registerTypeHierarchyAdapter(classOf[Option[_]], new ScalaOptionTypeAdapter)
60+
.registerTypeHierarchyAdapter(classOf[List[_]], new ScalaListTypeAdapter)

‎effekt/jvm/src/main/scala/effekt/Server.scala‎

Lines changed: 32 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -222,25 +222,6 @@ class Server(config: EffektConfig, compileOnChange: Boolean=false) extends Langu
222222
this.client = client
223223
}
224224

225-
/**
226-
* Launch a language server using provided input/output streams.
227-
* This allows tests to connect via in-memory pipes.
228-
*/
229-
def launch(client: EffektLanguageClient, in: InputStream, out: OutputStream): Launcher[EffektLanguageClient] = {
230-
val executor = Executors.newSingleThreadExecutor()
231-
val launcher =
232-
new LSPLauncher.Builder()
233-
.setLocalService(this)
234-
.setRemoteInterface(classOf[EffektLanguageClient])
235-
.setInput(in)
236-
.setOutput(out)
237-
.setExecutorService(executor)
238-
.create()
239-
this.connect(client)
240-
launcher.startListening()
241-
launcher
242-
}
243-
244225
// LSP Document Lifecycle
245226
//
246227
//
@@ -538,6 +519,34 @@ class Server(config: EffektConfig, compileOnChange: Boolean=false) extends Langu
538519
Some(value.getAsString)
539520
}
540521

522+
/**
523+
* Launch a language server using provided input/output streams.
524+
* This allows tests to connect via in-memory pipes.
525+
*/
526+
def launch(getClient: Launcher[EffektLanguageClient] => EffektLanguageClient, in: InputStream, out: OutputStream, trace: Boolean = false): Launcher[EffektLanguageClient] = {
527+
// Create a single-threaded executor to serialize all requests.
528+
val executor: ExecutorService = Executors.newSingleThreadExecutor()
529+
530+
val builder =
531+
new LSPLauncher.Builder()
532+
.setLocalService(this)
533+
.setRemoteInterface(classOf[EffektLanguageClient])
534+
.setInput(in)
535+
.setOutput(out)
536+
.setExecutorService(executor)
537+
// This line makes sure that the List and Option Scala types serialize correctly
538+
.configureGson(_.withScalaSupport)
539+
540+
if (trace) {
541+
builder.traceMessages(new PrintWriter(System.err, true))
542+
}
543+
544+
val launcher = builder.create()
545+
this.connect(getClient(launcher))
546+
launcher.startListening()
547+
launcher
548+
}
549+
541550
/**
542551
* Launch a language server with a given `ServerConfig`
543552
*/
@@ -549,32 +558,9 @@ class Server(config: EffektConfig, compileOnChange: Boolean=false) extends Langu
549558
val serverSocket = new ServerSocket(config.debugPort)
550559
System.err.println(s"Starting language server in debug mode on port ${config.debugPort}")
551560
val socket = serverSocket.accept()
552-
553-
val launcher =
554-
new LSPLauncher.Builder()
555-
.setLocalService(this)
556-
.setRemoteInterface(classOf[EffektLanguageClient])
557-
.setInput(socket.getInputStream)
558-
.setOutput(socket.getOutputStream)
559-
.setExecutorService(executor)
560-
.traceMessages(new PrintWriter(System.err, true))
561-
.create()
562-
val client = launcher.getRemoteProxy
563-
this.connect(client)
564-
launcher.startListening()
561+
launch(_.getRemoteProxy, socket.getInputStream, socket.getOutputStream, trace = true)
565562
} else {
566-
val launcher =
567-
new LSPLauncher.Builder()
568-
.setLocalService(this)
569-
.setRemoteInterface(classOf[EffektLanguageClient])
570-
.setInput(System.in)
571-
.setOutput(System.out)
572-
.setExecutorService(executor)
573-
.create()
574-
575-
val client = launcher.getRemoteProxy
576-
this.connect(client)
577-
launcher.startListening()
563+
launch(_.getRemoteProxy, System.in, System.out)
578564
}
579565
}
580566
}
@@ -621,8 +607,8 @@ case class EffektHoleInfo(id: String,
621607
range: LSPRange,
622608
innerType: Option[String],
623609
expectedType: Option[String],
624-
importedTerms: Seq[Intelligence.TermBinding], importedTypes: Seq[Intelligence.TypeBinding],
625-
terms: Seq[Intelligence.TermBinding], types: Seq[Intelligence.TypeBinding])
610+
importedTerms: List[Intelligence.TermBinding], importedTypes: List[Intelligence.TypeBinding],
611+
terms: List[Intelligence.TermBinding], types: List[Intelligence.TypeBinding])
626612

627613
object EffektHoleInfo {
628614
def fromHoleInfo(info: Intelligence.HoleInfo): EffektHoleInfo = {
@@ -638,4 +624,3 @@ object EffektHoleInfo {
638624
)
639625
}
640626
}
641-

‎effekt/jvm/src/test/scala/effekt/LSPTests.scala‎

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package effekt
22

3-
import com.google.gson.{JsonElement, JsonParser}
3+
import com.google.gson.{Gson, GsonBuilder, JsonElement, JsonParser}
44
import effekt.Intelligence.{TermBinding, TypeBinding}
55
import munit.FunSuite
66
import org.eclipse.lsp4j.{CodeAction, CodeActionKind, CodeActionParams, Command, DefinitionParams, Diagnostic, DiagnosticSeverity, DidChangeConfigurationParams, DidChangeTextDocumentParams, DidCloseTextDocumentParams, DidOpenTextDocumentParams, DidSaveTextDocumentParams, DocumentSymbol, DocumentSymbolParams, Hover, HoverParams, InitializeParams, InitializeResult, InlayHint, InlayHintKind, InlayHintParams, MarkupContent, MessageActionItem, MessageParams, Position, PublishDiagnosticsParams, Range, ReferenceContext, ReferenceParams, SaveOptions, ServerCapabilities, SetTraceParams, ShowMessageRequestParams, SymbolInformation, SymbolKind, TextDocumentContentChangeEvent, TextDocumentItem, TextDocumentSyncKind, TextDocumentSyncOptions, TextEdit, VersionedTextDocumentIdentifier, WorkspaceEdit}
@@ -40,7 +40,7 @@ class LSPTests extends FunSuite {
4040
val mockClient = new MockLanguageClient()
4141
server.connect(mockClient)
4242

43-
val launcher = server.launch(mockClient, serverIn, serverOut)
43+
val launcher = server.launch(_ => mockClient, serverIn, serverOut)
4444

4545
testBlock(mockClient, server)
4646
}
@@ -1192,21 +1192,21 @@ class LSPTests extends FunSuite {
11921192
qualifier = List(),
11931193
name = "x",
11941194
`type` = Some(
1195-
value = "Int"
1195+
"Int"
11961196
)
11971197
),
11981198
TermBinding(
11991199
qualifier = List(),
12001200
name = "bar",
12011201
`type` = Some(
1202-
value = "String => Int"
1202+
"String => Int"
12031203
)
12041204
),
12051205
TermBinding(
12061206
qualifier = List(),
12071207
name = "foo",
12081208
`type` = Some(
1209-
value = "Int => Bool"
1209+
"Int => Bool"
12101210
)
12111211
)
12121212
)
@@ -1217,7 +1217,7 @@ class LSPTests extends FunSuite {
12171217
assertEquals(receivedHoles.head.holes(0).id, "foo0")
12181218
assertEquals(receivedHoles.head.holes(0).innerType, Some("Int"))
12191219
assertEquals(receivedHoles.head.holes(0).expectedType, Some("Bool"))
1220-
assertEquals(receivedHoles.head.holes(0).terms.toList, termsFoo.toList)
1220+
assertEquals(receivedHoles.head.holes(0).terms, termsFoo)
12211221
assertEquals(receivedHoles.head.holes(0).types, List(
12221222
TypeBinding(
12231223
qualifier = Nil,
@@ -1261,6 +1261,50 @@ class LSPTests extends FunSuite {
12611261
}
12621262
}
12631263

1264+
/**
1265+
* By default, Scala types such as List and Option show pathological (de)serialization behavior with Gson.
1266+
* We use a custom extension method `withScalaSupport` which adds support for Scala collections, fixing serialization.
1267+
*/
1268+
test("Hole info serializes to expected JSON") {
1269+
val holeInfo = EffektHoleInfo(
1270+
id = "foo_bar0",
1271+
range = new Range(
1272+
new Position(0, 0),
1273+
new Position(0, 0)
1274+
),
1275+
innerType = None,
1276+
expectedType = Some("Bool"),
1277+
importedTerms = Nil,
1278+
importedTypes = Nil,
1279+
terms = List(
1280+
TermBinding(
1281+
qualifier = Nil,
1282+
name = "x",
1283+
`type` = Some("Int")
1284+
)
1285+
),
1286+
types = List(
1287+
TypeBinding(
1288+
qualifier = Nil,
1289+
name = "MyInt",
1290+
definition = "type MyInt = Int"
1291+
)
1292+
)
1293+
)
1294+
1295+
val expectedJsonStr =
1296+
"""{"id":"foo_bar0","range":{"start":{"line":0,"character":0},"end":{"line":0,"character":0}},"expectedType":"Bool","importedTerms":[],"importedTypes":[],"terms":[{"qualifier":[],"name":"x","type":"Int"}],"types":[{"qualifier":[],"name":"MyInt","definition":"type MyInt = Int"}]}""".stripMargin
1297+
1298+
val expectedJson: JsonElement = JsonParser.parseString(expectedJsonStr)
1299+
1300+
val gson: Gson = new GsonBuilder().withScalaSupport.create()
1301+
1302+
val actualJson: JsonElement = gson.toJsonTree(holeInfo)
1303+
1304+
assertEquals(actualJson, expectedJson)
1305+
}
1306+
1307+
12641308
// Text document DSL
12651309
//
12661310
//

‎effekt/shared/src/main/scala/effekt/Intelligence.scala‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -321,13 +321,13 @@ object Intelligence {
321321
span: Span,
322322
innerType: Option[String],
323323
expectedType: Option[String],
324-
importedTerms: Seq[TermBinding], importedTypes: Seq[TypeBinding],
325-
terms: Seq[TermBinding], types: Seq[TypeBinding]
324+
importedTerms: List[TermBinding], importedTypes: List[TypeBinding],
325+
terms: List[TermBinding], types: List[TypeBinding]
326326
)
327327

328-
case class TermBinding(qualifier: Seq[String], name: String, `type`: Option[String])
328+
case class TermBinding(qualifier: List[String], name: String, `type`: Option[String])
329329

330-
case class TypeBinding(qualifier: Seq[String], name: String, definition: String)
330+
case class TypeBinding(qualifier: List[String], name: String, definition: String)
331331

332332
case class BindingInfo(
333333
importedTerms: Iterable[TermBinding],

0 commit comments

Comments
 (0)