Convert Clasp into a source package #72237

dibarbet · 2024-02-23T03:40:37Z

Converts clasp into a source package so we can make breaking changes without breaking consumers in VS. This package is not used for cross component communication (its an internal utility package).

In order to avoid a triple insertion, we still create a normal dll for insertion into VS. This can be removed when all consumers have updated to the source package version. That and additional cleanup is tracked here - #72251

Passing RPS - https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/532565

dibarbet · 2024-02-23T22:24:18Z

...es/LanguageServer/Microsoft.CommonLanguageServerProtocol.Framework/AbstractLanguageServer.cs

    protected readonly ILspLogger _logger;
+#pragma warning restore IDE1006 // Naming Styles


similar to the public api, modifying this would be a breaking change. So suppressing until after all consumers switch to the source package.

dibarbet · 2024-02-23T22:30:59Z

...uageServer/Microsoft.CommonLanguageServerProtocol.Framework/xlf/LSPFrameworkResources.cs.xlf

@@ -1,42 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>


These were no longer used. the strings are in the protocol project. Additionally there was no resx file for any of these.

dibarbet · 2024-02-23T22:32:09Z

src/EditorFeatures/Core/Microsoft.CodeAnalysis.EditorFeatures.csproj

@@ -36,6 +36,9 @@
    <PackageReference Include="Microsoft.ServiceHub.Client" />
    <PackageReference Include="Microsoft.VisualStudio.Debugger.Contracts" />
  </ItemGroup>
+  <ItemGroup>
+    <Compile Include="..\..\Compilers\Core\Portable\InternalUtilities\IsExternalInit.cs" Link="Utilities\IsExternalInit.cs" />


seems like a bunch of projects were picking up this from the clasp project. Had to add it in a few places now that its become a source package.

dibarbet · 2024-02-24T00:27:58Z

...s/LanguageServer/Microsoft.CommonLanguageServerProtocol.Framework/AbstractHandlerProvider.cs

 public abstract class AbstractHandlerProvider
+#else
+internal abstract class AbstractHandlerProvider


The source package versions of these types need to be internal. Otherwise someone referencing Clasp directly, and MS.CA.LanguageServer.Protocol (for example, Razor) will get conflicting type definitions if they're public.

However, for binary compatibility in VS on the old non-source package versions of clasp, these APIs need to be public. So until we move all consumers to the source only version of the package, we have to ship the dll in VS with public accessibility.

dibarbet · 2024-02-28T23:52:17Z

...s/LanguageServer/Microsoft.CommonLanguageServerProtocol.Framework/AbstractHandlerProvider.cs

@@ -2,6 +2,9 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.

+// This is consumed as 'generated' code in a source package and therefore requires an explicit nullable enable
+#nullable enable


we ship an .editorconfig in the package that marks all the code in here as generated code. This ensures that analyzers do not report diagnostics for these files when included as a source package (compiler warnings are still reported - unfortunately there is no way to disable that).

And generated code requires explicit nullable defines in order to use nullable annotations.

dibarbet · 2024-02-29T17:58:50Z

src/Tools/ExternalAccess/Xaml/External/IXamlRequestHandler.cs


 namespace Microsoft.CodeAnalysis.ExternalAccess.Xaml;

 internal interface IXamlRequestHandler<TRequest, TResponse>
-    : IRequestHandler<TRequest, TResponse, XamlRequestContext>


This was incorrectly exposing the clasp type directly to XAML. making it internal broke their handlers. However XAML already wrapped this handler into a Roslyn typed handler (XamlRequestHandlerBase), so all we need to do here is copy the old signature directly here and remove the inheritance.

...geServerProtocol.Framework/Microsoft.CommonLanguageServerProtocol.Framework.Shared.projitems

dibarbet · 2024-03-11T17:50:58Z

/azp run roslyn-integration-CI

azure-pipelines · 2024-03-11T17:51:08Z

Azure Pipelines successfully started running 1 pipeline(s).

ToddGrun · 2024-03-11T17:55:41Z

...uageServerProtocol.Framework/Microsoft.CommonLanguageServerProtocol.Framework.Package.csproj

+    <PackageReference Include="StreamJsonRpc" />
+  </ItemGroup>
+  <ItemGroup>
+    <Compile Include="..\..\..\Compilers\Core\Portable\InternalUtilities\IsExternalInit.cs" Link="Utilities\IsExternalInit.cs" />


Consuming the ISExternalInit package still isn't an option, I assume?

this one? https://www.nuget.org/packages/IsExternalInit. I guess probably possible, but we don't own it and its one less dependency to manage, so I'd rather just use our version.

ToddGrun · 2024-03-11T17:59:23Z

