Skip to content

Commit

Permalink
Expose Roslyn LSP types to XAML and add readme
Browse files Browse the repository at this point in the history
  • Loading branch information
dibarbet committed Jun 11, 2024
1 parent d9c433e commit 1cc34e2
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 103 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@
<RestrictedInternalsVisibleTo Include="Microsoft.CodeAnalysis.Remote.Razor" Namespace="Roslyn.LanguageServer.Protocol" Partner="Razor" Key="$(RazorKey)" />
<RestrictedInternalsVisibleTo Include="Microsoft.VisualStudio.LanguageServices.Razor" Namespace="Roslyn.LanguageServer.Protocol" Partner="Razor" Key="$(RazorKey)" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.Xaml" />
<!-- Restricted IVT is direct for protocol types only -->
<RestrictedInternalsVisibleTo Include="Microsoft.VisualStudio.DesignTools.CodeAnalysis" Namespace="Roslyn.LanguageServer.Protocol" Partner="Xaml" Key="$(VisualStudioKey)" />
<RestrictedInternalsVisibleTo Include="DesignTools.Tests.Component.CodeAnalysis" Namespace="Roslyn.LanguageServer.Protocol" Partner="Xaml" Key="$(VisualStudioKey)" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.CSharp.EditorFeatures" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.CSharp.Features.UnitTests" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests" />
Expand Down

This file was deleted.

This file was deleted.

103 changes: 103 additions & 0 deletions src/LanguageServer/Protocol/Protocol/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
## Summary
The files in this folder defines C# types for [LSP protocol definitions](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/) and custom VS LSP protocol definitions.

These types are shared via restricted IVTs to Razor and XAML, as they run inside the C# Roslyn LSP server.

## Breaking Changes
Ensuring that these types are not binary breaking on changes is important, as this dll is shared between Roslyn, Razor, and XAML in both VSCode and VS. They export handlers that are used in our Roslyn LSP server using protocol types.

In general, the LSP specification itself generally does not make JSON protocol breaking changes. New additions are controlled by capabilities, properties are only added, etc. Most of the time these kinds of changes are not binary breaking for our type definitions either - it's totally fine to add new properties, methods, etc. to our type definitions

However, some protocol changes can result in binary breaking changes. The main scenario for this is when the protocol changes a property to a union or adds another definition to a union type. For example, if initially the server capabilities type defined a `hoverProvider`:
```json
hoverProvider?: boolean;
```
In our C# type definitions this would be defined as
```csharp
[JsonPropertyName("hoverProvider")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public bool? HoverProvider
{
get;
set;
}
```

It is totally legal (and not breaking) in the LSP protocol to modify this type definition into a union type, controlling what is defined based on a capability.
```json
hoverProvider?: boolean | HoverOptions;
```
and in C# this would be defined as:
```csharp
[JsonPropertyName("hoverProvider")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public SumType<bool, HoverOptions>? HoverProvider
{
get;
set;
}
```

which is now a breaking change due to the property type changing from `bool?` to `SumType<bool, HoverOptions>?`. And this same logic applies to adding a new value to a union type as going from `SumType<T, U>` to `SumType<T, U, V>` is also a breaking change.

### Handling breaking changes

Generally, changes that cause binary breaking changes are relatively rare (adding new types to a union). Additionally, we only need to be careful about binary breaking changes if our partners actually use the API being changed. If no one uses it, we can just update the property with a breaking change.

However, if a partner is using the type, we need to handle breaking changes to it carefully. We can support this by adding a new intermediate property for the new union type definition, obsoleting the old one, switching our partners to the new property, then move everything back:

### 1. Add new intermediate property.
First, we add a new property representing the union version of the type and move the serialization attributes to it. The old property is then implemented by accessing the correct value of the new union type. This is safe as the new `HoverOptions` is not provided unless explicitly opted in via capabilities.

```csharp
[JsonIgnore]
[Obsolete("Use HoverProviderUnion instead")]
public bool? HoverProvider
{
get => HoverProviderUnion?.First;
set => HoverProviderUnion = value;
}

[JsonPropertyName("hoverProvider")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public SumType<bool, HoverOptions>? HoverProviderUnion
{
get;
set;
}
```

### 2. Update partners to new version.
Update Razor / XAML to consume the new union type property.

### 3. Switch original property to use union type, obsolete intermediate property.
After partners have switched, we can now change the type of the original property and obsolete the intermediate one:
```csharp
[JsonPropertyName("hoverProvider")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public SumType<bool, HoverOptions>? HoverProvider
{
get;
set;
}

[JsonIgnore]
[Obsolete("Use HoverProvider instead")]
public SumType<bool, HoverOptions>? HoverProviderUnion
{
get => HoverProvider?.First;
set => HoverProvider = value;
}
```

### 4. Delete the intermediate property.
After partners have switched again, we can delete the intermediate property.
```csharp
[JsonPropertyName("hoverProvider")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public SumType<bool, HoverOptions>? HoverProvider
{
get;
set;
}
```
7 changes: 7 additions & 0 deletions src/LanguageServer/Protocol/Protocol/ServerCapabilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ public SumType<bool, HoverOptions>? HoverProvider
set;
}

[JsonIgnore]
public bool? HoverProvider2
{
get => HoverProvider?.First;
set => HoverProvider = value;
}

/// <summary>
/// Gets or sets the value which indicates if signature help is supported.
/// </summary>
Expand Down

0 comments on commit 1cc34e2

Please sign in to comment.