dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.12k stars 4.04k forks source link

LSIF Generator can generate invalid JSON in hover results #64050

Closed NTaylorMullen closed 2 years ago

NTaylorMullen commented 2 years ago

Version Used: 4.4.0-1.22372.4

Steps to Reproduce:

  1. Generate LSIF for this file in dotnet/razor-tooling

Expected Behavior: No 0x07 control character generated

Actual Behavior:

Scroll to the right in value to see the invalid 0x07 control characters

{
    "result": {
        "contents": {
            "kind": "plaintext",
            "value": "class Microsoft.AspNetCore.Razor.Test.Common.UseExportProviderAttribute\r\nThis attribute supports tests that need to use a MEF container (ExportProvider) directly or indirectly during the test sequence. It ensures production code uniformly handles the export provider created during a test, and cleans up the state before the test completes.\r\n\r\nThis attribute serves several important functions for tests that use state variables which are otherwise shared at runtime:\r\n\r\n Ensures HostServices implementations all use the same ExportProvider, which is the one created by the test.\r\n Clears static cached values in production code holding instances of HostServices, or any object obtained from it or one of its related interfaces such as HostLanguageServices.\r\n Isolates tests by waiting for asynchronous operations to complete before a test is considered complete.\r\n When required, provides a separate ExportProvider for the RemoteWorkspace executing in the test process. If this provider is created during testing, it is cleaned up with the primary export provider during test teardown.\r\n"
        },
        "range": {
            "start": {
                "line": 32,
                "character": 17
            },
            "end": {
                "line": 32,
                "character": 43
            }
        }
    },
    "id": 2734,
    "type": "vertex",
    "label": "hoverResult"
}

When deserializing this with System.Text.Json you get: image

And similarly this fails JSON validation on all web validators

NTaylorMullen commented 2 years ago

Hmm, something seems to be amiss. Looking at LSIF generated / uploaded from the task it looks like hover results aren't being escaped properly during json serialization possibly?

Looking at this hover result:

{"result":{"contents":{"kind":"plaintext","value":"interface Microsoft.VisualStudio.RpcContracts.Editor.ITextEditorSynchronizationServiceContract\r\nThis is a private brokered service running in the main VS process and representing "source of truth" regarding text document content. It provides callers with document content of specific versions and can provide updates as documents are changing. It also applies changes submitted by extensions to documents and text views.\r\n"},"range":{"start":{"line":12,"character":23},"end":{"line":12,"character":64}}},"id":712625,"type":"vertex","label":"hoverResult"}

The quotes "source of truth" aren't escaped which results in it breaking json deserialization entirely: image

Following up to ensure this isn't on the RichNav side

NTaylorMullen commented 2 years ago

@jasonmalinowski I could reproduce this directly by calling the C# LSIF generator. In the above case the quotes in the docs aren't normal quotes. They're "word" quotes maybe? "fancy" quotes? Lol

image

NTaylorMullen commented 2 years ago

Dug into why and you're missing Console.OutputEncoding = Encoding.UTF8; after setting the console to the output method: https://github.com/dotnet/roslyn/blob/2824abb2fabdc29ede627470b2800ba0d15f787a/src/Features/Lsif/Generator/Program.cs#L46

CyrusNajmabadi commented 2 years ago

@NTaylorMullen might be fastest to just submit a PR :)

NTaylorMullen commented 2 years ago

@NTaylorMullen might be fastest to just submit a PR :)

Already on it, Roslyn takes forever to build though. Maybe I just wont build and wing it😆

jasonmalinowski commented 2 years ago

Roslyn takes forever to build though.

image