...geServerProtocol.Framework/Microsoft.CommonLanguageServerProtocol.Framework.Shared.projitems

+    <Import_RootNamespace>Microsoft.CommonLanguageServerProtocol.Framework.Shared</Import_RootNamespace>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildThisFileDirectory)AbstractHandlerProvider.cs" />


AbstractHandlerProvider.cs

Out of curiosity, why not an all-inclusive glob?

Was autogenerated by VS - also seems to match what other shared projects do (probably because they were also autogenerated)
https://github.com/dotnet/roslyn/blob/main/src/Dependencies/PooledObjects/Microsoft.CodeAnalysis.PooledObjects.projitems

ToddGrun

### Summary of the changes Now that dotnet/roslyn#72237 has merged on the Roslyn side, we need to update Razor to consume the source package version. This will allow us to delete the nuget package version - Fixes:

dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Feb 23, 2024

dibarbet mentioned this pull request Feb 23, 2024

Cleanup Microsoft.CommonLanguageServerProtocol.Framework #72251

Closed

dibarbet commented Feb 23, 2024

View reviewed changes

dibarbet force-pushed the clasp_source_package branch 2 times, most recently from bb7a37b to a4c1d86 Compare February 24, 2024 00:24

dibarbet commented Feb 24, 2024

View reviewed changes

dibarbet force-pushed the clasp_source_package branch from a4c1d86 to 9430a78 Compare February 24, 2024 00:46

build-analysis bot mentioned this pull request Feb 24, 2024

The active test run was aborted. Reason: Test host process crashed dotnet/dnceng#451

Open

3 tasks

dibarbet force-pushed the clasp_source_package branch 7 times, most recently from 206091f to eead0f9 Compare February 28, 2024 23:49

dibarbet commented Feb 28, 2024

View reviewed changes

runfoapp bot mentioned this pull request Feb 29, 2024

invokenavigatetoforgeneratedfile_infolder failures #69686

Open

dibarbet marked this pull request as ready for review February 29, 2024 17:45

dibarbet requested review from 333fred and a team as code owners February 29, 2024 17:45

dibarbet requested review from davidwengier and ToddGrun February 29, 2024 17:45

dibarbet commented Feb 29, 2024

View reviewed changes

davidwengier approved these changes Feb 29, 2024

View reviewed changes

...geServerProtocol.Framework/Microsoft.CommonLanguageServerProtocol.Framework.Shared.projitems Outdated Show resolved Hide resolved

dibarbet mentioned this pull request Mar 8, 2024

[LSP] Untangle retrieving the document identifier from the executing handler #71114

Open

ToddGrun reviewed Mar 11, 2024

View reviewed changes

ToddGrun approved these changes Mar 11, 2024

View reviewed changes

dibarbet force-pushed the clasp_source_package branch from eead0f9 to fc6a1e4 Compare March 11, 2024 18:35

Convert Clasp into a source package

e3a92bf

dibarbet force-pushed the clasp_source_package branch from fc6a1e4 to e3a92bf Compare March 11, 2024 22:18

RikkiGibson self-assigned this Mar 14, 2024

dibarbet merged commit bfa6269 into dotnet:main Mar 20, 2024
27 checks passed

dibarbet deleted the clasp_source_package branch March 20, 2024 21:42

dotnet-policy-service bot added this to the Next milestone Mar 20, 2024

dibarbet mentioned this pull request Mar 21, 2024

Switch Razor to using clasp as a source package dotnet/razor#10139

Merged

RikkiGibson modified the milestones: Next, 17.10 P3 Mar 25, 2024

dotnet-bot mentioned this pull request Mar 26, 2024

[Automated] PRs inserted in VS build main-34725.195 #72740

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert Clasp into a source package #72237

Convert Clasp into a source package #72237

dibarbet commented Feb 23, 2024 •

edited

Loading

dibarbet Feb 23, 2024

dibarbet Feb 23, 2024

dibarbet Feb 23, 2024

dibarbet Feb 24, 2024

dibarbet Feb 28, 2024

dibarbet Feb 29, 2024

dibarbet commented Mar 11, 2024

azure-pipelines bot commented Mar 11, 2024

ToddGrun Mar 11, 2024

dibarbet Mar 11, 2024

ToddGrun Mar 11, 2024

dibarbet Mar 11, 2024

ToddGrun left a comment

		protected readonly ILspLogger _logger;
		#pragma warning restore IDE1006 // Naming Styles

Convert Clasp into a source package #72237

Convert Clasp into a source package #72237

Conversation

dibarbet commented Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dibarbet commented Mar 11, 2024

azure-pipelines bot commented Mar 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ToddGrun left a comment

Choose a reason for hiding this comment

dibarbet commented Feb 23, 2024 •

edited

Loading